Merge pull request #9802 from afbjorklund/podman-host-cleanup

Some more cleanup of missing code for podman-env

pkg/drivers/kic/oci/oci.go

@@ -522,7 +522,6 @@ func ListContainersByLabel(ociBin string, label string, warnSlow ...bool) ([]str
 // PointToHostDockerDaemon will unset env variables that point to docker inside minikube
 // to make sure it points to the docker daemon installed by user.
 func PointToHostDockerDaemon() error {
-
 	if p := os.Getenv(constants.MinikubeActiveDockerdEnv); p != "" {
 		klog.Infof("shell is pointing to dockerd inside minikube. will unset to use host")
 		for _, e := range constants.DockerDaemonEnvs {
@@ -531,7 +530,6 @@ func PointToHostDockerDaemon() error {
 			}
 		}
 	}
-
 	return nil
 }
 
@@ -551,18 +549,13 @@ func resetEnv(key string) error {
 
 // PointToHostPodman will unset env variables that point to podman inside minikube
 func PointToHostPodman() error {
-	p := os.Getenv(constants.MinikubeActivePodmanEnv)
+	if p := os.Getenv(constants.MinikubeActivePodmanEnv); p != "" {
-	if p != "" {
 		klog.Infof("shell is pointing to podman inside minikube. will unset to use host")
-	}
+		for _, e := range constants.PodmanRemoteEnvs {
-
+			if err := resetEnv(e); err != nil {
-	for i := range constants.PodmanRemoteEnvs {
+				return err
-		e := constants.PodmanRemoteEnvs[i]
+			}
-		err := os.Setenv(e, "")
-		if err != nil {
-			return errors.Wrapf(err, "resetting %s env", e)
 		}
-
 	}
 	return nil
 }
@@ -642,17 +635,25 @@ func iptablesFileExists(ociBin string, nameOrID string) bool {
 }
 
 // DaemonHost returns the ip/hostname where OCI daemon service for driver is running
-// For Podman it's always DefaultBindIPV4
+// For Podman return the host part of CONTAINER_HOST environment variable if set
 // For Docker return the host part of DOCKER_HOST environment variable if set
 // or DefaultBindIPV4 otherwise
 func DaemonHost(driver string) string {
-	if driver != Docker {
+	if driver == Podman {
-		return DefaultBindIPV4
+		if dh := os.Getenv(constants.PodmanContainerHostEnv); dh != "" {
+			if u, err := url.Parse(dh); err == nil {
+				if u.Host != "" {
+					return u.Hostname()
+				}
+			}
+		}
 	}
-	if dh := os.Getenv(constants.DockerHostEnv); dh != "" {
+	if driver == Docker {
-		if u, err := url.Parse(dh); err == nil {
+		if dh := os.Getenv(constants.DockerHostEnv); dh != "" {
-			if u.Host != "" {
+			if u, err := url.Parse(dh); err == nil {
-				return u.Hostname()
+				if u.Host != "" {
+					return u.Hostname()
+				}
 			}
 		}
 	}
@@ -660,15 +661,21 @@ func DaemonHost(driver string) string {
 }
 
 // IsExternalDaemonHost returns whether or not the OCI runtime is running on an external/virtual host
-// For Podman driver it's always false for now
+// For Podman driver return true if CONTAINER_HOST is set to a URI, and the URI contains a host item
 // For Docker driver return true if DOCKER_HOST is set to a URI, and the URI contains a host item
 func IsExternalDaemonHost(driver string) bool {
-	if driver != Docker {
+	if driver == Podman {
-		return false
+		if dh := os.Getenv(constants.PodmanContainerHostEnv); dh != "" {
+			if u, err := url.Parse(dh); err == nil {
+				return u.Host != ""
+			}
+		}
 	}
-	if dh := os.Getenv(constants.DockerHostEnv); dh != "" {
+	if driver == Docker {
-		if u, err := url.Parse(dh); err == nil {
+		if dh := os.Getenv(constants.DockerHostEnv); dh != "" {
-			return u.Host != ""
+			if u, err := url.Parse(dh); err == nil {
+				return u.Host != ""
+			}
 		}
 	}
 	return false

pkg/drivers/kic/oci/oci_test.go

@@ -71,21 +71,66 @@ func TestPointToHostDockerDaemon(t *testing.T) {
 	}
 }
 
+func TestPointToHostPodmanEmpty(t *testing.T) {
+	_ = os.Setenv("CONTAINER_HOST", "foo_host")
+	_ = os.Setenv("MINIKUBE_ACTIVE_PODMAN", "minikube")
+
+	_ = os.Unsetenv("MINIKUBE_EXISTING_CONTAINER_HOST")
+
+	if err := PointToHostPodman(); err != nil {
+		t.Fatalf("failed to set podman environment: got %v", err)
+	}
+
+	for _, key := range []string{
+		"CONTAINER_HOST",
+	} {
+		if v, set := os.LookupEnv(key); set {
+			t.Errorf("%v env variable should not be set. got: %v", key, v)
+		}
+	}
+}
+
+func TestPointToHostPodman(t *testing.T) {
+	_ = os.Setenv("CONTAINER_HOST", "foo_host")
+
+	_ = os.Setenv("MINIKUBE_EXISTING_CONTAINER_HOST", "bar_host")
+
+	if err := PointToHostPodman(); err != nil {
+		t.Fatalf("failed to set podman environment: got %v", err)
+	}
+
+	expected := []struct {
+		key, value string
+	}{
+		{"CONTAINER_HOST", "bar_host"},
+	}
+	for _, exp := range expected {
+		if v := os.Getenv(exp.key); v != exp.value {
+			t.Errorf("invalid %v env variable. got: %v, want: %v", exp.value, v, exp.value)
+		}
+	}
+}
+
 func TestDaemonHost(t *testing.T) {
 	tests := []struct {
 		driver           string
+		containerHost    string
 		dockerHost       string
 		expectedAddr     string
 		expectedExternal bool
 	}{
-		{"", "", "127.0.0.1", false},
+		{"", "", "", "127.0.0.1", false},
-		{"docker", "tcp://1.1.1.1:2222/foo", "1.1.1.1", true},
+		{"docker", "", "tcp://1.1.1.1:2222/foo", "1.1.1.1", true},
-		{"podman", "tcp://1.1.1.1:2222/foo", "127.0.0.1", false},
+		{"docker", "ssh://127.0.0.1/bar", "", "127.0.0.1", false},
-		{"_invalid_", "tcp://1.1.1.1:2222/foo", "127.0.0.1", false},
+		{"podman", "", "tcp://1.1.1.1:2222/foo", "127.0.0.1", false},
-		{"docker", "unix:///var/run/something", "127.0.0.1", false},
+		{"podman", "ssh://127.0.0.1/bar", "", "127.0.0.1", true},
-		{"docker", "tcp://127.0.0.1/foo", "127.0.0.1", true},
+		{"_invalid_", "", "tcp://1.1.1.1:2222/foo", "127.0.0.1", false},
+		{"docker", "", "unix:///var/run/something", "127.0.0.1", false},
+		{"docker", "", "tcp://127.0.0.1/foo", "127.0.0.1", true},
+		{"docker", "", "ssh://127.0.0.1/bar", "127.0.0.1", true},
 	}
 	for _, test := range tests {
+		_ = os.Setenv("CONTAINER_HOST", test.containerHost)
 		_ = os.Setenv("DOCKER_HOST", test.dockerHost)
 		if v := IsExternalDaemonHost(test.driver); v != test.expectedExternal {
 			t.Errorf("invalid result of IsExternalDaemonHost. got: %v, want: %v", v, test.expectedExternal)

pkg/minikube/constants/constants.go

@@ -115,7 +115,7 @@ var (
 	ExistingDockerDaemonEnvs = [3]string{ExistingDockerHostEnv, ExistingDockerTLSVerifyEnv, ExistingDockerCertPathEnv}
 
 	// PodmanRemoteEnvs is list of podman-remote related environment variables.
-	PodmanRemoteEnvs = [1]string{PodmanVarlinkBridgeEnv}
+	PodmanRemoteEnvs = [2]string{PodmanVarlinkBridgeEnv, PodmanContainerHostEnv}
 
 	// DefaultMinipath is the default minikube path (under the home directory)
 	DefaultMinipath = filepath.Join(homedir.HomeDir(), ".minikube")