From 1e36e346dbcdacefa5313252133bed8003afea1d Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Mon, 23 May 2016 11:13:12 -0700 Subject: [PATCH] Fix already active error on restoring from ungraceful restart MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On killing the processes that were still running after ungraceful restart, containerd can’t fully exit the process if the fifos have not been fully read. In this case process was just marked as exited and not removed from the internal containers array. Fixes #22913 Signed-off-by: Tonis Tiigi --- ...docker_cli_daemon_not_experimental_test.go | 20 +++++++++++++++++++ libcontainerd/client_shutdownrestore_linux.go | 5 +++++ libcontainerd/container_linux.go | 16 +++++++++++++++ 3 files changed, 41 insertions(+) diff --git a/integration-cli/docker_cli_daemon_not_experimental_test.go b/integration-cli/docker_cli_daemon_not_experimental_test.go index 3c987741fb..8c5634cc6d 100644 --- a/integration-cli/docker_cli_daemon_not_experimental_test.go +++ b/integration-cli/docker_cli_daemon_not_experimental_test.go @@ -37,3 +37,23 @@ func (s *DockerDaemonSuite) TestCleanupMountsAfterDaemonKill(c *check.C) { comment = check.Commentf("%s is still mounted from older daemon start:\nDaemon root repository %s\n%s", id, s.d.folder, mountOut) c.Assert(strings.Contains(string(mountOut), id), check.Equals, false, comment) } + +// #22913 +func (s *DockerDaemonSuite) TestContainerStartAfterDaemonKill(c *check.C) { + testRequires(c, DaemonIsLinux) + c.Assert(s.d.StartWithBusybox(), check.IsNil) + + // the application is chosen so it generates output and doesn't react to SIGTERM + out, err := s.d.Cmd("run", "-d", "busybox", "sh", "-c", "while true;do date;done") + c.Assert(err, check.IsNil, check.Commentf("Output: %s", out)) + id := strings.TrimSpace(out) + c.Assert(s.d.cmd.Process.Signal(os.Kill), check.IsNil) + + // restart daemon. + if err := s.d.Restart(); err != nil { + c.Fatal(err) + } + + out, err = s.d.Cmd("start", id) + c.Assert(err, check.IsNil, check.Commentf("Output: %s", out)) +} diff --git a/libcontainerd/client_shutdownrestore_linux.go b/libcontainerd/client_shutdownrestore_linux.go index 52ea2a6180..1b4a2bc58f 100644 --- a/libcontainerd/client_shutdownrestore_linux.go +++ b/libcontainerd/client_shutdownrestore_linux.go @@ -20,6 +20,8 @@ func (clnt *client) Restore(containerID string, options ...CreateOption) error { clnt.appendContainer(container) clnt.unlock(cont.Id) + container.discardFifos() + if err := clnt.Signal(containerID, int(syscall.SIGTERM)); err != nil { logrus.Errorf("error sending sigterm to %v: %v", containerID, err) } @@ -37,5 +39,8 @@ func (clnt *client) Restore(containerID string, options ...CreateOption) error { return nil } } + + clnt.deleteContainer(containerID) + return clnt.setExited(containerID) } diff --git a/libcontainerd/container_linux.go b/libcontainerd/container_linux.go index 7fa6770b58..8a49cde2fa 100644 --- a/libcontainerd/container_linux.go +++ b/libcontainerd/container_linux.go @@ -2,6 +2,7 @@ package libcontainerd import ( "encoding/json" + "io" "io/ioutil" "os" "path/filepath" @@ -191,3 +192,18 @@ func (ctr *container) handleEvent(e *containerd.Event) error { } return nil } + +// discardFifos attempts to fully read the container fifos to unblock processes +// that may be blocked on the writer side. +func (ctr *container) discardFifos() { + for _, i := range []int{syscall.Stdout, syscall.Stderr} { + f := ctr.fifo(i) + c := make(chan struct{}) + go func() { + close(c) // this channel is used to not close the writer too early, before readonly open has been called. + io.Copy(ioutil.Discard, openReaderFromFifo(f)) + }() + <-c + closeReaderFifo(f) // avoid blocking permanently on open if there is no writer side + } +}