From 12f2f03fd2e1cd64a73f5ed70564569e43060462 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 23 Mar 2023 11:57:46 -0700 Subject: [PATCH] [1.1] runc run: refuse a non-empty cgroup for systemd driver As this is currently not possible to add a PID into an existing systemd unit, plus this feature will be deprected in runc 1.2 (see commit d08bc0c1b3bb2 ("runc run: warn on non-empty cgroup"), let's reject sharing a systemd unit between two containers, and fix the test case accordingly. We still allow this to happen in case cgroupfs driver is used, to minimize the potential compatibility issues in a stable branch. This is an adaptation of main branch commit 82bc89cd10e for 1.1. Signed-off-by: Kir Kolyshkin --- libcontainer/factory_linux.go | 6 ++++++ tests/integration/cgroups.bats | 13 ++++++++++--- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/libcontainer/factory_linux.go b/libcontainer/factory_linux.go index e6c71ac34..1df87e2e5 100644 --- a/libcontainer/factory_linux.go +++ b/libcontainer/factory_linux.go @@ -179,6 +179,12 @@ func (l *LinuxFactory) Create(id string, config *configs.Config) (Container, err return nil, fmt.Errorf("unable to get cgroup PIDs: %w", err) } if len(pids) != 0 { + if config.Cgroups.Systemd { + // systemd cgroup driver can't add a pid to an + // existing systemd unit and will return an + // error anyway, so let's error out early. + return nil, fmt.Errorf("container's cgroup is not empty: %d process(es) found", len(pids)) + } // TODO: return an error. logrus.Warnf("container's cgroup is not empty: %d process(es) found", len(pids)) logrus.Warn("DEPRECATED: running container in a non-empty cgroup won't be supported in runc 1.2; https://github.com/opencontainers/runc/issues/3132") diff --git a/tests/integration/cgroups.bats b/tests/integration/cgroups.bats index ff7cf6d0b..7fd2e30af 100644 --- a/tests/integration/cgroups.bats +++ b/tests/integration/cgroups.bats @@ -348,7 +348,7 @@ function setup() { [ "$output" = "ok" ] } -@test "runc run/create should warn about a non-empty cgroup" { +@test "runc run/create should error/warn about a non-empty cgroup" { if [[ "$ROOTLESS" -ne 0 ]]; then requires rootless_cgroup fi @@ -358,14 +358,21 @@ function setup() { runc run -d --console-socket "$CONSOLE_SOCKET" ct1 [ "$status" -eq 0 ] + # When systemd driver is used, runc can't add PID to an existing unit, + # so runc returns an error. For backward compatibility, we still allow + # such configuration in 1.1, but only when systemd driver is NOT used. + # See https://github.com/opencontainers/runc/issues/3780. + local exp=0 + [[ -n "${RUNC_USE_SYSTEMD}" ]] && exp=1 + # Run a second container sharing the cgroup with the first one. runc --debug run -d --console-socket "$CONSOLE_SOCKET" ct2 - [ "$status" -eq 0 ] + [ "$status" -eq "$exp" ] [[ "$output" == *"container's cgroup is not empty"* ]] # Same but using runc create. runc create --console-socket "$CONSOLE_SOCKET" ct3 - [ "$status" -eq 0 ] + [ "$status" -eq "$exp" ] [[ "$output" == *"container's cgroup is not empty"* ]] }