Fix bash completion for kubectl symlinked to minikube by not adding --cluster flag for the kubectl __complete subcommand
#15850
Conversation
|
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: ben-krieger The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Welcome @ben-krieger! |
k8s-ci-robot
added
cncf-cla: no
|
Hi @ben-krieger. 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. |
k8s-ci-robot
added
the
size/XS
|
Can one of the admins verify this patch? |
k8s-ci-robot
added
cncf-cla: yes
cmd/minikube/cmd/kubectl.go
Outdated
| @@ -96,7 +96,7 @@ host. Please be aware that when using --ssh all paths will apply to the remote m | |||
| os.Exit(1) | |||
| } | |||
|
|
|||
| if len(args) > 1 && args[0] != "--help" { | |||
| if len(args) > 1 && args[0] != "--help" && args[0] != "__complete" { | |||
There was a problem hiding this comment.
Thanks for fixing this issue, @ben-krieger
I am curious,
can we guarantee the __complete would always be args[0] ? if not maybe we can loop over the args and flag it as skip dash cluster... ?
There was a problem hiding this comment.
So put another way, "Are there any valid flags for kubectl when using the __complete subcommand?" That's a good question. I assumed not, but I'll check the docs/code. Then there's the question of whether we think that any such flags may get added in the future...
There was a problem hiding this comment.
The global flags are namespace, context, cluster, and user (https://github.com/kubernetes/kubectl/blob/master/pkg/cmd/cmd.go#L499-L520). None of those make sense as flags to __complete.
But more importantly, __complete is a plugin (I guess?) and for the plugin handler code https://github.com/kubernetes/kubectl/blob/31af32762da0400245ada0c342327da4b78eb182/pkg/cmd/cmd.go#L227-L239 it guarantees that there are no flags before the plugin name.
I'm not familiar enough with the plugin architecture, but this seems like a strong guarantee that __complete will always be args[0].
There was a problem hiding this comment.
ok thats fair enough for me ! and thanks for this contribution ! I look forward to see more contributions from you
|
/ok-to-test |
k8s-ci-robot
added
ok-to-test
|
kvm2 driver with docker runtime Times for minikube start: 53.3s 56.0s 54.6s 56.4s 50.2s Times for minikube ingress: 27.7s 25.7s 28.2s 27.7s 27.7s docker driver with docker runtime Times for minikube ingress: 50.1s 21.1s 21.6s 21.6s 82.1s Times for minikube start: 26.5s 26.6s 28.2s 26.9s 26.9s docker driver with containerd runtime Times for minikube start: 21.6s 22.2s 21.6s 21.5s 22.7s Times for minikube (PR 15850) ingress: 31.6s 33.1s 34.1s 32.6s 32.6s |
This comment was marked as outdated.
This comment was marked as outdated.
|
/retest-this-please |
This comment was marked as outdated.
This comment was marked as outdated.
|
kvm2 driver with docker runtime Times for minikube (PR 15850) start: 51.5s 52.8s 56.5s 50.7s 55.5s Times for minikube (PR 15850) ingress: 24.7s 28.2s 28.8s 25.3s 24.7s docker driver with docker runtime Times for minikube start: 25.8s 25.4s 27.5s 25.7s 26.4s Times for minikube ingress: 81.6s 22.1s 21.1s 21.1s 22.1s docker driver with containerd runtime Times for minikube start: 22.1s 22.0s 22.6s 21.6s 21.7s Times for minikube ingress: 31.6s 20.6s 32.6s 32.6s 30.6s |
ben-krieger
force-pushed
the
fix-minikube-kubectl-bash-completion
branch
from
f580d35
to
6be3ba1
Compare
|
These are the flake rates of all failed tests.
To see the flake rates of all tests by environment, click here. |
|
These are the flake rates of all failed tests.
Too many tests failed - See test logs for more details. To see the flake rates of all tests by environment, click here. |
|
kvm2 driver with docker runtime Times for minikube ingress: 27.8s 25.3s 25.7s 25.2s 29.2s Times for minikube start: 58.3s 53.2s 52.8s 59.3s 53.5s docker driver with docker runtime Times for minikube ingress: 22.1s 21.1s 22.1s 22.1s 21.1s Times for minikube start: 26.8s 29.1s 26.3s 27.6s 27.9s docker driver with containerd runtime Times for minikube (PR 15850) start: 22.6s 22.4s 23.4s 23.0s 22.3s Times for minikube ingress: 32.6s 32.6s 79.6s 20.6s 19.6s |
|
These are the flake rates of all failed tests.
To see the flake rates of all tests by environment, click here. |
…--cluster` flag for the `kubectl __complete` subcommand Signed-off-by: Ben Krieger <ben.krieger@intel.com>
ben-krieger
force-pushed
the
fix-minikube-kubectl-bash-completion
branch
from
6be3ba1
to
313cabe
Compare
|
kvm2 driver with docker runtime Times for minikube start: 54.1s 50.5s 57.0s 52.8s 55.3s Times for minikube ingress: 25.7s 25.7s 25.2s 25.7s 23.7s docker driver with docker runtime Times for minikube ingress: 21.6s 22.1s 21.6s 21.1s 21.6s Times for minikube start: 26.7s 26.8s 27.3s 27.7s 27.1s docker driver with containerd runtime Times for minikube start: 22.7s 23.1s 22.2s 22.2s 22.8s Times for minikube ingress: 31.6s 19.6s 79.6s 79.6s 31.6s |
|
These are the flake rates of all failed tests.
To see the flake rates of all tests by environment, click here. |
Fixes #14959
Before:
$ go run ./cmd/minikube kubectl __complete ge Error: flags cannot be placed before plugin name: --cluster exit status 1After:
Signed-off-by: Ben Krieger ben.krieger@intel.com