Skip to content

Fix systemd cgroups implementation#19149

Closed
mrunalp wants to merge 1 commit intomoby:masterfrom
mrunalp:systemd_cgroup_fix
Closed

Fix systemd cgroups implementation#19149
mrunalp wants to merge 1 commit intomoby:masterfrom
mrunalp:systemd_cgroup_fix

Conversation

@mrunalp
Copy link
Contributor

@mrunalp mrunalp commented Jan 6, 2016

The cgroup parent was getting set to /docker by default which doesn't work
for the systemd cgroup implementation. It isn't necessary to set the default
as we set the correct value in the template code.

Signed-off-by: Mrunal Patel [email protected]

@mrunalp
Fix systemd cgroups implementation
9a81d18 
The cgroup parent was getting set to /docker by default which doesn't work
for the systemd cgroup implementation. It isn't necessary to set the default
as we set the correct value in the template code.

Signed-off-by: Mrunal Patel <[email protected]>
@mrunalp mrunalp mentioned this pull request Jan 6, 2016
Merged
@mrunalp
Copy link
Contributor Author

mrunalp commented Jan 6, 2016

@LK4D4 PTAL

Without the fix

[root@localhost ~]# /root/gosrc/src/github.com/docker/docker/bundles/1.10.0-dev/dynbinary/docker run -it --rm  busybox sh                                    
docker: Error response from daemon: Cannot start container 3a662894ac578c2dabf743e75f3c873ad711f0ab8e644a34798c3ddd576ef655: [9] System error: Invalid slice name /docker.

With the fix

[root@localhost ~]# /root/gosrc/src/github.com/docker/docker/bundles/1.10.0-dev/dynbinary/docker run -it --rm  busybox sh
/ # cat /proc/self/cgroup 
10:freezer:/system.slice/docker-e760336ac2d9cb721fd6fd3b6c9e3028785d27fd8d85251a8f89e97c5cc8eb22.scope
9:cpuset:/system.slice/docker-e760336ac2d9cb721fd6fd3b6c9e3028785d27fd8d85251a8f89e97c5cc8eb22.scope
8:devices:/system.slice/docker-e760336ac2d9cb721fd6fd3b6c9e3028785d27fd8d85251a8f89e97c5cc8eb22.scope
7:memory:/system.slice/docker-e760336ac2d9cb721fd6fd3b6c9e3028785d27fd8d85251a8f89e97c5cc8eb22.scope
6:hugetlb:/system.slice/docker-e760336ac2d9cb721fd6fd3b6c9e3028785d27fd8d85251a8f89e97c5cc8eb22.scope
5:cpu,cpuacct:/system.slice/docker-e760336ac2d9cb721fd6fd3b6c9e3028785d27fd8d85251a8f89e97c5cc8eb22.scope
4:blkio:/system.slice/docker-e760336ac2d9cb721fd6fd3b6c9e3028785d27fd8d85251a8f89e97c5cc8eb22.scope
3:perf_event:/system.slice/docker-e760336ac2d9cb721fd6fd3b6c9e3028785d27fd8d85251a8f89e97c5cc8eb22.scope
2:net_cls,net_prio:/system.slice/docker-e760336ac2d9cb721fd6fd3b6c9e3028785d27fd8d85251a8f89e97c5cc8eb22.scope
1:name=systemd:/system.slice/docker-e760336ac2d9cb721fd6fd3b6c9e3028785d27fd8d85251a8f89e97c5cc8eb22.scope


[root@localhost ~]# /root/gosrc/src/github.com/docker/docker/bundles/1.10.0-dev/dynbinary/docker run --cgroup-parent=containers.slice -it --rm  busybox sh
/ # cat /proc/self/cgroup 
10:freezer:/containers.slice/docker-a995029510c4330395b66047a6178d40a88e4a7a4cdbdb1ae3682c2e59c43b56.scope
9:cpuset:/containers.slice/docker-a995029510c4330395b66047a6178d40a88e4a7a4cdbdb1ae3682c2e59c43b56.scope
8:devices:/containers.slice/docker-a995029510c4330395b66047a6178d40a88e4a7a4cdbdb1ae3682c2e59c43b56.scope
7:memory:/containers.slice/docker-a995029510c4330395b66047a6178d40a88e4a7a4cdbdb1ae3682c2e59c43b56.scope
6:hugetlb:/containers.slice/docker-a995029510c4330395b66047a6178d40a88e4a7a4cdbdb1ae3682c2e59c43b56.scope
5:cpu,cpuacct:/containers.slice/docker-a995029510c4330395b66047a6178d40a88e4a7a4cdbdb1ae3682c2e59c43b56.scope
4:blkio:/containers.slice/docker-a995029510c4330395b66047a6178d40a88e4a7a4cdbdb1ae3682c2e59c43b56.scope
3:perf_event:/containers.slice/docker-a995029510c4330395b66047a6178d40a88e4a7a4cdbdb1ae3682c2e59c43b56.scope
2:net_cls,net_prio:/containers.slice/docker-a995029510c4330395b66047a6178d40a88e4a7a4cdbdb1ae3682c2e59c43b56.scope
1:name=systemd:/containers.slice/docker-a995029510c4330395b66047a6178d40a88e4a7a4cdbdb1ae3682c2e59c43b56.scope

@mrunalp
Copy link
Contributor Author

mrunalp commented Jan 6, 2016

@LK4D4 This was broken in 2e3186a

@LK4D4
Copy link
Contributor

LK4D4 commented Jan 6, 2016

@mrunalp but it's sorta reverts my commit about "/docker" default, no? :)

@LK4D4
Copy link
Contributor

LK4D4 commented Jan 6, 2016

Check out #19144 also

@mrunalp
Copy link
Contributor Author

mrunalp commented Jan 6, 2016

@LK4D4 Yes, that's what came to my mind after posting this PR. However, this PR will still work. It doesn't break the docker daemon feature :) It is upto you which one you want to merge. I am happy with either :)

@LK4D4
Copy link
Contributor

LK4D4 commented Jan 7, 2016

@mrunalp I don't know what to choose. Your PR is simpler, but mine looks less magical :/

@mrunalp
Copy link
Contributor Author

mrunalp commented Jan 7, 2016

@LK4D4 I prefer your PR. It simplifies the code which was a bit wonky.

@jessfraz
Copy link
Contributor

jessfraz commented Jan 7, 2016

so I think we can close this now?

@jessfraz jessfraz closed this Jan 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

status/0-triage

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mrunalp @LK4D4 @jessfraz @GordonTheTurtle