kubernetes-sigs / kind

Kubernetes IN Docker - local clusters for testing Kubernetes
https://kind.sigs.k8s.io/
Apache License 2.0
13.44k stars 1.56k forks source link

proxy info from docker config is not respected #1175

Open BenTheElder opened 4 years ago

BenTheElder commented 4 years ago

This should be an easy fix, thanks to @sudeshsh for debugging this with me.

We just need to detect and respect https://docs.docker.com/network/proxy/#configure-the-docker-client

passing in env explicitly will still be expected, but docker will automagically plumb through proxy env, just missing the things kind would otherwise append to noProxy.

/assign /lifecycle active

BenTheElder commented 4 years ago

filed https://github.com/kubernetes-sigs/kind/pull/1176

tao12345666333 commented 4 years ago

Just to clarify, the #1176 fix was as described in the https://docs.docker.com/config/daemon/systemd/#httphttps-proxy documentation, not the client side proxy. :smile_cat:

BenTheElder commented 4 years ago

@tao12345666333 I'm not sure what you mean by:

Just to clarify, the #1176 fix was as described in the https://docs.docker.com/config/daemon/systemd/#httphttps-proxy documentation, not the client side proxy. smile_cat

?

This fixes the case that you do not set environment variables and instead configure the docker config with proxy details.

BenTheElder commented 4 years ago

We're obtaining whatever proxy settings docker info reports. It is on docker to report sane values there imo :-)

tao12345666333 commented 4 years ago

https://github.com/kubernetes-sigs/kind/blob/1978aeb46a1d6452e1a61f3bdcc17c82fdf8de4d/pkg/cluster/internal/providers/provider/common/proxy.go#L82-L91

I mean, the proxy value read by docker info is based on the configuration of the docker daemon, not the.docker / config.

(MoeLove) ➜  ~ grep httpProxy ~/.docker/config.json
            "httpProxy": "http://127.0.0.1:9999"
(MoeLove) ➜  ~ docker info --format {{.HTTPProxy}}

(MoeLove) ➜  ~ docker run --rm alpine:3.10 env |grep -i http_proxy
HTTP_PROXY=http://127.0.0.1:9999
http_proxy=http://127.0.0.1:9999
BenTheElder commented 4 years ago

: |

aojea commented 4 years ago

I really think that is easier to configure environment variables ~than try to cover this corner case~ :/

https://github.com/kubernetes-sigs/kind/issues/1175#issuecomment-566841289

EDIT wrong comment :)

BenTheElder commented 4 years ago

This is not a corner case. This is a well documented mechanism to configure proxy for your containers that we have been ignorant of.

aojea commented 4 years ago

This is not a corner case. This is a well documented mechanism to configure proxy for your containers that we have been ignorant of.

ups, you're totally right https://docs.docker.com/network/proxy/#configure-the-docker-client I was confused because of the docker info thing :sweat_smile:

BenTheElder commented 4 years ago

Yeah, the info thing is an incorrect approach obtaining the wrong data, I'll correct it before unholding.

On Wed, Dec 18, 2019, 04:06 Antonio Ojea notifications@github.com wrote:

This is not a corner case. This is a well documented mechanism to configure proxy for your containers that we have been ignorant of.

ups, you're totally right https://docs.docker.com/network/proxy/#configure-the-docker-client I was confused because of the docker info thing 😅

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/kubernetes-sigs/kind/issues/1175?email_source=notifications&email_token=AAHADK6JZNTQVIVX5SQF7BTQZIG4ZA5CNFSM4J4DPVI2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHF46TA#issuecomment-567005004, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHADK4OFARKT3QRKRXJFM3QZIG4ZANCNFSM4J4DPVIQ .

BenTheElder commented 4 years ago

punting for a moment to handle some other major improvements but I still intend to finish this.

it looks like unfortunately we need to write some logic to find and parse the docker config accurately.

not a huge deal but I'd hoped to find a way to get docker ... to at least spit out the client config so we don't have to try to locate it the same way... doesn't look like you can.

