search

karmada-io / karmada

Open, Multi-Cloud, Multi-Cluster Kubernetes Orchestration
https://karmada.io
Apache License 2.0
4.12k stars 807 forks source link

Deprecate name/namespace labels of pp/cpp #4743

Closed whitewindmills closed 1 week ago

whitewindmills commented 1 month ago

What type of PR is this? /kind feature /kind cleanup

What this PR does / why we need it:

propagationpolicy.karmada.io/namespace, propagationpolicy.karmada.io/name -> propagationpolicy.karmada.io/permanent-id
clusterpropagationpolicy.karmada.io/name -> clusterpropagationpolicy.karmada.io/permanent-id

Which issue(s) this PR fixes: part of issue

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

karmada-controller-manager: deprecate name/namespace labels of pp/cpp.
codecov-commenter commented 1 month ago

Codecov Report

Attention: Patch coverage is 0.70423% with 141 lines in your changes are missing coverage. Please review.

Project coverage is 53.11%. Comparing base (1754e73) to head (59b8835).

Files Patch % Lines
pkg/detector/detector.go 0.00% 69 Missing :warning:
pkg/detector/policy.go 0.00% 46 Missing :warning:
pkg/detector/preemption.go 0.00% 16 Missing :warning:
pkg/util/annotation.go 0.00% 7 Missing :warning:
pkg/scheduler/event_handler.go 0.00% 2 Missing :warning:
pkg/util/worker.go 0.00% 1 Missing :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #4743 +/- ## ========================================== + Coverage 53.07% 53.11% +0.04% ========================================== Files 250 250 Lines 20371 20346 -25 ========================================== - Hits 10811 10806 -5 + Misses 8843 8821 -22 - Partials 717 719 +2 ``` | [Flag](https://app.codecov.io/gh/karmada-io/karmada/pull/4743/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=karmada-io) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/karmada-io/karmada/pull/4743/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=karmada-io) | `53.11% <0.70%> (+0.04%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=karmada-io#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

XiShanYongYe-Chang commented 1 month ago

Wow, you submitted a big PR. I found that my newly submitted PR #4748 is part of yours (pp/cpp).

whitewindmills commented 1 month ago

@XiShanYongYe-Chang Maybe it is not easy to split this task. It is easy for conflicts to arise between the codes we submitted. I found that there are some things in your PR that I missed, so how about we co-author this PR?

XiShanYongYe-Chang commented 1 month ago

@XiShanYongYe-Chang Maybe it is not easy to split this task. It is easy for conflicts to arise between the codes we submitted. I found that there are some things in your PR that I missed, so how about we co-author this PR?

Sure :). What is the specific cooperation mode?

whitewindmills commented 1 month ago

What is the specific cooperation mode?

If you still have unfinished code submission, you can continue to do this work. After the submission is completed, I will cherry-pick your PR and add Co-authored-by: changzhen <changzhen5@huawei.com> to this PR. Finally, we will review this PR together.

XiShanYongYe-Chang commented 1 month ago

The code has been submitted and is now waiting for the CI to finish. If the CI fails, the CI may need to be fixed.

You can directly cherry-pick and then we modify the CI together.

whitewindmills commented 1 month ago

@XiShanYongYe-Chang okay, thanks for your remainder.

XiShanYongYe-Chang commented 1 month ago

A priority preemption ci error has occurred. Please help take a look: https://github.com/karmada-io/karmada/actions/runs/8419825470/job/23053518691?pr=4748

/cc @whitewindmills

karmada-bot commented 1 month ago

@XiShanYongYe-Chang: GitHub didn't allow me to request PR reviews from the following users: whitewindmills.

Note that only karmada-io members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to [this](https://github.com/karmada-io/karmada/pull/4743#issuecomment-2017999700): >A priority preemption ci error has occurred. Please help take a look: https://github.com/karmada-io/karmada/actions/runs/8419825470/job/23053518691?pr=4748 > >/cc @whitewindmills Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
whitewindmills commented 1 month ago

@XiShanYongYe-Chang Could you help check if there are any omissions?

XiShanYongYe-Chang commented 1 month ago

@XiShanYongYe-Chang Could you help check if there are any omissions?

of course /assign

whitewindmills commented 1 month ago

Wait for merging PR. /hold

RainbowMango commented 1 month ago

Wait for merging PR. /hold

/unhold I don't know why this should wait #4751, but it has been merged.

RainbowMango commented 1 month ago

Oh, I get it. You might need to rebase to make the CI happy.

whitewindmills commented 1 month ago

I don't know why this should wait https://github.com/karmada-io/karmada/pull/4751

thanks, because the preemption E2E case will fail before merging https://github.com/karmada-io/karmada/pull/4751. I'll rebase it.

whitewindmills commented 1 month ago

ready to review. /cc @XiShanYongYe-Chang @RainbowMango

XiShanYongYe-Chang commented 1 month ago

Hi @whitewindmills Some users reported that they used the namespace/name labels of PP/CPP in secondary development. Can we not remove these labels in the current PR? These labels are still reserved for resource templates and binding, but they should no longer used in the processing logic. Do you think that's okay?

whitewindmills commented 1 month ago

Can we not remove these labels in the current PR? These labels are still reserved for resource templates and binding, but they should no longer used in the processing logic.

okay, this is a reasonable request.

RainbowMango commented 1 month ago

Do we need another PR?

XiShanYongYe-Chang commented 1 month ago

Do we need another PR?

It may be needed. We can handle the deletion of labels separately.

whitewindmills commented 1 month ago

We can handle the deletion of labels separately.

I'll separate this PR.

whitewindmills commented 3 weeks ago

@XiShanYongYe-Chang @RainbowMango PTAL the deletion of labels will be handled by another PR.

whitewindmills commented 3 weeks ago

@XiShanYongYe-Chang @RainbowMango In order to speed up the review, this PR is only responsible for the label deprecation of PP/CPP.

XiShanYongYe-Chang commented 3 weeks ago

Thanks @whitewindmills, I will take a review ASAP.

RainbowMango commented 3 weeks ago

Thanks @whitewindmills I'll look at it ASAP. I'll try my best to do it by this week.

RainbowMango commented 3 weeks ago

@whitewindmills Can you help to rebase this PR? I need to check something made in #4811.

whitewindmills commented 3 weeks ago

Can you help to rebase this PR?

okay

whitewindmills commented 2 weeks ago

Wait for https://github.com/karmada-io/karmada/pull/4836. /hold

RainbowMango commented 2 weeks ago

/hold cancel Please help to rebase this PR as #4836 has been merged.

whitewindmills commented 2 weeks ago

@RainbowMango pls take a look again.

XiShanYongYe-Chang commented 1 week ago

How about going on to move forward with this? /cc @RainbowMango @chaunceyjiang

karmada-bot commented 1 week ago

[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

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/karmada-io/karmada/blob/master/OWNERS)~~ [RainbowMango] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment