-
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
Invoke kubectl if minikube binary is named 'kubectl' #8872
Conversation
Hi @kadern0. 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. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: kadern0 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 |
Can one of the admins verify this patch? |
Travis tests have failedHey @kadern0, 1st Buildmake test
TravisBuddy Request Identifier: f2ca6770-d13e-11ea-b8d2-e37d5f97d09d |
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.
I feel like we could add somewhere in the website about this undocumented feature..
How about adding an item to f.a.q page with a question and then answer .like how to use minikube's kubectl ...
@medyagh I'd say adding an entry to the f.a.q pages and also documenting it here? https://github.com/kubernetes/minikube/blob/master/site/content/en/docs/handbook/kubectl.md What do you think? |
/ok-to-test |
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.
do you mind adding an integration test for this? we already have a kubectl integraiton test
we can add something like this to it (it could be a Linux Only Test if it is hard to implement for windows)
dir, err := ioutil.WhateverMakeTempDirectory()
dest := , filepath.Join(dir, "kubectl")
copy.Copy("minikube", dest)
exec("PATH=%s", dir, "kubectl get po -A")
kvm2 Driver Times for Minikube (PR 8872): [62.85187793299999 61.867400352 63.215824725000004] Averages Time Per Log
docker Driver Times for Minikube (PR 8872): [26.884199201 27.341777836000002 25.818418553] Averages Time Per Log
|
Instead of copying the binary, I've created a hard link. |
kvm2 Driver Times for Minikube (PR 8872): [62.526084555 65.221789642 64.34176651199999] Averages Time Per Log
docker Driver Times for Minikube (PR 8872): [27.673202739000004 26.427186228999997 26.080740510000002] Averages Time Per Log
|
Fixes issue kubernetes#8857 Signed-off-by: Pablo Caderno <[email protected]>
Signed-off-by: Pablo Caderno <[email protected]>
Travis tests have failedHey @kadern0, 1st Buildmake test
TravisBuddy Request Identifier: caaaa5e0-e078-11ea-94f1-792e7c9a1c39 |
Signed-off-by: Pablo Caderno <[email protected]>
Signed-off-by: Pablo Caderno <[email protected]>
Travis tests have failedHey @kadern0, 1st Buildmake test
TravisBuddy Request Identifier: b96d8370-e07c-11ea-94f1-792e7c9a1c39 |
Travis tests have failedHey @kadern0, 1st Buildmake test
TravisBuddy Request Identifier: 60f76020-e07d-11ea-94f1-792e7c9a1c39 |
Signed-off-by: Pablo Caderno <[email protected]>
kvm2 Driver Times for Minikube (PR 8872): [63.117536169000005 62.041956058 63.327481860000006] Averages Time Per Log
docker Driver Times for Minikube (PR 8872): [28.118795371 26.725539343 28.292362939] Averages Time Per Log
|
Signed-off-by: Pablo Caderno <[email protected]>
kvm2 Driver Times for Minikube (PR 8872): [61.958398914 61.311528751000004 60.71134397499999] Averages Time Per Log
docker Driver Times for Minikube (PR 8872): [40.594636910999995 27.827522944 26.274620815] Averages Time Per Log
|
@medyagh it seems to be working fine on mac too:
|
kvm2 Driver Times for Minikube (PR 8872): 64.2s 62.6s 62.2s Averages Time Per Log
docker Driver Times for Minikube (PR 8872): 32.9s 28.1s 28.2s Averages Time Per Log
|
Fixes issue #8857
Signed-off-by: Pablo Caderno [email protected]