From c3feb046b9b1eb606cc7e853d020799eb3800e0e Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Sun, 9 Jul 2017 09:34:14 -0400 Subject: [PATCH] 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 --- daemon/daemon.go | 36 ------------------------ daemon/kill.go | 16 +++++++---- integration-cli/docker_cli_pause_test.go | 11 ++++++++ integration-cli/docker_utils_test.go | 2 +- 4 files changed, 23 insertions(+), 42 deletions(-) diff --git a/daemon/daemon.go b/daemon/daemon.go index cdaa5c9a41..8359ef31ca 100644 --- a/daemon/daemon.go +++ b/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) diff --git a/daemon/kill.go b/daemon/kill.go index a2220715a3..b118160f84 100644 --- a/daemon/kill.go +++ b/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), } diff --git a/integration-cli/docker_cli_pause_test.go b/integration-cli/docker_cli_pause_test.go index 5822329f2a..682384fc1a 100644 --- a/integration-cli/docker_cli_pause_test.go +++ b/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) +} diff --git a/integration-cli/docker_utils_test.go b/integration-cli/docker_utils_test.go index 1488c93b46..0c0a164837 100644 --- a/integration-cli/docker_utils_test.go +++ b/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