From d2e99882b92ef965f1c219301227c4004504e57c Mon Sep 17 00:00:00 2001 From: Lei Jitang Date: Wed, 4 Jan 2017 20:42:17 -0500 Subject: [PATCH] Follow up #29365, fix fail to remove container after restart Call daemon.Mount will increase the refcount of mounted path, for those previous running containers, `Mount` call will make the refcount to 2. see https://github.com/docker/docker/blob/v1.13.0-rc4/daemon/graphdriver/counter.go#L38 ``` if !m.check { m.check = true if c.checker.IsMounted(path) { m.count++ } } m.count++ ``` graphdrive could restore on reboot after #22541, call daemon.Mount to resore the graphdriver is not necessary. And call `daemon.Mount` on restorting will mount all the containers mounted layer even if it was stop. This fix call Mount and then Unmount to get `BaseFs` Signed-off-by: Lei Jitang --- daemon/daemon.go | 26 ++++++++++------ integration-cli/docker_cli_daemon_test.go | 36 ++++++++++++++++++++++- 2 files changed, 52 insertions(+), 10 deletions(-) diff --git a/daemon/daemon.go b/daemon/daemon.go index 07fcc811f0..f0a1760fda 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -147,15 +147,6 @@ func (daemon *Daemon) restore() error { continue } container.RWLayer = rwlayer - if err := daemon.Mount(container); err != nil { - // The mount is unlikely to fail. However, in case mount fails - // the container should be allowed to restore here. Some functionalities - // (like docker exec -u user) might be missing but container is able to be - // stopped/restarted/removed. - // See #29365 for related information. - // The error is only logged here. - logrus.Warnf("Failed to mount container %v: %v", id, err) - } logrus.Debugf("Loaded container %v", container.ID) containers[container.ID] = container @@ -213,6 +204,23 @@ func (daemon *Daemon) restore() error { logrus.Errorf("Failed to restore %s with containerd: %s", c.ID, err) return } + + // we call Mount and then Unmount to get BaseFs of the container + if err := daemon.Mount(c); err != nil { + // The mount is unlikely to fail. However, in case mount fails + // the container should be allowed to restore here. Some functionalities + // (like docker exec -u user) might be missing but container is able to be + // stopped/restarted/removed. + // See #29365 for related information. + // The error is only logged here. + logrus.Warnf("Failed to mount container on getting BaseFs path %v: %v", c.ID, err) + } else { + // if mount success, then unmount it + if err := daemon.Unmount(c); err != nil { + logrus.Warnf("Failed to umount container on getting BaseFs path %v: %v", c.ID, err) + } + } + c.ResetRestartManager(false) if !c.HostConfig.NetworkMode.IsContainer() && c.IsRunning() { options, err := daemon.buildSandboxOptions(c) diff --git a/integration-cli/docker_cli_daemon_test.go b/integration-cli/docker_cli_daemon_test.go index 720d31553b..3a74fe215f 100644 --- a/integration-cli/docker_cli_daemon_test.go +++ b/integration-cli/docker_cli_daemon_test.go @@ -3,6 +3,7 @@ package main import ( + "bufio" "bytes" "encoding/json" "fmt" @@ -2936,7 +2937,7 @@ func (s *DockerDaemonSuite) TestExecWithUserAfterLiveRestore(c *check.C) { out, err := s.d.Cmd("run", "-d", "--name=top", "busybox", "sh", "-c", "addgroup -S test && adduser -S -G test test -D -s /bin/sh && top") c.Assert(err, check.IsNil, check.Commentf("Output: %s", out)) - waitRun("top") + s.d.waitRun("top") out1, err := s.d.Cmd("exec", "-u", "test", "top", "id") // uid=100(test) gid=101(test) groups=101(test) @@ -2952,3 +2953,36 @@ func (s *DockerDaemonSuite) TestExecWithUserAfterLiveRestore(c *check.C) { out, err = s.d.Cmd("stop", "top") c.Assert(err, check.IsNil, check.Commentf("Output: %s", out)) } + +func (s *DockerDaemonSuite) TestRemoveContainerAfterLiveRestore(c *check.C) { + testRequires(c, DaemonIsLinux, overlayFSSupported, SameHostDaemon) + s.d.StartWithBusybox("--live-restore", "--storage-driver", "overlay") + out, err := s.d.Cmd("run", "-d", "--name=top", "busybox", "top") + c.Assert(err, check.IsNil, check.Commentf("Output: %s", out)) + + s.d.waitRun("top") + + // restart daemon. + s.d.Restart("--live-restore", "--storage-driver", "overlay") + + out, err = s.d.Cmd("stop", "top") + c.Assert(err, check.IsNil, check.Commentf("Output: %s", out)) + + // test if the rootfs mountpoint still exist + mountpoint, err := s.d.inspectFilter("top", ".GraphDriver.Data.MergedDir") + c.Assert(err, check.IsNil) + f, err := os.Open("/proc/self/mountinfo") + c.Assert(err, check.IsNil) + defer f.Close() + sc := bufio.NewScanner(f) + for sc.Scan() { + line := sc.Text() + if strings.Contains(line, mountpoint) { + c.Fatalf("mountinfo should not include the mountpoint of stop container") + } + } + + out, err = s.d.Cmd("rm", "top") + c.Assert(err, check.IsNil, check.Commentf("Output: %s", out)) + +}