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 state: ensure that identical manual virtual IP updates result in not bumping the modify indexes into release/1.20.x #21969

Conversation

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

Backport

This PR is auto-generated from #21909 to be assessed for backporting due to the inclusion of the label backport/1.20.

The below text is copied from the body of the original PR.


Description

The consul-k8s endpoints controller issues catalog register and manual virtual ip updates without first checking to see if the updates would be effectively not changing anything. This is supposed to be reasonable because the state store functions do the check for a no-op update and should discard repeat updates so that downstream blocking queries watching one of the resources don't fire pointlessly (and CPU wastefully).

While this is true for the check/service/node catalog updates, it is not true for the "manual virtual ip" updates triggered by the PUT /v1/internal/service-virtual-ip. Forcing the connect injector pod to recycle while watching some lightly modified FSM code can show that a lot of updates are of the update list of ips from [A] to [A]. Immediately following this stray update you can see a lot of activity in proxycfg and xds packages waking up due to blocking queries triggered by this.

This PR skips updates that change nothing both:

  • at the RPC layer before passing it to raft (ideally)
  • if the write does make it through raft and get applied to the FSM (failsafe)

Testing & Reproduction steps

  • Deployed a small 1-node cluster using consul-k8s + kind with 2 connect-enabled services
  • Watched the server debug logs before/during/after recycling the connect injector pod.
  • Before you could see the api PUTs immediately preceding proxycfg/xds activity.
  • After you no longer see these as often.

PR Checklist

  • updated test coverage
  • [ ] external facing docs updated
  • appropriate backport labels added
  • not a security concern

Overview of commits

Copy link
Collaborator

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

@github-actions github-actions bot added the theme/internals Serf, Raft, SWIM, Lifeguard, Anti-Entropy, locking topics label Nov 22, 2024
@rboyer rboyer force-pushed the backport/rboyer/fix-manual-vip-writes/jointly-sweet-dingo branch from 8f1fd66 to 3155b2f Compare November 22, 2024 20:39
@rboyer rboyer merged commit d335aa3 into release/1.20.x Nov 25, 2024
93 checks passed
@rboyer rboyer deleted the backport/rboyer/fix-manual-vip-writes/jointly-sweet-dingo branch November 25, 2024 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme/internals Serf, Raft, SWIM, Lifeguard, Anti-Entropy, locking topics
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants