Backport of Fix issue with streaming service health watches. into release/1.16.x#17776
Merged
hc-github-team-consul-core merged 2 commits intoJun 15, 2023
Conversation
2d04da6 to
335462c
Compare
799b28b to
66b1f28
Compare
github-team-consul-core-pr-approver
approved these changes
Jun 15, 2023
Collaborator
github-team-consul-core-pr-approver
left a comment
There was a problem hiding this comment.
Auto approved Consul Bot automated PR
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport
This PR is auto-generated from #17775 to be assessed for backporting due to the inclusion of the label backport/1.16.
The below text is copied from the body of the original PR.
This commit fixes an issue where the health streams were unaware of service export changes. Whenever an exported-services config entry is modified, it is effectively an ACL change.
The bug would be triggered by the following situation:
In the situation above, the streaming backend does not trigger a refresh of its data. This means that any events that were supposed to have been received prior to the export are NOT backfilled, and the watches never see service X spawning.
We currently have decided to not trigger a stream refresh in this situation due to the potential for a thundering herd effect (touching exports would cause a re-fetch of all watches for that partition, potentially). Therefore, a local blocking-query approach was added by this commit for agentless.
It's also worth noting that the streaming subscription is currently bypassed most of the time with agentful, because proxycfg has a
req.Source.Node != ""which prevents thestreamingEnabledcheck from passing. This means that while agents should technically have this same issue, they don't experience it with mesh health watches.Note that this PR does not fully fix the issue for service-discovery and simply aims to resolve the problem for mesh proxycfg queries in the short-term.
Overview of commits