Add leader election to reconciler#672
Add leader election to reconciler#672AMacedoP wants to merge 1 commit intok8snetworkplumbingwg:masterfrom
Conversation
Guard the reconciler execution behind a leader election lock to ensure that it runs on one pod of the daemonset. This prevents high memory usage on the whereabouts pod and apiserver when running on large clusters. Fixes k8snetworkplumbingwg#386. Signed-off-by: Alejandro Macedo <alex.macedopereira@gmail.com>
Summary of ChangesHello @AMacedoP, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a robust leader election mechanism for the IP reconciler component of Whereabouts. By leveraging Kubernetes' native leader election capabilities, it guarantees that the reconciler's scheduled tasks, such as IP reconciliation, are executed by a single, active leader pod within the daemonset. This enhancement significantly improves the stability and correctness of the IP management by preventing concurrent modifications and ensuring consistent state across the cluster. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces leader election for the reconciler, ensuring it runs on a single pod within the daemonset. A security review found no immediate vulnerabilities. The implementation correctly utilizes the client-go leader election mechanism and is well-organized, with one minor suggestion for code clarity and adherence to idiomatic Go practices.
| electionCtx, cancelElection := context.WithCancel(ctx) | ||
| err = runReconcilerLeaderElection( | ||
| electionCtx, | ||
| k8sClientSet, | ||
| namespace, | ||
| identity, | ||
| errorChan, | ||
| cancelElection, | ||
| ) | ||
| cancelElection() | ||
| if err != nil { | ||
| errorChan <- err | ||
| } |
There was a problem hiding this comment.
The call to cancelElection() on line 125 is redundant. The runReconcilerLeaderElection function is a blocking call that only returns after its context (electionCtx) has been canceled. Therefore, calling cancelElection() again is a no-op. Using defer is a more idiomatic and cleaner way to handle context cancellation, which improves code maintainability.
electionCtx, cancelElection := context.WithCancel(ctx)
defer cancelElection()
if err := runReconcilerLeaderElection(
electionCtx,
k8sClientSet,
namespace,
identity,
errorChan,
cancelElection,
); err != nil {
errorChan <- err
}
What this PR does / why we need it:
Adds leader election to the reconciler execution to ensure that it runs on only one pod of the whereabouts daemonset.
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #386.
Special notes for your reviewer (optional):