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

Enhance the capabilities of the karmadactl get command #5254

Merged
merged 2 commits into from
Aug 19, 2024

Conversation

hulizhe
Copy link
Contributor

@hulizhe hulizhe commented Jul 26, 2024

What type of PR is this?
/kind feature

What this PR does / why we need it:

  • Fix the issue of incomplete output when the resource does not found

before:

$ karmadactl get secret 
No resources found in  namespace.

after:

$ karmadactl get secret 
No resources found in default namespace.
  • Enhance karmadactl to provide a complete resource view of the Karmada control plane and member clusters.

Given that the Karmada control plane has a deployment named nginx, which is propagated to member1 and member2.

// display deployment in Karmada control plane
$ _output/bin/linux/amd64/karmadactl get deployment             
NAME    CLUSTER   READY   UP-TO-DATE   AVAILABLE   AGE   ADOPTION
nginx   Karmada   0/2     4            0           17s   -

// display deployment in Karmada control plane and all member clusters
$ _output/bin/linux/amd64/karmadactl get deployment --operation-scope all
NAME    CLUSTER   READY   UP-TO-DATE   AVAILABLE   AGE   ADOPTION
nginx   Karmada   0/2     4            0           26s   -
nginx   member1   0/2     2            0           26s   Y
nginx   member2   0/2     2            0           26s   Y

// display deployment in Karmada control plane and member1
$ _output/bin/linux/amd64/karmadactl get deployment --operation-scope all -Cmember1     
NAME    CLUSTER   READY   UP-TO-DATE   AVAILABLE   AGE   ADOPTION
nginx   Karmada   0/2     4            0           37s   -
nginx   member1   0/2     2            0           37s   Y

// only display deployment in member clustes
$ _output/bin/linux/amd64/karmadactl get deployment --operation-scope members
NAME    CLUSTER   READY   UP-TO-DATE   AVAILABLE   AGE   ADOPTION
nginx   member1   2/2     2            2           65s   Y
nginx   member2   0/2     2            0           65s   Y

// only display deployment in member1
$ _output/bin/linux/amd64/karmadactl get deployment --operation-scope members -Cmember1
NAME    CLUSTER   READY   UP-TO-DATE   AVAILABLE   AGE   ADOPTION
nginx   member1   2/2     2            2           65s   Y

Which issue(s) this PR fixes:
Parts of #5249

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

`karmadactl`: The `get` command can show Karmada resources now.

@karmada-bot karmada-bot added the kind/feature Categorizes issue or PR as related to a new feature. label Jul 26, 2024
@karmada-bot karmada-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jul 26, 2024
@codecov-commenter
Copy link

codecov-commenter commented Jul 26, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 0% with 101 lines in your changes missing coverage. Please review.

Project coverage is 29.35%. Comparing base (bcefb22) to head (37a6dc9).
Report is 2 commits behind head on master.

Files Patch % Lines
pkg/karmadactl/get/get.go 0.00% 72 Missing ⚠️
pkg/karmadactl/options/global.go 0.00% 29 Missing ⚠️

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

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5254      +/-   ##
==========================================
- Coverage   29.41%   29.35%   -0.06%     
==========================================
  Files         632      632              
  Lines       43835    43901      +66     
==========================================
- Hits        12892    12889       -3     
- Misses      30003    30071      +68     
- Partials      940      941       +1     
Flag Coverage Δ
unittests 29.35% <0.00%> (-0.06%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@hulizhe hulizhe force-pushed the karmadactlget branch 2 times, most recently from 2c2fae2 to fedd361 Compare August 2, 2024 09:03
@zhzhuang-zju
Copy link
Contributor

@zhzhuang-zju
Copy link
Contributor

/retest

@zhzhuang-zju
Copy link
Contributor

[FAIL] [resource-status collection] resource status collection testing PodDisruptionBudget collection testing [It] pdb status collection testing

The probability of this e2e tests failing has been quite high recently.

/retest

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.

/assign

@zhzhuang-zju
Copy link
Contributor

The probability of this e2e tests failing has been quite high recently.

tracked by #5382

/retest

pkg/karmadactl/get/get.go Outdated Show resolved Hide resolved
pkg/karmadactl/get/get.go Outdated Show resolved Hide resolved
pkg/karmadactl/get/get.go Outdated Show resolved Hide resolved
pkg/karmadactl/get/get.go Outdated Show resolved Hide resolved
pkg/karmadactl/get/get.go Outdated Show resolved Hide resolved
pkg/karmadactl/get/get.go Outdated Show resolved Hide resolved
pkg/karmadactl/get/get.go Show resolved Hide resolved
pkg/karmadactl/get/get.go Outdated Show resolved Hide resolved
pkg/karmadactl/options/global.go Outdated Show resolved Hide resolved
pkg/karmadactl/options/global.go Outdated Show resolved Hide resolved
@hulizhe hulizhe force-pushed the karmadactlget branch 2 times, most recently from 2f7dfb5 to 054c22b Compare August 17, 2024 02:57
pkg/karmadactl/get/get.go Outdated Show resolved Hide resolved
flags.StringVarP(options.DefaultConfigFlags.Namespace, "namespace", "n", *options.DefaultConfigFlags.Namespace, "If present, the namespace scope for this CLI request")
flags.StringVarP(&o.LabelSelector, "labels", "l", "", "-l=label or -l label")
flags.StringSliceVarP(&o.Clusters, "clusters", "C", []string{}, "-C=member1,member2")
flags.StringSliceVarP(&o.Clusters, "clusters", "C", []string{}, "Used to specify target member clusters and only takes effect when the command's operation scope includes member clusters, for example: --operation-scope=all -C=member1,member2")
Copy link
Member

Choose a reason for hiding this comment

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

How about this one?

Signed-off-by: hulizhe <pompeii.hu@gmail.com>
Signed-off-by: hulizhe <pompeii.hu@gmail.com>
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.

/lgtm
/approve

@karmada-bot karmada-bot added the lgtm Indicates that a PR is ready to be merged. label Aug 19, 2024
@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 19, 2024
@zhzhuang-zju
Copy link
Contributor

/retest

@karmada-bot karmada-bot merged commit f3ae66b into karmada-io:master Aug 19, 2024
12 checks passed
@RainbowMango RainbowMango added this to the v1.11 milestone Aug 31, 2024
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/feature Categorizes issue or PR as related to a new feature. 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.

5 participants