-
Notifications
You must be signed in to change notification settings - Fork 94
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
🌱 Remove unnecessary code for removing managed cluster resources #330
Conversation
cluster resources Signed-off-by: xuezhaojun <zxue@redhat.com>
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: xuezhaojun The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #330 +/- ##
==========================================
- Coverage 61.75% 61.64% -0.12%
==========================================
Files 132 133 +1
Lines 13992 14070 +78
==========================================
+ Hits 8641 8673 +32
- Misses 4585 4628 +43
- Partials 766 769 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
/assign @qiujian16 |
hrm I do not thin we remove ns right now... @skeeey ? |
we will keep the ns when a managed cluster is denied, only remove https://github.com/open-cluster-management-io/ocm/blob/main/pkg/registration/hub/managedcluster/controller.go#L31 |
I see a related TODO in the code:
Is this means we tend to add namespace back to staicFiles? |
we may, but it's difficult to handle the namespace, because there are too many cases we need consider (like the acm import controller does) |
Summary
When hubAcceptsClient toggle between
true
andfalse
, we want to keep the resources.The resources we want to keep are:
If the namespace of the spoke cluster removed, then all manifestwork applied in that namespace will also gone. We want to avoid that.
Related issue(s)
Fixes #