diff --git a/delete.go b/delete.go index dd3041f8722..682101ccad8 100644 --- a/delete.go +++ b/delete.go @@ -14,10 +14,10 @@ import ( ) func killContainer(container *libcontainer.Container) error { - _ = container.Signal(unix.SIGKILL, false) + _ = container.Signal(unix.SIGKILL) for i := 0; i < 100; i++ { time.Sleep(100 * time.Millisecond) - if err := container.Signal(unix.Signal(0), false); err != nil { + if err := container.Signal(unix.Signal(0)); err != nil { destroy(container) return nil } diff --git a/kill.go b/kill.go index e5b13b1230d..ac1e47a5b19 100644 --- a/kill.go +++ b/kill.go @@ -1,10 +1,12 @@ package main import ( + "errors" "fmt" "strconv" "strings" + "github.com/opencontainers/runc/libcontainer" "github.com/urfave/cli" "golang.org/x/sys/unix" ) @@ -24,8 +26,9 @@ signal to the init process of the "ubuntu01" container: # runc kill ubuntu01 KILL`, Flags: []cli.Flag{ cli.BoolFlag{ - Name: "all, a", - Usage: "send the specified signal to all processes inside the container", + Name: "all, a", + Usage: "(obsoleted, do not use)", + Hidden: true, }, }, Action: func(context *cli.Context) error { @@ -49,7 +52,11 @@ signal to the init process of the "ubuntu01" container: if err != nil { return err } - return container.Signal(signal, context.Bool("all")) + err = container.Signal(signal) + if errors.Is(err, libcontainer.ErrNotRunning) && context.Bool("all") { + err = nil + } + return err }, } diff --git a/libcontainer/container_linux.go b/libcontainer/container_linux.go index 27ba0597d7e..6f0ccd9a21f 100644 --- a/libcontainer/container_linux.go +++ b/libcontainer/container_linux.go @@ -357,32 +357,18 @@ func (c *Container) start(process *Process) (retErr error) { return nil } -// Signal sends a specified signal to container's init, or, if all is true, -// true, to all container's processes (as determined by container's cgroup). +// Signal sends a specified signal to container's init. // -// Note all=true is implied when s is SIGKILL and the container does not have -// its own PID namespace. In this scenario, the libcontainer user may be required -// to implement a proper child reaper. -func (c *Container) Signal(s os.Signal, all bool) error { +// When s is SIGKILL and the container does not have its own PID namespace, all +// the container's processes are killed. In this scenario, the libcontainer +// user may be required to implement a proper child reaper. +func (c *Container) Signal(s os.Signal) error { c.m.Lock() defer c.m.Unlock() status, err := c.currentStatus() if err != nil { return err } - if all { - sig, ok := s.(unix.Signal) - if !ok { - return errors.New("unsupported signal type") - } - if status == Stopped && !c.cgroupManager.Exists() { - // Avoid calling signalAllProcesses which may print - // a warning trying to freeze a non-existing cgroup. - return nil - } - return c.ignoreCgroupError(signalAllProcesses(c.cgroupManager, sig)) - } - // To avoid a PID reuse attack, don't kill non-running container. switch status { case Running, Created, Paused: diff --git a/libcontainer/integration/exec_test.go b/libcontainer/integration/exec_test.go index e1ee5f7b4ed..03d16639067 100644 --- a/libcontainer/integration/exec_test.go +++ b/libcontainer/integration/exec_test.go @@ -1400,7 +1400,7 @@ func testPidnsInitKill(t *testing.T, config *configs.Config) { ok(t, err) // Kill the container. - err = container.Signal(syscall.SIGKILL, false) + err = container.Signal(syscall.SIGKILL) ok(t, err) _, err = process1.Wait() if err == nil { diff --git a/man/runc-kill.8.md b/man/runc-kill.8.md index 684666396bd..0d4cb68e2ff 100644 --- a/man/runc-kill.8.md +++ b/man/runc-kill.8.md @@ -4,7 +4,7 @@ **runc-kill** - send a specified signal to container # SYNOPSIS -**runc kill** [**--all**|**-a**] _container-id_ [_signal_] +**runc kill** _container-id_ [_signal_] # DESCRIPTION @@ -15,15 +15,6 @@ A different signal can be specified either by its name (with or without the **SIG** prefix), or its numeric value. Use **kill**(1) with **-l** option to list available signals. -# OPTIONS -**--all**|**-a** -: Send the signal to all processes inside the container, rather than -the container's init only. This option is implied when the _signal_ is **KILL** -and the container does not have its own PID namespace. - -: When this option is set, no error is returned if the container is stopped -or does not exist. - # EXAMPLES The following will send a **KILL** signal to the init process of the diff --git a/tests/integration/kill.bats b/tests/integration/kill.bats index 2a78778bdee..8f2f4a7b4e8 100644 --- a/tests/integration/kill.bats +++ b/tests/integration/kill.bats @@ -22,7 +22,12 @@ function teardown() { [ "$status" -eq 0 ] wait_for_container 10 1 test_busybox stopped - # we should ensure kill work after the container stopped + # Check that kill errors on a stopped container. + runc kill test_busybox 0 + [ "$status" -ne 0 ] + [[ "$output" == *"container not running"* ]] + + # Check that -a (now obsoleted) makes kill return no error for a stopped container. runc kill -a test_busybox 0 [ "$status" -eq 0 ] @@ -31,7 +36,7 @@ function teardown() { } # This is roughly the same as TestPIDHostInitProcessWait in libcontainer/integration. -@test "kill --all KILL [host pidns]" { +@test "kill KILL [host pidns]" { # kill -a currently requires cgroup freezer. requires cgroups_freezer @@ -65,7 +70,7 @@ function teardown() { kill -0 "$p" done - runc kill -a test_busybox KILL + runc kill test_busybox KILL [ "$status" -eq 0 ] wait_for_container 10 1 test_busybox stopped