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

core: add support to connect to docker-machine #83

Merged
merged 2 commits into from
Feb 24, 2017

Conversation

pmenglund
Copy link
Contributor

Fixes #82
Signed-off-by: Martin Englund martin@englund.nu

Signed-off-by: Martin Englund <martin@englund.nu>
@coveralls
Copy link

coveralls commented Feb 20, 2017

Coverage Status

Coverage increased (+0.1%) to 69.767% when pulling 8ebdf4f on pmenglund:docker-machine into 0a82db1 on ory:v3.

dockertest.go Outdated
if endpoint == "" {
if os.Getenv("DOCKER_URL") != "" {
endpoint = os.Getenv("DOCKER_URL")
} else if os.Getenv("DOCKER_MACHINE_NAME") != "" {
Copy link
Member

Choose a reason for hiding this comment

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

Is this env var set per spec on the host running docker machine?

Copy link
Contributor Author

@pmenglund pmenglund Feb 21, 2017

Choose a reason for hiding this comment

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

What do you mean by "per spec"?

You set the environment on your computer using eval $(docker-machine env default) when sets the following environment:

export DOCKER_TLS_VERIFY="1"
export DOCKER_HOST="tcp://192.168.174.132:2376"
export DOCKER_CERT_PATH="/Users/martin/.docker/machine/machines/default"
export DOCKER_MACHINE_NAME="default"

Copy link
Member

Choose a reason for hiding this comment

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

I see, thanks for clarifying!

dockertest.go Outdated
if endpoint == "" {
if os.Getenv("DOCKER_URL") != "" {
endpoint = os.Getenv("DOCKER_URL")
} else if os.Getenv("DOCKER_MACHINE_NAME") != "" {
client, err = dc.NewClientFromEnv()
goto error_check
Copy link
Member

Choose a reason for hiding this comment

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

Please handle the error here directly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, do you want to keep the goto for the return, or do it directly here too?

Copy link
Member

Choose a reason for hiding this comment

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

Do it directly here :)

Signed-off-by: Martin Englund <martin@englund.nu>
@pmenglund
Copy link
Contributor Author

I don't think I can write a test case for the new code, without some major refactoring as it can't call dc.NewClientFromEnv() without having docker-machine running (which isn't available in travis), and setting the environment variables by hand in the test won't work either, as it expects the docker connection to be tcp://...

@aeneasr
Copy link
Member

aeneasr commented Feb 24, 2017

Ok, thanks!

@aeneasr aeneasr merged commit 0944e3b into ory:v3 Feb 24, 2017
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.

3 participants