Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Automatically propagate proxy environment variables to docker env #3834

Merged
merged 1 commit into from
Mar 21, 2019
Merged

Automatically propagate proxy environment variables to docker env #3834

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion cmd/minikube/cmd/start.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,9 @@ var (
apiServerNames []string
apiServerIPs []net.IP
extraOptions pkgutil.ExtraOptionSlice

// proxyVars are variables we plumb through to the underlying container runtime
proxyVars = []string{"HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY"}
)

func init() {
Expand Down Expand Up @@ -260,6 +263,17 @@ func generateConfig(cmd *cobra.Command, kVersion string) (cfg.Config, error) {
}
}

// Feed Docker our host proxy environment by default, so that it can pull images
if _, ok := r.(*cruntime.Docker); ok {
if !cmd.Flags().Changed("docker-env") {
for _, k := range proxyVars {
if v := os.Getenv(k); v != "" {
dockerEnv = append(dockerEnv, fmt.Sprintf("%s=%s", k, v))
Copy link
Contributor

@tejal29 tejal29 Mar 12, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so, potentially user can have multiple dockerEnv variables set.
In case, users have NO_PROXY as well as "HTTPS_PROXY" set it shd error out right? or is this handled later?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to have something more portable as, well ?

Yes, I agree. This is an introductory PR that only solves the problem for most (99%?) of users who use the defaults. Other runtimes so far don't have a mechanism for introducing environment variables, but I suspect that #3835 should cover them nicely, which adds them to /etc/environment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a user manually sets --docker-env, this PR is disabled.

NO_PROXY and HTTPS_PROXY are complimentary, and thus plumbed into the environment as expected.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool.

}
}
}
}

cfg := cfg.Config{
MachineConfig: cfg.MachineConfig{
MinikubeISO: viper.GetString(isoURL),
Expand Down Expand Up @@ -370,7 +384,7 @@ func validateNetwork(h *host.Host) string {
console.OutStyle("connectivity", "%q IP address is %s", cfg.GetMachineName(), ip)

optSeen := false
for _, k := range []string{"HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY"} {
for _, k := range proxyVars {
if v := os.Getenv(k); v != "" {
if !optSeen {
console.OutStyle("internet", "Found network options:")
Expand Down