Skip to content
This repository has been archived by the owner on Dec 13, 2018. It is now read-only.

cgroups: Make systemd cgroup parent support use specified slices. #549

Closed
wants to merge 1 commit into from

Conversation

mrunalp
Copy link
Contributor

@mrunalp mrunalp commented Apr 21, 2015

Today, we just change the prefix name of the scope file. This PR allows one to specify a slice to use as parent.

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

@mrunalp
Copy link
Contributor Author

mrunalp commented Apr 21, 2015

[root@localhost busybox]# nsinit exec --systemd -t sh
/ # cat /proc/self/cgroup 
10:hugetlb:/
9:perf_event:/
8:blkio:/system.slice/nsinit-busybox.scope
7:net_cls,net_prio:/
6:freezer:/system.slice/nsinit-busybox.scope
5:devices:/system.slice/nsinit-busybox.scope
4:memory:/system.slice/nsinit-busybox.scope
3:cpu,cpuacct:/system.slice/nsinit-busybox.scope
2:cpuset:/system.slice/nsinit-busybox.scope
1:name=systemd:/system.slice/nsinit-busybox.scope
/ # exit
[root@localhost busybox]# nsinit exec --cgroup-parent user.slice --systemd -t sh
/ # cat /proc/self/cgroup 
10:hugetlb:/
9:perf_event:/
8:blkio:/user.slice/nsinit-busybox.scope
7:net_cls,net_prio:/
6:freezer:/user.slice/nsinit-busybox.scope
5:devices:/user.slice/nsinit-busybox.scope
4:memory:/user.slice/nsinit-busybox.scope
3:cpu,cpuacct:/user.slice/nsinit-busybox.scope
2:cpuset:/user.slice/nsinit-busybox.scope
1:name=systemd:/user.slice/nsinit-busybox.scope
/ # exit

@ibuildthecloud
Copy link

Is there no way to get the container to launch in an existing service? The original proposal was allowing the ability to set the cgroup parent was specifically to allow this.

@mrunalp
Copy link
Contributor Author

mrunalp commented Apr 22, 2015

@ibuildthecloud I think might have to add the pid to the service's cgroup cpuset to make that work.

@mrunalp
Copy link
Contributor Author

mrunalp commented Apr 22, 2015

@vishh @crosbymichael ping

@vishh
Copy link
Contributor

vishh commented Apr 22, 2015

I get the intent, but, overloading a cgroup fs path to represent systemd slices doesn't sound good to me. May be its just me :) I would prefer moving in the direction of handling systemd explicitly. I wonder what other maintainers think - @vmarmol @crosbymichael @LK4D4.

@mrunalp
Copy link
Contributor Author

mrunalp commented Apr 22, 2015

@vishh Thanks for taking a look :) We could use another property do it, but this could be used to run multiple containers under the same slice. And there could be slices under slices so it gives us the hierarchy that we need to support the notion of a parent in systemd.

@rhatdan
Copy link
Contributor

rhatdan commented Apr 22, 2015

I would like to see there be an easy way to say run the container under the clients cgroup.

--cgroup-parent=self

Or something like that. (I actually wish this was the default).

Admins expect the constraints on a service or user executing the docker client to apply to the container. Most envision the container to be a child of the client. (Which is another story).

If I set up a systemd unit file with a constraint of only having 500M or memory, I would figure the container would run with just 500M of memory. To make this work currently you need to modify the docker run/create command.

@rhatdan
Copy link
Contributor

rhatdan commented Apr 22, 2015

I do like the idea of easily running multiple containers under the same cgroup constraints.

@vishh
Copy link
Contributor

vishh commented Apr 22, 2015

Docker feature request was to have the ability to group docker containers
under a systemd service. Although slices would achieve the same goal, I
wonder if that's what users want?

On Wed, Apr 22, 2015 at 11:39 AM, Mrunal Patel [email protected]
wrote:

@vishh https://github.com/vishh Thanks for taking a look :) We could
use another property do it, but this could be used to run multiple
containers under the same slice. And there could be slices under slices so
it gives us the hierarchy that we need to support the notion of a parent in
systemd.


