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

document nuances of replayDeletion: true in Usage API #873

Open
jbw976 opened this issue Feb 4, 2025 · 3 comments
Open

document nuances of replayDeletion: true in Usage API #873

jbw976 opened this issue Feb 4, 2025 · 3 comments
Labels
enhancement New feature or request P2 Should fix, important issues. Some user impact

Comments

@jbw976
Copy link
Member

jbw976 commented Feb 4, 2025

What's Missing?

The Usage API has been promoted to Beta and is on by default in v1.19.0. We had hoped to set replayDeletion: true as the default because of the performance boost during resource cleanup, but after discovering some potentially damaging and undesirable behavior, we decided to not make this the default. @turkenh captured this decision and behavior in:
crossplane/crossplane#6140 (comment)

We should capture this in the Usage docs page, so users can make an informed decision about when to use replayDeletion in their environment.
https://docs.crossplane.io/master/concepts/usages/#replay-blocked-deletion-attempt

@jbw976 jbw976 added enhancement New feature or request P2 Should fix, important issues. Some user impact labels Feb 4, 2025
@bobh66
Copy link
Collaborator

bobh66 commented Feb 5, 2025

I wonder if replayDeletion only makes sense when there is a by resource specified? In that case there is generally a composite resource that is already marked for deletion and the GC is trying to delete it's children and is being blocked by the Usage webhook. replyDeletion is attempting to speed up this process by not waiting for the GC to retry deletion, since it may have backed off to the maximum delay (1000 seconds if I recall). If we didn't replay the deletion then GC would eventually do it anyway, which is completely out of anyone's control because the parent composite is marked deleted.

In the case where a Usage has no by attribute and is solely being used to prevent deletion of a resource, there is no parent composite that would be marked deleted and cause GC to eventually retry the deletion. So maybe it doesn't make sense to allow replayDeletion at all when there is no by specified? @turkenh ?

@bobh66
Copy link
Collaborator

bobh66 commented Feb 5, 2025

This could even be taken one step further and we could check for an ownerReference on the Usage and follow it to see if the owner is marked deleted. If it is then replayDeletion makes sense, but if it isn't or there is no owner then it probably doesn't.

@bobh66
Copy link
Collaborator

bobh66 commented Feb 6, 2025

I created crossplane/crossplane#6279 to consider restricting the application of replayDeletion

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request P2 Should fix, important issues. Some user impact
Projects
None yet
Development

No branches or pull requests

2 participants