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 service tags not added to health check. Part two #3845

Merged
merged 1 commit into from Feb 6, 2018

Conversation

42wim
Copy link
Contributor

@42wim 42wim commented Jan 29, 2018

This fixes the issue in my comment
#3259 (comment) on #3259

It also adds the ServiceTags to the check, otherwise the check will differ because the local one has a nil entry for ServiceTags but the server state has the actual ServiceTags in the check.

Related to #3642

@slackpad slackpad added this to the Next milestone Jan 29, 2018
@slackpad
Copy link
Contributor

Nice - thank you @42wim! I'm going to see if I can add a better unit test to cover this and then will merge shortly.

@pierresouchay
Copy link
Contributor

@42wim what was the impact then? The heathcheck result being sent to leader?

@42wim
Copy link
Contributor Author

42wim commented Jan 30, 2018

@pierresouchay Yes and the fact that this causes to change/update the ModifyIndex of this check. Couple this with something like fabio that reacts on a health state change :)

fboismenu pushed a commit to criteo-forks/consul that referenced this pull request Jan 30, 2018
fboismenu pushed a commit to criteo-forks/consul that referenced this pull request Jan 30, 2018
@slackpad slackpad modified the milestones: Next, 1.0.4 Feb 5, 2018
@slackpad slackpad merged commit 533f65b into hashicorp:master Feb 6, 2018
slackpad added a commit that referenced this pull request Feb 6, 2018
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

3 participants