Closed jwcesign closed 2 weeks ago
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 51.82%. Comparing base (
6534ffb
) to head (a542ad5
).
:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
/cc @XiShanYongYe-Chang @chaosi-zju @chaunceyjiang
/cancel lgtm
It looks good to initialize the label by webhook, however, I think there is no rush to do it. As the label name might be changed.
I feel the label name might not be prefixed with a resource kind
, for example, the label name on PropagationPolicy
now is:
propagationpolicy.karmada.io/permanent-id: xxxx
But it should be something like:
resource.karmada.io/permanent-id: xxxx
I feel it isn't right for each PP
/CPP
/RB
/CRB
/Work
to have a dedicated label.
/hold for a while.
@RainbowMango, any plan to move forward? Is it currently on hold for more discussions? We are waiting for a solution for issue.
Current progress on the issue: https://github.com/karmada-io/karmada/issues/4683#issuecomment-1996604867
It looks good to initialize the label by webhook, however, I think there is no rush to do it. As the label name might be changed.
I feel the label name might not be prefixed with a
resource kind
, for example, the label name onPropagationPolicy
now is:propagationpolicy.karmada.io/permanent-id: xxxx
But it should be something like:
resource.karmada.io/permanent-id: xxxx
I feel it isn't right for each
PP
/CPP
/RB
/CRB
/Work
to have a dedicated label.
If we want to change the label name, we can remove the previously used namespace/name related labels at least in the next version (v1.11), and solve the problem described in #4000. Is this progress as expected?
If we want to change the label name, we can remove the previously used namespace/name related labels at least in the next version (v1.11), and solve the problem described in https://github.com/karmada-io/karmada/issues/4000. Is this progress as expected?
Yeah, thanks for the reminder. Let's move forward and if we decide to rename the label in the future, we can run another deprecation cycle.
@jwcesign Can you help to rebase this PR?
And, shall we update the MutatingWebhookConfiguration along with this?
This PR is missing the webhook-configuration file.
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: RainbowMango
The full list of commands accepted by this bot can be found here.
The pull request process is described here
@XiShanYongYe-Chang The test is failing, please take a look.
@XiShanYongYe-Chang The test is failing, please take a look.
I'm working on it.
The current PR can be pushed after the modification of karmadactl init
in #4789 is complete.
/cc @RainbowMango It is ready to merge now.
Uh, please propose a release notes... And this must be something that should be put in upgrading docs, do we have a task for it?
Uh, please propose a release notes...
How about add permanentID label for PP/CPP/RB/CRB/Work resources in karmada-webhook
?
And this must be something that should be put in upgrading docs, do we have a task for it?
I added a sub-task to the task item of #4711 to describe this.
/cc @RainbowMango
Kindly ping @RainbowMango @chaunceyjiang
/assign @chaunceyjiang @RainbowMango
/hold cancel
What type of PR is this? /kind feature
What this PR does / why we need it: Please referring to: https://github.com/karmada-io/karmada/issues/4000 and https://github.com/karmada-io/karmada/pull/4199#discussion_r1393842038
Which issue(s) this PR fixes: Part of https://github.com/karmada-io/karmada/issues/4000
Special notes for your reviewer: The corresponding yaml configuration will be done in another PR.
Does this PR introduce a user-facing change?: