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

Fix: Use Patch instead of Update to add/remove finalizers #1801

Merged
merged 2 commits into from
Dec 30, 2024

Conversation

Baarsgaard
Copy link
Contributor

@Baarsgaard Baarsgaard commented Dec 18, 2024

Managed to solve the perpetual drift noted in #1776 that is rooted in us applying finalizers and updating unintended fields on CRs.

I first attempted to use JSONPatch, but I could not for the life of me get the libraries to comply, despite handcrafting working patches with kubectl.
MergePatch worked immediately.

Currently, the implementation is hardcoded to only support arrays of one finalizer. More work can be done to make this support multiple if the need ever arises.

@Baarsgaard Baarsgaard marked this pull request as ready for review December 18, 2024 11:45
@Baarsgaard Baarsgaard force-pushed the fix_finalizer_cfg_drift branch from 172cf54 to 1fa382c Compare December 20, 2024 23:49
@Baarsgaard Baarsgaard changed the title Refactor(Internal): Use Patch instead of Update to set/remove finalizers Fix: Use Patch instead of Update to add/remove finalizers Dec 20, 2024
controllers/controller_shared.go Outdated Show resolved Hide resolved
@Baarsgaard Baarsgaard force-pushed the fix_finalizer_cfg_drift branch 2 times, most recently from cdde804 to b778a52 Compare December 27, 2024 23:31
@Baarsgaard Baarsgaard requested a review from weisdd December 27, 2024 23:35
@Baarsgaard
Copy link
Contributor Author

Baarsgaard commented Dec 28, 2024

Once this is merged, I will likely add finalizers to GrafanaFolder before I start refactoring the reconcile loops for all other CRs one by one

@Baarsgaard Baarsgaard force-pushed the fix_finalizer_cfg_drift branch from b778a52 to a99dc26 Compare December 28, 2024 18:34
@weisdd weisdd added this pull request to the merge queue Dec 30, 2024
Merged via the queue into grafana:master with commit 24ba0ca Dec 30, 2024
14 checks passed
@Baarsgaard Baarsgaard deleted the fix_finalizer_cfg_drift branch December 30, 2024 15:23
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.

2 participants