-
Notifications
You must be signed in to change notification settings - Fork 187
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
Allow control of finalization garbage collection with .spec.deletionPolicy
#1314
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a new section to the API docs right after https://github.com/fluxcd/kustomize-controller/blob/main/docs/spec/v1/kustomizations.md#prune
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this contribution! I think it's a really valuable one.
0cba19c
to
90c3721
Compare
Ok, it seems like we are approaching something here. 😸 I have just rebased and squashed all commits. The PR should be ready to merge if you are satisfied with the proposed changes. Thanks for all the reviews! 🙏 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good! Just some minor nitpicks/thoughts regarding text.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great stuff, I'm ready to give my approval after all the open discussions are resolved!
Signed-off-by: Erik Godding Boye <[email protected]> Co-authored-by: Stefan Prodan <[email protected]> Co-authored-by: Amund Tenstad <[email protected]>
a681e26
to
c38ebab
Compare
@matheuscscp All discussions should now be resolved. Can you please approve CI to see if I made any mistakes in the test refactoring? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great contribution! Thanks @erikgb
.spec.deletionPolicy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thanks @erikgb 🏅
The
prune
setting of Kustomizations has a two-fold behavior. When set totrue
FLux will:We would like to have 1, but not 2, and this is currently not possible. This PR introduces a new API field allowing control over finalization garbage collection independently of
prune
.Some background/motivation for this proposed enhancement:
We are using Flux to provision review environments in a multi-tenant environment (shared cluster), and our tenants are not allowed to create Kubernetes namespaces directly. Instead they have to use a custom namespace-scoped resource as a controller for the namespace (and RBAC). This indirect management of namespaces makes it hard to create Kustomizations inside review namespaces. To overcome this, we use the
targetNamespace
field. This works well when the review namespace is created. But when the review namespace is teared down by our review environment automation, there is a finalization race condition between Flux and our namespace controller. If Flux looses this race, the Kustomization is stuck in non-finalized state (because the namespace is gone) and requires manual intervention from a cluster-admin. This is something we definitely would like to avoid! Since the namespace is controlled and eventually deleted by our namespace controller, no resources are in fact orphaned if we disable FLux garbage collection.Orphaning resources is generally a bad idea, but is sometimes required. 😉
Once we agree on the naming, I will submit a PR to update the docs in the website repo.Close #662