Browse code

Improvements to the original sharing implementation.

- Print the mount table as in /proc/self/mountinfo
- Do not exit prematurely when one of the ipc mounts doesn't exist.
- Do not exit prematurely when one of the ipc mounts cannot be unmounted.
- Add a unit test to see if the cleanup really works.
- Use syscall.MNT_DETACH to cleanup mounts after a crash.
- Unmount IPC mounts when the daemon unregisters an old running container.

Signed-off-by: David Calavera <david.calavera@gmail.com>

David Calavera authored on 2015/08/26 21:00:01
Showing 6 changed files
... ...
@@ -296,7 +296,7 @@ func (container *Container) Start() (err error) {
296 296
 		return err
297 297
 	}
298 298
 
299
-	if !(container.hostConfig.IpcMode.IsContainer() || container.hostConfig.IpcMode.IsHost()) {
299
+	if !container.hostConfig.IpcMode.IsContainer() && !container.hostConfig.IpcMode.IsHost() {
300 300
 		if err := container.setupIpcDirs(); err != nil {
301 301
 			return err
302 302
 		}
... ...
@@ -366,11 +366,11 @@ func (container *Container) cleanup() {
366 366
 	container.releaseNetwork()
367 367
 
368 368
 	if err := container.unmountIpcMounts(); err != nil {
369
-		logrus.Errorf("%v: Failed to umount ipc filesystems: %v", container.ID, err)
369
+		logrus.Errorf("%s: Failed to umount ipc filesystems: %v", container.ID, err)
370 370
 	}
371 371
 
372 372
 	if err := container.Unmount(); err != nil {
373
-		logrus.Errorf("%v: Failed to umount filesystem: %v", container.ID, err)
373
+		logrus.Errorf("%s: Failed to umount filesystem: %v", container.ID, err)
374 374
 	}
375 375
 
376 376
 	for _, eConfig := range container.execCommands.s {
... ...
@@ -1242,10 +1242,10 @@ func (container *Container) removeMountPoints(rm bool) error {
1242 1242
 }
1243 1243
 
1244 1244
 func (container *Container) shmPath() (string, error) {
1245
-	return container.GetRootResourcePath("shm")
1245
+	return container.getRootResourcePath("shm")
1246 1246
 }
1247 1247
 func (container *Container) mqueuePath() (string, error) {
1248
-	return container.GetRootResourcePath("mqueue")
1248
+	return container.getRootResourcePath("mqueue")
1249 1249
 }
1250 1250
 
1251 1251
 func (container *Container) setupIpcDirs() error {
... ...
@@ -1258,7 +1258,7 @@ func (container *Container) setupIpcDirs() error {
1258 1258
 		return err
1259 1259
 	}
1260 1260
 
1261
-	if err := syscall.Mount("shm", shmPath, "tmpfs", uintptr(syscall.MS_NOEXEC|syscall.MS_NOSUID|syscall.MS_NODEV), label.FormatMountLabel("mode=1777,size=65536k", container.GetMountLabel())); err != nil {
1261
+	if err := syscall.Mount("shm", shmPath, "tmpfs", uintptr(syscall.MS_NOEXEC|syscall.MS_NOSUID|syscall.MS_NODEV), label.FormatMountLabel("mode=1777,size=65536k", container.getMountLabel())); err != nil {
1262 1262
 		return fmt.Errorf("mounting shm tmpfs: %s", err)
1263 1263
 	}
1264 1264
 
... ...
@@ -1283,22 +1283,32 @@ func (container *Container) unmountIpcMounts() error {
1283 1283
 		return nil
1284 1284
 	}
1285 1285
 
1286
+	var errors []string
1286 1287
 	shmPath, err := container.shmPath()
1287 1288
 	if err != nil {
1288
-		return fmt.Errorf("shm path does not exist %v", err)
1289
-	}
1289
+		logrus.Error(err)
1290
+		errors = append(errors, err.Error())
1291
+	} else {
1292
+		if err := detachMounted(shmPath); err != nil {
1293
+			logrus.Errorf("failed to umount %s: %v", shmPath, err)
1294
+			errors = append(errors, err.Error())
1295
+		}
1290 1296
 
1291
-	if err := syscall.Unmount(shmPath, syscall.MNT_DETACH); err != nil {
1292
-		return fmt.Errorf("failed to umount %s filesystem %v", shmPath, err)
1293 1297
 	}
1294 1298
 
1295 1299
 	mqueuePath, err := container.mqueuePath()
1296 1300
 	if err != nil {
1297
-		return fmt.Errorf("mqueue path does not exist %v", err)
1301
+		logrus.Error(err)
1302
+		errors = append(errors, err.Error())
1303
+	} else {
1304
+		if err := detachMounted(mqueuePath); err != nil {
1305
+			logrus.Errorf("failed to umount %s: %v", mqueuePath, err)
1306
+			errors = append(errors, err.Error())
1307
+		}
1298 1308
 	}
1299 1309
 
1300
-	if err := syscall.Unmount(mqueuePath, syscall.MNT_DETACH); err != nil {
1301
-		return fmt.Errorf("failed to umount %s filesystem %v", mqueuePath, err)
1310
+	if len(errors) > 0 {
1311
+		return fmt.Errorf("failed to cleanup ipc mounts:\n%v", strings.Join(errors, "\n"))
1302 1312
 	}
1303 1313
 
1304 1314
 	return nil
... ...
@@ -1322,3 +1332,7 @@ func (container *Container) ipcMounts() []execdriver.Mount {
1322 1322
 	})
1323 1323
 	return mounts
1324 1324
 }
1325
+
1326
+func detachMounted(path string) error {
1327
+	return syscall.Unmount(path, syscall.MNT_DETACH)
1328
+}
... ...
@@ -209,6 +209,9 @@ func (daemon *Daemon) Register(container *Container) error {
209 209
 		}
210 210
 		daemon.execDriver.Terminate(cmd)
211 211
 
212
+		if err := container.unmountIpcMounts(); err != nil {
213
+			logrus.Errorf("%s: Failed to umount ipc filesystems: %v", container.ID, err)
214
+		}
212 215
 		if err := container.Unmount(); err != nil {
213 216
 			logrus.Debugf("unmount error %s", err)
214 217
 		}
... ...
@@ -756,13 +759,14 @@ func NewDaemon(config *Config, registryService *registry.Service) (daemon *Daemo
756 756
 	d.EventsService = eventsService
757 757
 	d.volumes = volStore
758 758
 	d.root = config.Root
759
-	go d.execCommandGC()
760 759
 
761
-	if err := d.restore(); err != nil {
760
+	if err := d.cleanupMounts(); err != nil {
762 761
 		return nil, err
763 762
 	}
764 763
 
765
-	if err := d.cleanupMounts(); err != nil {
764
+	go d.execCommandGC()
765
+
766
+	if err := d.restore(); err != nil {
766 767
 		return nil, err
767 768
 	}
768 769
 
... ...
@@ -2,12 +2,13 @@ package daemon
2 2
 
3 3
 import (
4 4
 	"bufio"
5
+	"fmt"
6
+	"io"
5 7
 	"os"
6 8
 	"path/filepath"
7 9
 	"strings"
8 10
 
9 11
 	"github.com/Sirupsen/logrus"
10
-	"github.com/docker/docker/pkg/mount"
11 12
 )
12 13
 
13 14
 // cleanupMounts umounts shm/mqueue mounts for old containers
... ...
@@ -19,17 +20,24 @@ func (daemon *Daemon) cleanupMounts() error {
19 19
 	}
20 20
 	defer f.Close()
21 21
 
22
-	sc := bufio.NewScanner(f)
22
+	return daemon.cleanupMountsFromReader(f, detachMounted)
23
+}
24
+
25
+func (daemon *Daemon) cleanupMountsFromReader(reader io.Reader, unmount func(target string) error) error {
26
+	sc := bufio.NewScanner(reader)
27
+	var errors []string
23 28
 	for sc.Scan() {
24 29
 		line := sc.Text()
25
-		fields := strings.Split(line, " ")
30
+		fields := strings.Fields(line)
26 31
 		if strings.HasPrefix(fields[4], daemon.repository) {
32
+			logrus.Debugf("Mount base: %v, repository %s", fields[4], daemon.repository)
27 33
 			mnt := fields[4]
28 34
 			mountBase := filepath.Base(mnt)
29 35
 			if mountBase == "mqueue" || mountBase == "shm" {
30
-				logrus.Debugf("Unmounting %+v", mnt)
31
-				if err := mount.Unmount(mnt); err != nil {
32
-					return err
36
+				logrus.Debugf("Unmounting %v", mnt)
37
+				if err := unmount(mnt); err != nil {
38
+					logrus.Error(err)
39
+					errors = append(errors, err.Error())
33 40
 				}
34 41
 			}
35 42
 		}
... ...
@@ -39,6 +47,10 @@ func (daemon *Daemon) cleanupMounts() error {
39 39
 		return err
40 40
 	}
41 41
 
42
+	if len(errors) > 0 {
43
+		return fmt.Errorf("Error cleaningup mounts:\n%v", strings.Join(errors, "\n"))
44
+	}
45
+
42 46
 	logrus.Debugf("Cleaning up old shm/mqueue mounts: done.")
43 47
 	return nil
44 48
 }
45 49
new file mode 100644
... ...
@@ -0,0 +1,58 @@
0
+// +build linux
1
+
2
+package daemon
3
+
4
+import (
5
+	"strings"
6
+	"testing"
7
+)
8
+
9
+func TestCleanupMounts(t *testing.T) {
10
+	fixture := `230 138 0:60 / / rw,relatime - overlay overlay rw,lowerdir=/var/lib/docker/overlay/0ef9f93d5d365c1385b09d54bbee6afff3d92002c16f22eccb6e1549b2ff97d8/root,upperdir=/var/lib/docker/overlay/dfac036ce135a8914e292cb2f6fea114f7339983c186366aa26d0051e93162cb/upper,workdir=/var/lib/docker/overlay/dfac036ce135a8914e292cb2f6fea114f7339983c186366aa26d0051e93162cb/work
11
+231 230 0:56 / /proc rw,nosuid,nodev,noexec,relatime - proc proc rw
12
+232 230 0:57 / /dev rw,nosuid - tmpfs tmpfs rw,mode=755
13
+233 232 0:58 / /dev/pts rw,nosuid,noexec,relatime - devpts devpts rw,gid=5,mode=620,ptmxmode=666
14
+234 232 0:59 / /dev/shm rw,nosuid,nodev,noexec,relatime - tmpfs shm rw,size=65536k
15
+235 232 0:55 / /dev/mqueue rw,nosuid,nodev,noexec,relatime - mqueue mqueue rw
16
+236 230 0:61 / /sys rw,nosuid,nodev,noexec,relatime - sysfs sysfs rw
17
+237 236 0:62 / /sys/fs/cgroup rw,nosuid,nodev,noexec,relatime - tmpfs tmpfs rw
18
+238 237 0:21 /system.slice/docker.service /sys/fs/cgroup/systemd rw,nosuid,nodev,noexec,relatime - cgroup cgroup rw,xattr,release_agent=/lib/systemd/systemd-cgroups-agent,name=systemd
19
+239 237 0:23 /docker/dfac036ce135a8914e292cb2f6fea114f7339983c186366aa26d0051e93162cb /sys/fs/cgroup/perf_event rw,nosuid,nodev,noexec,relatime - cgroup cgroup rw,perf_event
20
+240 237 0:24 /docker/dfac036ce135a8914e292cb2f6fea114f7339983c186366aa26d0051e93162cb /sys/fs/cgroup/cpuset rw,nosuid,nodev,noexec,relatime - cgroup cgroup rw,cpuset,clone_children
21
+241 237 0:25 /docker/dfac036ce135a8914e292cb2f6fea114f7339983c186366aa26d0051e93162cb /sys/fs/cgroup/devices rw,nosuid,nodev,noexec,relatime - cgroup cgroup rw,devices
22
+242 237 0:26 /docker/dfac036ce135a8914e292cb2f6fea114f7339983c186366aa26d0051e93162cb /sys/fs/cgroup/freezer rw,nosuid,nodev,noexec,relatime - cgroup cgroup rw,freezer
23
+243 237 0:27 /docker/dfac036ce135a8914e292cb2f6fea114f7339983c186366aa26d0051e93162cb /sys/fs/cgroup/cpu,cpuacct rw,nosuid,nodev,noexec,relatime - cgroup cgroup rw,cpu,cpuacct
24
+244 237 0:28 /docker/dfac036ce135a8914e292cb2f6fea114f7339983c186366aa26d0051e93162cb /sys/fs/cgroup/blkio rw,nosuid,nodev,noexec,relatime - cgroup cgroup rw,blkio
25
+245 237 0:29 /docker/dfac036ce135a8914e292cb2f6fea114f7339983c186366aa26d0051e93162cb /sys/fs/cgroup/net_cls,net_prio rw,nosuid,nodev,noexec,relatime - cgroup cgroup rw,net_cls,net_prio
26
+246 237 0:30 /docker/dfac036ce135a8914e292cb2f6fea114f7339983c186366aa26d0051e93162cb /sys/fs/cgroup/hugetlb rw,nosuid,nodev,noexec,relatime - cgroup cgroup rw,hugetlb
27
+247 237 0:31 /docker/dfac036ce135a8914e292cb2f6fea114f7339983c186366aa26d0051e93162cb /sys/fs/cgroup/memory rw,nosuid,nodev,noexec,relatime - cgroup cgroup rw,memory
28
+248 230 253:1 /var/lib/docker/volumes/510cc41ac68c48bd4eac932e3e09711673876287abf1b185312cfbfe6261a111/_data /var/lib/docker rw,relatime - ext4 /dev/disk/by-uuid/ba70ea0c-1a8f-4ee4-9687-cb393730e2b5 rw,errors=remount-ro,data=ordered
29
+250 230 253:1 /var/lib/docker/containers/dfac036ce135a8914e292cb2f6fea114f7339983c186366aa26d0051e93162cb/hostname /etc/hostname rw,relatime - ext4 /dev/disk/by-uuid/ba70ea0c-1a8f-4ee4-9687-cb393730e2b5 rw,errors=remount-ro,data=ordered
30
+251 230 253:1 /var/lib/docker/containers/dfac036ce135a8914e292cb2f6fea114f7339983c186366aa26d0051e93162cb/hosts /etc/hosts rw,relatime - ext4 /dev/disk/by-uuid/ba70ea0c-1a8f-4ee4-9687-cb393730e2b5 rw,errors=remount-ro,data=ordered
31
+252 232 0:13 /1 /dev/console rw,nosuid,noexec,relatime - devpts devpts rw,gid=5,mode=620,ptmxmode=000
32
+139 236 0:11 / /sys/kernel/security rw,relatime - securityfs none rw
33
+140 230 0:54 / /tmp rw,relatime - tmpfs none rw
34
+145 230 0:3 / /run/docker/netns/default rw - nsfs nsfs rw
35
+130 140 0:45 / /tmp/docker_recursive_mount_test312125472/tmpfs rw,relatime - tmpfs tmpfs rw
36
+131 230 0:3 / /run/docker/netns/47903e2e6701 rw - nsfs nsfs rw
37
+133 230 0:55 / /go/src/github.com/docker/docker/bundles/1.9.0-dev/test-integration-cli/d45526097/graph/containers/47903e2e67014246eba27607809d5f5c2437c3bf84c2986393448f84093cc40b/mqueue rw,nosuid,nodev,noexec,relatime - mqueue mqueue rw`
38
+
39
+	d := &Daemon{
40
+		repository: "/go/src/github.com/docker/docker/bundles/1.9.0-dev/test-integration-cli/d45526097/graph/containers/",
41
+	}
42
+
43
+	expected := "/go/src/github.com/docker/docker/bundles/1.9.0-dev/test-integration-cli/d45526097/graph/containers/47903e2e67014246eba27607809d5f5c2437c3bf84c2986393448f84093cc40b/mqueue"
44
+	var unmounted bool
45
+	unmount := func(target string) error {
46
+		if target == expected {
47
+			unmounted = true
48
+		}
49
+		return nil
50
+	}
51
+
52
+	d.cleanupMountsFromReader(strings.NewReader(fixture), unmount)
53
+
54
+	if !unmounted {
55
+		t.Fatalf("Expected to unmount the mqueue")
56
+	}
57
+}
... ...
@@ -1474,9 +1474,11 @@ func (s *DockerDaemonSuite) TestCleanupMountsAfterCrash(c *check.C) {
1474 1474
 	id := strings.TrimSpace(out)
1475 1475
 	c.Assert(s.d.cmd.Process.Signal(os.Kill), check.IsNil)
1476 1476
 	c.Assert(s.d.Start(), check.IsNil)
1477
-	mountOut, err := exec.Command("mount").CombinedOutput()
1477
+	mountOut, err := ioutil.ReadFile("/proc/self/mountinfo")
1478 1478
 	c.Assert(err, check.IsNil, check.Commentf("Output: %s", mountOut))
1479
-	c.Assert(strings.Contains(string(mountOut), id), check.Equals, false, check.Commentf("Something mounted from older daemon start: %s", mountOut))
1479
+
1480
+	comment := check.Commentf("%s is still mounted from older daemon start:\nDaemon root repository %s\n%s", id, s.d.folder, mountOut)
1481
+	c.Assert(strings.Contains(string(mountOut), id), check.Equals, false, comment)
1480 1482
 }
1481 1483
 
1482 1484
 func (s *DockerDaemonSuite) TestRunContainerWithBridgeNone(c *check.C) {