runc exec --cgroup #3059

kolyshkin · 2021-07-02T18:30:49Z

This does two things

Adds minimal hybrid hierarchy support to cgroup v1 fs driver. It seems it's needed when systemd is used, also makes it possible for cgroup v1 drivers to use the new cgroup.kill kernel API (in case hybrid hierarchy is available). This part is a carry of cgroups/systemd: add cgroup-v2 path to the list when using hybrid mode #2087.
Implements runc exec --cgroup to be able to run a process in a sub-cgroup of a container (as per [RFC] runc exec --cgroup #3040).

Fixes: #3040.
Closes: #2087.

Usage (option for runc exec):

--cgroup path | controller[,controller...]:path

Execute a process in a sub-cgroup. If the specified cgroup does not exist, an
error is returned. Default is empty path, which means to use container's top
level cgroup.

For cgroup v1 only, a particular controller (or multiple comma-separated
controllers) can be specified, and the option can be used multiple times to set
different paths for different controllers.

Note for cgroup v2, in case the process can't join the top level cgroup,
runc exec fallback is to try joining the cgroup of container's init.
This fallback can be disabled by using --cgroup /.

Proposed changelog entry

New features:
 - runc exec --cgroup: an option to specify a (non-top) in-container cgroup
   to use for the process being executed (#3040, #3059)
 - cgroup v1 controllers now support hybrid hierarchy (i.e. when on a cgroup v1
   machine a cgroup2 filesystem is mounted to /sys/fs/cgroup/unified, runc run/exec
   now adds the container to the appropriate cgroup under it) (#2087, #3059)

kolyshkin · 2021-07-22T00:14:31Z

# runc exec test_busybox sh -euc ! grep -v ':/$' /proc/self/cgroup (status=1):
# 0::/../../system.slice/runner-provisioner.service

This must be a unified path for hybrid cgroup v1+v2, which we do not handle well. Looks like I see what needs to be fixed (not a problem in this PR).

kolyshkin · 2021-07-22T00:38:29Z

This must be a unified path for hybrid cgroup v1+v2, which we do not handle well. Looks like I see what needs to be fixed (not a problem in this PR).

This is presumably fixed by #2087. Let me check...

kolyshkin · 2021-08-02T19:58:37Z

This is presumably fixed by #2087. Let me check...

Yes, but it requires criu 3.16+, so draft until it's released.

tests/integration/exec.bats

kolyshkin · 2021-08-17T01:58:51Z

Rebased; made the cgroup v1 test case work without cgroupns.

kolyshkin · 2021-08-31T04:11:22Z

Looks like criu 3.16 won't be in time for runc 1.1.0 release, so moving the milestone to 1.2.0.

kolyshkin · 2021-09-07T03:46:39Z

Looks like criu 3.16 won't be in time for runc 1.1.0 release, so moving the milestone to 1.2.0.

Or maybe it will -- if it is, I'll move the milestone back to 1.1.0, as this is quite important (not the feature per se, but the hybrid cgroup entering code from #2087).

kolyshkin · 2021-09-22T23:54:41Z

Rebased; this is supposed to fail until criu PPA will have CRIU 3.16 (I hope soon).

No need to have an intermediate variable here. Signed-off-by: Kir Kolyshkin <[email protected]>

No need to add a file name to the error messages, as errors from OpenFile and (*os.File).Write both contain the file name already. Signed-off-by: Kir Kolyshkin <[email protected]>

The function used here, cgroups.EnterPid, silently skips non-existing paths, and it does not look like a good idea to do so for an existing container with already configured cgroups. Switch to cgroups.WriteCgroupProc which does not do that, so in case a cgroup does not exist, we'll get an error. Signed-off-by: Kir Kolyshkin <[email protected]>

@kolyshkin

Currently the parent process of the container is moved to the right cgroup v2 tree when systemd is using a hybrid model (last line with 0::): $ runc --systemd-cgroup run myid / # cat /proc/self/cgroup 12:cpuset:/system.slice/runc-myid.scope 11:blkio:/system.slice/runc-myid.scope 10:devices:/system.slice/runc-myid.scope 9:hugetlb:/system.slice/runc-myid.scope 8:memory:/system.slice/runc-myid.scope 7:rdma:/ 6:perf_event:/system.slice/runc-myid.scope 5:net_cls,net_prio:/system.slice/runc-myid.scope 4:freezer:/system.slice/runc-myid.scope 3:pids:/system.slice/runc-myid.scope 2:cpu,cpuacct:/system.slice/runc-myid.scope 1:name=systemd:/system.slice/runc-myid.scope 0::/system.slice/runc-myid.scope However, if a second process is executed in the same container, it is not moved to the right cgroup v2 tree: $ runc exec myid /bin/sh -c 'cat /proc/self/cgroup' 12:cpuset:/system.slice/runc-myid.scope 11:blkio:/system.slice/runc-myid.scope 10:devices:/system.slice/runc-myid.scope 9:hugetlb:/system.slice/runc-myid.scope 8:memory:/system.slice/runc-myid.scope 7:rdma:/ 6:perf_event:/system.slice/runc-myid.scope 5:net_cls,net_prio:/system.slice/runc-myid.scope 4:freezer:/system.slice/runc-myid.scope 3:pids:/system.slice/runc-myid.scope 2:cpu,cpuacct:/system.slice/runc-myid.scope 1:name=systemd:/system.slice/runc-myid.scope 0::/user.slice/user-1000.slice/session-8.scope This commit makes that processes executed with exec are placed into the right cgroup v2 tree. The implementation checks if systemd is using a hybrid mode (by checking if cgroups v2 is mounted in /sys/fs/cgroup/unified), if yes, the path of the cgroup v2 slice for this container is saved into the cgroup path list. The fs group driver has a similar issue, in this case none of the runc run or runc exec commands put the process in the right cgroups v2. This commit also fixes that. Having the processes of the container in its own cgroup v2 is useful for any BPF programs that rely on bpf_get_current_cgroup_id(), like https://github.com/kinvolk/inspektor-gadget/ for instance. [@kolyshkin: rebased] Signed-off-by: Mauricio Vásquez <[email protected]> Signed-off-by: Kir Kolyshkin <[email protected]>

h-vetinari · 2021-09-26T03:01:13Z

Should this have a changelog entry?

kolyshkin · 2021-09-27T17:09:07Z

Should this have a changelog entry?

@h-vetinari added, PTAL

tests/integration/cgroups.bats

Check that runc run and runc exec put the process on the same cgroups v2 when using hybrid mode. Signed-off-by: Mauricio Vásquez <[email protected]> Signed-off-by: Kir Kolyshkin <[email protected]>

In some setups, multiple cgroups are used inside a container, and sometime there is a need to execute a process in a particular sub-cgroup (in case of cgroup v1, for a particular controller). This is what this commit implements. Signed-off-by: Kir Kolyshkin <[email protected]>

cyphar

LGTM. My only minor concern is that someone will think that --cgroup applies to the host cgroup paths, but since it's well documented hopefully that won't happen.

kolyshkin · 2021-10-07T01:02:40Z

I guess we can merge this one.

kolyshkin mentioned this pull request Jul 2, 2021

[RFC] runc exec --cgroup #3040

Closed

This comment has been minimized.

Sign in to view

kolyshkin force-pushed the cgroup-clean branch 5 times, most recently from 2532b02 to f850fd5 Compare July 12, 2021 23:26

kolyshkin force-pushed the cgroup-clean branch from f850fd5 to accfadf Compare July 21, 2021 23:26

kolyshkin force-pushed the cgroup-clean branch from accfadf to 33b44c1 Compare July 22, 2021 00:38

kolyshkin mentioned this pull request Jul 29, 2021

[WIP] [carry #2087] cgroups/systemd: add cgroup-v2 path to the list when using hybrid mode #3107

Closed

kolyshkin force-pushed the cgroup-clean branch 3 times, most recently from 6f831f2 to 0a0c868 Compare July 29, 2021 02:00

AkihiroSuda reviewed Aug 10, 2021

View reviewed changes

tests/integration/exec.bats Show resolved Hide resolved

kolyshkin force-pushed the cgroup-clean branch from 0a0c868 to 902be21 Compare August 17, 2021 01:58

kolyshkin force-pushed the cgroup-clean branch from 902be21 to ee2f532 Compare August 17, 2021 17:27

kolyshkin requested a review from AkihiroSuda August 24, 2021 21:26

kolyshkin added this to the 1.2.0 milestone Aug 31, 2021

kolyshkin mentioned this pull request Aug 31, 2021

cpuset: option for updating cpus and mems in child groups #3189

Open

kolyshkin force-pushed the cgroup-clean branch 2 times, most recently from c9d45a7 to 229b86a Compare August 31, 2021 17:22

kolyshkin mentioned this pull request Aug 31, 2021

cgroups/systemd: add cgroup-v2 path to the list when using hybrid mode #2087

Closed

kolyshkin force-pushed the cgroup-clean branch from 229b86a to cc3905d Compare September 2, 2021 15:11

kolyshkin mentioned this pull request Sep 3, 2021

Processes launched via docker exec are not placed into the correct cgroup moby/moby#42704

Closed

kolyshkin force-pushed the cgroup-clean branch from 8f672ef to a6eb8fe Compare September 22, 2021 23:51

kolyshkin mentioned this pull request Sep 23, 2021

CI/GHA: switch to OBS criu repo, bring in criu 3.16 #3222

Merged

kolyshkin force-pushed the cgroup-clean branch from a6eb8fe to 5d711c1 Compare September 23, 2021 20:40

kolyshkin and others added 4 commits September 23, 2021 19:26

exec.go: nit

cc1d746

No need to have an intermediate variable here. Signed-off-by: Kir Kolyshkin <[email protected]>

libct/cg.WriteCgroupProcs: improve errors

7d446c6

No need to add a file name to the error messages, as errors from OpenFile and (*os.File).Write both contain the file name already. Signed-off-by: Kir Kolyshkin <[email protected]>

kolyshkin force-pushed the cgroup-clean branch from 5d711c1 to 1747cc0 Compare September 24, 2021 02:29

kolyshkin marked this pull request as ready for review September 24, 2021 02:40

kolyshkin modified the milestones: 1.2.0, 1.1.0 Sep 24, 2021

kolyshkin added area/cgroupv1 area/cgroupv2 area/checkpoint-restore labels Sep 27, 2021

kolyshkin commented Sep 27, 2021

View reviewed changes

tests/integration/cgroups.bats Outdated Show resolved Hide resolved

mauriciovasquezbernal and others added 2 commits September 27, 2021 10:25

tests: add integration test for cgroups hybrid

cc15b88

Check that runc run and runc exec put the process on the same cgroups v2 when using hybrid mode. Signed-off-by: Mauricio Vásquez <[email protected]> Signed-off-by: Kir Kolyshkin <[email protected]>

kolyshkin force-pushed the cgroup-clean branch from 1747cc0 to 0202c39 Compare September 27, 2021 17:25

kolyshkin mentioned this pull request Sep 27, 2021

Release 1.1.0 #3195

Closed

cyphar approved these changes Sep 28, 2021

View reviewed changes

AkihiroSuda approved these changes Oct 1, 2021

View reviewed changes

AkihiroSuda requested a review from thaJeztah October 1, 2021 11:54

webcompat-bot mentioned this pull request Oct 6, 2021

github.com - design is broken webcompat/web-bugs#89182

Closed

kolyshkin merged commit f594ede into opencontainers:master Oct 7, 2021

kolyshkin mentioned this pull request Oct 7, 2021

run create/run/exec: refuse bad cgroup #3223

Merged

giuseppe mentioned this pull request Oct 8, 2021

support exec --cgroup containers/crun#749

Closed

runc exec --cgroup #3059

runc exec --cgroup #3059

Uh oh!

Conversation

kolyshkin commented Jul 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Usage (option for runc exec):

Proposed changelog entry

Uh oh!

This comment has been minimized.

kolyshkin commented Jul 22, 2021

Uh oh!

kolyshkin commented Jul 22, 2021

Uh oh!

kolyshkin commented Aug 2, 2021

Uh oh!

Uh oh!

kolyshkin commented Aug 17, 2021

Uh oh!

kolyshkin commented Aug 31, 2021

Uh oh!

kolyshkin commented Sep 7, 2021

Uh oh!

kolyshkin commented Sep 22, 2021

Uh oh!

h-vetinari commented Sep 26, 2021

Uh oh!

kolyshkin commented Sep 27, 2021

Uh oh!

Uh oh!

cyphar left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kolyshkin commented Oct 7, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

kolyshkin commented Jul 2, 2021 •

edited

Loading

cyphar left a comment •

edited

Loading