From bbf8eff818fc03f45d96e30a86a0b3f40ec3335f Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Fri, 20 Oct 2023 12:47:28 -0700 Subject: [PATCH 1/3] tests/int: fix "runc run (hugetlb limits)" Recent commit 4a7d3ae5cd had a bug (extra period). Fixes: 4a7d3ae5cd Signed-off-by: Kir Kolyshkin --- tests/integration/cgroups.bats | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/cgroups.bats b/tests/integration/cgroups.bats index 8daf7420d0f..790108ba0b4 100644 --- a/tests/integration/cgroups.bats +++ b/tests/integration/cgroups.bats @@ -243,7 +243,7 @@ convert_hugetlb_size() { [ "$status" -eq 0 ] lim="max" - [ -v CGROUP_V1 ] && lim=".limit_in_bytes" + [ -v CGROUP_V1 ] && lim="limit_in_bytes" optional=("") # Add rsvd, if available. From 5ea7c60f334e4a714ef7a3a5d952f03935d469d6 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Fri, 20 Oct 2023 12:33:20 -0700 Subject: [PATCH 2/3] 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 --- 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 7e6399a47b8..811f817ab44 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 616fe809b9c..5a3dc7f0563 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 < Date: Fri, 20 Oct 2023 16:17:46 -0700 Subject: [PATCH 3/3] libct/cg/fs.Set: fix error message There is no point in showing the underlying error when path == "", because it is ENOENT. Revert the change done in commit e1584831b6835d. Fixes: e1584831b6835d3e8b9a239cb10503bcf6b9b9e8 Signed-off-by: Kir Kolyshkin --- libcontainer/cgroups/fs/fs.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libcontainer/cgroups/fs/fs.go b/libcontainer/cgroups/fs/fs.go index e2c425d0cd0..d2decb127ca 100644 --- a/libcontainer/cgroups/fs/fs.go +++ b/libcontainer/cgroups/fs/fs.go @@ -191,7 +191,7 @@ func (m *Manager) Set(r *configs.Resources) error { if path == "" { // We never created a path for this cgroup, so we cannot set // limits for it (though we have already tried at this point). - return fmt.Errorf("cannot set %s limit: container could not join or create cgroup, and the error is %w", sys.Name(), err) + return fmt.Errorf("cannot set %s limit: container could not join or create cgroup", sys.Name()) } return err }