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.

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

Apply newly loaded envvars to "DockerCli" and "APIClient" #9745

Merged
merged 2 commits into from
Aug 18, 2022

Conversation

ulyssessouza
Copy link
Collaborator

What I did
Re-evaluate DockerCli and APIClient after reading the environment file.
I can contain DOCKER_HOST and/or DOCKER_CONTEXT so the DockerCli passed by docker/cli has to be re-evaluated.

Related issue
Resolves #9210

Copy link
Contributor

@milas milas left a comment

Choose a reason for hiding this comment

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

LGTM

pkg/api/proxy.go Outdated Show resolved Hide resolved
This forces a re-evaluation of the environment variables.

Signed-off-by: Ulysses Souza <[email protected]>
Just moving it up to make clearer

Signed-off-by: Ulysses Souza <[email protected]>
@ulyssessouza ulyssessouza merged commit 6fe34c4 into docker:v2 Aug 18, 2022
@markdascher
Copy link

Hate to be that guy 😄 But this might've broken something, at least on MacOS. I recently updated to docker-compose v2.10 and started getting this error for any docker-compose or docker compose command:

Error response from daemon: Client sent an HTTP request to an HTTPS server.

Using these env vars:

DOCKER_BUILDKIT="1"
DOCKER_TLS_VERIFY="1"
DOCKER_HOST="tcp://192.168…:2376"
DOCKER_CERT_PATH="…/certs"

Tried moving these env vars into an .env file but it didn't help. (Might've been doing it wrong though, since I've never used .env files with docker before.)

git bisect pointed to this commit, and reverting it solved the issue.

@markdascher
Copy link

Found out a little more detail here. TLSOptions are only set in one spot, inside of InstallFlags. That method isn't called by the flags.NewClientOptions() added in this PR, which results in dockerCli.dockerEndpoint.EndpointMeta.TLSData becoming nil.

InstallFlags seems like a strange place for that particular code to be tucked away. Also, that file calls os.Getenv in global initializers, which will always be incompatible with the runtime override mechanism added in this PR.

AFAICT there's no workaround, so I just have to use an older version. 2.7.0 is the newest one without a big warning on it, so that's probably the one I'll pick. But I don't know if the issue is really with this PR or if the code in docker/cli should be reworked?

@nicksieger
Copy link
Member

@markdascher thanks for the sleuthing, that's helpful. I'd like to see your issue addressed in some way, whether a kludge or rolling back this PR or some other intervention. I filed #9789 to track this.

milas added a commit to milas/compose that referenced this pull request Aug 26, 2022
…loaded-envvars"

This reverts commit 6fe34c4, reversing
changes made to 10cfd55.

Signed-off-by: Milas Bowman <[email protected]>
milas added a commit that referenced this pull request Aug 26, 2022
…9792)

Revert "Merge pull request #9745 from ulyssessouza/apply-newly-loaded-envvars"

This reverts commit 6fe34c4, reversing
changes made to 10cfd55.

Signed-off-by: Milas Bowman <[email protected]>
@trcoelho
Copy link

trcoelho commented Sep 5, 2022

Hello, when this change (docker desktop version) will be available?

Thank you.

@milas
Copy link
Contributor

milas commented Sep 6, 2022

Copying #9210 (comment) here:

Re-opening because we had to revert #9745 in #9792 because it caused issues with TLS-enabled Docker hosts.

As a result, this is not currently available in the latest release of Docker Compose.

@dmorrison
Copy link

I've been struggling for a long time with the "Client sent an HTTP request to an HTTPS server." error in a local dev setup, and @markdascher's suggestion to use an older version fixed my issue! Thank you!!!!!!

@markdascher
Copy link

Glad to hear my comment was helpful! But the latest version (or anything after v2.10.2) already reverted this particular change. I'm currently running version 2.18.1 and there's no "Client sent an HTTP request to an HTTPS server" error. So that might be worth a shot, too.

@dmorrison
Copy link

Yes, it's strange for me. I was actually on Compose 2.18.1 (included with Docker Desktop 4.20.1) and getting that error. I was on a slightly older version before that (and still getting the error), so I tried upgrading to the latest version, but still got the error. Downgrading to Docker Desktop 4.11.0 (with Compose 2.7.0) fixed it. ¯_(ツ)_/¯

For context, it's for a local web development setup at my company. I tried troubleshooting many parts of it, but downgrading is the only thing that has worked so far. My specific issue was that the local dev app would work when plugged in to a USB to ethernet adapter, but not on wifi.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support setting DOCKER_HOST in .env file
6 participants