Merge pull request #8198 from medyagh/fix_proxy

fix proxy env not being passed to docker engine

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

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"
 )
@@ -340,8 +341,8 @@ 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()
+	if _, ok := r.(*cruntime.Docker); ok {
+		proxy.SetDockerEnv()
 	}
 
 	var kubeNodeName string

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)
 		}
 	}
 

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,25 @@ 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])
+		}
+	}
+
 	o := engine.Options{
-		Env:              cfg.DockerEnv,
+		Env:              uniqueEnvs,
 		InsecureRegistry: append([]string{constants.DefaultServiceCIDR}, cfg.InsecureRegistry...),
 		RegistryMirror:   cfg.RegistryMirror,
 		ArbitraryFlags:   cfg.DockerOpt,

pkg/minikube/node/start.go

@@ -407,7 +407,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
 			}
 		}

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,34 @@ 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))
+		}
+	}
+
+	// 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
+}