From 5cfd6e8127fb50b0d94c905881eb2bf9bfa79f8f Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 21 Sep 2023 18:41:46 -0700 Subject: [PATCH 1/3] tests/int: fix "runc run (cgroup v2 resources.unified override)" This test case checks that unified resources override those set by conventional means, but it does not set conventional pids limit. Fix this. Signed-off-by: Kir Kolyshkin --- tests/integration/cgroups.bats | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/integration/cgroups.bats b/tests/integration/cgroups.bats index 8daf7420d0f..8c59d205118 100644 --- a/tests/integration/cgroups.bats +++ b/tests/integration/cgroups.bats @@ -309,6 +309,7 @@ convert_hugetlb_size() { set_cgroups_path # CPU shares of 3333 corresponds to CPU weight of 128. update_config ' .linux.resources.memory |= {"limit": 33554432} + | .linux.resources.pids.limit |= 100 | .linux.resources.cpu |= { "shares": 3333, "quota": 40000, From 36e3e426889cfd441e695951a5ff1f37a33c1b94 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 21 Sep 2023 18:52:24 -0700 Subject: [PATCH 2/3] tests/int/update: use systemd_version() Signed-off-by: Kir Kolyshkin --- tests/integration/update.bats | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/integration/update.bats b/tests/integration/update.bats index 616fe809b9c..4dd43bd8f7c 100644 --- a/tests/integration/update.bats +++ b/tests/integration/update.bats @@ -54,8 +54,7 @@ function setup() { fi SD_UNLIMITED="infinity" - SD_VERSION=$(systemctl --version | awk '{print $2; exit}') - if [ "$SD_VERSION" -lt 227 ]; then + if [ "$(systemd_version)" -lt 227 ]; then SD_UNLIMITED="18446744073709551615" fi From 4c61f6822920e64bd1249f811037bccd08728353 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 21 Sep 2023 19:22:35 -0700 Subject: [PATCH 3/3] runc run: treat pids.limit=0 as unlimited It has been pointed out that runc incorrectly ignores pids.limit=0 set in the runtime spec. This happens because runtime-spec says "default is unlimited" and also allows for Pids to not be set at all, thus distinguishing unset (Resources.Pids == nil) from unlimited (Resources.Pids.Limit <= 0). Internally, runc also distinguishes unset from unlimited, but since we do not use a pointer, we treat 0 as unset and -1 as unlimited. Add a conversion code to libcontainer/specconv. Add a test case to check that starting a container with pids.limit=0 results in no pids limit (the test fails before the fix when systemd cgroup manager is used, as systemd apparently uses parent's TasksMax). NOTE that runc update still treats 0 as "unset". Finally, fix/update the documentation here and there. Should fix issue 4014. Reported-by: Peter Hunt Signed-off-by: Kir Kolyshkin --- libcontainer/configs/cgroup_linux.go | 2 +- libcontainer/specconv/spec_linux.go | 8 +++++++- man/runc-update.8.md | 2 +- tests/integration/cgroups.bats | 14 ++++++++++++++ update.go | 4 ++-- 5 files changed, 25 insertions(+), 5 deletions(-) diff --git a/libcontainer/configs/cgroup_linux.go b/libcontainer/configs/cgroup_linux.go index 4a34cf76fc5..cbe970672e3 100644 --- a/libcontainer/configs/cgroup_linux.go +++ b/libcontainer/configs/cgroup_linux.go @@ -90,7 +90,7 @@ type Resources struct { // cgroup SCHED_IDLE CPUIdle *int64 `json:"cpu_idle,omitempty"` - // Process limit; set <= `0' to disable limit. + // Maximum number of tasks; 0 for unset, -1 for max/unlimited. PidsLimit int64 `json:"pids_limit"` // Specifies per cgroup weight, range is from 10 to 1000. diff --git a/libcontainer/specconv/spec_linux.go b/libcontainer/specconv/spec_linux.go index c5553832776..7e0f069dce8 100644 --- a/libcontainer/specconv/spec_linux.go +++ b/libcontainer/specconv/spec_linux.go @@ -775,8 +775,14 @@ func CreateCgroupConfig(opts *CreateOpts, defaultDevs []*devices.Device) (*confi c.Resources.CpusetMems = r.CPU.Mems c.Resources.CPUIdle = r.CPU.Idle } + // Convert pids limit from the runtime-spec value (where any value <= 0 means "unlimited") + // to internal runc value (where -1 is "unlimited", and 0 is "unset"). if r.Pids != nil { - c.Resources.PidsLimit = r.Pids.Limit + if r.Pids.Limit > 0 { + c.Resources.PidsLimit = r.Pids.Limit + } else { + c.Resources.PidsLimit = -1 + } } if r.BlockIO != nil { if r.BlockIO.Weight != nil { diff --git a/man/runc-update.8.md b/man/runc-update.8.md index 0e95d85ded1..3b54f9785b3 100644 --- a/man/runc-update.8.md +++ b/man/runc-update.8.md @@ -85,7 +85,7 @@ stdin. If this option is used, all other options are ignored. (i.e. use unlimited swap). **--pids-limit** _num_ -: Set the maximum number of processes allowed in the container. +: Set the maximum number of tasks. Use **-1** for unlimited. **--l3-cache-schema** _value_ : Set the value for Intel RDT/CAT L3 cache schema. diff --git a/tests/integration/cgroups.bats b/tests/integration/cgroups.bats index 8c59d205118..6e553de0835 100644 --- a/tests/integration/cgroups.bats +++ b/tests/integration/cgroups.bats @@ -263,6 +263,20 @@ convert_hugetlb_size() { done } +# https://github.com/opencontainers/runc/issues/4014. +@test "runc run (pids.limit=0 means unlimited)" { + [ $EUID -ne 0 ] && requires rootless_cgroup + + set_cgroups_path + update_config '.linux.resources.pids.limit |= 0' + + runc run -d --console-socket "$CONSOLE_SOCKET" test_pids + [ "$status" -eq 0 ] + check_cgroup_value "pids.max" "max" + # systemd < v227 shows UINT64_MAX instead of "infinity". + check_systemd_value "TasksMax" "infinity" "18446744073709551615" +} + @test "runc run (cgroup v2 resources.unified only)" { requires root cgroups_v2 diff --git a/update.go b/update.go index fc2d656abbf..7a390a6c94f 100644 --- a/update.go +++ b/update.go @@ -122,11 +122,11 @@ other options are ignored. }, cli.StringFlag{ Name: "memory-swap", - Usage: "Total memory usage (memory + swap); set '-1' to enable unlimited swap", + Usage: "Total memory usage (memory + swap); use '-1' to enable unlimited swap", }, cli.IntFlag{ Name: "pids-limit", - Usage: "Maximum number of pids allowed in the container", + Usage: "Maximum number of tasks; use '-1' for unlimited", }, cli.StringFlag{ Name: "l3-cache-schema",