Skip to content

Commit

Permalink
runc delete: refuse to delete container with non-empty cgroup
Browse files Browse the repository at this point in the history
When we delete the container, we no longer have any knowledge about it.

In case when the container does not have own private PID namespace, this
knowledge (in particular, container's cgroup path) might be crucial, because
otherwise we won't be able to kill the leftover container processes.

Therefore, let's error out when runc destroy called on a stopped container
without private PID namespace and there are processes in its cgroup,
suggesting to use runc delete -f or runc kill.

Add a test case.

Signed-off-by: Kir Kolyshkin <[email protected]>
  • Loading branch information
kolyshkin committed Nov 6, 2023
1 parent 6c3f2e2 commit 99d14ef
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 1 deletion.
9 changes: 9 additions & 0 deletions libcontainer/state_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,15 @@ func (b *stoppedState) transition(s containerState) error {
}

func (b *stoppedState) destroy() error {
// Normally, when a container init is gone, all other processes in its
// cgroup are killed by the kernel. This is not the case for a shared
// PID namespace container, which may have some leftover processes.
if !b.c.config.Namespaces.IsPrivate(configs.NEWPID) {
// Get container pids on a best-effort basis.
if pids, _ := b.c.cgroupManager.GetAllPids(); len(pids) > 0 {
return fmt.Errorf("container is in stopped state (init is gone), but its cgroup still has %d processes; use runc delete -f or runc kill", len(pids))
}
}
return destroy(b.c)
}

Expand Down
8 changes: 7 additions & 1 deletion tests/integration/delete.bats
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ function teardown() {
[ "$status" -eq 0 ]
}

# Issue 4047, case "runc delete -f".
# Issue 4047, cases "runc delete" and "runc delete -f".
# See also: "kill KILL [host pidns + init gone]" test in kill.bats.
@test "runc delete --force [host pidns + init gone]" {
requires cgroups_freezer
Expand Down Expand Up @@ -113,6 +113,12 @@ function teardown() {
kill -0 "$p"
done

# runc delete must warn that container cgroup is not empty.
runc delete test_busybox
[ "$status" -ne 0 ]
[[ "$output" == *"container is in stopped state (init is gone), but its cgroup still has 5 processes"* ]]

# runc delete -f must kill those processes and remove container.
runc delete -f test_busybox
[ "$status" -eq 0 ]

Expand Down

0 comments on commit 99d14ef

Please sign in to comment.