-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
local: mark service as InSync when added to local agent state #9284
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great Daniel. I could be wrong but I think we need to also add the IsSame
check for Checks that are part of the registration.
I think this would be easy to confirm by adding a service-level check to the registration in the test - I think if you do that you'll see it fail again as the check would still be marked as InSync = false
even when it hasn't changed so will cause an extra RPC.
1b2829c
to
3b3e907
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect! Looks great!
If the existing service and checks are the same as the new registration.
3b3e907
to
33b8106
Compare
🍒 If backport labels were added before merging, cherry-picking will start automatically. To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/289221. |
🍒✅ Cherry pick of commit 9f0f2bd onto |
local: mark service as InSync when added to local agent state
Do we want to manually backport this to 1.8 and 1.7? |
This was a relatively minor problem that we only discovered when testing at a large scale, and even then it wasn't really breaking anything it was just surprising. So I think we shouldn't spend the time to manually backport unless it becomes a problem for someone. |
When the existing service is the same as the new service.
Fixes #9266