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 systemd cgroups implementation #19149

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
4 participants
@mrunalp
Contributor

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 mrunalp@gmail.com

Fix systemd cgroups implementation
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 <mrunalp@gmail.com>
@mrunalp

This comment has been minimized.

Show comment
Hide comment
@mrunalp

mrunalp Jan 6, 2016

Contributor

@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

Contributor

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

This comment has been minimized.

Show comment
Hide comment
@mrunalp

mrunalp Jan 6, 2016

Contributor

@LK4D4 This was broken in 2e3186a

Contributor

mrunalp commented Jan 6, 2016

@LK4D4 This was broken in 2e3186a

@LK4D4

This comment has been minimized.

Show comment
Hide comment
@LK4D4

LK4D4 Jan 6, 2016

Contributor

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

Contributor

LK4D4 commented Jan 6, 2016

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

@LK4D4

This comment has been minimized.

Show comment
Hide comment
@LK4D4

LK4D4 Jan 6, 2016

Contributor

Check out #19144 also

Contributor

LK4D4 commented Jan 6, 2016

Check out #19144 also

@mrunalp

This comment has been minimized.

Show comment
Hide comment
@mrunalp

mrunalp Jan 6, 2016

Contributor

@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 :)

Contributor

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

This comment has been minimized.

Show comment
Hide comment
@LK4D4

LK4D4 Jan 7, 2016

Contributor

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

Contributor

LK4D4 commented Jan 7, 2016

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

@mrunalp

This comment has been minimized.

Show comment
Hide comment
@mrunalp

mrunalp Jan 7, 2016

Contributor

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

Contributor

mrunalp commented Jan 7, 2016

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

@jessfraz

This comment has been minimized.

Show comment
Hide comment
@jessfraz

jessfraz Jan 7, 2016

Contributor

so I think we can close this now?

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