Fix docker-env and add unit and integration tests (#6604)

* Fix docker cert env in docker-env for docker driver

* add unit tests

* add integration tests for docker-env cmd

* lint

* fix lint

* merge conflict

* improve docker-env integraiton test

* linting

Author: medyagh

Diff for: cmd/minikube/cmd/docker-env.go

@@ -29,7 +29,6 @@ import (
 
 	"github.com/docker/machine/libmachine/drivers"
 	"github.com/docker/machine/libmachine/state"
-	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
 	"k8s.io/minikube/pkg/drivers/kic/oci"
@@ -182,11 +181,22 @@ var dockerEnvCmd = &cobra.Command{
 		sh := shell.EnvConfig{
 			Shell: shell.ForceShell,
 		}
+
+		port := constants.DockerDaemonPort
+		if driver.IsKIC(host.DriverName) { // for kic we need to find what port docker/podman chose for us
+			hostIP = oci.DefaultBindIPV4
+			port, err = oci.HostPortBinding(host.DriverName, profile, port)
+			if err != nil {
+				exit.WithCodeT(exit.Failure, "Error getting port binding for '{{.driver_name}} driver: {{.error}}", out.V{"driver_name": host.DriverName, "error": err})
+			}
+		}
+
 		ec := DockerEnvConfig{
 			EnvConfig: sh,
 			profile:   profile,
 			driver:    host.DriverName,
 			hostIP:    hostIP,
+			port:      port,
 			certsDir:  localpath.MakeMiniPath("certs"),
 			noProxy:   noProxy,
 		}
@@ -217,16 +227,14 @@ type DockerEnvConfig struct {
 	profile  string
 	driver   string
 	hostIP   string
+	port     int
 	certsDir string
 	noProxy  bool
 }
 
 // dockerSetScript writes out a shell-compatible 'docker-env' script
 func dockerSetScript(ec DockerEnvConfig, w io.Writer) error {
-	envVars, err := dockerEnvVars(ec)
-	if err != nil {
-		return err
-	}
+	envVars := dockerEnvVars(ec)
 	return shell.SetScript(ec.EnvConfig, w, dockerEnvTmpl, dockerShellCfgSet(ec, envVars))
 }
 
@@ -255,22 +263,15 @@ func dockerURL(ip string, port int) string {
 }
 
 // dockerEnvVars gets the necessary docker env variables to allow the use of minikube's docker daemon
-func dockerEnvVars(ec DockerEnvConfig) (map[string]string, error) {
+func dockerEnvVars(ec DockerEnvConfig) map[string]string {
 	env := map[string]string{
 		constants.DockerTLSVerifyEnv:       "1",
-		constants.DockerHostEnv:            dockerURL(ec.hostIP, constants.DockerDaemonPort),
+		constants.DockerHostEnv:            dockerURL(ec.hostIP, ec.port),
 		constants.DockerCertPathEnv:        ec.certsDir,
 		constants.MinikubeActiveDockerdEnv: ec.profile,
 	}
 
-	if driver.IsKIC(ec.driver) { // for kic we need to find out what port docker allocated during creation
-		port, err := oci.HostPortBinding(ec.driver, ec.profile, constants.DockerDaemonPort)
-		if err != nil {
-			return nil, errors.Wrapf(err, "get hostbind port for %d", constants.DockerDaemonPort)
-		}
-		env[constants.DockerCertPathEnv] = dockerURL(oci.DefaultBindIPV4, port)
-	}
-	return env, nil
+	return env
 }
 
 func init() {

Diff for: cmd/minikube/cmd/docker-env_test.go

@@ -42,7 +42,22 @@ func TestGenerateDockerScripts(t *testing.T) {
 	}{
 		{
 			"bash",
-			DockerEnvConfig{profile: "bash", driver: "kvm2", hostIP: "127.0.0.1", certsDir: "/certs"},
+			DockerEnvConfig{profile: "dockerdrver", driver: "docker", hostIP: "127.0.0.1", port: 32842, certsDir: "/certs"},
+			nil,
+			`export DOCKER_TLS_VERIFY="1"
+export DOCKER_HOST="tcp://127.0.0.1:32842"
+export DOCKER_CERT_PATH="/certs"
+export MINIKUBE_ACTIVE_DOCKERD="dockerdrver"
+
+# To point your shell to minikube's docker-daemon, run:
+# eval $(minikube -p dockerdrver docker-env)
+`,
+			`unset DOCKER_TLS_VERIFY DOCKER_HOST DOCKER_CERT_PATH MINIKUBE_ACTIVE_DOCKERD
+`,
+		},
+		{
+			"bash",
+			DockerEnvConfig{profile: "bash", driver: "kvm2", hostIP: "127.0.0.1", port: 2376, certsDir: "/certs"},
 			nil,
 			`export DOCKER_TLS_VERIFY="1"
 export DOCKER_HOST="tcp://127.0.0.1:2376"
@@ -57,7 +72,7 @@ export MINIKUBE_ACTIVE_DOCKERD="bash"
 		},
 		{
 			"bash",
-			DockerEnvConfig{profile: "ipv6", driver: "kvm2", hostIP: "fe80::215:5dff:fe00:a903", certsDir: "/certs"},
+			DockerEnvConfig{profile: "ipv6", driver: "kvm2", hostIP: "fe80::215:5dff:fe00:a903", port: 2376, certsDir: "/certs"},
 			nil,
 			`export DOCKER_TLS_VERIFY="1"
 export DOCKER_HOST="tcp://[fe80::215:5dff:fe00:a903]:2376"
@@ -72,7 +87,7 @@ export MINIKUBE_ACTIVE_DOCKERD="ipv6"
 		},
 		{
 			"fish",
-			DockerEnvConfig{profile: "fish", driver: "kvm2", hostIP: "127.0.0.1", certsDir: "/certs"},
+			DockerEnvConfig{profile: "fish", driver: "kvm2", hostIP: "127.0.0.1", port: 2376, certsDir: "/certs"},
 			nil,
 			`set -gx DOCKER_TLS_VERIFY "1"
 set -gx DOCKER_HOST "tcp://127.0.0.1:2376"
@@ -90,7 +105,7 @@ set -e MINIKUBE_ACTIVE_DOCKERD
 		},
 		{
 			"powershell",
-			DockerEnvConfig{profile: "powershell", driver: "hyperv", hostIP: "192.168.0.1", certsDir: "/certs"},
+			DockerEnvConfig{profile: "powershell", driver: "hyperv", hostIP: "192.168.0.1", port: 2376, certsDir: "/certs"},
 			nil,
 			`$Env:DOCKER_TLS_VERIFY = "1"
 $Env:DOCKER_HOST = "tcp://192.168.0.1:2376"
@@ -105,7 +120,7 @@ $Env:MINIKUBE_ACTIVE_DOCKERD = "powershell"
 		},
 		{
 			"cmd",
-			DockerEnvConfig{profile: "cmd", driver: "hyperv", hostIP: "192.168.0.1", certsDir: "/certs"},
+			DockerEnvConfig{profile: "cmd", driver: "hyperv", hostIP: "192.168.0.1", port: 2376, certsDir: "/certs"},
 			nil,
 			`SET DOCKER_TLS_VERIFY=1
 SET DOCKER_HOST=tcp://192.168.0.1:2376
@@ -123,7 +138,7 @@ SET MINIKUBE_ACTIVE_DOCKERD=
 		},
 		{
 			"emacs",
-			DockerEnvConfig{profile: "emacs", driver: "hyperv", hostIP: "192.168.0.1", certsDir: "/certs"},
+			DockerEnvConfig{profile: "emacs", driver: "hyperv", hostIP: "192.168.0.1", port: 2376, certsDir: "/certs"},
 			nil,
 			`(setenv "DOCKER_TLS_VERIFY" "1")
 (setenv "DOCKER_HOST" "tcp://192.168.0.1:2376")
@@ -140,7 +155,7 @@ SET MINIKUBE_ACTIVE_DOCKERD=
 		},
 		{
 			"bash",
-			DockerEnvConfig{profile: "bash-no-proxy", driver: "kvm2", hostIP: "127.0.0.1", certsDir: "/certs", noProxy: true},
+			DockerEnvConfig{profile: "bash-no-proxy", driver: "kvm2", hostIP: "127.0.0.1", port: 2376, certsDir: "/certs", noProxy: true},
 			&FakeNoProxyGetter{"NO_PROXY", "127.0.0.1"},
 			`export DOCKER_TLS_VERIFY="1"
 export DOCKER_HOST="tcp://127.0.0.1:2376"
@@ -157,7 +172,7 @@ export NO_PROXY="127.0.0.1"
 		},
 		{
 			"bash",
-			DockerEnvConfig{profile: "bash-no-proxy-lower", driver: "kvm2", hostIP: "127.0.0.1", certsDir: "/certs", noProxy: true},
+			DockerEnvConfig{profile: "bash-no-proxy-lower", driver: "kvm2", hostIP: "127.0.0.1", port: 2376, certsDir: "/certs", noProxy: true},
 			&FakeNoProxyGetter{"no_proxy", "127.0.0.1"},
 			`export DOCKER_TLS_VERIFY="1"
 export DOCKER_HOST="tcp://127.0.0.1:2376"
@@ -174,7 +189,7 @@ export no_proxy="127.0.0.1"
 		},
 		{
 			"powershell",
-			DockerEnvConfig{profile: "powershell-no-proxy-idempotent", driver: "hyperv", hostIP: "192.168.0.1", certsDir: "/certs", noProxy: true},
+			DockerEnvConfig{profile: "powershell-no-proxy-idempotent", driver: "hyperv", hostIP: "192.168.0.1", port: 2376, certsDir: "/certs", noProxy: true},
 			&FakeNoProxyGetter{"no_proxy", "192.168.0.1"},
 			`$Env:DOCKER_TLS_VERIFY = "1"
 $Env:DOCKER_HOST = "tcp://192.168.0.1:2376"
@@ -190,7 +205,7 @@ $Env:no_proxy = "192.168.0.1"
 		},
 		{
 			"bash",
-			DockerEnvConfig{profile: "sh-no-proxy-add", driver: "kvm2", hostIP: "127.0.0.1", certsDir: "/certs", noProxy: true},
+			DockerEnvConfig{profile: "sh-no-proxy-add", driver: "kvm2", hostIP: "127.0.0.1", port: 2376, certsDir: "/certs", noProxy: true},
 			&FakeNoProxyGetter{"NO_PROXY", "192.168.0.1,10.0.0.4"},
 			`export DOCKER_TLS_VERIFY="1"
 export DOCKER_HOST="tcp://127.0.0.1:2376"

Diff for: test/integration/functional_test.go

@@ -104,6 +104,7 @@ func TestFunctional(t *testing.T) {
 			{"MySQL", validateMySQL},
 			{"FileSync", validateFileSync},
 			{"UpdateContextCmd", validateUpdateContextCmd},
+			{"DockerEnv", validateDockerEnv},
 		}
 		for _, tc := range tests {
 			tc := tc
@@ -115,6 +116,36 @@ func TestFunctional(t *testing.T) {
 	})
 }
 
+// check functionality of minikube after evaling docker-env
+func validateDockerEnv(ctx context.Context, t *testing.T, profile string) {
+	mctx, cancel := context.WithTimeout(ctx, 13*time.Second)
+	defer cancel()
+	// we should be able to get minikube status with a bash which evaled docker-env
+	c := exec.CommandContext(mctx, "/bin/bash", "-c", "eval $("+Target()+" -p "+profile+" docker-env) && "+Target()+" status -p "+profile)
+	rr, err := Run(t, c)
+	if err != nil {
+		t.Fatalf("Failed to do minikube status after eval-ing docker-env %s", err)
+	}
+	if !strings.Contains(rr.Output(), "Running") {
+		t.Fatalf("Expected status output to include 'Running' after eval docker-env but got \n%s", rr.Output())
+	}
+
+	mctx, cancel = context.WithTimeout(ctx, 13*time.Second)
+	defer cancel()
+	// do a eval $(minikube -p profile docker-env) and check if we are point to docker inside minikube
+	c = exec.CommandContext(mctx, "/bin/bash", "-c", "eval $("+Target()+" -p "+profile+" docker-env) && docker images")
+	rr, err = Run(t, c)
+	if err != nil {
+		t.Fatalf("Failed to test eval docker-evn %s", err)
+	}
+
+	expectedImgInside := "gcr.io/k8s-minikube/storage-provisioner"
+	if !strings.Contains(rr.Output(), expectedImgInside) {
+		t.Fatalf("Expected 'docker ps' to have %q from docker-daemon inside minikube. the docker ps output is:\n%q\n", expectedImgInside, rr.Output())
+	}
+
+}
+
 func validateStartWithProxy(ctx context.Context, t *testing.T, profile string) {
 	srv, err := startHTTPProxy(t)
 	if err != nil {