From 5ea7c60f334e4a714ef7a3a5d952f03935d469d6 Mon Sep 17 00:00:00 2001
From: Kir Kolyshkin <kolyshkin@gmail.com>
Date: Fri, 20 Oct 2023 12:33:20 -0700
Subject: [PATCH] tests/int: fix cgroup tests

Commit e1584831b6835d did two modifications to check_cgroup_value():

1. It now skips the test if the file is not found.

2. If the comparison failed, a second comparison, with value divided by 1000,
   is performed.

These modifications were only needed for cpu.burst, but instead were done
in a generic function used from many cgroup tests. As a result, we can
no longer be sure about the test coverage (item 1) and the check being
correct (item 2) anymore. In fact, part of "update cgroup cpu limits"
test is currently skipped on CentOS 7 and 8 because of item 1.

To fix:
 - replace item 1 with a new "cgroups_cpu_burst" argument for "requires",
   and move the test to a separate case;
 - replace item 2 with a local change in check_cpu_burst.

Fixes: e1584831b6835d
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
---
 tests/integration/helpers.bash | 22 ++++++++++++++++------
 tests/integration/update.bats  | 23 +++++++++++++++++------
 2 files changed, 33 insertions(+), 12 deletions(-)

diff --git a/tests/integration/helpers.bash b/tests/integration/helpers.bash
index 7e6399a47..811f817ab 100755
--- a/tests/integration/helpers.bash
+++ b/tests/integration/helpers.bash
@@ -260,16 +260,11 @@ function get_cgroup_value() {
 # Helper to check a if value in a cgroup file matches the expected one.
 function check_cgroup_value() {
 	local current
-	local cgroup
-	cgroup="$(get_cgroup_path "$1")"
-	if [ ! -f "$cgroup/$1" ]; then
-		skip "$cgroup/$1 does not exist"
-	fi
 	current="$(get_cgroup_value "$1")"
 	local expected=$2
 
 	echo "current $current !? $expected"
-	[ "$current" = "$expected" ] || [ "$current" = "$((expected / 1000))" ]
+	[ "$current" = "$expected" ]
 }
 
 # Helper to check a value in systemd.
@@ -318,6 +313,7 @@ function check_cpu_quota() {
 function check_cpu_burst() {
 	local burst=$1
 	if [ -v CGROUP_V2 ]; then
+		burst=$((burst / 1000))
 		check_cgroup_value "cpu.max.burst" "$burst"
 	else
 		check_cgroup_value "cpu.cfs_burst_us" "$burst"
@@ -438,6 +434,20 @@ function requires() {
 				skip_me=1
 			fi
 			;;
+		cgroups_cpu_burst)
+			local p f
+			init_cgroup_paths
+			if [ -v CGROUP_V1 ]; then
+				p="$CGROUP_CPU_BASE_PATH"
+				f="cpu.cfs_burst_us"
+			elif [ -v CGROUP_V2 ]; then
+				p="$CGROUP_BASE_PATH"
+				f="cpu.max.burst"
+			fi
+			if [ -z "$(find "$p" -name "$f" -print -quit)" ]; then
+				skip_me=1
+			fi
+			;;
 		cgroupns)
 			if [ ! -e "/proc/self/ns/cgroup" ]; then
 				skip_me=1
diff --git a/tests/integration/update.bats b/tests/integration/update.bats
index 616fe809b..5a3dc7f05 100644
--- a/tests/integration/update.bats
+++ b/tests/integration/update.bats
@@ -288,12 +288,6 @@ EOF
 	runc update test_update --cpu-share 200
 	[ "$status" -eq 0 ]
 	check_cpu_shares 200
-	runc update test_update --cpu-period 900000 --cpu-burst 500000
-	[ "$status" -eq 0 ]
-	check_cpu_burst 500000
-	runc update test_update --cpu-period 900000 --cpu-burst 0
-	[ "$status" -eq 0 ]
-	check_cpu_burst 0
 
 	# Revert to the test initial value via json on stding
 	runc update -r - test_update <<EOF
@@ -338,6 +332,23 @@ EOF
 	check_cpu_shares 100
 }
 
+@test "cpu burst" {
+	[ $EUID -ne 0 ] && requires rootless_cgroup
+	requires cgroups_cpu_burst
+
+	runc run -d --console-socket "$CONSOLE_SOCKET" test_update
+	[ "$status" -eq 0 ]
+	check_cpu_burst 0
+
+	runc update test_update --cpu-period 900000 --cpu-burst 500000
+	[ "$status" -eq 0 ]
+	check_cpu_burst 500000
+
+	runc update test_update --cpu-period 900000 --cpu-burst 0
+	[ "$status" -eq 0 ]
+	check_cpu_burst 0
+}
+
 @test "set cpu period with no quota" {
 	[ $EUID -ne 0 ] && requires rootless_cgroup