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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for background cascading deletion #2529

Open
chirauki opened this issue Aug 8, 2023 · 5 comments
Open

Support for background cascading deletion #2529

chirauki opened this issue Aug 8, 2023 · 5 comments
Labels
kind/enhancement Improvements or new features

Comments

@chirauki
Copy link

chirauki commented Aug 8, 2023

Hello!

  • Vote on this issue by adding a 馃憤 reaction
  • If you want to implement this feature, comment to let us know (we'll work with you on design, scheduling, etc.)

Issue details

I can't find any reference in docs, but it looks like this provider only supports foreground cascading deletion:

https://github.com/pulumi/pulumi-kubernetes/blob/master/provider/pkg/await/await.go#L675-L683

This causes problems with some custom controllers from third-party providers that do not support well this method, but handle correctly background cascading deletion. Since background cascading deletion is the default in kubectl and Kubernetes API, this leads to some time troubleshooting as the Pulumi provider cannot delete resources that kubectl can.

@chirauki chirauki added kind/enhancement Improvements or new features needs-triage Needs attention from the triage team labels Aug 8, 2023
@aq17 aq17 removed the needs-triage Needs attention from the triage team label Aug 10, 2023
@aq17
Copy link

aq17 commented Aug 10, 2023

Thanks for opening this feature request @chirauki , I've added it to our project board.

@aq17 aq17 closed this as completed Aug 10, 2023
@aq17 aq17 reopened this Aug 10, 2023
@EronWright
Copy link
Contributor

@chirauki could you answer some clarifying questions?

  1. Do you have an example of a thirty-party controller that would have a problem with how Pulumi deletes resources?
  2. When you say 'background cascading deletion', do you perhaps mean that Pulumi should delete the resource but not wait for it to be finalized?

Pulumi has support for skipping the await logic using the pulumi.com/skipAwait annotation, see this blog post.

To my understanding, the default behavior of kubectl is to call delete on the object and wait for it to be finalized, and rely on background cascading deletion to cleanup the object's dependents. It is true that Pulumi uses foreground cascading deletion (as of v3.27, previously it used background deletion, see #2379). It doesn't seem to be customizable, and it would help if you could elaborate on the importance.

@chirauki
Copy link
Author

chirauki commented Oct 3, 2023

Hi!

Do you have an example of a thirty-party controller that would have a problem with how Pulumi deletes resources?

I found this with a controller developed by the company I work with, but it's not open source, so I'm not at liberty to share. I found some references to older versions of cert-manager having a similar problem, but I don't know of any other controller that might have this problem.

When you say 'background cascading deletion', do you perhaps mean that Pulumi should delete the resource but not wait for it to be finalized?

I don't think it has to do with waiting according to docs: https://kubernetes.io/docs/concepts/architecture/garbage-collection/#cascading-deletion. As you can see, there are some differences between foreground and background cascading deletion, and I think the controller that watches those resources needs to account for that (which means we need to fix our controller to work both ways).

In our case, I tested both methods using kubectl (as described in https://kubernetes.io/docs/tasks/administer-cluster/use-cascading-deletion/#use-foreground-cascading-deletion) and I could verify that using background cascading deletion (the default in kubectl), the object could be deleted, but when forcing kubectl to use foreground cascading deletion (the one pulumi enforces), the object could not be deleted (again, due to our controller not handling it well).

So, from our side, we've added a pulumi local command that runs kubectl delete so that it will run before pulumi tries to delete such an object to avoid the pulumi destroy to become stall.

@EronWright
Copy link
Contributor

I do have a theory on how foreground deletion could be a problem for some controllers:

With foreground cascading deletion, the controller must be careful to not finalize the object prematurely, that is, when there's multiple finalizers. A given object could have finalizers: ["foregroundDeletion", "operator.example.com"] and the operator shouldn't do anything until the foregroundDeletion is gone, otherwise the "parent" would be prematurely disturbed.

Here's a similar issue in ArgoCD: argoproj/argo-cd#5724

@chirauki this comment seems to answer the mystery of cert-manager.

I accept there's good reason to use background deletion in some cases, and would agree that Pulumi could support it with an override.

@lkt82
Copy link

lkt82 commented Mar 25, 2024

Hi I ran into this issues using the mariadb-operator version 0.27.0. Hope it can be fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Improvements or new features
Projects
None yet
Development

No branches or pull requests

4 participants