Skip to content

Add CurrentContainerID into Docker struct #158

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

Merged
merged 9 commits into from
Feb 24, 2016

Conversation

baptistedonaux
Copy link
Contributor

Add CurrentContainerID into Docker struct. This improvement allow the possibility to compare current container networks and others containers networks.

@md5 I watched your comments and I edited my code to propose a best version.

Linked with nginx-proxy/nginx-proxy#337

@baptistedonaux
Copy link
Contributor Author

Success Travis !

@md5
Copy link
Contributor

md5 commented Feb 8, 2016

This is looking pretty good. I'm curious about baptistedonaux@b3b09d7 though. If different Docker versions have a different name for the cgroups, it might be good to support both formats. I've only seen the slash version looking at Docker 1.9 and 1.10.

I still think this code needs to have a test or two.

Also, I'd recommend initializing the regex as a const instead of compiling it each time the function runs.

@baptistedonaux
Copy link
Contributor Author

About baptistedonaux@b3b09d7 I had Docker 1.9.1 and works on Debian Jessie. Unfortunately I can't reproduce.

I will change my code with your comments. For the test, it's tests in code or unit test ?

@baptistedonaux
Copy link
Contributor Author

Travis success. Ready to be merged ! @jwilder @md5


if err != nil {
if os.IsExist(err) {
log.Printf("Fail to open /proc/self/cgroup: %s\n", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

This log line is going to make docker-gen noisy when it runs outside a container. I don't think it's necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Edited !

jwilder added a commit that referenced this pull request Feb 24, 2016
Add CurrentContainerID into Docker struct
@jwilder jwilder merged commit 5f59705 into nginx-proxy:master Feb 24, 2016
@jwilder
Copy link
Collaborator

jwilder commented Feb 24, 2016

Thanks @baptistedonaux @md5!

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