Skip to content

add resourceCleanup featureGate introduction #473

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

Merged

Conversation

zhiweiyin318
Copy link
Member

No description provided.

@openshift-ci openshift-ci bot requested review from dhaiducek and qiujian16 May 8, 2025 15:20
Copy link

netlify bot commented May 8, 2025

Deploy Preview for open-cluster-management ready!

Name Link
🔨 Latest commit 2194bcc
🔍 Latest deploy log https://app.netlify.com/projects/open-cluster-management/deploys/683713c43cea3f0008afea88
😎 Deploy Preview https://deploy-preview-473--open-cluster-management.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@zhiweiyin318 zhiweiyin318 force-pushed the add-gc branch 4 times, most recently from be5a047 to add0d9f Compare May 8, 2025 15:25
@@ -316,3 +316,33 @@ Check the klusterlet is removed from the managed cluster.
kubectl get klusterlet --context ${CTX_MANAGED_CLUSTER}
error: the server doesn't have a resource type "klusterlet
```

### ResourceCleanup FeatureGate

Copy link
Member

Choose a reason for hiding this comment

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

add a short description that

when the user deletes the managedcluster resource, the related resource under the cluster namespace should also be removed, including managedclusteraddon, manifestworks and the rolebindings for the klusterlet agent. There is a certain sequence for cleanup to ensure no resource will be blocked in the deleting state.

Copy link
Member Author

Choose a reason for hiding this comment

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

updated

**Deletion Sequence:**
1. managedClusterAddons are deleted first.
2. manifestWorks are removed subsequently after there is no managedClusterAddon.
3. Custom ordering can be defined using the `open-cluster-management.io/cleanup-priority` annotation:
Copy link
Member

Choose a reason for hiding this comment

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

We need to explain why this order is needed, some example use case would be helpful for user the understand.

Copy link
Member Author

Choose a reason for hiding this comment

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

added


1. managedClusterAddons are deleted first.
2. manifestWorks are removed subsequently after all managedClusterAddons are deleted.
3. Custom deletion ordering can be defined using the `open-cluster-management.io/cleanup-priority` annotation:
Copy link
Member

Choose a reason for hiding this comment

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

also need to clarify which resource the annotation should be added.

Copy link
Member Author

Choose a reason for hiding this comment

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

updated

@qiujian16
Copy link
Member

/approve
/lgtm

Copy link
Contributor

openshift-ci bot commented May 28, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: qiujian16, zhiweiyin318

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

@openshift-merge-bot openshift-merge-bot bot merged commit ddf9520 into open-cluster-management-io:main May 28, 2025
7 checks passed
@zhiweiyin318 zhiweiyin318 deleted the add-gc branch May 28, 2025 14:31
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.

2 participants