Skip to content

Commit

Permalink
tests/int: fix cgroup tests
Browse files Browse the repository at this point in the history
Commit e158483 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: e158483
Signed-off-by: Kir Kolyshkin <[email protected]>
  • Loading branch information
kolyshkin committed Oct 23, 2023
1 parent bbf8eff commit 5ea7c60
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 12 deletions.
22 changes: 16 additions & 6 deletions tests/integration/helpers.bash
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
23 changes: 17 additions & 6 deletions tests/integration/update.bats
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down

0 comments on commit 5ea7c60

Please sign in to comment.