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

Old vendored fsouza/go-dockerclient doesn't have fix for failing docker check connection #3254

Closed
davidkemper opened this issue Jul 11, 2017 · 10 comments
Assignees
Labels
theme/health-checks Health Check functionality

Comments

@davidkemper
Copy link

Operating system and Environment details

A vagrant single machine setup based on Ubuntu.

Description of the Issue (and unexpected/desired result)

I'm using Consul 0.8.4 on a single vm for dev testing. I'm trying to set up a docker check by creating a .json configuration for it.

I've volume-mounted the docker socket into the docker container in which I'm running consul, and am expecting the consul agent to resolve to the desired container. In the consul GUI I see the error

Unable to create Exec, error: Post http://172.17.0.1:2376/containers/djk/exec: EOF

Here's what I think is going on:

As per fsouza/go-dockerclient#622 there is a fix to an underlying issue in go-cleanhttp. go-dockerclient has a fix for it as of fsouza/go-dockerclient#621 on Feb 11, 2017.

You have not yet vendored that fix: as per https://github.com/hashicorp/consul/blob/master/vendor/vendor.json you are vendoring go-dockerclient revision a53ba79627e888ef775bdcf15813f07d7a232867 with a revisionTime of 2016-08-09T01:24:47Z.

This does not appear to be updated in Consul 0.8.5 either; it's possible that other libraries or functionality is affected.

(This is my first created issue; apologies in advance if I'm short-cutting, but I'm hoping this gives you enough bread crumbs.)

@magiconair
Copy link
Contributor

On it.

@magiconair
Copy link
Contributor

@davidkemper Thx for reporting this.

I've updated the go-dockerclient lib in #3257 to contain the fix for fsouza/go-dockerclient#622. Pulling in the latest version would require upgrading docker and pulling in more dependencies. I need to check whether that is necessary.

Can you check whether this works for you?

You can check out the branch and run make dev to build consul. Pls use go1.8.3.

@davidkemper
Copy link
Author

Thanks for the quick turn-around. I'll try to verify today; our CI builds on released archives, so I'll need to take a little time building from source and integrating into our systems.

@davidkemper
Copy link
Author

Here's my git pull:

git status
HEAD detached at 83b7e4a3
nothing to commit, working tree clean
fatal: no tag exactly matches '83b7e4a33bd387245b16cdec2b6ce39ac215bd2a'

I've tried building this on OSX using go 1.8.3 installed with brew, and in an Ubuntu docker container with go, git, etc added. I consistently get build errors of this flavor:

On OSX

--> linux/amd64 error: exit status 1
Stderr: vendor/github.com/fsouza/go-dockerclient/misc.go:12:2: cannot find package "github.com/docker/docker/api/types/swarm" in any of:
	/Users/davidkemper/go/src/github.com/hashicorp/consul/vendor/github.com/docker/docker/api/types/swarm (vendor tree)
	/usr/local/Cellar/go/1.8.3/libexec/src/github.com/docker/docker/api/types/swarm (from $GOROOT)
	/Users/davidkemper/go/src/github.com/docker/docker/api/types/swarm (from $GOPATH)
vendor/github.com/fsouza/go-dockerclient/client.go:34:2: cannot find package "github.com/docker/docker/pkg/jsonmessage" in any of:
	/Users/davidkemper/go/src/github.com/hashicorp/consul/vendor/github.com/docker/docker/pkg/jsonmessage (vendor tree)
	/usr/local/Cellar/go/1.8.3/libexec/src/github.com/docker/docker/pkg/jsonmessage (from $GOROOT)
	/Users/davidkemper/go/src/github.com/docker/docker/pkg/jsonmessage (from $GOPATH)

On Ubuntu:

--> linux/arm error: exit status 1
Stderr: /go/src/github.com/hashicorp/consul/vendor/github.com/fsouza/go-dockerclient/misc.go:12:2: cannot find package "github.com/docker/docker/api/types/swarm" in any of:
	/go/src/github.com/hashicorp/consul/vendor/github.com/docker/docker/api/types/swarm (vendor tree)
	/usr/local/go/src/github.com/docker/docker/api/types/swarm (from $GOROOT)
	/go/src/github.com/docker/docker/api/types/swarm (from $GOPATH)
/go/src/github.com/hashicorp/consul/vendor/github.com/fsouza/go-dockerclient/client.go:34:2: cannot find package "github.com/docker/docker/pkg/jsonmessage" in any of:
	/go/src/github.com/hashicorp/consul/vendor/github.com/docker/docker/pkg/jsonmessage (vendor tree)
	/usr/local/go/src/github.com/docker/docker/pkg/jsonmessage (from $GOROOT)
	/go/src/github.com/docker/docker/pkg/jsonmessage (from $GOPATH)
main.go:11:2: cannot find package "github.com/mitchellh/cli" in any of:
	/usr/local/go/src/github.com/mitchellh/cli (from $GOROOT)
	/go/src/github.com/mitchellh/cli (from $GOPATH)

This is also what your CI server is reporting: https://travis-ci.org/hashicorp/consul/builds/252375875?utm_source=github_status&utm_medium=notification

@magiconair
Copy link
Contributor

my bad. will fix

magiconair added a commit that referenced this issue Jul 11, 2017
Update go-dockerclient and dependant dependencies
to add fix for go-dockerclient#622.

Fixes #3254
@magiconair
Copy link
Contributor

Fixing this required a somewhat insane amount of dependency update including with a naming clash for the github.com/sirupsen/logrus lib. I will have a look to implement this feature in a different way so that we can drop these dependencies altogether.

Could you have another look, please?

@davidkemper
Copy link
Author

Sure thing; it will have to be tomorrow, though. I'll let you know how it goes.

@magiconair magiconair added the theme/health-checks Health Check functionality label Jul 12, 2017
@davidkemper
Copy link
Author

I was able to build this change set and patch it into our systems. I did not get that error.

I wasn't able to get a docker check running end-to-end because we have dockerized consul and have a custom consul user/group as well, and I'm searching for the happy path through the various layers of isolation and TLS. Thus I think it's unrelated to the specific problem that I reported.

Thank you again for the rapid response.

@magiconair
Copy link
Contributor

@davidkemper I've merged the library update to fix the immediate problem but also started rewriting that piece of code since it used a significant amount of dependencies for a tiny feature. The change is in #3270. This is functional but I still need to finish the tests. Would appreciate if you can test this at some point.

magiconair added a commit that referenced this issue Jul 14, 2017
This patch replaces the Docker client which is used
for health checks with a simplified version tailored
for that purpose.

See #3254
See #3257
Fixes #3270
magiconair added a commit that referenced this issue Jul 16, 2017
This patch replaces the Docker client which is used
for health checks with a simplified version tailored
for that purpose.

See #3254
See #3257
Fixes #3270
magiconair added a commit that referenced this issue Jul 18, 2017
This patch replaces the Docker client which is used
for health checks with a simplified version tailored
for that purpose.

See #3254
See #3257
Fixes #3270
magiconair added a commit that referenced this issue Jul 18, 2017
This patch replaces the Docker client which is used
for health checks with a simplified version tailored
for that purpose.

See #3254
See #3257
Fixes #3270
@davidkemper
Copy link
Author

BTW, thanks again for the fix. I look forward to trying it again at our next upstream integration point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme/health-checks Health Check functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants