From d384343c39c2c8878d22246cfe8b174a19447e50 Mon Sep 17 00:00:00 2001 From: Ilya Zuyev Date: Mon, 19 Oct 2020 22:02:42 -0700 Subject: [PATCH 01/18] When configuring docker environment, use MINIKUBE_EXISTING_* variables if set --- pkg/drivers/kic/oci/oci.go | 46 ++++++++++++------- pkg/drivers/kic/oci/oci_test.go | 71 +++++++++++++++++++++++++++++ pkg/minikube/constants/constants.go | 2 + 3 files changed, 102 insertions(+), 17 deletions(-) create mode 100644 pkg/drivers/kic/oci/oci_test.go diff --git a/pkg/drivers/kic/oci/oci.go b/pkg/drivers/kic/oci/oci.go index 96783e58c2..e55cfd147b 100644 --- a/pkg/drivers/kic/oci/oci.go +++ b/pkg/drivers/kic/oci/oci.go @@ -17,26 +17,25 @@ limitations under the License. package oci import ( - "context" - "os" - "time" - "bufio" "bytes" + "context" + "fmt" + "os" + "os/exec" + "runtime" + "strconv" + "strings" + "time" "github.com/docker/machine/libmachine/state" "github.com/pkg/errors" "k8s.io/klog/v2" + "k8s.io/minikube/pkg/minikube/constants" "k8s.io/minikube/pkg/minikube/out" "k8s.io/minikube/pkg/util/retry" - - "fmt" - "os/exec" - "runtime" - "strconv" - "strings" ) // DeleteContainersByLabel deletes all containers that have a specific label @@ -521,18 +520,31 @@ 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 { - p := os.Getenv(constants.MinikubeActiveDockerdEnv) - if p != "" { + + if p := os.Getenv(constants.MinikubeActiveDockerdEnv); p != "" { klog.Infof("shell is pointing to dockerd inside minikube. will unset to use host") } - for i := range constants.DockerDaemonEnvs { - e := constants.DockerDaemonEnvs[i] - err := os.Setenv(e, "") - if err != nil { - return errors.Wrapf(err, "resetting %s env", e) + for _, e := range constants.DockerDaemonEnvs { + if err := resetEnv(e); err != nil { + return err } + } + return nil +} + +func resetEnv(key string) error { + v := os.Getenv(constants.MinikubeExistingPrefix + key) + if v == "" { + if err := os.Unsetenv(key); err != nil { + return errors.Wrapf(err, "resetting %s env", key) + } + return nil + } + + if err := os.Setenv(key, v); err != nil { + return errors.Wrapf(err, "resetting %s env", key) } return nil } diff --git a/pkg/drivers/kic/oci/oci_test.go b/pkg/drivers/kic/oci/oci_test.go new file mode 100644 index 0000000000..897762345d --- /dev/null +++ b/pkg/drivers/kic/oci/oci_test.go @@ -0,0 +1,71 @@ +/* +Copyright 2020 The Kubernetes Authors All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package oci + +import ( + "os" + "testing" +) + +func TestPointToHostDockerDaemonEmpty(t *testing.T) { + _ = os.Setenv("DOCKER_HOST", "foo_host") + _ = os.Setenv("DOCKER_CERT_PATH", "foo_cert_path") + _ = os.Setenv("DOCKER_TLS_VERIFY", "foo_tls_verify") + + _ = os.Unsetenv("MINIKUBE_EXISTING_DOCKER_HOST") + _ = os.Unsetenv("MINIKUBE_EXISTING_DOCKER_CERT_PATH") + _ = os.Unsetenv("MINIKUBE_EXISTING_DOCKER_TLS_VERIFY") + + if err := PointToHostDockerDaemon(); err != nil { + t.Fatalf("failed to set docker environment: got %v", err) + } + + for _, key := range []string{ + "DOCKER_HOST", "DOCKER_CERT_PATH", "DOCKER_TLS_VERIFY", + } { + if v, set := os.LookupEnv(key); set { + t.Errorf("%v env variable should not be set. got: %v", key, v) + } + } +} + +func TestPointToHostDockerDaemon(t *testing.T) { + _ = os.Setenv("DOCKER_HOST", "foo_host") + _ = os.Setenv("DOCKER_CERT_PATH", "foo_cert_path") + _ = os.Setenv("DOCKER_TLS_VERIFY", "foo_tls_verify") + + _ = os.Setenv("MINIKUBE_EXISTING_DOCKER_HOST", "bar_host") + _ = os.Setenv("MINIKUBE_EXISTING_DOCKER_CERT_PATH", "bar_cert_path") + _ = os.Setenv("MINIKUBE_EXISTING_DOCKER_TLS_VERIFY", "bar_tls_verify") + + if err := PointToHostDockerDaemon(); err != nil { + t.Fatalf("failed to set docker environment: got %v", err) + } + + expected := []struct { + key, value string + }{ + {"DOCKER_HOST", "bar_host"}, + {"DOCKER_CERT_PATH", "bar_cert_path"}, + {"DOCKER_TLS_VERIFY", "bar_tls_verify"}, + } + 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) + } + } +} diff --git a/pkg/minikube/constants/constants.go b/pkg/minikube/constants/constants.go index fd330a96d9..54238fe6bb 100644 --- a/pkg/minikube/constants/constants.go +++ b/pkg/minikube/constants/constants.go @@ -75,6 +75,8 @@ const ( MinikubeForceSystemdEnv = "MINIKUBE_FORCE_SYSTEMD" // TestDiskUsedEnv is used in integration tests for insufficient storage with 'minikube status' TestDiskUsedEnv = "MINIKUBE_TEST_STORAGE_CAPACITY" + // MinikubeExistingPrefix is used to save the original environment when executing docker-env + MinikubeExistingPrefix = "MINIKUBE_EXISTING_" ) var ( From 85d45be207b49848ca4aa462aebfd35bccb933b3 Mon Sep 17 00:00:00 2001 From: Ilya Zuyev Date: Tue, 20 Oct 2020 23:10:08 -0700 Subject: [PATCH 02/18] Fix docker-env command --- cmd/minikube/cmd/docker-env.go | 52 +++++++++++++++++++++--- pkg/drivers/kic/kic.go | 9 +++-- pkg/drivers/kic/oci/env.go | 47 +++++++++++++++++++++ pkg/drivers/kic/oci/oci.go | 10 ++--- pkg/drivers/kic/oci/oci_test.go | 1 + pkg/minikube/constants/constants.go | 11 +++++ pkg/minikube/shell/shell.go | 63 +++++++++++++++++++++-------- pkg/minikube/shell/shell_test.go | 11 +++-- 8 files changed, 168 insertions(+), 36 deletions(-) create mode 100644 pkg/drivers/kic/oci/env.go diff --git a/cmd/minikube/cmd/docker-env.go b/cmd/minikube/cmd/docker-env.go index 5f34398ddd..f27513a471 100644 --- a/cmd/minikube/cmd/docker-env.go +++ b/cmd/minikube/cmd/docker-env.go @@ -30,6 +30,7 @@ import ( "github.com/spf13/cobra" "k8s.io/klog/v2" + "k8s.io/minikube/pkg/drivers/kic/oci" "k8s.io/minikube/pkg/minikube/command" "k8s.io/minikube/pkg/minikube/constants" @@ -43,7 +44,31 @@ import ( "k8s.io/minikube/pkg/minikube/sysinit" ) -var dockerEnvTmpl = fmt.Sprintf("{{ .Prefix }}%s{{ .Delimiter }}{{ .DockerTLSVerify }}{{ .Suffix }}{{ .Prefix }}%s{{ .Delimiter }}{{ .DockerHost }}{{ .Suffix }}{{ .Prefix }}%s{{ .Delimiter }}{{ .DockerCertPath }}{{ .Suffix }}{{ .Prefix }}%s{{ .Delimiter }}{{ .MinikubeDockerdProfile }}{{ .Suffix }}{{ if .NoProxyVar }}{{ .Prefix }}{{ .NoProxyVar }}{{ .Delimiter }}{{ .NoProxyValue }}{{ .Suffix }}{{end}}{{ .UsageHint }}", constants.DockerTLSVerifyEnv, constants.DockerHostEnv, constants.DockerCertPathEnv, constants.MinikubeActiveDockerdEnv) +var dockerSetEnvTmpl = fmt.Sprintf( + "{{ .Prefix }}%s{{ .Delimiter }}{{ .DockerTLSVerify }}{{ .Suffix }}"+ + "{{ .Prefix }}%s{{ .Delimiter }}{{ .DockerHost }}{{ .Suffix }}"+ + "{{ .Prefix }}%s{{ .Delimiter }}{{ .DockerCertPath }}{{ .Suffix }}"+ + "{{ if .ExistingDockerTLSVerify }}"+ + "{{ .Prefix }}%s{{ .Delimiter }}{{ .ExistingDockerTLSVerify }}{{ .Suffix }}"+ + "{{ end }}"+ + "{{ if .ExistingDockerHost }}"+ + "{{ .Prefix }}%s{{ .Delimiter }}{{ .ExistingDockerHost }}{{ .Suffix }}"+ + "{{ end }}"+ + "{{ if .ExistingDockerCertPath }}"+ + "{{ .Prefix }}%s{{ .Delimiter }}{{ .ExistingDockerCertPath }}{{ .Suffix }}"+ + "{{ end }}"+ + "{{ .Prefix }}%s{{ .Delimiter }}{{ .MinikubeDockerdProfile }}{{ .Suffix }}"+ + "{{ if .NoProxyVar }}"+ + "{{ .Prefix }}{{ .NoProxyVar }}{{ .Delimiter }}{{ .NoProxyValue }}{{ .Suffix }}"+ + "{{ end }}"+ + "{{ .UsageHint }}", + constants.DockerTLSVerifyEnv, + constants.DockerHostEnv, + constants.DockerCertPathEnv, + constants.ExistingDockerTLSVerifyEnv, + constants.ExistingDockerHostEnv, + constants.ExistingDockerCertPathEnv, + constants.MinikubeActiveDockerdEnv) // DockerShellConfig represents the shell config for Docker type DockerShellConfig struct { @@ -54,6 +79,10 @@ type DockerShellConfig struct { MinikubeDockerdProfile string NoProxyVar string NoProxyValue string + + ExistingDockerCertPath string + ExistingDockerHost string + ExistingDockerTLSVerify string } var ( @@ -81,6 +110,11 @@ func dockerShellCfgSet(ec DockerEnvConfig, envMap map[string]string) *DockerShel s.DockerCertPath = envMap[constants.DockerCertPathEnv] s.DockerHost = envMap[constants.DockerHostEnv] s.DockerTLSVerify = envMap[constants.DockerTLSVerifyEnv] + + s.ExistingDockerCertPath = envMap[constants.ExistingDockerCertPathEnv] + s.ExistingDockerHost = envMap[constants.ExistingDockerHostEnv] + s.ExistingDockerTLSVerify = envMap[constants.ExistingDockerTLSVerifyEnv] + s.MinikubeDockerdProfile = envMap[constants.MinikubeActiveDockerdEnv] if ec.noProxy { @@ -228,7 +262,7 @@ type DockerEnvConfig struct { // dockerSetScript writes out a shell-compatible 'docker-env' script func dockerSetScript(ec DockerEnvConfig, w io.Writer) error { envVars := dockerEnvVars(ec) - return shell.SetScript(ec.EnvConfig, w, dockerEnvTmpl, dockerShellCfgSet(ec, envVars)) + return shell.SetScript(ec.EnvConfig, w, dockerSetEnvTmpl, dockerShellCfgSet(ec, envVars)) } // dockerSetScript writes out a shell-compatible 'docker-env unset' script @@ -246,7 +280,6 @@ func dockerUnsetScript(ec DockerEnvConfig, w io.Writer) error { vars = append(vars, k) } } - return shell.UnsetScript(ec.EnvConfig, w, vars) } @@ -257,14 +290,21 @@ 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 { - env := map[string]string{ + rt := map[string]string{ constants.DockerTLSVerifyEnv: "1", constants.DockerHostEnv: dockerURL(ec.hostIP, ec.port), constants.DockerCertPathEnv: ec.certsDir, constants.MinikubeActiveDockerdEnv: ec.profile, } - - return env + if os.Getenv(constants.MinikubeActiveDockerdEnv) == "" { + for _, env := range constants.DockerDaemonEnvs { + if v := oci.InitialEnv(env); v != "" { + key := constants.MinikubeExistingPrefix + env + rt[key] = v + } + } + } + return rt } // dockerEnvVarsList gets the necessary docker env variables to allow the use of minikube's docker daemon to be used in a exec.Command diff --git a/pkg/drivers/kic/kic.go b/pkg/drivers/kic/kic.go index 10c5aebdcc..0f13b64085 100644 --- a/pkg/drivers/kic/kic.go +++ b/pkg/drivers/kic/kic.go @@ -95,10 +95,11 @@ func (d *Driver) Create() error { } // control plane specific options - params.PortMappings = append(params.PortMappings, oci.PortMapping{ - ListenAddress: oci.DefaultBindIPV4, - ContainerPort: int32(params.APIServerPort), - }, + params.PortMappings = append(params.PortMappings, + oci.PortMapping{ + ListenAddress: oci.DefaultBindIPV4, + ContainerPort: int32(params.APIServerPort), + }, oci.PortMapping{ ListenAddress: oci.DefaultBindIPV4, ContainerPort: constants.SSHPort, diff --git a/pkg/drivers/kic/oci/env.go b/pkg/drivers/kic/oci/env.go new file mode 100644 index 0000000000..6899016f11 --- /dev/null +++ b/pkg/drivers/kic/oci/env.go @@ -0,0 +1,47 @@ +/* +Copyright 2020 The Kubernetes Authors All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package oci + +import ( + "os" + + "k8s.io/minikube/pkg/minikube/constants" +) + +var initialEnvs = make(map[string]string) + +func init() { + for _, env := range constants.DockerDaemonEnvs { + if v, set := os.LookupEnv(env); set { + initialEnvs[env] = v + } + exEnv := constants.MinikubeExistingPrefix + env + if v, set := os.LookupEnv(exEnv); set { + initialEnvs[exEnv] = v + } + } +} + +// InitialEnv returns the value of the environment variable env before any environment changes made by minikube +func InitialEnv(env string) string { + return initialEnvs[env] +} + +// LookupInitialEnv returns the value of the environment variable env before any environment changes made by minikube +func LookupInitialEnv(env string) (string, bool) { + v, set := initialEnvs[env] + return v, set +} diff --git a/pkg/drivers/kic/oci/oci.go b/pkg/drivers/kic/oci/oci.go index e55cfd147b..ced79caf08 100644 --- a/pkg/drivers/kic/oci/oci.go +++ b/pkg/drivers/kic/oci/oci.go @@ -523,11 +523,10 @@ 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 { - if err := resetEnv(e); err != nil { - return err + for _, e := range constants.DockerDaemonEnvs { + if err := resetEnv(e); err != nil { + return err + } } } @@ -542,7 +541,6 @@ func resetEnv(key string) error { } return nil } - if err := os.Setenv(key, v); err != nil { return errors.Wrapf(err, "resetting %s env", key) } diff --git a/pkg/drivers/kic/oci/oci_test.go b/pkg/drivers/kic/oci/oci_test.go index 897762345d..164793537f 100644 --- a/pkg/drivers/kic/oci/oci_test.go +++ b/pkg/drivers/kic/oci/oci_test.go @@ -25,6 +25,7 @@ func TestPointToHostDockerDaemonEmpty(t *testing.T) { _ = os.Setenv("DOCKER_HOST", "foo_host") _ = os.Setenv("DOCKER_CERT_PATH", "foo_cert_path") _ = os.Setenv("DOCKER_TLS_VERIFY", "foo_tls_verify") + _ = os.Setenv("MINIKUBE_ACTIVE_DOCKERD", "minikube") _ = os.Unsetenv("MINIKUBE_EXISTING_DOCKER_HOST") _ = os.Unsetenv("MINIKUBE_EXISTING_DOCKER_CERT_PATH") diff --git a/pkg/minikube/constants/constants.go b/pkg/minikube/constants/constants.go index 54238fe6bb..00896b69f5 100644 --- a/pkg/minikube/constants/constants.go +++ b/pkg/minikube/constants/constants.go @@ -75,8 +75,16 @@ const ( MinikubeForceSystemdEnv = "MINIKUBE_FORCE_SYSTEMD" // TestDiskUsedEnv is used in integration tests for insufficient storage with 'minikube status' TestDiskUsedEnv = "MINIKUBE_TEST_STORAGE_CAPACITY" + // MinikubeExistingPrefix is used to save the original environment when executing docker-env MinikubeExistingPrefix = "MINIKUBE_EXISTING_" + + // ExistingDockerHostEnv is used to save original docker environment + ExistingDockerHostEnv = MinikubeExistingPrefix + "DOCKER_HOST" + // ExistingDockerCertPathEnv is used to save original docker environment + ExistingDockerCertPathEnv = MinikubeExistingPrefix + "DOCKER_CERT_PATH" + // ExistingDockerTLSVerifyEnv is used to save original docker environment + ExistingDockerTLSVerifyEnv = MinikubeExistingPrefix + "DOCKER_TLS_VERIFY" ) var ( @@ -92,6 +100,9 @@ var ( // DockerDaemonEnvs is list of docker-daemon related environment variables. DockerDaemonEnvs = [3]string{DockerHostEnv, DockerTLSVerifyEnv, DockerCertPathEnv} + // ExistingDockerDaemonEnvs is list of docker-daemon related environment variables. + ExistingDockerDaemonEnvs = [3]string{ExistingDockerHostEnv, ExistingDockerTLSVerifyEnv, ExistingDockerCertPathEnv} + // PodmanRemoteEnvs is list of podman-remote related environment variables. PodmanRemoteEnvs = [1]string{PodmanVarlinkBridgeEnv} diff --git a/pkg/minikube/shell/shell.go b/pkg/minikube/shell/shell.go index 5d562ba653..f224c41c9c 100644 --- a/pkg/minikube/shell/shell.go +++ b/pkg/minikube/shell/shell.go @@ -24,12 +24,21 @@ import ( "io" "os" "runtime" - "strings" "text/template" "github.com/docker/machine/libmachine/shell" + + "k8s.io/minikube/pkg/minikube/constants" ) +var unsetEnvTmpl = "{{ $root := .}}" + + "{{ range .Unset }}" + + "{{ $root.UnsetPrefix }}{{ . }}{{ $root.UnsetDelimiter }}{{ $root.UnsetSuffix }}" + + "{{ end }}" + + "{{ range .Set }}" + + "{{ $root.SetPrefix }}{{ .Env }}{{ $root.SetDelimiter }}{{ .Value }}{{ $root.SetSuffix }}" + + "{{ end }}" + // Config represents the shell config type Config struct { Prefix string @@ -107,7 +116,7 @@ REM @FOR /f "tokens=*" %%i IN ('%s') DO @%%i suffix: "\"\n", delimiter: "=\"", unsetPrefix: "unset ", - unsetSuffix: "\n", + unsetSuffix: ";\n", unsetDelimiter: "", usageHint: func(s ...interface{}) string { return fmt.Sprintf(` @@ -181,24 +190,46 @@ func SetScript(ec EnvConfig, w io.Writer, envTmpl string, data interface{}) erro return tmpl.Execute(w, data) } +type unsetConfigItem struct { + Env, Value string +} +type unsetConfig struct { + Set []unsetConfigItem + Unset []string + SetPrefix string + SetDelimiter string + SetSuffix string + UnsetPrefix string + UnsetDelimiter string + UnsetSuffix string +} + // UnsetScript writes out a shell-compatible unset script func UnsetScript(ec EnvConfig, w io.Writer, vars []string) error { - var sb strings.Builder shellCfg := ec.getShell() - pfx, sfx, delim := shellCfg.unsetPrefix, shellCfg.unsetSuffix, shellCfg.unsetDelimiter - switch ec.Shell { - case "cmd", "emacs", "fish": - break - case "powershell": - vars = []string{strings.Join(vars, " Env:\\\\")} - default: - vars = []string{strings.Join(vars, " ")} + cfg := unsetConfig{ + SetPrefix: shellCfg.prefix, + SetDelimiter: shellCfg.delimiter, + SetSuffix: shellCfg.suffix, + UnsetPrefix: shellCfg.unsetPrefix, + UnsetDelimiter: shellCfg.unsetDelimiter, + UnsetSuffix: shellCfg.unsetSuffix, } - for _, v := range vars { - if _, err := sb.WriteString(fmt.Sprintf("%s%s%s%s", pfx, v, delim, sfx)); err != nil { - return err + var tempUnset []string + for _, env := range vars { + exEnv := constants.MinikubeExistingPrefix + env + if v := os.Getenv(exEnv); v == "" { + cfg.Unset = append(cfg.Unset, env) + } else { + cfg.Set = append(cfg.Set, unsetConfigItem{ + Env: env, + Value: v, + }) + tempUnset = append(tempUnset, exEnv) } } - _, err := w.Write([]byte(sb.String())) - return err + cfg.Unset = append(cfg.Unset, tempUnset...) + + tmpl := template.Must(template.New("unsetEnv").Parse(unsetEnvTmpl)) + return tmpl.Execute(w, &cfg) } diff --git a/pkg/minikube/shell/shell_test.go b/pkg/minikube/shell/shell_test.go index 43dcdcaa71..cf8de26c26 100644 --- a/pkg/minikube/shell/shell_test.go +++ b/pkg/minikube/shell/shell_test.go @@ -87,16 +87,19 @@ func TestUnsetScript(t *testing.T) { ec EnvConfig expected string }{ - {[]string{"baz", "bar"}, EnvConfig{""}, `unset baz bar`}, - {[]string{"baz", "bar"}, EnvConfig{"bash"}, `unset baz bar`}, - {[]string{"baz", "bar"}, EnvConfig{"powershell"}, `Remove-Item Env:\\baz Env:\\bar`}, + {[]string{"baz", "bar"}, EnvConfig{""}, `unset baz; +unset bar;`}, + {[]string{"baz", "bar"}, EnvConfig{"bash"}, `unset baz; +unset bar;`}, + {[]string{"baz", "bar"}, EnvConfig{"powershell"}, `Remove-Item Env:\\baz +Remove-Item Env:\\bar`}, {[]string{"baz", "bar"}, EnvConfig{"cmd"}, `SET baz= SET bar=`}, {[]string{"baz", "bar"}, EnvConfig{"fish"}, `set -e baz; set -e bar;`}, {[]string{"baz", "bar"}, EnvConfig{"emacs"}, `(setenv "baz" nil) (setenv "bar" nil)`}, - {[]string{"baz", "bar"}, EnvConfig{"none"}, `baz bar`}, + {[]string{"baz", "bar"}, EnvConfig{"none"}, "baz\nbar"}, } for _, tc := range testCases { tc := tc From 34a2dc76864986587b4b837d207d44aecad4aac8 Mon Sep 17 00:00:00 2001 From: Ilya Zuyev Date: Thu, 22 Oct 2020 13:37:05 -0700 Subject: [PATCH 03/18] Delete test file --- pkg/drivers/kic/oci/test | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 pkg/drivers/kic/oci/test diff --git a/pkg/drivers/kic/oci/test b/pkg/drivers/kic/oci/test deleted file mode 100644 index e69de29bb2..0000000000 From eaa9ecfd31c064453d0e75e14d2b4e102e179253 Mon Sep 17 00:00:00 2001 From: Ilya Zuyev Date: Thu, 22 Oct 2020 13:41:01 -0700 Subject: [PATCH 04/18] Experiments --- pkg/drivers/kic/kic.go | 10 ++++++++++ pkg/drivers/kic/oci/network.go | 4 ++-- pkg/drivers/kic/oci/oci.go | 3 ++- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/pkg/drivers/kic/kic.go b/pkg/drivers/kic/kic.go index 0f13b64085..ecca7c3aa4 100644 --- a/pkg/drivers/kic/kic.go +++ b/pkg/drivers/kic/kic.go @@ -19,6 +19,8 @@ package kic import ( "fmt" "net" + "net/url" + "os" "os/exec" "strconv" "strings" @@ -230,6 +232,14 @@ func (d *Driver) GetExternalIP() (string, error) { // GetSSHHostname returns hostname for use with ssh func (d *Driver) GetSSHHostname() (string, error) { + // TODO + if dh := os.Getenv(constants.DockerHostEnv); dh != "" { + if u, err := url.Parse(dh); err == nil { + if u.Host != "" { + return u.Hostname(), nil + } + } + } return oci.DefaultBindIPV4, nil } diff --git a/pkg/drivers/kic/oci/network.go b/pkg/drivers/kic/oci/network.go index 274f01917d..b7938808ec 100644 --- a/pkg/drivers/kic/oci/network.go +++ b/pkg/drivers/kic/oci/network.go @@ -164,6 +164,6 @@ func dockerContainerIP(name string) (string, string, error) { if len(ips) != 2 { return "", "", errors.Errorf("container addresses should have 2 values, got %d values: %+v", len(ips), ips) } - - return ips[0], ips[1], nil + return "34.72.91.111", "", nil + //return ips[0], ips[1], nil } diff --git a/pkg/drivers/kic/oci/oci.go b/pkg/drivers/kic/oci/oci.go index ced79caf08..638dd07fb9 100644 --- a/pkg/drivers/kic/oci/oci.go +++ b/pkg/drivers/kic/oci/oci.go @@ -469,7 +469,8 @@ func generatePortMappings(portMappings ...PortMapping) []string { for _, pm := range portMappings { // let docker pick a host port by leaving it as :: // example --publish=127.0.0.17::8443 will get a random host port for 8443 - publish := fmt.Sprintf("--publish=%s::%d", pm.ListenAddress, pm.ContainerPort) + publish := fmt.Sprintf("--publish=%s::%d", "", pm.ContainerPort) + //publish := fmt.Sprintf("--publish=%s::%d", pm.ListenAddress, pm.ContainerPort) result = append(result, publish) } return result From 89bf9ac01c601db259262bf805543ea5ad44086d Mon Sep 17 00:00:00 2001 From: Ilya Zuyev Date: Thu, 22 Oct 2020 22:10:07 -0700 Subject: [PATCH 05/18] Update certs and API endpoints --- pkg/drivers/kic/kic.go | 27 +++++++++++---------------- pkg/drivers/kic/oci/cli_runner.go | 2 ++ pkg/drivers/kic/oci/network.go | 3 +-- pkg/drivers/kic/oci/oci.go | 19 +++++++++++++++++-- pkg/minikube/bootstrapper/certs.go | 2 +- pkg/minikube/driver/endpoint.go | 3 ++- 6 files changed, 34 insertions(+), 22 deletions(-) diff --git a/pkg/drivers/kic/kic.go b/pkg/drivers/kic/kic.go index ecca7c3aa4..55c79a308c 100644 --- a/pkg/drivers/kic/kic.go +++ b/pkg/drivers/kic/kic.go @@ -19,7 +19,6 @@ package kic import ( "fmt" "net" - "net/url" "os" "os/exec" "strconv" @@ -32,6 +31,7 @@ import ( "github.com/docker/machine/libmachine/state" "github.com/pkg/errors" "k8s.io/klog/v2" + pkgdrivers "k8s.io/minikube/pkg/drivers" "k8s.io/minikube/pkg/drivers/kic/oci" "k8s.io/minikube/pkg/minikube/assets" @@ -95,23 +95,26 @@ func (d *Driver) Create() error { klog.Infof("calculated static IP %q for the %q container", ip.String(), d.NodeConfig.MachineName) params.IP = ip.String() } - + listAddr := oci.DefaultBindIPV4 + if d.DriverName() == oci.Docker && os.Getenv(constants.DockerHostEnv) != "" { + listAddr = "0.0.0.0" + } // control plane specific options params.PortMappings = append(params.PortMappings, oci.PortMapping{ - ListenAddress: oci.DefaultBindIPV4, + ListenAddress: listAddr, ContainerPort: int32(params.APIServerPort), }, oci.PortMapping{ - ListenAddress: oci.DefaultBindIPV4, + ListenAddress: listAddr, ContainerPort: constants.SSHPort, }, oci.PortMapping{ - ListenAddress: oci.DefaultBindIPV4, + ListenAddress: listAddr, ContainerPort: constants.DockerDaemonPort, }, oci.PortMapping{ - ListenAddress: oci.DefaultBindIPV4, + ListenAddress: listAddr, ContainerPort: constants.RegistryAddonPort, }, ) @@ -227,20 +230,12 @@ func (d *Driver) GetIP() (string, error) { // GetExternalIP returns an IP which is accessible from outside func (d *Driver) GetExternalIP() (string, error) { - return oci.DefaultBindIPV4, nil + return oci.DockerHost(d.DriverName()), nil } // GetSSHHostname returns hostname for use with ssh func (d *Driver) GetSSHHostname() (string, error) { - // TODO - if dh := os.Getenv(constants.DockerHostEnv); dh != "" { - if u, err := url.Parse(dh); err == nil { - if u.Host != "" { - return u.Hostname(), nil - } - } - } - return oci.DefaultBindIPV4, nil + return oci.DockerHost(d.DriverName()), nil } // GetSSHPort returns port for use with ssh diff --git a/pkg/drivers/kic/oci/cli_runner.go b/pkg/drivers/kic/oci/cli_runner.go index eb7b5abdb4..d2d18d32ba 100644 --- a/pkg/drivers/kic/oci/cli_runner.go +++ b/pkg/drivers/kic/oci/cli_runner.go @@ -27,6 +27,7 @@ import ( "time" "k8s.io/klog/v2" + "k8s.io/minikube/pkg/minikube/out" "k8s.io/minikube/pkg/minikube/style" ) @@ -128,6 +129,7 @@ func runCmd(cmd *exec.Cmd, warnSlow ...bool) (*RunResult, error) { cmd.Stderr = errb start := time.Now() + klog.Infof("Running %v", cmd.Args) err := cmd.Run() elapsed := time.Since(start) if warn { diff --git a/pkg/drivers/kic/oci/network.go b/pkg/drivers/kic/oci/network.go index b7938808ec..de4e4414da 100644 --- a/pkg/drivers/kic/oci/network.go +++ b/pkg/drivers/kic/oci/network.go @@ -164,6 +164,5 @@ func dockerContainerIP(name string) (string, string, error) { if len(ips) != 2 { return "", "", errors.Errorf("container addresses should have 2 values, got %d values: %+v", len(ips), ips) } - return "34.72.91.111", "", nil - //return ips[0], ips[1], nil + return ips[0], ips[1], nil } diff --git a/pkg/drivers/kic/oci/oci.go b/pkg/drivers/kic/oci/oci.go index 638dd07fb9..25fd842eb3 100644 --- a/pkg/drivers/kic/oci/oci.go +++ b/pkg/drivers/kic/oci/oci.go @@ -21,6 +21,7 @@ import ( "bytes" "context" "fmt" + "net/url" "os" "os/exec" "runtime" @@ -469,8 +470,7 @@ func generatePortMappings(portMappings ...PortMapping) []string { for _, pm := range portMappings { // let docker pick a host port by leaving it as :: // example --publish=127.0.0.17::8443 will get a random host port for 8443 - publish := fmt.Sprintf("--publish=%s::%d", "", pm.ContainerPort) - //publish := fmt.Sprintf("--publish=%s::%d", pm.ListenAddress, pm.ContainerPort) + publish := fmt.Sprintf("--publish=%s::%d", pm.ListenAddress, pm.ContainerPort) result = append(result, publish) } return result @@ -639,3 +639,18 @@ func iptablesFileExists(ociBin string, nameOrID string) bool { } return true } + +func DockerHost(driver string) string { + if driver != Docker { + return DefaultBindIPV4 + } + + if dh := os.Getenv(constants.DockerHostEnv); dh != "" { + if u, err := url.Parse(dh); err == nil { + if u.Host != "" { + return u.Hostname() + } + } + } + return DefaultBindIPV4 +} diff --git a/pkg/minikube/bootstrapper/certs.go b/pkg/minikube/bootstrapper/certs.go index f5181b8ce7..8d7045db17 100644 --- a/pkg/minikube/bootstrapper/certs.go +++ b/pkg/minikube/bootstrapper/certs.go @@ -196,7 +196,7 @@ func generateProfileCerts(k8s config.KubernetesConfig, n config.Node, ccs CACert apiServerIPs := append( k8s.APIServerIPs, - []net.IP{net.ParseIP(n.IP), serviceIP, net.ParseIP(oci.DefaultBindIPV4), net.ParseIP("10.0.0.1")}...) + []net.IP{net.ParseIP(n.IP), serviceIP, net.ParseIP(oci.DockerHost(k8s.ContainerRuntime)), net.ParseIP("10.0.0.1")}...) apiServerNames := append(k8s.APIServerNames, k8s.APIServerName, constants.ControlPlaneAlias) apiServerAlternateNames := append( apiServerNames, diff --git a/pkg/minikube/driver/endpoint.go b/pkg/minikube/driver/endpoint.go index 1f8ac92ff9..45f7352244 100644 --- a/pkg/minikube/driver/endpoint.go +++ b/pkg/minikube/driver/endpoint.go @@ -29,7 +29,8 @@ import ( func ControlPlaneEndpoint(cc *config.ClusterConfig, cp *config.Node, driverName string) (string, net.IP, int, error) { if NeedsPortForward(driverName) { port, err := oci.ForwardedPort(cc.Driver, cc.Name, cp.Port) - hostname := oci.DefaultBindIPV4 + hostname := oci.DockerHost(driverName) + ip := net.ParseIP(hostname) if ip == nil { return hostname, ip, port, fmt.Errorf("failed to parse ip for %q", hostname) From 6e027532041adbbf7ef9ee8d7034c561b81cb139 Mon Sep 17 00:00:00 2001 From: Ilya Zuyev Date: Thu, 22 Oct 2020 22:41:58 -0700 Subject: [PATCH 06/18] Forward ports for docker machine on Linux --- pkg/drivers/kic/oci/oci.go | 13 ++++++++++++- pkg/minikube/driver/driver.go | 8 +++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/pkg/drivers/kic/oci/oci.go b/pkg/drivers/kic/oci/oci.go index 25fd842eb3..9fffd43e5b 100644 --- a/pkg/drivers/kic/oci/oci.go +++ b/pkg/drivers/kic/oci/oci.go @@ -640,11 +640,22 @@ func iptablesFileExists(ociBin string, nameOrID string) bool { return true } +// TODO: comment func DockerHost(driver string) string { if driver != Docker { return DefaultBindIPV4 } + if v := DockerMachineHost(driver); v != "" { + return v + } + return DefaultBindIPV4 +} +// TODO: comment +func DockerMachineHost(driver string) string { + if driver != Docker { + return "" + } if dh := os.Getenv(constants.DockerHostEnv); dh != "" { if u, err := url.Parse(dh); err == nil { if u.Host != "" { @@ -652,5 +663,5 @@ func DockerHost(driver string) string { } } } - return DefaultBindIPV4 + return "" } diff --git a/pkg/minikube/driver/driver.go b/pkg/minikube/driver/driver.go index 8d876f1c83..4e30faea08 100644 --- a/pkg/minikube/driver/driver.go +++ b/pkg/minikube/driver/driver.go @@ -148,8 +148,14 @@ func NeedsRoot(name string) bool { // NeedsPortForward returns true if driver is unable provide direct IP connectivity func NeedsPortForward(name string) bool { + if !IsKIC(name) { + return false + } + if oci.DockerMachineHost(name) != "" { + return true + } // Docker for Desktop - return IsKIC(name) && (runtime.GOOS == "darwin" || runtime.GOOS == "windows" || IsMicrosoftWSL()) + return (runtime.GOOS == "darwin" || runtime.GOOS == "windows" || IsMicrosoftWSL()) } // IsMicrosoftWSL will return true if process is running in WSL in windows From 6b3bf1fee5472252e343ef776dbdf0345185f3d2 Mon Sep 17 00:00:00 2001 From: Ilya Zuyev Date: Fri, 23 Oct 2020 15:05:31 -0700 Subject: [PATCH 07/18] Add docker machine to cert hosts --- pkg/provision/provision.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/pkg/provision/provision.go b/pkg/provision/provision.go index fe6e05c93d..713f94ea4a 100644 --- a/pkg/provision/provision.go +++ b/pkg/provision/provision.go @@ -36,6 +36,7 @@ import ( "github.com/docker/machine/libmachine/swarm" "github.com/pkg/errors" "k8s.io/klog/v2" + "k8s.io/minikube/pkg/minikube/assets" "k8s.io/minikube/pkg/minikube/command" "k8s.io/minikube/pkg/minikube/config" @@ -96,12 +97,17 @@ func configureAuth(p miniProvisioner) error { return errors.Wrap(err, "error getting ip during provisioning") } + hostIP, err := driver.GetSSHHostname() + if err != nil { + return errors.Wrap(err, "error getting ssh hostname during provisioning") + } + if err := copyHostCerts(authOptions); err != nil { return err } // The Host IP is always added to the certificate's SANs list - hosts := append(authOptions.ServerCertSANs, ip, "localhost", "127.0.0.1", "minikube", machineName) + hosts := append(authOptions.ServerCertSANs, ip, hostIP, "localhost", "127.0.0.1", "minikube", machineName) klog.Infof("generating server cert: %s ca-key=%s private-key=%s org=%s san=%s", authOptions.ServerCertPath, authOptions.CaCertPath, From acbdafa4e19f22d7d4f2757eea4ee2ef38131c32 Mon Sep 17 00:00:00 2001 From: Ilya Zuyev Date: Tue, 27 Oct 2020 16:07:50 -0700 Subject: [PATCH 08/18] Fix func naming --- pkg/drivers/kic/kic.go | 10 +++++---- pkg/drivers/kic/oci/oci.go | 35 ++++++++++++++++++------------ pkg/minikube/bootstrapper/certs.go | 2 +- pkg/minikube/driver/driver.go | 4 ++-- pkg/minikube/driver/endpoint.go | 2 +- 5 files changed, 31 insertions(+), 22 deletions(-) diff --git a/pkg/drivers/kic/kic.go b/pkg/drivers/kic/kic.go index 55c79a308c..189ab4cfb9 100644 --- a/pkg/drivers/kic/kic.go +++ b/pkg/drivers/kic/kic.go @@ -19,7 +19,6 @@ package kic import ( "fmt" "net" - "os" "os/exec" "strconv" "strings" @@ -95,10 +94,13 @@ func (d *Driver) Create() error { klog.Infof("calculated static IP %q for the %q container", ip.String(), d.NodeConfig.MachineName) params.IP = ip.String() } + drv := d.DriverName() listAddr := oci.DefaultBindIPV4 - if d.DriverName() == oci.Docker && os.Getenv(constants.DockerHostEnv) != "" { + if oci.IsExternalRuntimeHost(drv) { + klog.Infof("Listening 0.0.0.0 on external docker host %v", oci.RuntimeHost(drv)) listAddr = "0.0.0.0" } + // control plane specific options params.PortMappings = append(params.PortMappings, oci.PortMapping{ @@ -230,12 +232,12 @@ func (d *Driver) GetIP() (string, error) { // GetExternalIP returns an IP which is accessible from outside func (d *Driver) GetExternalIP() (string, error) { - return oci.DockerHost(d.DriverName()), nil + return oci.RuntimeHost(d.DriverName()), nil } // GetSSHHostname returns hostname for use with ssh func (d *Driver) GetSSHHostname() (string, error) { - return oci.DockerHost(d.DriverName()), nil + return oci.RuntimeHost(d.DriverName()), nil } // GetSSHPort returns port for use with ssh diff --git a/pkg/drivers/kic/oci/oci.go b/pkg/drivers/kic/oci/oci.go index 9fffd43e5b..a30496daf2 100644 --- a/pkg/drivers/kic/oci/oci.go +++ b/pkg/drivers/kic/oci/oci.go @@ -640,22 +640,14 @@ func iptablesFileExists(ociBin string, nameOrID string) bool { return true } -// TODO: comment -func DockerHost(driver string) string { +// RuntimeHost returns the ip/hostname where OCI daemon service for driver is running +// For Podman it's always DefaultBindIPV4 +// For Docker return the host part of DOCKER_HOST environment variable if set +// or DefaultBindIPV4 otherwise +func RuntimeHost(driver string) string { if driver != Docker { return DefaultBindIPV4 } - if v := DockerMachineHost(driver); v != "" { - return v - } - return DefaultBindIPV4 -} - -// TODO: comment -func DockerMachineHost(driver string) string { - if driver != Docker { - return "" - } if dh := os.Getenv(constants.DockerHostEnv); dh != "" { if u, err := url.Parse(dh); err == nil { if u.Host != "" { @@ -663,5 +655,20 @@ func DockerMachineHost(driver string) string { } } } - return "" + return DefaultBindIPV4 +} + +// IsExternalRuntimeHost returns whether or not the OCI runtime is running on an external/virtual host +// For Podman driver it's always false for now +// For Docker driver return true if DOCKER_HOST is set to a URI, and the URI contains a host item +func IsExternalRuntimeHost(driver string) bool { + if driver != Docker { + return false + } + if dh := os.Getenv(constants.DockerHostEnv); dh != "" { + if u, err := url.Parse(dh); err == nil { + return u.Host != "" + } + } + return false } diff --git a/pkg/minikube/bootstrapper/certs.go b/pkg/minikube/bootstrapper/certs.go index 8d7045db17..8caa899353 100644 --- a/pkg/minikube/bootstrapper/certs.go +++ b/pkg/minikube/bootstrapper/certs.go @@ -196,7 +196,7 @@ func generateProfileCerts(k8s config.KubernetesConfig, n config.Node, ccs CACert apiServerIPs := append( k8s.APIServerIPs, - []net.IP{net.ParseIP(n.IP), serviceIP, net.ParseIP(oci.DockerHost(k8s.ContainerRuntime)), net.ParseIP("10.0.0.1")}...) + []net.IP{net.ParseIP(n.IP), serviceIP, net.ParseIP(oci.RuntimeHost(k8s.ContainerRuntime)), net.ParseIP("10.0.0.1")}...) apiServerNames := append(k8s.APIServerNames, k8s.APIServerName, constants.ControlPlaneAlias) apiServerAlternateNames := append( apiServerNames, diff --git a/pkg/minikube/driver/driver.go b/pkg/minikube/driver/driver.go index 4e30faea08..a3ea7d1ec2 100644 --- a/pkg/minikube/driver/driver.go +++ b/pkg/minikube/driver/driver.go @@ -151,11 +151,11 @@ func NeedsPortForward(name string) bool { if !IsKIC(name) { return false } - if oci.DockerMachineHost(name) != "" { + if oci.IsExternalRuntimeHost(name) { return true } // Docker for Desktop - return (runtime.GOOS == "darwin" || runtime.GOOS == "windows" || IsMicrosoftWSL()) + return runtime.GOOS == "darwin" || runtime.GOOS == "windows" || IsMicrosoftWSL() } // IsMicrosoftWSL will return true if process is running in WSL in windows diff --git a/pkg/minikube/driver/endpoint.go b/pkg/minikube/driver/endpoint.go index 45f7352244..c2fd03e0dc 100644 --- a/pkg/minikube/driver/endpoint.go +++ b/pkg/minikube/driver/endpoint.go @@ -29,7 +29,7 @@ import ( func ControlPlaneEndpoint(cc *config.ClusterConfig, cp *config.Node, driverName string) (string, net.IP, int, error) { if NeedsPortForward(driverName) { port, err := oci.ForwardedPort(cc.Driver, cc.Name, cp.Port) - hostname := oci.DockerHost(driverName) + hostname := oci.RuntimeHost(driverName) ip := net.ParseIP(hostname) if ip == nil { From e0b4957fcc28ac28aeb3083793b26ba9661c44e1 Mon Sep 17 00:00:00 2001 From: Ilya Zuyev Date: Tue, 27 Oct 2020 16:48:20 -0700 Subject: [PATCH 09/18] Fix docker-env -u shell detection --- cmd/minikube/cmd/docker-env.go | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/cmd/minikube/cmd/docker-env.go b/cmd/minikube/cmd/docker-env.go index f27513a471..aa24fd96b7 100644 --- a/cmd/minikube/cmd/docker-env.go +++ b/cmd/minikube/cmd/docker-env.go @@ -168,8 +168,13 @@ var dockerEnvCmd = &cobra.Command{ Short: "Configure environment to use minikube's Docker daemon", Long: `Sets up docker env variables; similar to '$(docker-machine env)'.`, Run: func(cmd *cobra.Command, args []string) { + + shl, err := shell.Detect() + if err != nil { + exit.Error(reason.InternalShellDetect, "Error detecting shell", err) + } sh := shell.EnvConfig{ - Shell: shell.ForceShell, + Shell: shl, } if dockerUnset { @@ -201,7 +206,6 @@ var dockerEnvCmd = &cobra.Command{ mustRestartDocker(cname, co.CP.Runner) } - var err error port := constants.DockerDaemonPort if driver.NeedsPortForward(driverName) { port, err = oci.ForwardedPort(driverName, cname, port) @@ -220,13 +224,6 @@ var dockerEnvCmd = &cobra.Command{ noProxy: noProxy, } - if ec.Shell == "" { - ec.Shell, err = shell.Detect() - if err != nil { - exit.Error(reason.InternalShellDetect, "Error detecting shell", err) - } - } - dockerPath, err := exec.LookPath("docker") if err != nil { klog.Warningf("Unable to find docker in path - skipping connectivity check: %v", err) From 43a4b62b68167b053546387bbc87169ae64b026e Mon Sep 17 00:00:00 2001 From: Ilya Zuyev Date: Fri, 30 Oct 2020 13:27:29 -0700 Subject: [PATCH 10/18] Remove excessive logging --- pkg/drivers/kic/oci/cli_runner.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/drivers/kic/oci/cli_runner.go b/pkg/drivers/kic/oci/cli_runner.go index d2d18d32ba..b141305f7d 100644 --- a/pkg/drivers/kic/oci/cli_runner.go +++ b/pkg/drivers/kic/oci/cli_runner.go @@ -129,7 +129,6 @@ func runCmd(cmd *exec.Cmd, warnSlow ...bool) (*RunResult, error) { cmd.Stderr = errb start := time.Now() - klog.Infof("Running %v", cmd.Args) err := cmd.Run() elapsed := time.Since(start) if warn { From 72b710af03673f2f7f65e0c0b8f5f13237e0d2cc Mon Sep 17 00:00:00 2001 From: Ilya Zuyev Date: Fri, 30 Oct 2020 13:41:06 -0700 Subject: [PATCH 11/18] Fix warning about listening to 0.0.0.0 --- pkg/drivers/kic/kic.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/drivers/kic/kic.go b/pkg/drivers/kic/kic.go index 189ab4cfb9..0ffa70804f 100644 --- a/pkg/drivers/kic/kic.go +++ b/pkg/drivers/kic/kic.go @@ -97,7 +97,8 @@ func (d *Driver) Create() error { drv := d.DriverName() listAddr := oci.DefaultBindIPV4 if oci.IsExternalRuntimeHost(drv) { - klog.Infof("Listening 0.0.0.0 on external docker host %v", oci.RuntimeHost(drv)) + out.WarningT("Listening to 0.0.0.0 on external docker host {{.host}}. Please be advised", + out.V{"host": oci.RuntimeHost(drv)}) listAddr = "0.0.0.0" } From c97cd912f17559f1e505beb76f1b61c788b74b58 Mon Sep 17 00:00:00 2001 From: Ilya Zuyev Date: Fri, 30 Oct 2020 13:44:34 -0700 Subject: [PATCH 12/18] Rename oci.RuntimeHost() -> oci.DaemonHost() --- pkg/drivers/kic/kic.go | 8 ++++---- pkg/drivers/kic/oci/oci.go | 8 ++++---- pkg/minikube/bootstrapper/certs.go | 2 +- pkg/minikube/driver/driver.go | 2 +- pkg/minikube/driver/endpoint.go | 2 +- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/pkg/drivers/kic/kic.go b/pkg/drivers/kic/kic.go index 0ffa70804f..4c47a478cf 100644 --- a/pkg/drivers/kic/kic.go +++ b/pkg/drivers/kic/kic.go @@ -96,9 +96,9 @@ func (d *Driver) Create() error { } drv := d.DriverName() listAddr := oci.DefaultBindIPV4 - if oci.IsExternalRuntimeHost(drv) { + if oci.IsExternalDaemonHost(drv) { out.WarningT("Listening to 0.0.0.0 on external docker host {{.host}}. Please be advised", - out.V{"host": oci.RuntimeHost(drv)}) + out.V{"host": oci.DaemonHost(drv)}) listAddr = "0.0.0.0" } @@ -233,12 +233,12 @@ func (d *Driver) GetIP() (string, error) { // GetExternalIP returns an IP which is accessible from outside func (d *Driver) GetExternalIP() (string, error) { - return oci.RuntimeHost(d.DriverName()), nil + return oci.DaemonHost(d.DriverName()), nil } // GetSSHHostname returns hostname for use with ssh func (d *Driver) GetSSHHostname() (string, error) { - return oci.RuntimeHost(d.DriverName()), nil + return oci.DaemonHost(d.DriverName()), nil } // GetSSHPort returns port for use with ssh diff --git a/pkg/drivers/kic/oci/oci.go b/pkg/drivers/kic/oci/oci.go index a30496daf2..087d6224f4 100644 --- a/pkg/drivers/kic/oci/oci.go +++ b/pkg/drivers/kic/oci/oci.go @@ -640,11 +640,11 @@ func iptablesFileExists(ociBin string, nameOrID string) bool { return true } -// RuntimeHost returns the ip/hostname where OCI daemon service for driver is running +// DaemonHost returns the ip/hostname where OCI daemon service for driver is running // For Podman it's always DefaultBindIPV4 // For Docker return the host part of DOCKER_HOST environment variable if set // or DefaultBindIPV4 otherwise -func RuntimeHost(driver string) string { +func DaemonHost(driver string) string { if driver != Docker { return DefaultBindIPV4 } @@ -658,10 +658,10 @@ func RuntimeHost(driver string) string { return DefaultBindIPV4 } -// IsExternalRuntimeHost returns whether or not the OCI runtime is running on an external/virtual host +// 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 Docker driver return true if DOCKER_HOST is set to a URI, and the URI contains a host item -func IsExternalRuntimeHost(driver string) bool { +func IsExternalDaemonHost(driver string) bool { if driver != Docker { return false } diff --git a/pkg/minikube/bootstrapper/certs.go b/pkg/minikube/bootstrapper/certs.go index 8caa899353..37d88c5e2d 100644 --- a/pkg/minikube/bootstrapper/certs.go +++ b/pkg/minikube/bootstrapper/certs.go @@ -196,7 +196,7 @@ func generateProfileCerts(k8s config.KubernetesConfig, n config.Node, ccs CACert apiServerIPs := append( k8s.APIServerIPs, - []net.IP{net.ParseIP(n.IP), serviceIP, net.ParseIP(oci.RuntimeHost(k8s.ContainerRuntime)), net.ParseIP("10.0.0.1")}...) + []net.IP{net.ParseIP(n.IP), serviceIP, net.ParseIP(oci.DaemonHost(k8s.ContainerRuntime)), net.ParseIP("10.0.0.1")}...) apiServerNames := append(k8s.APIServerNames, k8s.APIServerName, constants.ControlPlaneAlias) apiServerAlternateNames := append( apiServerNames, diff --git a/pkg/minikube/driver/driver.go b/pkg/minikube/driver/driver.go index a3ea7d1ec2..7dc53f4aeb 100644 --- a/pkg/minikube/driver/driver.go +++ b/pkg/minikube/driver/driver.go @@ -151,7 +151,7 @@ func NeedsPortForward(name string) bool { if !IsKIC(name) { return false } - if oci.IsExternalRuntimeHost(name) { + if oci.IsExternalDaemonHost(name) { return true } // Docker for Desktop diff --git a/pkg/minikube/driver/endpoint.go b/pkg/minikube/driver/endpoint.go index c2fd03e0dc..e72e10c9fc 100644 --- a/pkg/minikube/driver/endpoint.go +++ b/pkg/minikube/driver/endpoint.go @@ -29,7 +29,7 @@ import ( func ControlPlaneEndpoint(cc *config.ClusterConfig, cp *config.Node, driverName string) (string, net.IP, int, error) { if NeedsPortForward(driverName) { port, err := oci.ForwardedPort(cc.Driver, cc.Name, cp.Port) - hostname := oci.RuntimeHost(driverName) + hostname := oci.DaemonHost(driverName) ip := net.ParseIP(hostname) if ip == nil { From d5a092c3c72d8da05bd43625eadf9fc207c14800 Mon Sep 17 00:00:00 2001 From: Ilya Zuyev Date: Fri, 30 Oct 2020 14:00:52 -0700 Subject: [PATCH 13/18] Add unit tests for oci.DaemonHost() --- pkg/drivers/kic/oci/oci_test.go | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/pkg/drivers/kic/oci/oci_test.go b/pkg/drivers/kic/oci/oci_test.go index 164793537f..cbe6b98cc0 100644 --- a/pkg/drivers/kic/oci/oci_test.go +++ b/pkg/drivers/kic/oci/oci_test.go @@ -70,3 +70,28 @@ func TestPointToHostDockerDaemon(t *testing.T) { } } } + +func TestDaemonHost(t *testing.T) { + tests := []struct { + driver string + dockerHost string + expectedAddr string + expectedExternal bool + }{ + {"", "", "127.0.0.1", false}, + {"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}, + {"_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}, + } + for _, test := range tests { + _ = 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) + } + if v := DaemonHost(test.driver); v != test.expectedAddr { + t.Errorf("invalid oci daemon host. got: %v, want: %v", v, test.expectedAddr) + } + } +} From 996b05bfc10dcd98515c7fd45363383821a79ff6 Mon Sep 17 00:00:00 2001 From: Ilya Zuyev Date: Fri, 30 Oct 2020 14:13:43 -0700 Subject: [PATCH 14/18] Add 127.0.0.1 to ssl cert even if external docker host is used --- pkg/minikube/bootstrapper/certs.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/pkg/minikube/bootstrapper/certs.go b/pkg/minikube/bootstrapper/certs.go index 37d88c5e2d..90fd9870fa 100644 --- a/pkg/minikube/bootstrapper/certs.go +++ b/pkg/minikube/bootstrapper/certs.go @@ -194,9 +194,13 @@ func generateProfileCerts(k8s config.KubernetesConfig, n config.Node, ccs CACert return nil, errors.Wrap(err, "getting service cluster ip") } - apiServerIPs := append( - k8s.APIServerIPs, - []net.IP{net.ParseIP(n.IP), serviceIP, net.ParseIP(oci.DaemonHost(k8s.ContainerRuntime)), net.ParseIP("10.0.0.1")}...) + apiServerIPs := append(k8s.APIServerIPs, + net.ParseIP(n.IP), serviceIP, net.ParseIP(oci.DefaultBindIPV4), net.ParseIP("10.0.0.1")) + + if v := oci.DaemonHost(k8s.ContainerRuntime); v != oci.DefaultBindIPV4 { + apiServerIPs = append(apiServerIPs, net.ParseIP(v)) + } + apiServerNames := append(k8s.APIServerNames, k8s.APIServerName, constants.ControlPlaneAlias) apiServerAlternateNames := append( apiServerNames, From 69c2b016609be48dafc4433e700585bdd8270838 Mon Sep 17 00:00:00 2001 From: Ilya Zuyev Date: Fri, 30 Oct 2020 14:39:59 -0700 Subject: [PATCH 15/18] Fix shell arg parsing --- cmd/minikube/cmd/docker-env.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/cmd/minikube/cmd/docker-env.go b/cmd/minikube/cmd/docker-env.go index aa24fd96b7..414af71490 100644 --- a/cmd/minikube/cmd/docker-env.go +++ b/cmd/minikube/cmd/docker-env.go @@ -168,10 +168,14 @@ var dockerEnvCmd = &cobra.Command{ Short: "Configure environment to use minikube's Docker daemon", Long: `Sets up docker env variables; similar to '$(docker-machine env)'.`, Run: func(cmd *cobra.Command, args []string) { + var err error - shl, err := shell.Detect() - if err != nil { - exit.Error(reason.InternalShellDetect, "Error detecting shell", err) + shl := shell.ForceShell + if shl == "" { + shl, err = shell.Detect() + if err != nil { + exit.Error(reason.InternalShellDetect, "Error detecting shell", err) + } } sh := shell.EnvConfig{ Shell: shl, From e58d5389033ddbb613ed693410e0adc58610b6d6 Mon Sep 17 00:00:00 2001 From: Ilya Zuyev Date: Mon, 9 Nov 2020 12:52:27 -0800 Subject: [PATCH 16/18] Fix linter warning --- pkg/drivers/kic/oci/env.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/drivers/kic/oci/env.go b/pkg/drivers/kic/oci/env.go index 6899016f11..8bedcf4d0c 100644 --- a/pkg/drivers/kic/oci/env.go +++ b/pkg/drivers/kic/oci/env.go @@ -13,6 +13,7 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. */ + package oci import ( From aa6a693142cd062840dc42af3d3c7d66ad026ff1 Mon Sep 17 00:00:00 2001 From: Ilya Zuyev Date: Mon, 9 Nov 2020 13:44:52 -0800 Subject: [PATCH 17/18] Fix unit tests for "[docker|podman]-env --unset" --- cmd/minikube/cmd/docker-env_test.go | 49 +++++++++++++++++++++++------ cmd/minikube/cmd/podman-env_test.go | 3 +- 2 files changed, 42 insertions(+), 10 deletions(-) diff --git a/cmd/minikube/cmd/docker-env_test.go b/cmd/minikube/cmd/docker-env_test.go index aaa1d97f5b..d34e51213e 100644 --- a/cmd/minikube/cmd/docker-env_test.go +++ b/cmd/minikube/cmd/docker-env_test.go @@ -52,7 +52,10 @@ export MINIKUBE_ACTIVE_DOCKERD="dockerdriver" # To point your shell to minikube's docker-daemon, run: # eval $(minikube -p dockerdriver docker-env) `, - `unset DOCKER_TLS_VERIFY DOCKER_HOST DOCKER_CERT_PATH MINIKUBE_ACTIVE_DOCKERD + `unset DOCKER_TLS_VERIFY; +unset DOCKER_HOST; +unset DOCKER_CERT_PATH; +unset MINIKUBE_ACTIVE_DOCKERD; `, }, { @@ -67,7 +70,10 @@ export MINIKUBE_ACTIVE_DOCKERD="bash" # To point your shell to minikube's docker-daemon, run: # eval $(minikube -p bash docker-env) `, - `unset DOCKER_TLS_VERIFY DOCKER_HOST DOCKER_CERT_PATH MINIKUBE_ACTIVE_DOCKERD + `unset DOCKER_TLS_VERIFY; +unset DOCKER_HOST; +unset DOCKER_CERT_PATH; +unset MINIKUBE_ACTIVE_DOCKERD; `, }, { @@ -82,7 +88,10 @@ export MINIKUBE_ACTIVE_DOCKERD="ipv6" # To point your shell to minikube's docker-daemon, run: # eval $(minikube -p ipv6 docker-env) `, - `unset DOCKER_TLS_VERIFY DOCKER_HOST DOCKER_CERT_PATH MINIKUBE_ACTIVE_DOCKERD + `unset DOCKER_TLS_VERIFY; +unset DOCKER_HOST; +unset DOCKER_CERT_PATH; +unset MINIKUBE_ACTIVE_DOCKERD; `, }, { @@ -115,7 +124,10 @@ $Env:MINIKUBE_ACTIVE_DOCKERD = "powershell" # & minikube -p powershell docker-env | Invoke-Expression `, - `Remove-Item Env:\\DOCKER_TLS_VERIFY Env:\\DOCKER_HOST Env:\\DOCKER_CERT_PATH Env:\\MINIKUBE_ACTIVE_DOCKERD + `Remove-Item Env:\\DOCKER_TLS_VERIFY +Remove-Item Env:\\DOCKER_HOST +Remove-Item Env:\\DOCKER_CERT_PATH +Remove-Item Env:\\MINIKUBE_ACTIVE_DOCKERD `, }, { @@ -167,7 +179,11 @@ export NO_PROXY="127.0.0.1" # eval $(minikube -p bash-no-proxy docker-env) `, - `unset DOCKER_TLS_VERIFY DOCKER_HOST DOCKER_CERT_PATH MINIKUBE_ACTIVE_DOCKERD NO_PROXY + `unset DOCKER_TLS_VERIFY; +unset DOCKER_HOST; +unset DOCKER_CERT_PATH; +unset MINIKUBE_ACTIVE_DOCKERD; +unset NO_PROXY; `, }, { @@ -184,7 +200,11 @@ export no_proxy="127.0.0.1" # eval $(minikube -p bash-no-proxy-lower docker-env) `, - `unset DOCKER_TLS_VERIFY DOCKER_HOST DOCKER_CERT_PATH MINIKUBE_ACTIVE_DOCKERD no_proxy + `unset DOCKER_TLS_VERIFY; +unset DOCKER_HOST; +unset DOCKER_CERT_PATH; +unset MINIKUBE_ACTIVE_DOCKERD; +unset no_proxy; `, }, { @@ -200,7 +220,11 @@ $Env:no_proxy = "192.168.0.1" # & minikube -p powershell-no-proxy-idempotent docker-env | Invoke-Expression `, - `Remove-Item Env:\\DOCKER_TLS_VERIFY Env:\\DOCKER_HOST Env:\\DOCKER_CERT_PATH Env:\\MINIKUBE_ACTIVE_DOCKERD Env:\\no_proxy + `Remove-Item Env:\\DOCKER_TLS_VERIFY +Remove-Item Env:\\DOCKER_HOST +Remove-Item Env:\\DOCKER_CERT_PATH +Remove-Item Env:\\MINIKUBE_ACTIVE_DOCKERD +Remove-Item Env:\\no_proxy `, }, { @@ -217,7 +241,11 @@ export NO_PROXY="192.168.0.1,10.0.0.4,127.0.0.1" # eval $(minikube -p sh-no-proxy-add docker-env) `, - `unset DOCKER_TLS_VERIFY DOCKER_HOST DOCKER_CERT_PATH MINIKUBE_ACTIVE_DOCKERD NO_PROXY + `unset DOCKER_TLS_VERIFY; +unset DOCKER_HOST; +unset DOCKER_CERT_PATH; +unset MINIKUBE_ACTIVE_DOCKERD; +unset NO_PROXY; `, }, { @@ -229,7 +257,10 @@ DOCKER_HOST=tcp://127.0.0.1:32842 DOCKER_CERT_PATH=/certs MINIKUBE_ACTIVE_DOCKERD=noneshell `, - `DOCKER_TLS_VERIFY DOCKER_HOST DOCKER_CERT_PATH MINIKUBE_ACTIVE_DOCKERD + `DOCKER_TLS_VERIFY +DOCKER_HOST +DOCKER_CERT_PATH +MINIKUBE_ACTIVE_DOCKERD `, }, } diff --git a/cmd/minikube/cmd/podman-env_test.go b/cmd/minikube/cmd/podman-env_test.go index 088867193a..8cfb7a8641 100644 --- a/cmd/minikube/cmd/podman-env_test.go +++ b/cmd/minikube/cmd/podman-env_test.go @@ -49,7 +49,8 @@ export MINIKUBE_ACTIVE_PODMAN="bash" # To point your shell to minikube's podman service, run: # eval $(minikube -p bash podman-env) `, - `unset PODMAN_VARLINK_BRIDGE MINIKUBE_ACTIVE_PODMAN + `unset PODMAN_VARLINK_BRIDGE; +unset MINIKUBE_ACTIVE_PODMAN; `, }, } From 76c62afddb070350350fb841cc651202fcba4933 Mon Sep 17 00:00:00 2001 From: Ilya Zuyev Date: Mon, 9 Nov 2020 14:21:42 -0800 Subject: [PATCH 18/18] Fix unit tests for "podman-env --unset" --- cmd/minikube/cmd/podman-env_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cmd/minikube/cmd/podman-env_test.go b/cmd/minikube/cmd/podman-env_test.go index e9d35cbffb..7215fe9a1f 100644 --- a/cmd/minikube/cmd/podman-env_test.go +++ b/cmd/minikube/cmd/podman-env_test.go @@ -63,7 +63,9 @@ export MINIKUBE_ACTIVE_PODMAN="bash" # To point your shell to minikube's podman service, run: # eval $(minikube -p bash podman-env) `, - `unset CONTAINER_HOST CONTAINER_SSHKEY MINIKUBE_ACTIVE_PODMAN + `unset CONTAINER_HOST; +unset CONTAINER_SSHKEY; +unset MINIKUBE_ACTIVE_PODMAN; `, }, }