shouldn't be hard at all, but we should be careful to mimic docker closely here to avoid surprises. in particular we should see how they lookup $HOME.

tao12345666333 commented 4 years ago

If we really need to do this, I personally think that we probably need to deal with the following:

BenTheElder commented 4 years ago

I don't think we should go past grabbing the client config. This won't be hard, but I'm deprioritizing it versus finishing proving out an approach to host restart support :-)

On Thu, Dec 19, 2019, 22:43 Jintao Zhang notifications@github.com wrote:

If we really need to do this, I personally think that we probably need to deal with the following:

-

check DOCKER_CONFIG env

check $HOME/.docker/config

parse docker config and get proxies

check DOCKER_HOST env etc to get docker's daemon host

using docker's daemon host to get value from proxies, if not found we can using default value.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/kubernetes-sigs/kind/issues/1175?email_source=notifications&email_token=AAHADKYSLDSQDOGPUHQVWR3QZRSPNA5CNFSM4J4DPVI2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHMBUFY#issuecomment-567810583, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHADK5YCLAT5MCJ3TG4C4LQZRSPNANCNFSM4J4DPVIQ .

BenTheElder commented 4 years ago

will have to circle back to this one

tao12345666333 commented 3 years ago

/remove-lifecycle stale

BenTheElder commented 3 years ago

it would be helpful to know more about how docker behaves here, e.g. does it try to merge these settings? If I add proxy details to docker but then I pass the http_proxy env to a container, what value is use?

tao12345666333 commented 3 years ago

Today or tomorrow I will describe this behavior of docker in detail here.

tao12345666333 commented 3 years ago

Sorry for delay.

First of all, it needs to be clear that the environment variables of the container can be set in two ways.

  1. Set directly through --env args.

  2. Configure via client config file ~/.docker/config

The first method has the highest priority.

e.g. set the client proxy to http://127.0.0.1:3001 but pass HTTP_PROXY=http://127.0.0.1:5001 env to container.

The container will have both HTTP_PROXY and http_proxy environment variables.

➜  ~ docker run --rm -e HTTP_PROXY=http://127.0.0.1:5001 alpine env 
PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
HOSTNAME=c569d42dd38c
HTTP_PROXY=http://127.0.0.1:5001
http_proxy=http://127.0.0.1:3001
HOME=/root

Secondly, the proxy set by the Docker daemon is not added to the container by default.

➜  ~ docker info --format {{.HTTPProxy}}
socks5://127.0.0.1:1080
➜  ~ docker run --rm  -e http_proxy=http://127.0.0.1:5001 alpine env
PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
HOSTNAME=a84c2638485f
http_proxy=http://127.0.0.1:5001
HOME=/root
BenTheElder commented 3 years ago

Thank you!

We still lack a way to read the settings of 2) ourselves except attempting to match docker config loading (we may have to just do that then).

Maybe we should read from 2) ourselves when using the docker node provider and use it if the HTTP* env are not set 🤔

That seems closest to the docker default behavior with normal containers, we can take what docker set and inject the additional kind no_proxy values. If it is set AND the CLI read the env, then the env are like the user --env to docker run and we can ignore / not bother to load the .docker/config.json

blacksails commented 2 years ago

I would like to contribute, and I think this could be a good first issue for me.

I don't think we should go past grabbing the client config.

Are you sure about this? In practice that would mean that you wouldn't be able to use the combination of a remote docker daemon, proxy config and kind (assuming that you would need other proxy settings for the remote docker daemon than what is defined in the default proxy config).

BenTheElder commented 2 years ago

Sorry, my own github inbox is a tire fire.

Are you sure about this? In practice that would mean that you wouldn't be able to use the combination of a remote docker daemon, proxy config and kind (assuming that you would need other proxy settings for the remote docker daemon than what is defined in the default proxy config).

Yes, it will be complex and brittle, and the user can always set the *proxy environment variables when invoking kind themselves.

At the moment we have very limited support for remote docker, we try not to break it, but we don't have much in the way of special support for it. kind is intended for use with local clusters, and an alternative to remote docker host is just running kind on the remote host.