Skip to content

Commit

Permalink
runc update: fix updating swap for cgroup v2
Browse files Browse the repository at this point in the history
This allows to do

	runc update $ID --memory=-1 --memory-swap=$VAL

for cgroup v2, i.e. set memory to unlimited and swap to a specific
value.

This was not possible because ConvertMemorySwapToCgroupV2Value rejected
memory=-1 ("unlimited"). In a hindsight, it was a mistake, because if
memory limit is unlimited, we should treat memory+swap limit as just swap
limit.

Revise the unit test; add description to each case.

Fixes: c86be8a ("cgroupv2: fix setting MemorySwap")
Signed-off-by: Kir Kolyshkin <[email protected]>
  • Loading branch information
kolyshkin committed Oct 23, 2024
1 parent cb9f3d6 commit 732806e
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 20 deletions.
7 changes: 5 additions & 2 deletions libcontainer/cgroups/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -427,8 +427,11 @@ func ConvertMemorySwapToCgroupV2Value(memorySwap, memory int64) (int64, error) {
case memorySwap == -1, memorySwap == 0:
// Treat -1 ("max") and 0 ("unset") swap as is.
return memorySwap, nil
case memory == 0, memory == -1:
// Unset or unlimited memory, can't calculate swap.
case memory == -1:
// Unlimited memory, so treat swap as is.
return memorySwap, nil
case memory == 0:
// Unset or unknown memory, can't calculate swap.
return 0, errors.New("unable to set swap limit without memory limit")
case memory < 0:
// Does not make sense to subtract a negative value.
Expand Down
51 changes: 33 additions & 18 deletions libcontainer/cgroups/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -554,82 +554,97 @@ func TestConvertCPUSharesToCgroupV2Value(t *testing.T) {

func TestConvertMemorySwapToCgroupV2Value(t *testing.T) {
cases := []struct {
descr string
memswap, memory int64
expected int64
expErr bool
}{
{
descr: "all unset",
memswap: 0,
memory: 0,
expected: 0,
},
{
descr: "unlimited memory+swap, unset memory",
memswap: -1,
memory: 0,
expected: -1,
},
{
descr: "unlimited memory",
memswap: 300,
memory: -1,
expected: 300,
},
{
descr: "all unlimited",
memswap: -1,
memory: -1,
expected: -1,
},
{
descr: "negative memory+swap",
memswap: -2,
memory: 0,
expErr: true,
},
{
descr: "unlimited memory+swap, set memory",
memswap: -1,
memory: 1000,
expected: -1,
},
{
descr: "memory+swap == memory",
memswap: 1000,
memory: 1000,
expected: 0,
},
{
descr: "memory+swap > memory",
memswap: 500,
memory: 200,
expected: 300,
},
{
descr: "memory+swap < memory",
memswap: 300,
memory: 400,
expErr: true,
},
{
descr: "unset memory",
memswap: 300,
memory: 0,
expErr: true,
},
{
descr: "negative memory",
memswap: 300,
memory: -300,
expErr: true,
},
{
memswap: 300,
memory: -1,
expErr: true,
},
}

for _, c := range cases {
swap, err := ConvertMemorySwapToCgroupV2Value(c.memswap, c.memory)
if c.expErr {
if err == nil {
t.Errorf("memswap: %d, memory %d, expected error, got %d, nil", c.memswap, c.memory, swap)
c := c
t.Run(c.descr, func(t *testing.T) {
swap, err := ConvertMemorySwapToCgroupV2Value(c.memswap, c.memory)
if c.expErr {
if err == nil {
t.Errorf("memswap: %d, memory %d, expected error, got %d, nil", c.memswap, c.memory, swap)
}
// No more checks.
return
}
// no more checks
continue
}
if err != nil {
t.Errorf("memswap: %d, memory %d, expected success, got error %s", c.memswap, c.memory, err)
}
if swap != c.expected {
t.Errorf("memswap: %d, memory %d, expected %d, got %d", c.memswap, c.memory, c.expected, swap)
}
if err != nil {
t.Errorf("memswap: %d, memory %d, expected success, got error %s", c.memswap, c.memory, err)
}
if swap != c.expected {
t.Errorf("memswap: %d, memory %d, expected %d, got %d", c.memswap, c.memory, c.expected, swap)
}
})
}
}

Expand Down

0 comments on commit 732806e

Please sign in to comment.