Fix calling appendKnownHelper when not needed

[spowelljr]

Related #16933 #15452

appendKnownHelper was being called when ssdAdd was 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:

$ minikube docker-env
export DOCKER_TLS_VERIFY="1"
export DOCKER_HOST="tcp://192.168.49.2:2376"
export DOCKER_CERT_PATH="/home/test/.minikube/certs"
export MINIKUBE_ACTIVE_DOCKERD="minikube"

# To point your shell to minikube's docker-daemon, run:
# eval $(minikube -p minikube docker-env)
Host added: /home/test/.ssh/known_hosts ([127.0.0.1]:32777)
OpenFile: open /home/test/.ssh/known_hosts: no such file or directory

$ echo $?
1

After:

$ minikube docker-env
export DOCKER_TLS_VERIFY="1"
export DOCKER_HOST="tcp://192.168.49.2:2376"
export DOCKER_CERT_PATH="/home/test/.minikube/certs"
export MINIKUBE_ACTIVE_DOCKERD="minikube"

# To point your shell to minikube's docker-daemon, run:
# eval $(minikube -p minikube docker-env)

$ echo $?
0

[spowelljr]

/ok-to-test

[ComradeProgrammer]

Well perhaps there is a small problem?
I think whether appendKnownHelper should be called, should depend on the flag --ssh-host instead of --ssh-add
--ssh-add means "Add SSH identity key to SSH authentication agent", it controlls whether we need to start a ssh-agent

--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 appendKnownHelper(nodeName, true) with if sshHost {}?

---

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

#17039

[spowelljr]

@ComradeProgrammer

To focus on the regression I wrapped appendKnownHelper with if cr == constants.Containerd so docker users can't invoke it, which may have untested outcomes, same with SSH_AUTH_SOCK & SSH_AGENT_PID. In your PR you can follow up on your comments once we have testing with using docker with the new functionality.

[minikube-pr-bot]

kvm2 driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 17030) |
+----------------+----------+---------------------+
| minikube start | 50.8s    | 51.9s               |
| enable ingress | 26.9s    | 26.5s               |
+----------------+----------+---------------------+

Times for minikube start: 51.1s 50.0s 49.7s 51.7s 51.2s
Times for minikube (PR 17030) start: 51.5s 51.3s 51.2s 51.7s 53.6s

Times for minikube ingress: 24.7s 27.3s 26.8s 27.3s 28.3s
Times for minikube (PR 17030) ingress: 24.2s 27.7s 27.3s 26.8s 26.7s

docker driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 17030) |
+----------------+----------+---------------------+
| minikube start | 23.9s    | 23.5s               |
| enable ingress | 20.9s    | 21.0s               |
+----------------+----------+---------------------+

Times for minikube start: 22.0s 21.9s 25.0s 24.8s 25.6s
Times for minikube (PR 17030) start: 25.2s 21.4s 25.4s 23.8s 21.9s

Times for minikube (PR 17030) ingress: 20.9s 21.4s 20.9s 20.9s 20.9s
Times for minikube ingress: 20.9s 21.4s 20.9s 20.4s 20.9s

docker driver with containerd runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 17030) |
+----------------+----------+---------------------+
| minikube start | 23.7s    | 21.7s               |
| enable ingress | 34.6s    | 28.9s               |
+----------------+----------+---------------------+

Times for minikube start: 23.2s 23.5s 23.8s 23.4s 24.3s
Times for minikube (PR 17030) start: 22.6s 20.5s 24.2s 20.4s 20.9s

Times for minikube ingress: 31.4s 47.4s 31.4s 31.4s 31.4s
Times for minikube (PR 17030) ingress: 18.9s 31.4s 31.4s 31.4s 31.4s

[minikube-pr-bot]

These are the flake rates of all failed tests.

Environment	Failed Tests	Flake Rate (%)
Docker_Linux_containerd_arm64	TestAddons/parallel/InspektorGadget (gopogh)	0.00 (chart)
Docker_Linux_crio	TestStartStop/group/no-preload/serial/FirstStart (gopogh)	0.00 (chart)
Docker_Linux_crio	TestStartStop/group/old-k8s-version/serial/FirstStart (gopogh)	0.00 (chart)
Docker_Windows	TestPause/serial/PauseAgain (gopogh)	0.00 (chart)
Hyperkit_macOS	TestStoppedBinaryUpgrade/MinikubeLogs (gopogh)	0.00 (chart)
KVM_Linux	TestSkaffold (gopogh)	0.71 (chart)
QEMU_macOS	TestAddons/serial/GCPAuth/Namespaces (gopogh)	1.59 (chart)
Docker_macOS	TestSkaffold (gopogh)	2.08 (chart)
KVM_Linux	TestRunningBinaryUpgrade (gopogh)	4.26 (chart)
Hyperkit_macOS	TestCertOptions (gopogh)	7.09 (chart)
Hyperkit_macOS	TestStoppedBinaryUpgrade/Upgrade (gopogh)	7.09 (chart)
QEMU_macOS	TestAddons/StoppedEnableDisable (gopogh)	7.35 (chart)
Hyperkit_macOS	TestRunningBinaryUpgrade (gopogh)	19.15 (chart)
KVM_Linux_crio	TestFunctional/serial/LogsFileCmd (gopogh)	31.43 (chart)
QEMU_macOS	TestFunctional/parallel/TunnelCmd/serial/RunSecondTunnel (gopogh)	46.21 (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: 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:

• OWNERS [medyagh,spowelljr]

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