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

daemon: ensure we set default options to stock runtime #24450

Merged
merged 1 commit into from
Jul 8, 2016

Conversation

runcom
Copy link
Member

@runcom runcom commented Jul 8, 2016

- What I did

Fix #24424
Make sure we set default stock runtime options in case we use systemd

- How I did it

set stock runtime options

- How to verify it

if you run w/o this patch and --exec-opts native.cgroupdriver=systemd you'll get:

$ docker run fedora cat /proc/self/cgroup
11:perf_event:/system.slice:docker:046f8702dd1dd72557f320bae3b77875cd7550799670ff06308ccd6f1f662a6b
10:memory:/system.slice/system.slice:docker:046f8702dd1dd72557f320bae3b77875cd7550799670ff06308ccd6f1f662a6b
9:pids:/system.slice/docker.service/system.slice:docker:046f8702dd1dd72557f320bae3b77875cd7550799670ff06308ccd6f1f662a6b
8:devices:/system.slice/docker.service/system.slice:docker:046f8702dd1dd72557f320bae3b77875cd7550799670ff06308ccd6f1f662a6b
7:hugetlb:/system.slice:docker:046f8702dd1dd72557f320bae3b77875cd7550799670ff06308ccd6f1f662a6b
6:blkio:/system.slice/system.slice:docker:046f8702dd1dd72557f320bae3b77875cd7550799670ff06308ccd6f1f662a6b
5:freezer:/system.slice:docker:046f8702dd1dd72557f320bae3b77875cd7550799670ff06308ccd6f1f662a6b
4:cpuset:/system.slice:docker:046f8702dd1dd72557f320bae3b77875cd7550799670ff06308ccd6f1f662a6b
3:cpu,cpuacct:/system.slice/system.slice:docker:046f8702dd1dd72557f320bae3b77875cd7550799670ff06308ccd6f1f662a6b
2:net_cls,net_prio:/system.slice:docker:046f8702dd1dd72557f320bae3b77875cd7550799670ff06308ccd6f1f662a6b
1:name=systemd:/system.slice/docker.service/system.slice:docker:046f8702dd1dd72557f320bae3b77875cd7550799670ff06308ccd6f1f662a6b

and you can notice cgroup paths are incorrect (too many colons)

Running with this patch:

$ docker run fedora cat /proc/self/cgroup   
11:perf_event:/system.slice/docker-28287451c5ac225e572a15a42f5e7e23bc820e49c293fe2debc95abf3cbb6374.scope
10:memory:/init.scope/system.slice/docker-28287451c5ac225e572a15a42f5e7e23bc820e49c293fe2debc95abf3cbb6374.scope
9:pids:/init.scope/system.slice/docker-28287451c5ac225e572a15a42f5e7e23bc820e49c293fe2debc95abf3cbb6374.scope
8:devices:/init.scope/system.slice/docker-28287451c5ac225e572a15a42f5e7e23bc820e49c293fe2debc95abf3cbb6374.scope
7:hugetlb:/system.slice/docker-28287451c5ac225e572a15a42f5e7e23bc820e49c293fe2debc95abf3cbb6374.scope
6:blkio:/init.scope/system.slice/docker-28287451c5ac225e572a15a42f5e7e23bc820e49c293fe2debc95abf3cbb6374.scope
5:freezer:/system.slice/docker-28287451c5ac225e572a15a42f5e7e23bc820e49c293fe2debc95abf3cbb6374.scope
4:cpuset:/system.slice/docker-28287451c5ac225e572a15a42f5e7e23bc820e49c293fe2debc95abf3cbb6374.scope
3:cpu,cpuacct:/init.scope/system.slice/docker-28287451c5ac225e572a15a42f5e7e23bc820e49c293fe2debc95abf3cbb6374.scope
2:net_cls,net_prio:/system.slice/docker-28287451c5ac225e572a15a42f5e7e23bc820e49c293fe2debc95abf3cbb6374.scope
1:name=systemd:/system.slice/docker-28287451c5ac225e572a15a42f5e7e23bc820e49c293fe2debc95abf3cbb6374.scope

- Description for the changelog

Ensure we set default options to stock runtime

- A picture of a cute animal (not mandatory but encouraged)

Signed-off-by: Antonio Murdaca runcom@redhat.com

Signed-off-by: Antonio Murdaca <runcom@redhat.com>
@runcom
Copy link
Member Author

runcom commented Jul 8, 2016

ping @LK4D4 @mlaventure @mrunalp

@runcom
Copy link
Member Author

runcom commented Jul 8, 2016

more background from the issue here #24424 (comment) and basically when we now retrieve the runtime and it happens to be the default we also get the options such as systemd (https://github.com/docker/docker/blob/master/daemon/start_linux.go#L19)

@runcom runcom added this to the 1.12.0 milestone Jul 8, 2016
@mrunalp
Copy link
Contributor

mrunalp commented Jul 8, 2016

This seems reasonable.

@mrunalp
Copy link
Contributor

mrunalp commented Jul 8, 2016

Yeah that is the basic issue and it should be fixed by setting default runtime args correctly in docker when none is specified. The containerd flags now aren't of any use for docker.

Sent from my iPhone

On Jul 8, 2016, at 7:13 AM, Antonio Murdaca notifications@github.com wrote:

The underlying issue is probably that we don't have to set a runtime on start if we don't need it because containerd is going to apply whatever defualt it was started with. @mlaventure wdyt?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@mlaventure
Copy link
Contributor

mlaventure commented Jul 8, 2016

LGTM (IANAM)

@LK4D4
Copy link
Contributor

LK4D4 commented Jul 8, 2016

I tried and it works.
LGTM

@LK4D4 LK4D4 merged commit 9d10221 into moby:master Jul 8, 2016
@runcom runcom deleted the fix-systemd-defaultRuntime branch July 8, 2016 15:55
@runcom
Copy link
Member Author

runcom commented Jul 8, 2016

\o/ ty all

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants