Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bug with defining connect healthcheck #272

Merged
merged 1 commit into from
Jun 8, 2020
Merged

Fix bug with defining connect healthcheck #272

merged 1 commit into from
Jun 8, 2020

Conversation

lkysow
Copy link
Member

@lkysow lkysow commented Jun 4, 2020

Previously we were setting the alias_service field of alias checks to
the service name instead of the service id. This check would then pass
because Consul would consider an alias for a non-existent service id to
be okay. With hashicorp/consul#7384, now the
check will fail if the service id doesn't exist and so those connect
services will be considered unhealthy and be unroutable.

Previously we were setting the alias_service field of alias checks to
the service name instead of the service id. This check would then pass
because Consul would consider an alias for a non-existent service id to
be okay. With hashicorp/consul#7384, now the
check will fail if the service id doesn't exist and so those connect
services will be considered unhealthy and be unroutable.
@lkysow lkysow requested a review from a team June 4, 2020 20:19
@lkysow
Copy link
Member Author

lkysow commented Jun 4, 2020

I've tested with consul from master that has hashicorp/consul#7384 merged and this check is now passing correctly. Without this patch all connect services will be failing and unroutable.

@lkysow lkysow added this to the Consul 1.8.0 milestone Jun 4, 2020
@lkysow
Copy link
Member Author

lkysow commented Jun 5, 2020

lkysow/consul-k8s-dev:jun04-svc-alias can be used with lkysow/consul-jun4 if you want to test

@lkysow
Copy link
Member Author

lkysow commented Jun 5, 2020

To reproduce:

  1. Install consul with lkysow/consul-jun4 consul image
  2. Deploy a connect service
  3. Notice that it's failing its alias check
  4. If you deploy another service and try to make a call between the two they'll fail.
  5. If you dig into the envoy logs at debug level you'll see it's not healthy

Then update consul-k8s image to lkysow/consul-k8s-dev:jun04-svc-alias and delete the pods so they redeploy. The check now passes.

Copy link
Contributor

@ishustava ishustava left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good! My tests were all successful.

@lkysow lkysow merged commit 5a98720 into master Jun 8, 2020
@lkysow lkysow deleted the alias-check branch June 8, 2020 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants