daemon: daemon.ContainerKill() accept stop-signal as string

This allows the postContainersKill() handler to pass values as-is. As part of
the rewrite, I also moved the daemon.GetContainer(name) call later in the
function, so that we can fail early if an invalid signal is passed, before
doing the (heavier) fetching of the container.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

api/server/router/container/backend.go

@@ -33,7 +33,7 @@ type copyBackend interface {
 // stateBackend includes functions to implement to provide container state lifecycle functionality.
 type stateBackend interface {
 	ContainerCreate(config types.ContainerCreateConfig) (container.CreateResponse, error)
-	ContainerKill(name string, sig uint64) error
+	ContainerKill(name string, signal string) error
 	ContainerPause(name string) error
 	ContainerRename(oldName, newName string) error
 	ContainerResize(name string, height, width int) error

api/server/router/container/container_routes.go

@@ -7,7 +7,6 @@ import (
 	"io"
 	"net/http"
 	"strconv"
-	"syscall"
 
 	"github.com/containerd/containerd/platforms"
 	"github.com/docker/docker/api/server/httpstatus"
@@ -254,18 +253,8 @@ func (s *containerRouter) postContainersKill(ctx context.Context, w http.Respons
 		return err
 	}
 
-	var sig syscall.Signal
 	name := vars["name"]
-
-	// If we have a signal, look at it. Otherwise, do nothing
-	if sigStr := r.Form.Get("signal"); sigStr != "" {
-		var err error
-		if sig, err = signal.ParseSignal(sigStr); err != nil {
-			return errdefs.InvalidParameter(err)
-		}
-	}
-
-	if err := s.backend.ContainerKill(name, uint64(sig)); err != nil {
+	if err := s.backend.ContainerKill(name, r.Form.Get("signal")); err != nil {
 		var isStopped bool
 		if errdefs.IsConflict(err) {
 			isStopped = true

builder/builder.go

@@ -65,7 +65,7 @@ type ExecBackend interface {
 	// ContainerRm removes a container specified by `id`.
 	ContainerRm(name string, config *types.ContainerRmConfig) error
 	// ContainerKill stops the container execution abruptly.
-	ContainerKill(containerID string, sig uint64) error
+	ContainerKill(containerID string, sig string) error
 	// ContainerStart starts a new container
 	ContainerStart(containerID string, hostConfig *container.HostConfig, checkpoint string, checkpointDir string) error
 	// ContainerWait stops processing until the given container is stopped.

builder/dockerfile/containerbackend.go

@@ -61,7 +61,7 @@ func (c *containerManager) Run(ctx context.Context, cID string, stdout, stderr i
 		select {
 		case <-ctx.Done():
 			logrus.Debugln("Build cancelled, killing and removing container:", cID)
-			c.backend.ContainerKill(cID, 0)
+			c.backend.ContainerKill(cID, "")
 			c.removeContainer(cID, stdout)
 			cancelErrCh <- errCancelled
 		case <-finished:

builder/dockerfile/mockbackend_test.go

@@ -46,7 +46,7 @@ func (m *MockBackend) CommitBuildStep(c backend.CommitConfig) (image.ID, error)
 	return "", nil
 }
 
-func (m *MockBackend) ContainerKill(containerID string, sig uint64) error {
+func (m *MockBackend) ContainerKill(containerID string, sig string) error {
 	return nil
 }
 
@@ -129,7 +129,7 @@ func (l *mockLayer) NewRWLayer() (builder.RWLayer, error) {
 }
 
 func (l *mockLayer) DiffID() layer.DiffID {
-	return layer.DiffID("abcdef")
+	return "abcdef"
 }
 
 type mockRWLayer struct {

daemon/cluster/executor/backend.go

@@ -45,7 +45,7 @@ type Backend interface {
 	ContainerInspectCurrent(name string, size bool) (*types.ContainerJSON, error)
 	ContainerWait(ctx context.Context, name string, condition containerpkg.WaitCondition) (<-chan containerpkg.StateStatus, error)
 	ContainerRm(name string, config *types.ContainerRmConfig) error
-	ContainerKill(name string, sig uint64) error
+	ContainerKill(name string, sig string) error
 	SetContainerDependencyStore(name string, store exec.DependencyGetter) error
 	SetContainerSecretReferences(name string, refs []*swarmtypes.SecretReference) error
 	SetContainerConfigReferences(name string, refs []*swarmtypes.ConfigReference) error

daemon/cluster/executor/container/adapter.go

@@ -417,7 +417,7 @@ func (c *containerAdapter) shutdown(ctx context.Context) error {
 }
 
 func (c *containerAdapter) terminate(ctx context.Context) error {
-	return c.backend.ContainerKill(c.container.name(), uint64(syscall.SIGKILL))
+	return c.backend.ContainerKill(c.container.name(), syscall.SIGKILL.String())
 }
 
 func (c *containerAdapter) remove(ctx context.Context) error {

daemon/kill.go

@@ -34,22 +34,29 @@ func isErrNoSuchProcess(err error) bool {
 }
 
 // ContainerKill sends signal to the container
-// If no signal is given (sig 0), then Kill with SIGKILL and wait
+// If no signal is given, then Kill with SIGKILL and wait
 // for the container to exit.
 // If a signal is given, then just send it to the container and return.
-func (daemon *Daemon) ContainerKill(name string, stopSignal uint64) error {
-	sig := syscall.Signal(stopSignal)
+func (daemon *Daemon) ContainerKill(name, stopSignal string) error {
+	var (
+		err error
+		sig = syscall.SIGKILL
+	)
+	if stopSignal != "" {
+		sig, err = signal.ParseSignal(stopSignal)
+		if err != nil {
+			return errdefs.InvalidParameter(err)
+		}
+		if !signal.ValidSignalForPlatform(sig) {
+			return errdefs.InvalidParameter(errors.Errorf("the %s daemon does not support signal %d", runtime.GOOS, sig))
+		}
+	}
 	container, err := daemon.GetContainer(name)
 	if err != nil {
 		return err
 	}
-
-	if sig != 0 && !signal.ValidSignalForPlatform(sig) {
-		return fmt.Errorf("The %s daemon does not support signal %d", runtime.GOOS, sig)
-	}
-
-	// If no signal is passed, or SIGKILL, perform regular Kill (SIGKILL + wait())
-	if sig == 0 || sig == syscall.SIGKILL {
+	if sig == syscall.SIGKILL {
+		// perform regular Kill (SIGKILL + wait())
 		return daemon.Kill(container)
 	}
 	return daemon.killWithSignal(container, sig)

libcontainerd/local/local_windows.go

@@ -688,7 +688,7 @@ func (c *client) Exec(ctx context.Context, containerID, processID string, spec *
 // SignalProcess handles `docker stop` on Windows. While Linux has support for
 // the full range of signals, signals aren't really implemented on Windows.
 // We fake supporting regular stop and -9 to force kill.
-func (c *client) SignalProcess(_ context.Context, containerID, processID string, signal int) error {
+func (c *client) SignalProcess(_ context.Context, containerID, processID string, signal syscall.Signal) error {
 	ctr, p, err := c.getProcess(containerID, processID)
 	if err != nil {
 		return err