Fix kubectl tab-completion and improve error messages

[oldium]

Try to fix the following issues when kubectl is an alias for minikube kubectl -- and tab-completion for kubectl is enabled:

1. When --cluster name form of command-line option is used, it conflicts with argument parsing, name is interpreted as plugin name. Call to kubectl --cluster test help leads to error Error: flags cannot be placed before plugin name: --cluster. Fix this by using --cluster=name form, which is correctly handled also in call kubectl --cluster=test help.
2. When --cluster=name is added as first argument (as seen in original code), it still leads to error mentioned above when the command is unknown like in the call kubectl --cluster=test unknown. Fix this by inserting --cluster=name after all (sub-)commands and before any flags/options.
3. The original code passed --cluster=name to normal kubectl calls, but omitted it for tab-completion calls. This might lead to wrong tab suggestions about pods, deployments and the like. Fix this by always adding --cluster=name option to the command line.
4. In case of tab-completion, put the --cluster=name immediately after __complete command to prevent any interference with incomplete commands (just in case). This works fine and does not generate any errors like mentioned in point 2.

Fixes #12938

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: oldium / name: Oldřich Jedlička (6d28d2a)

[k8s-ci-robot]

Welcome @oldium!

It looks like this is your first PR to kubernetes/minikube 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/minikube has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[k8s-ci-robot]

Hi @oldium. 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 /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

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.

[minikube-bot]

Can one of the admins verify this patch?

[medyagh]

@oldium thank you for this PR, have you checked if this PR will break other use cases?
we do not have enough unit-test or integration test arround this

[oldium]

I am not sure, really.

I figured out now that the error could be as well fixed when the --cluster cname parameter is replaced by --cluster=cname. In this form also --help works without issues. This should be a more safe fix then.

[oldium]

The actual fix could be:

                if len(args) > 1 && args[0] != "--help" {
                    cluster := []string{"--cluster=" + cname}
                    args = append(cluster, args...)
                }

or

                if len(args) > 0 {
                    cluster := []string{"--cluster=" + cname}
                    args = append(cluster, args...)
                }

or even

                cluster := []string{"--cluster=" + cname}
                args = append(cluster, args...)

Tested all versions (minikube kubectl --help, minikube kubectl -- --help, minikube kubectl __complete app, minikube kubectl __complete logs "") and the output looks reasonable.

For the code completion the --cluster argument is actually necessary to give correct hints.

[oldium]

Please advise which version should I put into the patch 😊

[oldium]

Updated patch to be the simplest version (no if for --help check), tested by installing built minikube-installer.exe, tested the following in PowerShell:

minikube.exe kubectl
minikube.exe kubectl --help
minikube.exe kubectl -- --help
minikube.exe kubectl __complete app
minikube.exe kubectl __complete logs `"`"
minikube.exe kubectl get pods

And the output looks good, plain --help is help from minikube, call with no arguments and with -- --help is help from kubectl, completion works, the pods from logs correspond to the get pods call.

[medyagh]

/ok-to-test

[oldium]

Updated patch to insert the cluster argument before other flags, but after commands to improve error message from kubectl. In case of code completion (__complete arg), insert cluster argument at the beginning.

[oldium]

Updated description and commit-message to contain the full reasoning behind the solution. Rebased to latest master.

If there is anything more to be solved in kubectl app or Cobra library (like mentioned by Ben), it can be done independently on this solution. This solution works without changes to other software components.

[spowelljr]

@oldium Sorry for the delay, I just rebased the PR, is this good to go now?

[minikube-pr-bot]

kvm2 driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 14868) |
+----------------+----------+---------------------+
| minikube start | 52.4s    | 52.5s               |
| enable ingress | 28.7s    | 28.9s               |
+----------------+----------+---------------------+

Times for minikube start: 52.0s 52.7s 52.0s 53.0s 52.2s
Times for minikube (PR 14868) start: 56.7s 50.5s 53.3s 50.8s 51.2s

Times for minikube ingress: 27.8s 27.4s 28.9s 29.3s 30.2s
Times for minikube (PR 14868) ingress: 28.3s 28.3s 27.8s 31.9s 28.4s

docker driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 14868) |
+----------------+----------+---------------------+
| minikube start | 24.8s    | 24.3s               |
| enable ingress | 20.8s    | 21.0s               |
+----------------+----------+---------------------+

Times for minikube start: 25.6s 25.8s 25.2s 25.1s 22.5s
Times for minikube (PR 14868) start: 23.1s 25.6s 24.5s 25.8s 22.6s

Times for minikube (PR 14868) ingress: 20.9s 20.9s 21.9s 19.9s 21.4s
Times for minikube ingress: 20.9s 20.9s 21.4s 20.9s 19.9s

docker driver with containerd runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 14868) |
+----------------+----------+---------------------+
| minikube start | 23.2s    | 21.9s               |
| enable ingress | 31.4s    | 31.2s               |
+----------------+----------+---------------------+

Times for minikube start: 23.5s 23.7s 21.5s 23.7s 23.5s
Times for minikube (PR 14868) start: 20.7s 20.3s 24.6s 21.1s 23.1s

Times for minikube ingress: 31.4s 31.4s 31.4s 31.4s 31.4s
Times for minikube (PR 14868) ingress: 31.4s 31.4s 31.4s 30.4s 31.4s

[oldium]

@oldium Sorry for the delay, I just rebased the PR, is this good to go now?

@spowelljr No problem with the delay, that is normal 😊 Looks good, ready to go 👍

[minikube-pr-bot]

These are the flake rates of all failed tests.

Environment	Failed Tests	Flake Rate (%)
Hyperkit_macOS	TestMinikubeProfile (gopogh)	1.38 (chart)
Docker_Linux_crio_arm64	TestPause/serial/SecondStartNoReconfiguration (gopogh)	27.81 (chart)

To see the flake rates of all tests by environment, click here.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: oldium, spowelljr

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 [spowelljr]

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