Allow stopping of paused container

When a container is paused, signals are sent once the container has been
unpaused.
Instead of forcing the user to unpause a container before they can ever
send a signal, allow the user to send the signals, and in the case of a
stop signal, automatically unpause the container afterwards.

This is much safer than unpausing the container first then sending a
signal (what a user is currently forced to do), as the container may be
paused for very good reasons and should not be unpaused except for
stopping.
Note that not even SIGKILL is possible while a process is paused,
but it is killed the instant it is unpaused.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>

daemon/daemon.go

@@ -43,7 +43,6 @@ import (
 	"github.com/docker/docker/pkg/idtools"
 	"github.com/docker/docker/pkg/plugingetter"
 	"github.com/docker/docker/pkg/registrar"
-	"github.com/docker/docker/pkg/signal"
 	"github.com/docker/docker/pkg/sysinfo"
 	"github.com/docker/docker/pkg/system"
 	"github.com/docker/docker/pkg/truncindex"
@@ -838,42 +837,7 @@ func (daemon *Daemon) waitForStartupDone() {
 
 func (daemon *Daemon) shutdownContainer(c *container.Container) error {
 	stopTimeout := c.StopTimeout()
-	// TODO(windows): Handle docker restart with paused containers
-	if c.IsPaused() {
-		// To terminate a process in freezer cgroup, we should send
-		// SIGTERM to this process then unfreeze it, and the process will
-		// force to terminate immediately.
-		logrus.Debugf("Found container %s is paused, sending SIGTERM before unpausing it", c.ID)
-		sig, ok := signal.SignalMap["TERM"]
-		if !ok {
-			return errors.New("System does not support SIGTERM")
-		}
-		if err := daemon.kill(c, int(sig)); err != nil {
-			return fmt.Errorf("sending SIGTERM to container %s with error: %v", c.ID, err)
-		}
-		if err := daemon.containerUnpause(c); err != nil {
-			return fmt.Errorf("Failed to unpause container %s with error: %v", c.ID, err)
-		}
-
-		ctx, cancel := context.WithTimeout(context.Background(), time.Duration(stopTimeout)*time.Second)
-		defer cancel()
 
-		// Wait with timeout for container to exit.
-		if status := <-c.Wait(ctx, container.WaitConditionNotRunning); status.Err() != nil {
-			logrus.Debugf("container %s failed to exit in %d second of SIGTERM, sending SIGKILL to force", c.ID, stopTimeout)
-			sig, ok := signal.SignalMap["KILL"]
-			if !ok {
-				return errors.New("System does not support SIGKILL")
-			}
-			if err := daemon.kill(c, int(sig)); err != nil {
-				logrus.Errorf("Failed to SIGKILL container %s", c.ID)
-			}
-			// Wait for exit again without a timeout.
-			// Explicitly ignore the result.
-			_ = <-c.Wait(context.Background(), container.WaitConditionNotRunning)
-			return status.Err()
-		}
-	}
 	// If container failed to exit in stopTimeout seconds of SIGTERM, then using the force
 	if err := daemon.containerStop(c, stopTimeout); err != nil {
 		return fmt.Errorf("Failed to stop container %s with error: %v", c.ID, err)

daemon/kill.go

@@ -60,15 +60,11 @@ func (daemon *Daemon) killWithSignal(container *containerpkg.Container, sig int)
 	container.Lock()
 	defer container.Unlock()
 
-	// We could unpause the container for them rather than returning this error
-	if container.Paused {
-		return fmt.Errorf("Container %s is paused. Unpause the container before stopping or killing", container.ID)
-	}
-
 	if !container.Running {
 		return errNotRunning{container.ID}
 	}
 
+	var unpause bool
 	if container.Config.StopSignal != "" && syscall.Signal(sig) != syscall.SIGKILL {
 		containerStopSignal, err := signal.ParseSignal(container.Config.StopSignal)
 		if err != nil {
@@ -76,9 +72,11 @@ func (daemon *Daemon) killWithSignal(container *containerpkg.Container, sig int)
 		}
 		if containerStopSignal == syscall.Signal(sig) {
 			container.ExitOnNext()
+			unpause = container.Paused
 		}
 	} else {
 		container.ExitOnNext()
+		unpause = container.Paused
 	}
 
 	if !daemon.IsShuttingDown() {
@@ -98,11 +96,19 @@ func (daemon *Daemon) killWithSignal(container *containerpkg.Container, sig int)
 		if strings.Contains(err.Error(), "container not found") ||
 			strings.Contains(err.Error(), "no such process") {
 			logrus.Warnf("container kill failed because of 'container not found' or 'no such process': %s", err.Error())
+			unpause = false
 		} else {
 			return err
 		}
 	}
 
+	if unpause {
+		// above kill signal will be sent once resume is finished
+		if err := daemon.containerd.Resume(container.ID); err != nil {
+			logrus.Warn("Cannot unpause container %s: %s", container.ID, err)
+		}
+	}
+
 	attributes := map[string]string{
 		"signal": fmt.Sprintf("%d", sig),
 	}

integration-cli/docker_cli_pause_test.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"strings"
+	"time"
 
 	"github.com/docker/docker/integration-cli/checker"
 	"github.com/docker/docker/integration-cli/cli"
@@ -65,3 +66,13 @@ func (s *DockerSuite) TestPauseFailsOnWindowsServerContainers(c *check.C) {
 	out, _, _ := dockerCmdWithError("pause", "test")
 	c.Assert(out, checker.Contains, "cannot pause Windows Server Containers")
 }
+
+func (s *DockerSuite) TestStopPausedContainer(c *check.C) {
+	testRequires(c, DaemonIsLinux)
+
+	id := runSleepingContainer(c)
+	cli.WaitRun(c, id)
+	cli.DockerCmd(c, "pause", id)
+	cli.DockerCmd(c, "stop", id)
+	cli.WaitForInspectResult(c, id, "{{.State.Running}}", "false", 30*time.Second)
+}

integration-cli/docker_utils_test.go

@@ -396,7 +396,7 @@ func runSleepingContainerInImage(c *check.C, image string, extraArgs ...string)
 	args = append(args, extraArgs...)
 	args = append(args, image)
 	args = append(args, sleepCommandForDaemonPlatform()...)
-	return cli.DockerCmd(c, args...).Combined()
+	return strings.TrimSpace(cli.DockerCmd(c, args...).Combined())
 }
 
 // minimalBaseImage returns the name of the minimal base image for the current