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

client: recreate script checks on Update #6265

Merged
merged 1 commit into from
Sep 5, 2019
Merged

Conversation

tgross
Copy link
Member

@tgross tgross commented Sep 4, 2019

This PR fixes a bug uncovered in e2e testing in #6252

Splitting the immutable and mutable components of the scriptCheck led to a bug where the environment interpolation wasn't being incorporated into the check's ID, which caused the UpdateTTL to update for a check ID that Consul didn't have (because our Consul client creates the ID from the structs.ServiceCheck each time we update).

@tgross tgross force-pushed the b-script-check-immutability branch from b01d447 to 7d5040b Compare September 4, 2019 20:44
Copy link
Member

@schmichael schmichael left a comment

Choose a reason for hiding this comment

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

Feel free to address the err check in a followup PR if it's an actual issue.

@tgross tgross mentioned this pull request Sep 5, 2019
@tgross tgross force-pushed the b-script-check-immutability branch from dbc4877 to da342e2 Compare September 5, 2019 13:45
@schmichael schmichael added this to the 0.10.0 milestone Sep 5, 2019
Splitting the immutable and mutable components of the scriptCheck led
to a bug where the environment interpolation wasn't being incorporated
into the check's ID, which caused the UpdateTTL to update for a check
ID that Consul didn't have (because our Consul client creates the ID
from the structs.ServiceCheck each time we update).

Task group services don't have access to a task environment at
creation, so their checks get registered before the check can be
interpolated. Use the original check ID so they can be updated.
@tgross tgross force-pushed the b-script-check-immutability branch from da342e2 to 38eea0a Compare September 5, 2019 15:11
@tgross tgross merged commit 088e381 into master Sep 5, 2019
@tgross tgross deleted the b-script-check-immutability branch September 5, 2019 15:43
@github-actions
Copy link

github-actions bot commented Feb 1, 2023

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants