From 2f2a8357a871ce50da18b7679e31980c53a3cc6d Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Mon, 18 May 2020 18:04:45 -0700 Subject: [PATCH 1/4] fix proxy envs not being passed to docker engine --- cmd/minikube/cmd/start.go | 19 ------------------- cmd/minikube/cmd/start_flags.go | 3 ++- pkg/minikube/machine/start.go | 22 +++++++++++++++++++++- pkg/minikube/node/start.go | 3 ++- pkg/minikube/proxy/proxy.go | 21 +++++++++++++++++++++ 5 files changed, 46 insertions(+), 22 deletions(-) diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go index 0d52db44ac6f..3435f7a88634 100644 --- a/cmd/minikube/cmd/start.go +++ b/cmd/minikube/cmd/start.go @@ -56,7 +56,6 @@ import ( "k8s.io/minikube/pkg/minikube/node" "k8s.io/minikube/pkg/minikube/notify" "k8s.io/minikube/pkg/minikube/out" - "k8s.io/minikube/pkg/minikube/proxy" "k8s.io/minikube/pkg/minikube/registry" "k8s.io/minikube/pkg/minikube/translate" "k8s.io/minikube/pkg/util" @@ -909,24 +908,6 @@ func createNode(cc config.ClusterConfig, kubeNodeName string, existing *config.C return cc, cp, nil } -// setDockerProxy sets the proxy environment variables in the docker environment. -func setDockerProxy() { - for _, k := range proxy.EnvVars { - if v := os.Getenv(k); v != "" { - // convert https_proxy to HTTPS_PROXY for linux - // TODO (@medyagh): if user has both http_proxy & HTTPS_PROXY set merge them. - k = strings.ToUpper(k) - if k == "HTTP_PROXY" || k == "HTTPS_PROXY" { - if strings.HasPrefix(v, "localhost") || strings.HasPrefix(v, "127.0") { - out.WarningT("Not passing {{.name}}={{.value}} to docker env.", out.V{"name": k, "value": v}) - continue - } - } - config.DockerEnv = append(config.DockerEnv, fmt.Sprintf("%s=%s", k, v)) - } - } -} - // autoSetDriverOptions sets the options needed for specific driver automatically. func autoSetDriverOptions(cmd *cobra.Command, drvName string) (err error) { err = nil diff --git a/cmd/minikube/cmd/start_flags.go b/cmd/minikube/cmd/start_flags.go index 582f85021c51..de41ab180aa3 100644 --- a/cmd/minikube/cmd/start_flags.go +++ b/cmd/minikube/cmd/start_flags.go @@ -36,6 +36,7 @@ import ( "k8s.io/minikube/pkg/minikube/driver" "k8s.io/minikube/pkg/minikube/exit" "k8s.io/minikube/pkg/minikube/out" + "k8s.io/minikube/pkg/minikube/proxy" pkgutil "k8s.io/minikube/pkg/util" "k8s.io/minikube/pkg/version" ) @@ -341,7 +342,7 @@ func generateClusterConfig(cmd *cobra.Command, existing *config.ClusterConfig, k // Feed Docker our host proxy environment by default, so that it can pull images // doing this for both new config and existing, in case proxy changed since previous start if _, ok := r.(*cruntime.Docker); ok && !cmd.Flags().Changed("docker-env") { - setDockerProxy() + proxy.SetDockerEnv() } var kubeNodeName string diff --git a/pkg/minikube/machine/start.go b/pkg/minikube/machine/start.go index e44ad685fd18..be0d1f84f8bd 100644 --- a/pkg/minikube/machine/start.go +++ b/pkg/minikube/machine/start.go @@ -39,6 +39,7 @@ import ( "k8s.io/minikube/pkg/minikube/driver" "k8s.io/minikube/pkg/minikube/localpath" "k8s.io/minikube/pkg/minikube/out" + "k8s.io/minikube/pkg/minikube/proxy" "k8s.io/minikube/pkg/minikube/registry" "k8s.io/minikube/pkg/minikube/vmpath" "k8s.io/minikube/pkg/util/lock" @@ -89,9 +90,28 @@ func StartHost(api libmachine.API, cfg config.ClusterConfig, n config.Node) (*ho return h, exists, err } +// engineOptions returns docker engine options for the dockerd running inside minikube func engineOptions(cfg config.ClusterConfig) *engine.Options { + // get docker env from user's proxy settings + dockerEnv := proxy.SetDockerEnv() + // get docker env from user specifiec config + dockerEnv = append(dockerEnv, cfg.DockerEnv...) + + // remove duplicates + seen := map[string]bool{} + uniqueEnvs := []string{} + for e := range dockerEnv { + if !seen[dockerEnv[e]] { + seen[dockerEnv[e]] = true + uniqueEnvs = append(uniqueEnvs, dockerEnv[e]) + } + } + + // config.DockerEnv is a global so we update that one too + config.DockerEnv = uniqueEnvs + o := engine.Options{ - Env: cfg.DockerEnv, + Env: uniqueEnvs, InsecureRegistry: append([]string{constants.DefaultServiceCIDR}, cfg.InsecureRegistry...), RegistryMirror: cfg.RegistryMirror, ArbitraryFlags: cfg.DockerOpt, diff --git a/pkg/minikube/node/start.go b/pkg/minikube/node/start.go index e8b90a66f007..24b0fc3c3886 100644 --- a/pkg/minikube/node/start.go +++ b/pkg/minikube/node/start.go @@ -410,7 +410,8 @@ func validateNetwork(h *host.Host, r command.Runner, imageRepository string) (st ipExcluded := proxy.IsIPExcluded(ip) // Skip warning if minikube ip is already in NO_PROXY k = strings.ToUpper(k) // for http_proxy & https_proxy if (k == "HTTP_PROXY" || k == "HTTPS_PROXY") && !ipExcluded && !warnedOnce { - out.WarningT("You appear to be using a proxy, but your NO_PROXY environment does not include the minikube IP ({{.ip_address}}). Please see {{.documentation_url}} for more details", out.V{"ip_address": ip, "documentation_url": "https://minikube.sigs.k8s.io/docs/handbook/vpn_and_proxy/"}) + out.WarningT("You appear to be using a proxy, but your NO_PROXY environment does not include the minikube IP ({{.ip_address}}).", out.V{"ip_address": ip}) + out.T(out.Documentation, "Please see {{.documentation_url}} for more details", out.V{"documentation_url": "https://minikube.sigs.k8s.io/docs/handbook/vpn_and_proxy/"}) warnedOnce = true } } diff --git a/pkg/minikube/proxy/proxy.go b/pkg/minikube/proxy/proxy.go index c6a539a8a38a..0c95f2325843 100644 --- a/pkg/minikube/proxy/proxy.go +++ b/pkg/minikube/proxy/proxy.go @@ -26,6 +26,8 @@ import ( "github.com/golang/glog" "github.com/pkg/errors" "k8s.io/client-go/rest" + "k8s.io/minikube/pkg/minikube/config" + "k8s.io/minikube/pkg/minikube/out" ) // EnvVars are variables we plumb through to the underlying container runtime @@ -149,3 +151,22 @@ func UpdateTransport(cfg *rest.Config) *rest.Config { } return cfg } + +// SetDockerEnv sets the proxy environment variables in the docker environment. +func SetDockerEnv() []string { + for _, k := range EnvVars { + if v := os.Getenv(k); v != "" { + // convert https_proxy to HTTPS_PROXY for linux + // TODO (@medyagh): if user has both http_proxy & HTTPS_PROXY set merge them. + k = strings.ToUpper(k) + if k == "HTTP_PROXY" || k == "HTTPS_PROXY" { + if strings.HasPrefix(v, "localhost") || strings.HasPrefix(v, "127.0") { + out.WarningT("Not passing {{.name}}={{.value}} to docker env.", out.V{"name": k, "value": v}) + continue + } + } + config.DockerEnv = append(config.DockerEnv, fmt.Sprintf("%s=%s", k, v)) + } + } + return config.DockerEnv +} From 80d1c8741d56834d812de016dc9d2f924341149b Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Mon, 18 May 2020 18:17:45 -0700 Subject: [PATCH 2/4] improve unit test log --- pkg/minikube/machine/cluster_test.go | 2 +- pkg/minikube/machine/start.go | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/pkg/minikube/machine/cluster_test.go b/pkg/minikube/machine/cluster_test.go index 3dfdc303e24a..3adbcf4a34f7 100644 --- a/pkg/minikube/machine/cluster_test.go +++ b/pkg/minikube/machine/cluster_test.go @@ -290,7 +290,7 @@ func TestStartHostConfig(t *testing.T) { for i := range h.HostOptions.EngineOptions.Env { if h.HostOptions.EngineOptions.Env[i] != cfg.DockerEnv[i] { - t.Fatal("Docker env variables were not set!") + t.Fatalf("Docker env variables were not set! got %+v but want %+v",h.HostOptions.EngineOptions.Env,cfg.DockerEnv) } } diff --git a/pkg/minikube/machine/start.go b/pkg/minikube/machine/start.go index be0d1f84f8bd..fc8d7d541da0 100644 --- a/pkg/minikube/machine/start.go +++ b/pkg/minikube/machine/start.go @@ -107,9 +107,6 @@ func engineOptions(cfg config.ClusterConfig) *engine.Options { } } - // config.DockerEnv is a global so we update that one too - config.DockerEnv = uniqueEnvs - o := engine.Options{ Env: uniqueEnvs, InsecureRegistry: append([]string{constants.DefaultServiceCIDR}, cfg.InsecureRegistry...), From 32bf4d46a7202ff0d0d7e1500ba0a49d6bf37c87 Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Mon, 18 May 2020 18:26:11 -0700 Subject: [PATCH 3/4] add unique --- cmd/minikube/cmd/start_flags.go | 2 +- pkg/minikube/proxy/proxy.go | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/cmd/minikube/cmd/start_flags.go b/cmd/minikube/cmd/start_flags.go index de41ab180aa3..075143845ee4 100644 --- a/cmd/minikube/cmd/start_flags.go +++ b/cmd/minikube/cmd/start_flags.go @@ -341,7 +341,7 @@ func generateClusterConfig(cmd *cobra.Command, existing *config.ClusterConfig, k // Feed Docker our host proxy environment by default, so that it can pull images // doing this for both new config and existing, in case proxy changed since previous start - if _, ok := r.(*cruntime.Docker); ok && !cmd.Flags().Changed("docker-env") { + if _, ok := r.(*cruntime.Docker); ok { proxy.SetDockerEnv() } diff --git a/pkg/minikube/proxy/proxy.go b/pkg/minikube/proxy/proxy.go index 0c95f2325843..63aea41a1638 100644 --- a/pkg/minikube/proxy/proxy.go +++ b/pkg/minikube/proxy/proxy.go @@ -168,5 +168,17 @@ func SetDockerEnv() []string { config.DockerEnv = append(config.DockerEnv, fmt.Sprintf("%s=%s", k, v)) } } + + // remove duplicates + seen := map[string]bool{} + uniqueEnvs := []string{} + for e := range config.DockerEnv { + if !seen[config.DockerEnv[e]] { + seen[config.DockerEnv[e]] = true + uniqueEnvs = append(uniqueEnvs, config.DockerEnv[e]) + } + } + config.DockerEnv = uniqueEnvs + return config.DockerEnv } From 5640da0cb48957f588eb3eb17213001098eebc89 Mon Sep 17 00:00:00 2001 From: Medya Gh Date: Mon, 18 May 2020 22:52:01 -0700 Subject: [PATCH 4/4] lint --- pkg/minikube/machine/cluster_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/minikube/machine/cluster_test.go b/pkg/minikube/machine/cluster_test.go index 3adbcf4a34f7..f45d7c412578 100644 --- a/pkg/minikube/machine/cluster_test.go +++ b/pkg/minikube/machine/cluster_test.go @@ -290,7 +290,7 @@ func TestStartHostConfig(t *testing.T) { for i := range h.HostOptions.EngineOptions.Env { if h.HostOptions.EngineOptions.Env[i] != cfg.DockerEnv[i] { - t.Fatalf("Docker env variables were not set! got %+v but want %+v",h.HostOptions.EngineOptions.Env,cfg.DockerEnv) + t.Fatalf("Docker env variables were not set! got %+v but want %+v", h.HostOptions.EngineOptions.Env, cfg.DockerEnv) } }