Fix calling appendKnownHelper when not needed #17030
Conversation
|
/ok-to-test |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Well perhaps there is a small problem? --ssh-host controls whether we use ssh connections to connect to docker daemon's unix socket or use http to connect to docker daemon's exposed port using append known which modifies known_hosts file, is for connecting to a remote host using ssh,and it has nothing to do with ssh agent, so whether we need to append known hosts should depend on whether --ssh-host is set. so perhaps just wrap And if we want to use ssh-add for the ssh-agent daemon we stored in config, we still need to set SSH_AUTH_SOCK and SSH_AGENT_PID. Actually perhaps the automation's inconsistent behavior between "containerd" and "docker runtime with ssh connection" is too confusing. I think we should unify the automation behavior and fix this bug as well |
|
To focus on the regression I wrapped |
|
kvm2 driver with docker runtime Times for minikube start: 51.1s 50.0s 49.7s 51.7s 51.2s Times for minikube ingress: 24.7s 27.3s 26.8s 27.3s 28.3s docker driver with docker runtime Times for minikube start: 22.0s 21.9s 25.0s 24.8s 25.6s Times for minikube (PR 17030) ingress: 20.9s 21.4s 20.9s 20.9s 20.9s docker driver with containerd runtime Times for minikube start: 23.2s 23.5s 23.8s 23.4s 24.3s Times for minikube ingress: 31.4s 47.4s 31.4s 31.4s 31.4s |
|
These are the flake rates of all failed tests.
To see the flake rates of all tests by environment, click here. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: medyagh, 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 |
Related #16933 #15452
appendKnownHelperwas being called whenssdAddwas not requested, causing #16933 to affect all docker-env commands, not just containerd usages.Also fixed adding envs to a command that might be empty
Before:
After: