tests/int: really randomize cgroup/unit names

Commit 41670e2 added some randomization to cgroup paths and (if systemd cgroup driver is used) systemd unit names, but - the randomization was done only if set_cgroups_path is called, which is not done by every test; - the randomization was per bats instance, not per test. Fix both issues by refactoring init_cgroups_path/set_cgroups_path (moving variable/random part to set_cgroups_path), and calling the latter from runc_spec, so it is now applicable to every container. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
opencontainers · Apr 1, 2021 · 0d60c46 · 0d60c46
1 parent f09a3e1
commit 0d60c46
Show file tree

Hide file tree

Showing 7 changed files with 28 additions and 44 deletions.
diff --git a/tests/integration/cgroups.bats b/tests/integration/cgroups.bats
@@ -14,8 +14,6 @@ function setup() {
 	[[ "$ROOTLESS" -ne 0 ]] && requires rootless_cgroup
 	requires cgroups_kmem
 
-	set_cgroups_path
-
 	# Set some initial known values
 	update_config '.linux.resources.memory |= {"kernel": 16777216, "kernelTCP": 11534336}'
 
@@ -41,8 +39,6 @@ function setup() {
 	[[ "$ROOTLESS" -ne 0 ]] && requires rootless_cgroup
 	requires cgroups_kmem
 
-	set_cgroups_path
-
 	# run a detached busybox to work with
 	runc run -d --console-socket "$CONSOLE_SOCKET" test_cgroups_kmem
 	[ "$status" -eq 0 ]
@@ -70,8 +66,6 @@ function setup() {
 	# systemd controls the permission, so error does not happen
 	requires no_systemd
 
-	set_cgroups_path
-
 	runc run -d --console-socket "$CONSOLE_SOCKET" test_cgroups_permissions
 	[ "$status" -eq 1 ]
 	[[ "$output" == *"applying cgroup configuration"*"permission denied"* ]]
@@ -94,7 +88,6 @@ function setup() {
 @test "runc create (limits + cgrouppath + permission on the cgroup dir) succeeds" {
 	[[ "$ROOTLESS" -ne 0 ]] && requires rootless_cgroup
 
-	set_cgroups_path
 	set_resources_limit
 
 	runc run -d --console-socket "$CONSOLE_SOCKET" test_cgroups_permissions
@@ -116,7 +109,6 @@ function setup() {
 @test "runc exec (limits + cgrouppath + permission on the cgroup dir) succeeds" {
 	[[ "$ROOTLESS" -ne 0 ]] && requires rootless_cgroup
 
-	set_cgroups_path
 	set_resources_limit
 
 	runc run -d --console-socket "$CONSOLE_SOCKET" test_cgroups_permissions
@@ -130,7 +122,6 @@ function setup() {
 @test "runc exec (cgroup v2 + init process in non-root cgroup) succeeds" {
 	requires root cgroups_v2
 
-	set_cgroups_path
 	set_cgroup_mount_writable
 
 	runc run -d --console-socket "$CONSOLE_SOCKET" test_cgroups_group
@@ -177,7 +168,6 @@ function setup() {
 @test "runc run (cgroup v1 + unified resources should fail)" {
 	requires root cgroups_v1
 
-	set_cgroups_path
 	set_resources_limit
 	update_config '.linux.resources.unified |= {"memory.min": "131072"}'
 
@@ -189,8 +179,7 @@ function setup() {
 @test "runc run (blkio weight)" {
 	requires root cgroups_v2
 
-	set_cgroups_path "$BUSYBOX_BUNDLE"
-	update_config '.linux.resources.blockIO |= {"weight": 750}' "${BUSYBOX_BUNDLE}"
+	update_config '.linux.resources.blockIO |= {"weight": 750}'
 
 	runc run -d --console-socket "$CONSOLE_SOCKET" test_cgroups_unified
 	[ "$status" -eq 0 ]
@@ -207,7 +196,6 @@ function setup() {
 @test "runc run (cgroup v2 resources.unified only)" {
 	requires root cgroups_v2
 
-	set_cgroups_path
 	update_config ' .linux.resources.unified |= {
 				"memory.min":   "131072",
 				"memory.low":   "524288",
@@ -247,7 +235,6 @@ function setup() {
 @test "runc run (cgroup v2 resources.unified override)" {
 	requires root cgroups_v2
 
-	set_cgroups_path
 	# CPU shares of 3333 corresponds to CPU weight of 128.
 	update_config '   .linux.resources.memory |= {"limit": 33554432}
 			| .linux.resources.memorySwap |= {"limit": 33554432}

diff --git a/tests/integration/delete.bats b/tests/integration/delete.bats
@@ -52,7 +52,6 @@ function teardown() {
 
 @test "runc delete --force in cgroupv1 with subcgroups" {
 	requires cgroups_v1 root cgroupns
-	set_cgroups_path
 	set_cgroup_mount_writable
 	# enable cgroupns
 	update_config '.linux.namespaces += [{"type": "cgroup"}]'
@@ -102,7 +101,6 @@ EOF
 
 @test "runc delete --force in cgroupv2 with subcgroups" {
 	requires cgroups_v2 root
-	set_cgroups_path
 	set_cgroup_mount_writable
 
 	# run busybox detached

diff --git a/tests/integration/events.bats b/tests/integration/events.bats
@@ -13,7 +13,6 @@ function teardown() {
 @test "events --stats" {
 	# XXX: currently cgroups require root containers.
 	requires root
-	init_cgroup_paths
 
 	# run busybox detached
 	runc run -d --console-socket "$CONSOLE_SOCKET" test_busybox
@@ -29,7 +28,6 @@ function teardown() {
 function test_events() {
 	# XXX: currently cgroups require root containers.
 	requires root
-	init_cgroup_paths
 
 	local status interval retry_every=1
 	if [ $# -eq 2 ]; then
@@ -73,7 +71,6 @@ function test_events() {
 @test "events oom" {
 	# XXX: currently cgroups require root containers.
 	requires root cgroups_swap
-	init_cgroup_paths
 
 	# we need the container to hit OOM, so disable swap
 	update_config '(.. | select(.resources? != null)) .resources.memory |= {"limit": 33554432, "swap": 33554432}'

diff --git a/tests/integration/helpers.bash b/tests/integration/helpers.bash
@@ -73,6 +73,8 @@ function runc_spec() {
 	if [[ "$ROOTLESS" -ne 0 ]] && [[ "$ROOTLESS_FEATURES" == *"cgroup"* ]]; then
 		runc_rootless_cgroup "$bundle"
 	fi
+
+	set_cgroups_path
 }
 
 # Helper function to reformat config.json file. Input uses jq syntax.
@@ -112,22 +114,6 @@ function init_cgroup_paths() {
 	# init once
 	test -n "$CGROUP_UNIFIED" && return
 
-	local rnd="$RANDOM"
-	if [ -n "${RUNC_USE_SYSTEMD}" ]; then
-		SD_UNIT_NAME="runc-cgroups-integration-test-${rnd}.scope"
-		if [ $(id -u) = "0" ]; then
-			REL_CGROUPS_PATH="/machine.slice/$SD_UNIT_NAME"
-			OCI_CGROUPS_PATH="machine.slice:runc-cgroups:integration-test-${rnd}"
-		else
-			REL_CGROUPS_PATH="/user.slice/user-$(id -u).slice/user@$(id -u).service/machine.slice/$SD_UNIT_NAME"
-			# OCI path doesn't contain "/user.slice/user-$(id -u).slice/user@$(id -u).service/" prefix
-			OCI_CGROUPS_PATH="machine.slice:runc-cgroups:integration-test-${rnd}"
-		fi
-	else
-		REL_CGROUPS_PATH="/runc-cgroups-integration-test/test-cgroup-${rnd}"
-		OCI_CGROUPS_PATH=$REL_CGROUPS_PATH
-	fi
-
 	if stat -f -c %t /sys/fs/cgroup | grep -qFw 63677270; then
 		CGROUP_UNIFIED=yes
 		# "pseudo" controllers do not appear in /sys/fs/cgroup/cgroup.controllers.
@@ -140,7 +126,6 @@ function init_cgroup_paths() {
 			echo devices
 		)
 		CGROUP_BASE_PATH=/sys/fs/cgroup
-		CGROUP_PATH=${CGROUP_BASE_PATH}${REL_CGROUPS_PATH}
 
 		# Find any cgroup.freeze files...
 		if [ -n "$(find "$CGROUP_BASE_PATH" -type f -name "cgroup.freeze" -print -quit)" ]; then
@@ -158,11 +143,34 @@ function init_cgroup_paths() {
 	fi
 }
 
-# Helper function to set cgroupsPath to the value of $OCI_CGROUPS_PATH
+# Randomize cgroup path(s), and update cgroupsPath in config.json.
+# This function sets a few cgroup-related variables.
 function set_cgroups_path() {
 	bundle="${1:-.}"
 	init_cgroup_paths
-	update_config '.linux.cgroupsPath |= "'"${OCI_CGROUPS_PATH}"'"' $bundle
+
+	local rnd="$RANDOM"
+	if [ -n "${RUNC_USE_SYSTEMD}" ]; then
+		SD_UNIT_NAME="runc-cgroups-integration-test-${rnd}.scope"
+		if [ "$(id -u)" = "0" ]; then
+			REL_CGROUPS_PATH="/machine.slice/$SD_UNIT_NAME"
+			OCI_CGROUPS_PATH="machine.slice:runc-cgroups:integration-test-${rnd}"
+		else
+			REL_CGROUPS_PATH="/user.slice/user-$(id -u).slice/user@$(id -u).service/machine.slice/$SD_UNIT_NAME"
+			# OCI path doesn't contain "/user.slice/user-$(id -u).slice/user@$(id -u).service/" prefix
+			OCI_CGROUPS_PATH="machine.slice:runc-cgroups:integration-test-${rnd}"
+		fi
+	else
+		REL_CGROUPS_PATH="/runc-cgroups-integration-test/test-cgroup-${rnd}"
+		OCI_CGROUPS_PATH=$REL_CGROUPS_PATH
+	fi
+
+	# Absolute path to container's cgroup v2.
+	if [ "$CGROUP_UNIFIED" == "yes" ]; then
+		CGROUP_PATH=${CGROUP_BASE_PATH}${REL_CGROUPS_PATH}
+	fi
+
+	update_config '.linux.cgroupsPath |= "'"${OCI_CGROUPS_PATH}"'"' "$bundle"
 }
 
 # Helper to check a value in cgroups.

diff --git a/tests/integration/pause.bats b/tests/integration/pause.bats
@@ -13,7 +13,6 @@ function teardown() {
 @test "runc pause and resume" {
 	if [[ "$ROOTLESS" -ne 0 ]]; then
 		requires rootless_cgroup
-		set_cgroups_path
 	fi
 	requires cgroups_freezer
 
@@ -41,7 +40,6 @@ function teardown() {
 @test "runc pause and resume with nonexist container" {
 	if [[ "$ROOTLESS" -ne 0 ]]; then
 		requires rootless_cgroup
-		set_cgroups_path
 	fi
 	requires cgroups_freezer
 

diff --git a/tests/integration/ps.bats b/tests/integration/ps.bats
@@ -63,7 +63,6 @@ function teardown() {
 @test "ps after the container stopped" {
 	# ps requires cgroups
 	[[ "$ROOTLESS" -ne 0 ]] && requires rootless_cgroup
-	set_cgroups_path
 
 	# start busybox detached
 	runc run -d --console-socket "$CONSOLE_SOCKET" test_busybox

diff --git a/tests/integration/update.bats b/tests/integration/update.bats
@@ -10,8 +10,6 @@ function teardown() {
 function setup() {
 	setup_busybox
 
-	set_cgroups_path
-
 	# Set some initial known values
 	update_config ' .linux.resources.memory |= {"limit": 33554432, "reservation": 25165824}
 			| .linux.resources.cpu |= {"shares": 100, "quota": 500000, "period": 1000000, "cpus": "0"}
@@ -31,7 +29,6 @@ function setup() {
 			fi
 		done
 	fi
-	init_cgroup_paths
 
 	# run a few busyboxes detached
 	runc run -d --console-socket "$CONSOLE_SOCKET" test_update