Skip to content
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

[karmadactl get] judge with label on the resources determine whether resource is adopted by Karmada #3910

Conversation

XiShanYongYe-Chang
Copy link
Member

@XiShanYongYe-Chang XiShanYongYe-Chang commented Aug 7, 2023

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

We have added labels to the resources in the member cluster to indicate whether the current resource is propagated by Karmada:

// ManagedByKarmadaLabel is a reserved karmada label to indicate whether resources are managed by karmada controllers.
ManagedByKarmadaLabel = "karmada.io/managed"
// ManagedByKarmadaLabelValue indicates that resources are managed by karmada controllers.
ManagedByKarmadaLabelValue = "true"

Therefore, we can directly process this label in the karmadactl get command, without needing to retrieve RB/CRB resources.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

One reason for this change is that we cannot assume that users using karmadactl by default have the authority to get rb/crb resources.

Does this PR introduce a user-facing change?:

NONE

@karmada-bot karmada-bot added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 7, 2023
@XiShanYongYe-Chang
Copy link
Member Author

/cc @lonelyCZ @QAQ-rookie

@karmada-bot
Copy link
Collaborator

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

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

In response to this:

/cc @lonelyCZ @QAQ-rookie

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/test-infra repository.

@codecov-commenter
Copy link

codecov-commenter commented Aug 7, 2023

Codecov Report

Merging #3910 (7fcd0e4) into master (a6ba598) will not change coverage.
The diff coverage is n/a.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@           Coverage Diff           @@
##           master    #3910   +/-   ##
=======================================
  Coverage   54.83%   54.83%           
=======================================
  Files         229      229           
  Lines       21946    21946           
=======================================
  Hits        12033    12033           
  Misses       9274     9274           
  Partials      639      639           
Flag Coverage Δ
unittests 54.83% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

see 1 file with indirect coverage changes

@lonelyCZ
Copy link
Member

lonelyCZ commented Aug 7, 2023

Looks nice, thanks for your work, I will look it ASAP.

/assign

@RainbowMango
Copy link
Member

One reason for this change is that we cannot assume that users using karmadactl by default have the authority to get rb/crb resources.

+1

Copy link
Member

@lonelyCZ lonelyCZ left a comment

Choose a reason for hiding this comment

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

I just tested it in my env, it worked fine.

LGTM!

pkg/karmadactl/get/get.go Outdated Show resolved Hide resolved
…and to determine whether resource is managed by Karmada

Signed-off-by: changzhen <changzhen5@huawei.com>
@lonelyCZ
Copy link
Member

/lgtm

cc @RainbowMango

@karmada-bot karmada-bot added the lgtm Indicates that a PR is ready to be merged. label Aug 15, 2023
Copy link
Member

@RainbowMango RainbowMango left a comment

Choose a reason for hiding this comment

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

/approve

@karmada-bot
Copy link
Collaborator

[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:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@karmada-bot karmada-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 16, 2023
@karmada-bot karmada-bot merged commit 27eb29c into karmada-io:master Aug 16, 2023
12 checks passed
@XiShanYongYe-Chang XiShanYongYe-Chang deleted the karmadactl-get-without-rb-info branch February 27, 2024 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants