Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

tests/int: swap-related fixes #4188

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
+45 −18
Open

tests/int: swap-related fixes #4188

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
be7de21
tests/int/update: fix v2 swap check
kolyshkin Feb 2, 2024
a459392
tests/int: fixup find statements
kolyshkin Feb 2, 2024
e5a1f80
tests/int/helpers: fix cgroups_swap check for v2
kolyshkin Feb 2, 2024
ea3c5a3
libct/cg/fs/v2: ignore setting swap in some cases
kolyshkin Mar 26, 2024
59ca3a8
tests/cgroups: separate cgroup v2 swap test
kolyshkin Mar 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 4 additions & 1 deletion libcontainer/cgroups/fs2/memory.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,10 @@ func setMemory(dirPath string, r *configs.Resources) error {
// never write empty string to `memory.swap.max`, it means set to 0.
if swapStr != "" {
if err := cgroups.WriteFile(dirPath, "memory.swap.max", swapStr); err != nil {
return err
// If swap is not enabled, silently ignore setting to max or disabling it.
if !(errors.Is(err, os.ErrNotExist) && (swapStr == "max" || swapStr == "0")) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your PR has indeed fixed the issue, but I think we should change the behavior according to what's doing in cgroup v1:

runc/libcontainer/cgroups/fs/memory.go

Lines 72 to 77 in 59ca3a8

if r.Memory == -1 && r.MemorySwap == 0 {
// Only set swap if it's enabled in kernel
if cgroups.PathExists(filepath.Join(path, cgroupMemorySwapLimit)) {
r.MemorySwap = -1
}
}

return err
}
}
}

Expand Down
26 changes: 23 additions & 3 deletions tests/integration/cgroups.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,6 @@ convert_hugetlb_size() {
"memory.low": "524288",
"memory.high": "5242880",
"memory.max": "20484096",
"memory.swap.max": "20971520",
"pids.max": "99",
"cpu.max": "10000 100000",
"cpu.weight": "42"
Expand All @@ -289,20 +288,41 @@ convert_hugetlb_size() {
echo "$output" | grep -q '^memory.low:524288$'
echo "$output" | grep -q '^memory.high:5242880$'
echo "$output" | grep -q '^memory.max:20484096$'
echo "$output" | grep -q '^memory.swap.max:20971520$'
echo "$output" | grep -q '^pids.max:99$'
echo "$output" | grep -q '^cpu.max:10000 100000$'

check_systemd_value "MemoryMin" 131072
check_systemd_value "MemoryLow" 524288
check_systemd_value "MemoryHigh" 5242880
check_systemd_value "MemoryMax" 20484096
check_systemd_value "MemorySwapMax" 20971520
check_systemd_value "TasksMax" 99
check_cpu_quota 10000 100000 "100ms"
check_cpu_weight 42
}

@test "runc run (cgroup v2 resources.unified swap)" {
requires root cgroups_v2 cgroups_swap

set_cgroups_path
update_config ' .linux.resources.unified |= {
"memory.max": "20484096",
"memory.swap.max": "20971520"
}'

runc run -d --console-socket "$CONSOLE_SOCKET" test_cgroups_unified
[ "$status" -eq 0 ]

runc exec test_cgroups_unified sh -c 'cd /sys/fs/cgroup && grep . *.max'
[ "$status" -eq 0 ]
echo "$output"

echo "$output" | grep -q '^memory.max:20484096$'
echo "$output" | grep -q '^memory.swap.max:20971520$'

check_systemd_value "MemoryMax" 20484096
check_systemd_value "MemorySwapMax" 20971520
}

