Automatically propagate proxy environment variables to docker env #3834
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
the
size/S
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: tstromberg The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
|
Closing in preference to #3835 - which is admittedly a bit more advanced. |
|
Reopening, since this is a good introductory approach. |
|
It would be nice to have something more portable as, well ? i.e. those proxy variables are also needed for other runtimes |
| 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)) |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Currently, our documentation recommends that users use:
This makes the setting of --docker-env automatic if the environment variables are set, and partially resolves #3242