Skip to content

Commit

Permalink
libcontainer/ignoreTerminateError: ignore SIGKILL
Browse files Browse the repository at this point in the history
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 <kolyshkin@gmail.com>
  • Loading branch information
kolyshkin committed Sep 30, 2020
1 parent dc42459 commit a4d5e8a
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 4 deletions.
5 changes: 3 additions & 2 deletions libcontainer/container_linux.go
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/cgroups.bats
Expand Up @@ -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" {
Expand All @@ -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" {
Expand Down

0 comments on commit a4d5e8a

Please sign in to comment.