Add support for enabling systemd cgroups #667
Conversation
mrunalp
force-pushed
the
systemd_cgroups
branch
from
ed540e6
to
e77fd6b
Compare
|
@icecrime @crosbymichael @LK4D4 PTAL. This PR adds support to enable systemd cgroups for runc. |
|
Thanks! Cc @anusha-ragunathan @mlaventure. |
|
/me is slightly cautious, given how many issues we've had with systemd in the past. At least it's no longer automatic, though. |
|
LGTM |
| } else { | ||
| // Parse the path from expected "slice:prefix:name" | ||
| // for e.g. "system.slice:docker:1234" | ||
| parts := strings.Split(myCgroupPath, ":") |
There was a problem hiding this comment.
This is not how docker works today. Refer daemon/oci_linux.go:createSpec. Couple of issues:
- CgroupParent and containerID are combined using a filepath.Join. Not by a colon separator.
- There's no ScopePrefix appended to the CgroupsPath.
Are the requirements about cgroups path being in "slice:prefix:name" format part of the spec? If yes, we will need docker changes to go along with this commit.
There was a problem hiding this comment.
@anusha-ragunathan Yes, I am well aware of that. The choice to encode the parameters within cgroupsPath was deliberate. The reason is that systemd cgroups API doesn't work with full cgroups paths. See https://github.com/opencontainers/runc/blob/master/libcontainer/cgroups/systemd/apply_systemd.go#L227
It expects the unit name that is formed from the prefix and name and the parent slice name.
Since OCI doesn't support these as first class fields, these are encoded in the cgroupsPath using ":" as a separator as a practical choice.
Also, I don't think that these should be first class concepts in OCI and runc can add this to support systemd cgroups as it isn't bound to just supporting OCI.
There was a problem hiding this comment.
This is similar to how docker supported systemd cgroups earlier using a daemon option. When the native.cgroupdriver is set to systemd, then createSpec could special form cgroupsPath using separators.
|
LGTM |
| // for e.g. "system.slice:docker:1234" | ||
| parts := strings.Split(myCgroupPath, ":") | ||
| if len(parts) != 3 { | ||
| return nil, fmt.Errorf("expected cgroupsPath to be of format \"slice:prefix:name\" for systemd cgroups") |
There was a problem hiding this comment.
can we add this to the help text ?
There was a problem hiding this comment.
Updated help text and the man page.
Signed-off-by: Mrunal Patel <mrunalp@gmail.com>
Signed-off-by: Mrunal Patel <mrunalp@gmail.com>
Signed-off-by: Mrunal Patel <mrunalp@gmail.com>
mrunalp
force-pushed
the
systemd_cgroups
branch
from
e77fd6b
to
0e41708
Compare
|
@crosbymichael @LK4D4 ping |
|
LGTM |
This adds a flag to enable systemd cgroups in runc.
Signed-off-by: Mrunal Patel mrunalp@gmail.com