@test "runc run (cgroup v2 resources.unified override)" {
requires root cgroups_v2

Expand Down
16 changes: 11 additions & 5 deletions tests/integration/helpers.bash
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ function init_cgroup_paths() {
CGROUP_BASE_PATH=/sys/fs/cgroup

# Find any cgroup.freeze files...
if [ -n "$(find "$CGROUP_BASE_PATH" -type f -name "cgroup.freeze" -print -quit)" ]; then
if [ -n "$(find "$CGROUP_BASE_PATH" -maxdepth 2 -type f -name "cgroup.freeze" -print -quit)" ]; then
CGROUP_SUBSYSTEMS+=" freezer"
fi
else
Expand Down Expand Up @@ -451,16 +451,22 @@ function requires() {
;;
cgroups_swap)
init_cgroup_paths
if [ -v CGROUP_V1 ] && [ ! -e "${CGROUP_MEMORY_BASE_PATH}/memory.memsw.limit_in_bytes" ]; then
skip_me=1
if [ -v CGROUP_V1 ]; then
if [ ! -e "${CGROUP_MEMORY_BASE_PATH}/memory.memsw.limit_in_bytes" ]; then
skip_me=1
fi
elif [ -v CGROUP_V2 ]; then
if [ -z "$(find "$CGROUP_BASE_PATH" -maxdepth 2 -type f -name memory.swap.max -print -quit)" ]; then
skip_me=1
fi
fi
;;
cgroups_cpu_idle)
local p
init_cgroup_paths
[ -v CGROUP_V1 ] && p="$CGROUP_CPU_BASE_PATH"
[ -v CGROUP_V2 ] && p="$CGROUP_BASE_PATH"
if [ -z "$(find "$p" -name cpu.idle -print -quit)" ]; then
if [ -z "$(find "$p" -maxdepth 2 -type f -name cpu.idle -print -quit)" ]; then
skip_me=1
fi
;;
Expand All @@ -476,7 +482,7 @@ function requires() {
p="$CGROUP_BASE_PATH"
f="cpu.max.burst"
fi
if [ -z "$(find "$p" -name "$f" -print -quit)" ]; then
if [ -z "$(find "$p" -maxdepth 2 -type f -name "$f" -print -quit)" ]; then
skip_me=1
fi
;;
Expand Down
16 changes: 7 additions & 9 deletions tests/integration/update.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,6 @@ function setup() {
MEM_SWAP="memory.memsw.limit_in_bytes"
SD_MEM_SWAP="unsupported"
SYSTEM_MEM=$(cat "${CGROUP_MEMORY_BASE_PATH}/${MEM_LIMIT}")
HAVE_SWAP="no"
if [ -f "${CGROUP_MEMORY_BASE_PATH}/${MEM_SWAP}" ]; then
HAVE_SWAP="yes"
fi
else
MEM_LIMIT="memory.max"
SD_MEM_LIMIT="MemoryMax"
Expand All @@ -50,9 +46,11 @@ function setup() {
MEM_SWAP="memory.swap.max"
SD_MEM_SWAP="MemorySwapMax"
SYSTEM_MEM="max"
HAVE_SWAP="yes"
fi

unset HAVE_SWAP
get_cgroup_value $MEM_SWAP >/dev/null && HAVE_SWAP=yes

SD_UNLIMITED="infinity"
SD_VERSION=$(systemctl --version | awk '{print $2; exit}')
if [ "$SD_VERSION" -lt 227 ]; then
Expand Down Expand Up @@ -94,8 +92,8 @@ function setup() {
check_cgroup_value "$MEM_RESERVE" 33554432
check_systemd_value "$SD_MEM_RESERVE" 33554432

# Run swap memory tests if swap is available
if [ "$HAVE_SWAP" = "yes" ]; then
# Run swap memory tests if swap is available.
if [ -v HAVE_SWAP ]; then
# try to remove memory swap limit
runc update test_update --memory-swap -1
[ "$status" -eq 0 ]
Expand Down Expand Up @@ -127,7 +125,7 @@ function setup() {
check_systemd_value "$SD_MEM_LIMIT" "$SD_UNLIMITED"

# check swap memory limited is gone
if [ "$HAVE_SWAP" = "yes" ]; then
if [ -v HAVE_SWAP ]; then
check_cgroup_value "$MEM_SWAP" "$SYSTEM_MEM"
check_systemd_value "$SD_MEM_SWAP" "$SD_UNLIMITED"
fi
Expand Down Expand Up @@ -221,7 +219,7 @@ EOF
check_cgroup_value "pids.max" 20
check_systemd_value "TasksMax" 20

if [ "$HAVE_SWAP" = "yes" ]; then
if [ -v HAVE_SWAP ]; then
# Test case for https://github.com/opencontainers/runc/pull/592,
# checking libcontainer/cgroups/fs/memory.go:setMemoryAndSwap.

Expand Down