-
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
docker-env: improve detecting powershell if SSHed from linux #10722
Conversation
Hi @hetong07. 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. |
Can one of the admins verify this patch? |
/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.
thanks @hetong07 please see comment, lets see if this passes the windows tests
kvm2 Driver Times for Minikube (PR 10722): 71.9s 68.5s 71.8s Averages Time Per Log
docker Driver Times for Minikube (PR 10722): 26.9s 34.1s 32.2s Averages Time Per Log
|
This change seems to be better happen in https://github.com/machine-drivers/machine, so create a PR there machine-drivers/machine#33 |
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.
now that this is fixed in libmachine, please update the go.mod replace section to use latest libmachine fork
5973cfc
to
cb626a1
Compare
@medyagh Updated via |
kvm2 Driver Times for Minikube (PR 10722): 69.9s 69.9s 69.1s Averages Time Per Log
docker Driver Times for Minikube (PR 10722): 31.1s 36.4s 31.1s Averages Time Per Log
|
/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.
The dockerEnv test still fails
https://storage.googleapis.com/minikube-builds/logs/10722/cb626a1/Docker_Windows.html#fail_TestFunctional%2fparallel%2fDockerEnv
-- /stdout --
** stderr **
export : The term 'export' is not recognized as the name of a cmdlet, function, script file, or operable program.
Check the spelling of the name, or if a path was included, verify that the path is correct and try again.
At line:1 char:1
+ export DOCKER_TLS_VERIFY="1"
+ ~~~~~~
+ CategoryInfo : ObjectNotFound: (export:String) [], CommandNotFoundException
+ FullyQualifiedErrorId : CommandNotFoundException
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: hetong07 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 |
cb626a1
to
c7a4943
Compare
@medyagh This is because the PR to machine-drivers has not be correctly reflected. Solving this by
|
kvm2 Driver Times for Minikube (PR 10722): 62.2s 61.2s 63.0s Averages Time Per Log
docker Driver Times for Minikube (PR 10722): 27.8s 27.0s 26.7s Averages Time Per Log
|
@hetong07 this time we got passed initial error
|
kvm2 Driver Times for Minikube (PR 10722): 63.4s 64.9s 61.0s Averages Time Per Log
docker Driver Times for Minikube (PR 10722): 28.5s 26.6s 27.6s Averages Time Per Log
|
/retest-this-please |
/retest-this-please |
/ok-to-test |
a4c6908
to
cc7973a
Compare
kvm2 Driver Times for Minikube (PR 10722): 65.0s 65.7s 66.1s Averages Time Per Log
docker Driver Times for Minikube (PR 10722): 32.0s 27.6s 34.9s Averages Time Per Log
|
Fix #10626, #10627
Make the minikube correctly detect it is running in powershell. The actual fix is here and this PR is only to reflect it in minikube. Also fix a character bug in test.
Before PR:
After PR: