karmada
karmada copied to clipboard
karmada-io
Add annotation to the the resource template to prevent removal managed resources
What type of PR is this?
What this PR does / why we need it: Extend the API field orphaningDeletion to prevent removal managed resources
Which issue(s) this PR fixes: Fixes https://github.com/karmada-io/karmada/issues/4709
Special notes for your reviewer:
Does this PR introduce a user-facing change?:
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 53.34%. Comparing base (
d676996) to head (2e44070). Report is 6 commits behind head on master.
:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@ Coverage Diff @@
## master #4788 +/- ##
==========================================
+ Coverage 53.31% 53.34% +0.02%
==========================================
Files 252 253 +1
Lines 20539 20565 +26
==========================================
+ Hits 10951 10971 +20
- Misses 8862 8868 +6
Partials 726 726
| Flag | Coverage Δ | |
|---|---|---|
| unittests | 53.34% <100.00%> (+0.02%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Hi @CharlesQQ @chaunceyjiang Is there any blockage in this PR now? Can we move forward?
Hi @CharlesQQ @chaunceyjiang Is there any blockage in this PR now? Can we move forward?
https://github.com/karmada-io/karmada/pull/4788#discussion_r1582008275 Regarding this plan, it seems that there is no conclusion yet?
Let's invite @RainbowMango and @whitewindmills give some opinion. /cc @RainbowMango @whitewindmills
Regarding this plan, it seems that there is no conclusion yet?
Yes, To sum up, it seems inappropriate to put fields in pp/cpp
@XiShanYongYe-Chang @RainbowMango @CharlesQQ @whitewindmills Do we have a consensus on the current proposal?
We may not reached a unanimous conclusion yet. Work suspension might also can learn from this discussion. They are both processing policies for resource templates and can be decoupled from PP.
There should be three options at this point,
- Solution 1: Define a resource deletion policy in the PP.
- disadvantage:
- Coupled with the PP, the PP becomes larger and larger with the evolution.
- disadvantage:
- Solution 2: Add an annotation to the resource template to describe the deletion policy of the target resource.
- disadvantage:
- Poor API evolution.
- disadvantage:
- Solution 3: A new CRD is defined to describe a deletion policy of one or more resources.
- disadvantage:
- Increased learning costs and CRs
- disadvantage:
You can publish it, or we can have a meeting to discuss and decide.
@XiShanYongYe-Chang @RainbowMango @CharlesQQ @whitewindmills Do we have a consensus on the current proposal?
Can we set up a time to discuss this plan? When would be a good time for you, or maybe next Tuesday at the community meeting?
@buji-code Yeah, after discussion at the community meeting, we decided to try out solution 2 and 3. This work will start in the next release, early next month.
Hi guys @CharlesQQ @whitewindmills @chaunceyjiang @RainbowMango @buji-code
Can we proceed with solution 2: Add an annotation to the resource template to describe the deletion policy of the target resource first, and then consider designing and adding a CRD to describe the deletion policy as the scenario develops? (Of course, the use of annotations is not excluded as the ideal end use.)
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: Once this PR has been reviewed and has the lgtm label, please ask for approval from rainbowmango. For more information see the Kubernetes Code Review Process.
The full list of commands accepted by this bot can be found here.
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
happy to contribute if I can push this forward (design or implementation)
the latest proposal is here: https://github.com/karmada-io/karmada/pull/5101
now we stuck at: here are four optional resolutions, and we haven't decided which is best.
could you help sharing opinions about which is the best practice?
👋🏼 happy to contribute if I can push this forward (design or implementation)
Sure thing! Thank you Amir in advance. I'll look at the proposal and see how I can help with it.
Hi everyone, let's use umbrella issue https://github.com/karmada-io/karmada/issues/5577 to track this requirement. /close
@XiShanYongYe-Chang: Closed this PR.
In response to this:
Hi everyone, let's use umbrella issue https://github.com/karmada-io/karmada/issues/5577 to track this requirement. /close
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.