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

Fix name resolution in Systemd systems #220

Merged
merged 1 commit into from Sep 29, 2014
Merged

Conversation

vmarmol
Copy link
Contributor

@vmarmol vmarmol commented Sep 5, 2014

No description provided.

@vmarmol
Copy link
Contributor Author

vmarmol commented Sep 5, 2014

This is why Docker containers were not being registered in Systemd systems. It is still broken now because of a bug in libcontainer. Looking into that.

It is actually broken even worse now because we'll register the Docker driver, but can't get stats from it. Maybe hold off merging until I figure out the libcontainer issue :)

@vishh
Copy link
Contributor

vishh commented Sep 17, 2014

@vmarmol Is it safe to merge this PR now?

@vmarmol
Copy link
Contributor Author

vmarmol commented Sep 18, 2014

@vishh not yet, we still have the libcontainer issues.

@vmarmol
Copy link
Contributor Author

vmarmol commented Sep 24, 2014

@rjnagal @vishh PTAL. I made us always use the FS stats API even in systemd systems which makes this work. I already sent a PR to libcontainer, I'll update when that goes through.

Fixes #143

Only remaining issue is that the "system.slice" does not show the docker container name. One at a time :) all the exported stats and names should work, so it is only a UI issue at this point.

@vmarmol
Copy link
Contributor Author

vmarmol commented Sep 26, 2014

Ping @rjnagal @vishh :)

@vmarmol
Copy link
Contributor Author

vmarmol commented Sep 29, 2014

Ping :D


// Turn systemd cgroup name into Docker ID.
if useSystemd {
const systemdDockerPrefix = "docker-"
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we instead export "docker-" prefix from libcontainer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

libcontainer as in container/libcontainer/helpers.go?

I think the right place for this is here since it is a Docker concept and not a libcontainer one (as in, Docker prefixes it's containers with that).

Copy link
Contributor

Choose a reason for hiding this comment

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

I meant docker/libcontainer. If libcontainer is not the source of the prefix, then the current code is fine as is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

libcontainer does some sad mangling of container names I'm trying to remove in a PR :( once that goes in, Docker will be the one doing the prefixing.

@vishh
Copy link
Contributor

vishh commented Sep 29, 2014

LGTM

vishh added a commit that referenced this pull request Sep 29, 2014
Fix name resolution in Systemd systems
@vishh vishh merged commit 1ed9d12 into google:master Sep 29, 2014
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.

None yet

2 participants