From cfc801b7ed89555b5622bfc9a88ee731d0670e90 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 2 Aug 2023 18:54:43 -0700 Subject: [PATCH 1/4] Fix running tests under Docker/Podman and cgroup v2 For "make integration", the tests are run inside a Docker/Podman container. Problem is, if cgroup v2 is used, the in-container /sys/fs/cgroup/cgroup.subtree_control is empty. The added script, used as Docker entrypoint, moves the current process into a sub-cgroup, and then adds all controllers in top-level cgroup.subtree_control. Signed-off-by: Kir Kolyshkin --- Dockerfile | 4 ++++ script/prepare-cgroup-v2.sh | 17 +++++++++++++++++ 2 files changed, 21 insertions(+) create mode 100755 script/prepare-cgroup-v2.sh diff --git a/Dockerfile b/Dockerfile index 96ead8c4918..9fd29a59371 100644 --- a/Dockerfile +++ b/Dockerfile @@ -63,3 +63,7 @@ ENV PKG_CONFIG_PATH=/opt/libseccomp/lib/pkgconfig RUN git config --global --add safe.directory /go/src/github.com/opencontainers/runc WORKDIR /go/src/github.com/opencontainers/runc + +# Fixup for cgroup v2. +COPY script/prepare-cgroup-v2.sh / +ENTRYPOINT [ "/prepare-cgroup-v2.sh" ] diff --git a/script/prepare-cgroup-v2.sh b/script/prepare-cgroup-v2.sh new file mode 100755 index 00000000000..886c550ec46 --- /dev/null +++ b/script/prepare-cgroup-v2.sh @@ -0,0 +1,17 @@ +#!/bin/bash +# +# This script is used from ../Dockerfile as the ENTRYPOINT. It sets up cgroup +# delegation for cgroup v2 to make sure runc tests can be properly run inside +# a container. + +# Only do this for cgroup v2. +if [ -f /sys/fs/cgroup/cgroup.controllers ]; then + set -x + # Move the current process to a sub-cgroup. + mkdir /sys/fs/cgroup/init + echo 0 >/sys/fs/cgroup/init/cgroup.procs + # Enable all controllers. + sed 's/\b\w/+\0/g' <"/sys/fs/cgroup/cgroup.controllers" >"/sys/fs/cgroup/cgroup.subtree_control" +fi + +exec "$@" From 962019d64e578be4fbd3a2b2ce5c9f19be7346c0 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 2 Aug 2023 20:13:03 -0700 Subject: [PATCH 2/4] ci: fix TestNilResources when systemd not available Split the test into two -- for fs and systemd cgroup managers, and only run the second one if systemd is available. Prevents the following failure during `make unittest`: > === RUN TestNilResources > manager_test.go:27: systemd not running on this host, cannot use systemd cgroups manager > --- FAIL: TestNilResources (0.22s) Signed-off-by: Kir Kolyshkin --- libcontainer/cgroups/manager/manager_test.go | 65 ++++++++++++-------- 1 file changed, 38 insertions(+), 27 deletions(-) diff --git a/libcontainer/cgroups/manager/manager_test.go b/libcontainer/cgroups/manager/manager_test.go index b53e6f1761e..6f0c0703a60 100644 --- a/libcontainer/cgroups/manager/manager_test.go +++ b/libcontainer/cgroups/manager/manager_test.go @@ -3,6 +3,7 @@ package manager import ( "testing" + "github.com/opencontainers/runc/libcontainer/cgroups/systemd" "github.com/opencontainers/runc/libcontainer/configs" ) @@ -10,35 +11,45 @@ import ( // config.Resources is nil. While it does not make sense to use a // manager with no resources, it should not result in a panic. // -// This tests either v1 or v2 managers (both fs and systemd), -// depending on what cgroup version is available on the host. +// This tests either v1 or v2 fs cgroup manager, depending on which +// cgroup version is available. func TestNilResources(t *testing.T) { - for _, sd := range []bool{false, true} { - cg := &configs.Cgroup{} // .Resources is nil - cg.Systemd = sd - mgr, err := New(cg) + testNilResources(t, false) +} + +// TestNilResourcesSystemd is the same as TestNilResources, +// only checking the systemd cgroup manager. +func TestNilResourcesSystemd(t *testing.T) { + if !systemd.IsRunningSystemd() { + t.Skip("requires systemd") + } + testNilResources(t, true) +} + +func testNilResources(t *testing.T, systemd bool) { + cg := &configs.Cgroup{} // .Resources is nil + cg.Systemd = systemd + mgr, err := New(cg) + if err != nil { + // Some managers require non-nil Resources during + // instantiation -- provide and retry. In such case + // we're mostly testing Set(nil) below. + cg.Resources = &configs.Resources{} + mgr, err = New(cg) if err != nil { - // Some managers require non-nil Resources during - // instantiation -- provide and retry. In such case - // we're mostly testing Set(nil) below. - cg.Resources = &configs.Resources{} - mgr, err = New(cg) - if err != nil { - t.Error(err) - continue - } + t.Fatal(err) } - _ = mgr.Apply(-1) - _ = mgr.Set(nil) - _ = mgr.Freeze(configs.Thawed) - _ = mgr.Exists() - _, _ = mgr.GetAllPids() - _, _ = mgr.GetCgroups() - _, _ = mgr.GetFreezerState() - _ = mgr.Path("") - _ = mgr.GetPaths() - _, _ = mgr.GetStats() - _, _ = mgr.OOMKillCount() - _ = mgr.Destroy() } + _ = mgr.Apply(-1) + _ = mgr.Set(nil) + _ = mgr.Freeze(configs.Thawed) + _ = mgr.Exists() + _, _ = mgr.GetAllPids() + _, _ = mgr.GetCgroups() + _, _ = mgr.GetFreezerState() + _ = mgr.Path("") + _ = mgr.GetPaths() + _, _ = mgr.GetStats() + _, _ = mgr.OOMKillCount() + _ = mgr.Destroy() } From 5c6b334c88f761dca933ca58cd221cf32327e943 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 2 Aug 2023 20:21:24 -0700 Subject: [PATCH 3/4] ci: fix TestOpenat2 when no systemd is used A few cases relied on the fact that systemd is used, and thus /sys/fs/cgroup/user.slice is available. Guess what, in case of "make unittest" it might not be. Signed-off-by: Kir Kolyshkin --- libcontainer/cgroups/file_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/libcontainer/cgroups/file_test.go b/libcontainer/cgroups/file_test.go index dc2b0630cde..94f1a99bff0 100644 --- a/libcontainer/cgroups/file_test.go +++ b/libcontainer/cgroups/file_test.go @@ -58,8 +58,6 @@ func TestOpenat2(t *testing.T) { {"/sys/fs/cgroup", "/cgroup.controllers"}, {"/sys/fs/cgroup/", "cgroup.controllers"}, {"/sys/fs/cgroup/", "/cgroup.controllers"}, - {"/sys/fs/cgroup/user.slice", "cgroup.controllers"}, - {"/sys/fs/cgroup/user.slice/", "/cgroup.controllers"}, {"/", "/sys/fs/cgroup/cgroup.controllers"}, {"/", "sys/fs/cgroup/cgroup.controllers"}, {"/sys/fs/cgroup/cgroup.controllers", ""}, From f88a7654601a9c50ba5e0c47f49eb81134b23146 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 2 Aug 2023 20:38:09 -0700 Subject: [PATCH 4/4] ci: fix flaky test "update memory vs CheckBeforeUpdate" This test fails in CI sometimes with the following error: > `testcontainer test_update stopped' failed Give OOM killer some time to do its job. Signed-off-by: Kir Kolyshkin --- tests/integration/update.bats | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/update.bats b/tests/integration/update.bats index 1a16c9ae34c..5a87fa1500d 100644 --- a/tests/integration/update.bats +++ b/tests/integration/update.bats @@ -854,5 +854,5 @@ EOF # The container will be OOM killed, and runc might either succeed # or fail depending on the timing, so we don't check its exit code. runc update test_update --memory 1024 - testcontainer test_update stopped + wait_for_container 10 1 test_update stopped }