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

Add support for deleting resources in reverse order by stage. #2582

Merged

Conversation

hankfreund
Copy link
Member

Change description

This solution makes use of the ApplySet annotations for GroupKinds and Namespaces. When a facade deletion request is received, the ApplySet data is read from the Plan to know what GKs to find and delete, and which namespaces to search. The resources are further filtered by the ApplySet ID they are automatically labeled with.
Stage order information is added to the Plan so the deletion processing can iterate over them in reverse, and as a label on each expanded resource.

@hankfreund
Copy link
Member Author

/assign @barney-s

This solution makes use of the ApplySet annotations for GroupKinds and
Namespaces. When a facade deletion request is received, the ApplySet
data is read from the Plan to know what GKs to find and delete, and
which namespaces to search. The resources are further filtered by the
ApplySet ID they are automatically labeled with.
Stage order information is added to the Plan so the deletion processing
can iterate over them in reverse.
}
numFound += n
}
if numFound > 0 {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logic here implies if a resource in a particular stage is not deleted (for whatever reason), the deletion of rest of the resources are blocked (fits the mental model). This is great if the delete pending resource eventually is removed (O(mins)). Iam thinking if we need to add a timeout (by looking at delete timestamp of the resource) to make progress if a resource is stuck in deletion. We can proceed to delete other resources after a timeout. We dont have to delete the plan/inputcr.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can do it in another PR ?

@barney-s
Copy link
Collaborator

/lgtm
/approve

@google-oss-prow google-oss-prow bot added the lgtm label Aug 29, 2024
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: barney-s

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@google-oss-prow google-oss-prow bot merged commit 0784071 into GoogleCloudPlatform:master Aug 29, 2024
10 of 11 checks passed
@yuwenma yuwenma added this to the 1.122 milestone Oct 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants