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

Support the ENV variables used by boot2docker. #210

Merged
merged 1 commit into from
Nov 14, 2014

Conversation

pwnall
Copy link
Contributor

@pwnall pwnall commented Nov 7, 2014

Boot2docker currently installs docker 1.3, which uses https and certificates. It sets up the DOCKER_HOST and DOCKER_CERT_PATH environment variables. The docker client understands these variables, so it would be nice if docker-api understood them too.

This should fix #202 without the need for custom configuration.

I did my best to minimize changes and to get spec coverage for the code change. I would be happy to incorporate any feedback into the diff.

Please consider merging this, so docker-api can work with boot2docker on OSX out of the box! ❤️

@bfulton
Copy link
Contributor

bfulton commented Nov 7, 2014

@pwnall, awesome, thanks. Will review with an eye to merge soon.

@pwnall
Copy link
Contributor Author

pwnall commented Nov 8, 2014

@bfulton Thank you! I look forward to your feedback!

ENV['DOCKER_CERT_PATH'] = '/boot2dockert/cert/path'
end

its(:options) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This should have a should == before the hash, correct? I don't think /boot2dockert/cert/path/https is a correct value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for catching that!

Yup, I assumed that the test case is taking advantage of some RSpec shorthand.

I fixed this pattern everywhere in the spec for the Docker object.

Also, I had to copy-paste Docker.options = nil to every before block that changes environment vars, because the docker's before block executes before the context-specific before blocks, so options never actually changed before.

Previous tests didn't catch this because they were all testing for the same result, namely the empty hash.

@tlunter tlunter mentioned this pull request Nov 13, 2014
@pwnall
Copy link
Contributor Author

pwnall commented Nov 13, 2014

@tlunter Thank you very much for the feedback! Can you please take another look?

@tlunter
Copy link
Contributor

tlunter commented Nov 13, 2014

👍 Fantastic!

@bfulton
Copy link
Contributor

bfulton commented Nov 13, 2014

@pwnall looks good overall!

Is the order of precedence — DOCKER_URL then DOCKER_HOST then DOCKER_CERT_PATH — the same in the CLI?

@pwnall
Copy link
Contributor Author

pwnall commented Nov 14, 2014

@bfulton DOCKER_CERT_PATH is orthogonal to the other two. If it's set, it enables TLS connections.

I didn't check DOCKER_URL vs DOCKER_HOST -- the gem already had an opinion (expressed in both code and tests), and I just tried to not interfere with it. Would you like me to perform some experiments with the latest docker on OSX?

@bfulton
Copy link
Contributor

bfulton commented Nov 14, 2014

Ah, that's great. We checked the original precedence, and I just misunderstood that it wasn't changed and that DOCKER_CERT_PATH was a different dimension. Thanks for explaining.

bfulton added a commit that referenced this pull request Nov 14, 2014
Support the ENV variables used by boot2docker.
@bfulton bfulton merged commit 4fc0bb9 into upserve:master Nov 14, 2014
@pwnall
Copy link
Contributor Author

pwnall commented Nov 14, 2014

@bfulton Thank you for merging, and much gratitude for this gem! ❤️

@pwnall pwnall deleted the boot2docker branch November 14, 2014 16:45
@bfulton
Copy link
Contributor

bfulton commented Nov 14, 2014

@pwnall thank you!

This is released in v1.15.0, and pushed to RubyGems.

@pwnall
Copy link
Contributor Author

pwnall commented Nov 14, 2014

@bfulton Sweet! Thanks so much for the quick turnaround!!

I just confirmed that v1.15.0 works out of the box with boot2docker on OSX. 😄

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.

Excon::Errors::SocketError: end of file reached (EOFError)
3 participants