From a4d5e8a27bedc05bf122660933f7c948747bb85c Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 29 Sep 2020 19:37:33 -0700 Subject: [PATCH] libcontainer/ignoreTerminateError: ignore SIGKILL When we call terminate(), we kill the process, and wait returns the error indicating the process was killed. This is exactly what we expect here, so there is no reason to treat it as an error. Before this patch, when a container with invalid cgroup parameters is started: > WARN[0000] unable to terminate initProcess error="signal: killed" > ERRO[0000] container_linux.go:366: starting container process caused: process_linux.go:495: container init caused: process_linux.go:458: setting cgroup config for procHooks process caused: failed to write "555": open /sys/fs/cgroup/blkio/user.slice/xx33/blkio.weight: permission denied After: > ERRO[0000] container_linux.go:366: starting container process caused: process_linux.go:495: container init caused: process_linux.go:458: setting cgroup config for procHooks process caused: failed to write "555": open /sys/fs/cgroup/blkio/user.slice/xx33/blkio.weight: permission denied I.e. the useless warning is gone. NOTE this breaks a couple of integration test cases, since they were expecting a particular message in the second line, and now due to "signal: killed" removed it's in the first line. Fix those, too. Signed-off-by: Kir Kolyshkin --- libcontainer/container_linux.go | 5 +++-- tests/integration/cgroups.bats | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/libcontainer/container_linux.go b/libcontainer/container_linux.go index 27b5a618e..bd3e2c0f7 100644 --- a/libcontainer/container_linux.go +++ b/libcontainer/container_linux.go @@ -2050,8 +2050,9 @@ func ignoreTerminateErrors(err error) error { return nil } s := err.Error() - switch { - case strings.Contains(s, "process already finished"), strings.Contains(s, "Wait was already called"): + if strings.Contains(s, "signal: killed") || + strings.Contains(s, "process already finished") || + strings.Contains(s, "Wait was already called") { return nil } return err diff --git a/tests/integration/cgroups.bats b/tests/integration/cgroups.bats index 13c9c354d..bb8f4e144 100644 --- a/tests/integration/cgroups.bats +++ b/tests/integration/cgroups.bats @@ -79,7 +79,7 @@ function setup() { runc run -d --console-socket "$CONSOLE_SOCKET" test_cgroups_permissions [ "$status" -eq 1 ] - [[ ${lines[1]} == *"permission denied"* ]] + [[ ${lines[0]} == *"permission denied"* ]] } @test "runc create (rootless + limits + no cgrouppath + no permission) fails with informative error" { @@ -92,7 +92,7 @@ function setup() { runc run -d --console-socket "$CONSOLE_SOCKET" test_cgroups_permissions [ "$status" -eq 1 ] - [[ ${lines[1]} == *"rootless needs no limits + no cgrouppath when no permission is granted for cgroups"* ]] || [[ ${lines[1]} == *"cannot set pids limit: container could not join or create cgroup"* ]] + [[ ${lines[0]} == *"rootless needs no limits + no cgrouppath when no permission is granted for cgroups"* ]] || [[ ${lines[0]} == *"cannot set pids limit: container could not join or create cgroup"* ]] } @test "runc create (limits + cgrouppath + permission on the cgroup dir) succeeds" {