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

feature request: add support for providing propagationPolicy (i.e orphan) when deleting immutable resources #603

Open
mdnfiras opened this issue Jul 8, 2023 · 3 comments

Comments

@mdnfiras
Copy link

mdnfiras commented Jul 8, 2023

When setting the kustomize.toolkit.fluxcd.io/force: enabled label for statefulsets, flux will delete the statefulset if it detects an immutable field error. This will delete all the statefulset's pods as a result due to kubernetes garbage collection.

As a workaround, an admission webhook can mutate pods' metadata.ownerReferences to be empty during pod creation, but admission webhooks are risky: if they fail (service is down, or throwing unhandled exceptions) they block the creation of pods, and if set to be ignored upon failure then we would risk flux deleting a statefulset and its pods all at once.

I think that a proper solution would be to add support to set the propagationPolicy of the deleteOptions in the delete request that flux makes. This can be set for example through an annotation like kustomize.toolkit.fluxcd.io/propagationPolicy: orphan in the same resource that has kustomize.toolkit.fluxcd.io/force: enabled:

  • if the label kustomize.toolkit.fluxcd.io/force: enabled is set, then the annotation kustomize.toolkit.fluxcd.io/propagationPolicy is checked
  • if the annotation kustomize.toolkit.fluxcd.io/propagationPolicy does not exist, the default policy is used, which is background
  • if the annotation kustomize.toolkit.fluxcd.io/propagationPolicy is set to background, foreground or orphan then this value will be used in the delete request
  • if the annotation kustomize.toolkit.fluxcd.io/propagationPolicy is set to anything other than those values, the controller can throw an error during reconciliation dry-run.

what do you think about this feature?

@kingdonb
Copy link
Member

Is one of the intended or possible uses of a foreground propagationPolicy to be able to wait for deletes? I think I have seen users request control over the deletion order when it comes to dependency and dependsOn, I just wonder if this might address that concern when it gets into kustomize-controller.

@stefanprodan
Copy link
Member

stefanprodan commented Jul 12, 2023

No this will not address dependsOn delete ordering. Waiting for resources to be deleted is not a viable option IMO. I’m for adding an annotation to control the propagation policy, but it can’t be only for force as proposed here nor it can be blocking. The annotation would cover deletion no matter why it happens, force apply, GC, Kustomization deletion, etc.

@mdnfiras
Copy link
Author

@kingdonb actually my main concern is to have the orphan option, since I intend to use the new immutable field resource deletion feature of flux with statefulsets, but as flux deletes and re-creates a statefulset it will result in an instant deletion of all its pods at the same time.

@stefanprodan

Waiting for resources to be deleted is not a viable option IMO
The annotation would cover deletion no matter why it happens

I agree with both statements, so I guess foreground is out of the question since it waits for owned resources to be deleted first, so its kinda "blocking".

orphan is also a bit blocking since it should technically add the orphan finalizer on the resource to delete, then block resource deletion until another controller has removed the ownerReferences from owned resources. but this usually happens instantly, so in practice it doesn't really block

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

No branches or pull requests

3 participants