Reply to this email directly or view it on GitHub
#549 (comment).

@mrunalp
Copy link
Contributor Author

mrunalp commented Apr 22, 2015

@vishh @rhatdan I see your point w.r.t service file integration. I would argue that it is a separate use case from this one which could probably be enabled by --cgroup-parent=self or some other keyword.
I think both the use cases should be supported. I think that this PR gives us parity with the fs implementation. Further PRs could be made to support inheriting systemd service cgroups.

Systemd provides three unit types that are useful for the purpose of resource control:

Services encapsulate a number of processes that are started and stopped by systemd based on configuration. Services are named in the style of quux.service.

Scopes encapsulate a number of processes that are started and stopped by arbitrary processes via fork(), and then registered at runtime with PID1. Scopes are named in the style of wuff.scope.

Slices may be used to group a number of services and scopes together in a hierarchial tree. Slices do not contain processes themselves, but the services and slices contained in them do. Slices are named in the style of foobar-waldo.slice, where the path to the location of the slice in the tree is encoded in the name with "-" as separator for the path components (foobar-waldo.slice is hence a subslice of foobar.slice). There's one special slices defined, -.slice, which is the root slice of all slices (foobar.slice is hence subslice of -.slice). This is similar how in regular file paths, "/" denotes the root directory.

@ibuildthecloud
Copy link

The original intent was to allow the docker container to be launched in the service unit's cgroup and then let the client die. If you notify systemd of the container PID systemd will now be properly monitoring the container.

There's two hang ups that prevent this from working. First, systemd doesn't seem to support launching new process and placing them in an existing service. Second, if you get the processes in the right cgroup and tell systemd about the PID you still get some odd behavior because systemd is monitoring a grandchild and not a child.

The best route right now seems to be to use cgroup parent and then disable systemd code and use cgroupfs. With these two things you can actually write a decent wrapper to Docker that will make the two play a bit nicer with each other.

@rhatdan
Copy link
Contributor

rhatdan commented Apr 23, 2015

Except this will not work in the long run, I believe, when the kernel moves to the Unified Model of handling cgroups.

@mrunalp mrunalp changed the title cgroups: Make systemd cgroup parent support to use specified slices. cgroups: Make systemd cgroup parent support use specified slices. Apr 23, 2015
@mrunalp
Copy link
Contributor Author

mrunalp commented Apr 28, 2015

@crosbymichael @rhatdan Any thoughts whether this PR is worth pursuing. I think it does make sense.

@rhatdan
Copy link
Contributor

rhatdan commented Apr 28, 2015

@mrunalp Yes I think this would be very valuable.

@ibuildthecloud
Copy link

This PR is far more reasonable than the current behavior.

@mrunalp
Copy link
Contributor Author

mrunalp commented Apr 28, 2015

@ibuildthecloud yes, today it essentially has no effect other than changing the name of the scope file.

@LK4D4
Copy link
Contributor

LK4D4 commented May 16, 2015

@mrunalp Need rebase, sorry :/

@mrunalp mrunalp force-pushed the cgroup/systemd_parent branch from ebd5f9b to bd948ae Compare May 17, 2015 02:18
@mrunalp mrunalp force-pushed the cgroup/systemd_parent branch from bd948ae to 3aa8e3a Compare May 17, 2015 02:26
@mrunalp
Copy link
Contributor Author

mrunalp commented May 17, 2015

@LK4D4 done

@LK4D4
Copy link
Contributor

LK4D4 commented May 17, 2015

I don't see harm.
LGTM

@LK4D4
Copy link
Contributor

LK4D4 commented May 26, 2015

ping @crosbymichael

1 similar comment
@LK4D4
Copy link
Contributor

LK4D4 commented Jun 2, 2015

ping @crosbymichael

@LK4D4
Copy link
Contributor

LK4D4 commented Jul 14, 2015

closing as ported

@LK4D4 LK4D4 closed this Jul 14, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants