-
Notifications
You must be signed in to change notification settings - Fork 38.7k
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
feat: make user-defined plugins discoverable with e.g. kubectl help #116752
feat: make user-defined plugins discoverable with e.g. kubectl help #116752
Conversation
Welcome @xvzf! |
Hi @xvzf. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
/sig cli |
/ok-to-test |
Hi @xvzf Seems like this PR needs a rebase . can you do the same and check for further errors? |
Yes, I'll do it this week. Sorry for the long delay, have been busy with work stuff |
16b60c9
to
c47cdd0
Compare
Changelog suggestion: Changed `kubectl help` to display basic details for subcommands from plugins |
c47cdd0
to
babaff8
Compare
/retest |
/test pull-kubernetes-node-e2e-containerd |
/test pull-kubernetes-node-e2e-containerd |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great - thank you for your contribution! I left you one small comment, do you think you can fix that? Once you do, please ping me on slack to get it merged promptly.
streams := genericiooptions.IOStreams{ | ||
// registerPluginCommand allows adding Cobra command to the command tree or extracting them for usage in | ||
// e.g. the help function or for registering the completion function | ||
func registerPluginCommands(kubectl *cobra.Command, depth int, register bool) (cmds []*cobra.Command) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: rather than having 2 arguments depth
and register
, how about we just add list bool
and if that is set it means we only care about the highest level (ie. yours depth = 1
and register = false
), when false, we register like in the old days. I don't think we need that much control over the process.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just did! Lmk if there's anything else please
/assign |
Signed-off-by: Matthias Riegler <matthias.riegler@ankorstore.com>
Signed-off-by: Matthias Riegler <matthias.riegler@ankorstore.com>
babaff8
to
e14961f
Compare
Signed-off-by: Matthias Riegler <matthias.riegler@ankorstore.com>
e14961f
to
e1cd7a7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/approve
/label tide/merge-method-squash |
/remove-lgtm |
Signed-off-by: Matthias Riegler <matthias.riegler@ankorstore.com>
7200be5
to
f941d1c
Compare
/lgtm |
@xvzf: you cannot LGTM your own PR. In response to this:
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. |
/test pull-kubernetes-integration |
/test pull-kubernetes-e2e-gce |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
LGTM label has been added. Git tree hash: c7ab4eea20d9c8c0a1540a737a6c5a0b44bced30
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: soltysh, xvzf 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 |
/retest-required |
What type of PR is this?
/kind feature
What this PR does / why we need it:
Kubectl plugins are a powerful way for extending kubectl, but actually discovering available plugins is hard.
Right now, the only way is to use krew list
, check for
kubectl-<plugin-name>
executables or use tab-complete.This PR maps the same behavior of tab-complete to the output of
kubectl
andkubectl help
, including the new command groupUser-Installed Plugin Commands:
.An example output (with `krew` and `convert` as plugins):
Which issue(s) this PR fixes:
Fixes #116751
Special notes for your reviewer:
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:
I did not create a KEP for this change as it doesn't introduce breaking behavior, just a visual change. I'm planning on extending this to improve discovery of plugins by e.g. allowing user-defined descriptions as a KEP.