-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Fix pre command flags #13995
Fix pre command flags #13995
Conversation
/ok-to-test |
kvm2 driver with docker runtime
Times for minikube start: 55.9s 50.8s 50.3s 50.4s 51.4s Times for minikube ingress: 30.8s 38.6s 30.6s 30.0s 30.1s docker driver with docker runtime
Times for minikube ingress: 24.4s 22.9s 23.9s 24.0s 22.9s Times for minikube start: 28.9s 24.9s 25.3s 24.5s 24.7s docker driver with containerd runtime
Times for minikube ingress: 17.9s 17.9s 17.9s 17.9s 27.9s Times for minikube (PR 13995) start: 43.7s 39.3s 43.8s 39.4s 42.9s |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sharifelgamal, 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:
Approvers can indicate their approval by writing |
Fixes #13994
Problem:
Having a flag before the command (ex.
minikube --alsologtostderr start
) would break instances that were coded to look for the command usingos.Args[1]
. This would result in issues such as not creating alastStart.txt
and not properly detecting if a purge delete was occurring.Solution:
Replaced instances of
os.Args[1]
withpflag.Arg(0)
.pflag.Arg
returns all args fromos.Args[1:]
that aren't flags, resulting in only true args (ex.minikube --alsologtostderr start --driver docker
->pflag.Args() = ["start"]
).pflag.Arg()
will also return an empty string if trying to access a non-existing arg index.