Browse code

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 <leijitang@huawei.com>

Lei Jitang authored on 2016/12/29 10:08:03
Showing 3 changed files
... ...
@@ -147,15 +147,6 @@ func (daemon *Daemon) restore() error {
147 147
 				continue
148 148
 			}
149 149
 			container.RWLayer = rwlayer
150
-			if err := daemon.Mount(container); err != nil {
151
-				// The mount is unlikely to fail. However, in case mount fails
152
-				// the container should be allowed to restore here. Some functionalities
153
-				// (like docker exec -u user) might be missing but container is able to be
154
-				// stopped/restarted/removed.
155
-				// See #29365 for related information.
156
-				// The error is only logged here.
157
-				logrus.Warnf("Failed to mount container %v: %v", id, err)
158
-			}
159 150
 			logrus.Debugf("Loaded container %v", container.ID)
160 151
 
161 152
 			containers[container.ID] = container
... ...
@@ -213,6 +204,23 @@ func (daemon *Daemon) restore() error {
213 213
 					logrus.Errorf("Failed to restore %s with containerd: %s", c.ID, err)
214 214
 					return
215 215
 				}
216
+
217
+				// we call Mount and then Unmount to get BaseFs of the container
218
+				if err := daemon.Mount(c); err != nil {
219
+					// The mount is unlikely to fail. However, in case mount fails
220
+					// the container should be allowed to restore here. Some functionalities
221
+					// (like docker exec -u user) might be missing but container is able to be
222
+					// stopped/restarted/removed.
223
+					// See #29365 for related information.
224
+					// The error is only logged here.
225
+					logrus.Warnf("Failed to mount container on getting BaseFs path %v: %v", c.ID, err)
226
+				} else {
227
+					// if mount success, then unmount it
228
+					if err := daemon.Unmount(c); err != nil {
229
+						logrus.Warnf("Failed to umount container on getting BaseFs path %v: %v", c.ID, err)
230
+					}
231
+				}
232
+
216 233
 				c.ResetRestartManager(false)
217 234
 				if !c.HostConfig.NetworkMode.IsContainer() && c.IsRunning() {
218 235
 					options, err := daemon.buildSandboxOptions(c)
... ...
@@ -654,6 +654,11 @@ func (d *Daemon) ReadLogFile() ([]byte, error) {
654 654
 	return ioutil.ReadFile(d.logFile.Name())
655 655
 }
656 656
 
657
+// InspectField returns the field filter by 'filter'
658
+func (d *Daemon) InspectField(name, filter string) (string, error) {
659
+	return d.inspectFilter(name, filter)
660
+}
661
+
657 662
 func (d *Daemon) inspectFilter(name, filter string) (string, error) {
658 663
 	format := fmt.Sprintf("{{%s}}", filter)
659 664
 	out, err := d.Cmd("inspect", "-f", format, name)
... ...
@@ -3,6 +3,7 @@
3 3
 package main
4 4
 
5 5
 import (
6
+	"bufio"
6 7
 	"bytes"
7 8
 	"encoding/json"
8 9
 	"fmt"
... ...
@@ -2832,7 +2833,7 @@ func (s *DockerDaemonSuite) TestExecWithUserAfterLiveRestore(c *check.C) {
2832 2832
 	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")
2833 2833
 	c.Assert(err, check.IsNil, check.Commentf("Output: %s", out))
2834 2834
 
2835
-	waitRun("top")
2835
+	s.d.WaitRun("top")
2836 2836
 
2837 2837
 	out1, err := s.d.Cmd("exec", "-u", "test", "top", "id")
2838 2838
 	// uid=100(test) gid=101(test) groups=101(test)
... ...
@@ -2848,3 +2849,36 @@ func (s *DockerDaemonSuite) TestExecWithUserAfterLiveRestore(c *check.C) {
2848 2848
 	out, err = s.d.Cmd("stop", "top")
2849 2849
 	c.Assert(err, check.IsNil, check.Commentf("Output: %s", out))
2850 2850
 }
2851
+
2852
+func (s *DockerDaemonSuite) TestRemoveContainerAfterLiveRestore(c *check.C) {
2853
+	testRequires(c, DaemonIsLinux, overlayFSSupported, SameHostDaemon)
2854
+	s.d.StartWithBusybox(c, "--live-restore", "--storage-driver", "overlay")
2855
+	out, err := s.d.Cmd("run", "-d", "--name=top", "busybox", "top")
2856
+	c.Assert(err, check.IsNil, check.Commentf("Output: %s", out))
2857
+
2858
+	s.d.WaitRun("top")
2859
+
2860
+	// restart daemon.
2861
+	s.d.Restart(c, "--live-restore", "--storage-driver", "overlay")
2862
+
2863
+	out, err = s.d.Cmd("stop", "top")
2864
+	c.Assert(err, check.IsNil, check.Commentf("Output: %s", out))
2865
+
2866
+	// test if the rootfs mountpoint still exist
2867
+	mountpoint, err := s.d.InspectField("top", ".GraphDriver.Data.MergedDir")
2868
+	c.Assert(err, check.IsNil)
2869
+	f, err := os.Open("/proc/self/mountinfo")
2870
+	c.Assert(err, check.IsNil)
2871
+	defer f.Close()
2872
+	sc := bufio.NewScanner(f)
2873
+	for sc.Scan() {
2874
+		line := sc.Text()
2875
+		if strings.Contains(line, mountpoint) {
2876
+			c.Fatalf("mountinfo should not include the mountpoint of stop container")
2877
+		}
2878
+	}
2879
+
2880
+	out, err = s.d.Cmd("rm", "top")
2881
+	c.Assert(err, check.IsNil, check.Commentf("Output: %s", out))
2882
+
2883
+}