Skip to content

Chore(Helm): Align Helm with Kustomize by enabling leader-election. #1944

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Baarsgaard
Copy link
Collaborator

@Baarsgaard Baarsgaard commented Apr 8, 2025

Helm has leader election disabled per default.
That is not the case for the Kustomize deployment

@github-actions github-actions bot added the chore label Apr 8, 2025
@Baarsgaard Baarsgaard marked this pull request as ready for review April 8, 2025 17:22
@weisdd
Copy link
Collaborator

weisdd commented Apr 9, 2025

Leader election is needed to prevent two controllers from trying to manage the same set of resources at the same time.
If you disable elections, better to have spec.strategy.type set to Recreate. Although, I think it's rather helm chart that has suboptimal configuration.

@theSuess
Copy link
Member

theSuess commented Apr 9, 2025

We talked about this in the sync call after you left. We concluded that regular deployments should be fine as status updates no longer trigger reconciliation. So the only way simultaneous reconciliation can happen is by coinciding resyncPeriods

@weisdd
Copy link
Collaborator

weisdd commented Apr 11, 2025

@theSuess I was thinking rather about briefly having two pods during Rollout. - We have readiness probes, so the old pod will not be terminated for up to 15 seconds (initialDelaySeconds: 5 + periodSeconds: 10). Upon start, controllers would receive all CRs and watched owned resources.

@Baarsgaard Baarsgaard force-pushed the chore_disable_leader_election_in_kustomize branch from e9a9d86 to a5d5ef0 Compare April 29, 2025 16:27
@Baarsgaard Baarsgaard changed the title Chore(Kustomize): Align Kustomize with Helm by disabling leader-election. Chore(Kustomize): Align Helm with Kustomize by enabling leader-election. Apr 29, 2025
@Baarsgaard Baarsgaard force-pushed the chore_disable_leader_election_in_kustomize branch from a5d5ef0 to d780c17 Compare April 29, 2025 16:29
@Baarsgaard Baarsgaard changed the title Chore(Kustomize): Align Helm with Kustomize by enabling leader-election. Chore(Helm): Align Helm with Kustomize by enabling leader-election. Apr 30, 2025
@weisdd
Copy link
Collaborator

weisdd commented May 1, 2025

Briefly discussed this with @Baarsgaard over Slack. - There's an old bug in our chart which causes an additional ConfigMap with controller_manager_config.yaml to get created when leader election is on. - The values there are hardcoded, so they do not match whatever is passed through values.yaml.
We agreed that, as part of the PR, we should address templates/cm.yaml as well.

@Baarsgaard Baarsgaard force-pushed the chore_disable_leader_election_in_kustomize branch from 7a07d42 to b2aa3f5 Compare May 5, 2025 22:00
@Baarsgaard Baarsgaard closed this May 5, 2025
@Baarsgaard Baarsgaard deleted the chore_disable_leader_election_in_kustomize branch May 5, 2025 22:00
@Baarsgaard Baarsgaard restored the chore_disable_leader_election_in_kustomize branch May 5, 2025 22:00
@Baarsgaard Baarsgaard reopened this May 5, 2025
@Baarsgaard
Copy link
Collaborator Author

Baarsgaard commented May 5, 2025

After some digging I found both some solid documentation on Leader election and a bug 😅
Leader election is handled through leases per default according to the Manager docs

The bug is that we always trigger status sync on Grafanas regardless of the instance being elected as the leader.
I have a couple of ideas on fixing that in a separate PR.

PS: Transitioning from my fork to the main repo, accidentally deleted the branch in the wrong repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants