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

Backport of Fix issue with streaming service health watches. into release/1.16.x #17776

Conversation

hc-github-team-consul-core
Copy link
Contributor

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:

  • no services are exported
  • an upstream watch to service X is spawned
  • the streaming backend filters out data for service X (due to lack of exports)
  • service X is finally exported

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 the streamingEnabled check 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

@hc-github-team-consul-core hc-github-team-consul-core force-pushed the backport/derekm/NET-4506/fix-agentless-health/merely-promoted-moose branch from 2d04da6 to 335462c Compare June 15, 2023 17:47
@hc-github-team-consul-core hc-github-team-consul-core force-pushed the backport/derekm/NET-4506/fix-agentless-health/merely-promoted-moose branch from 799b28b to 66b1f28 Compare June 15, 2023 17:47
Copy link

Choose a reason for hiding this comment

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

Auto approved Consul Bot automated PR

@hc-github-team-consul-core hc-github-team-consul-core merged commit 359bea1 into release/1.16.x Jun 15, 2023
@hc-github-team-consul-core hc-github-team-consul-core deleted the backport/derekm/NET-4506/fix-agentless-health/merely-promoted-moose branch June 15, 2023 18:06
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.

3 participants