Skip to content

Commit b1d2f52

Browse files
calaveracalavera
committed
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>
1 parent c8291f7 commit b1d2f52

File tree

6 files changed

+114
-24
lines changed

6 files changed

+114
-24
lines changed

‎daemon/container.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,7 @@ func (container *Container) Start() (err error) {
296296
return err
297297
}
298298

299-
if !(container.hostConfig.IpcMode.IsContainer() || container.hostConfig.IpcMode.IsHost()) {
299+
if !container.hostConfig.IpcMode.IsContainer() && !container.hostConfig.IpcMode.IsHost() {
300300
if err := container.setupIpcDirs(); err != nil {
301301
return err
302302
}
@@ -366,11 +366,11 @@ func (container *Container) cleanup() {
366366
container.releaseNetwork()
367367

368368
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)
370370
}
371371

372372
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)
374374
}
375375

376376
for _, eConfig := range container.execCommands.s {

‎daemon/container_unix.go‎

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1242,10 +1242,10 @@ func (container *Container) removeMountPoints(rm bool) error {
12421242
}
12431243

12441244
func (container *Container) shmPath() (string, error) {
1245-
return container.GetRootResourcePath("shm")
1245+
return container.getRootResourcePath("shm")
12461246
}
12471247
func (container *Container) mqueuePath() (string, error) {
1248-
return container.GetRootResourcePath("mqueue")
1248+
return container.getRootResourcePath("mqueue")
12491249
}
12501250

12511251
func (container *Container) setupIpcDirs() error {
@@ -1258,7 +1258,7 @@ func (container *Container) setupIpcDirs() error {
12581258
return err
12591259
}
12601260

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 {
12621262
return fmt.Errorf("mounting shm tmpfs: %s", err)
12631263
}
12641264

@@ -1283,22 +1283,32 @@ func (container *Container) unmountIpcMounts() error {
12831283
return nil
12841284
}
12851285

1286+
var errors []string
12861287
shmPath, err := container.shmPath()
12871288
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+
}
12901296

1291-
if err := syscall.Unmount(shmPath, syscall.MNT_DETACH); err != nil {
1292-
return fmt.Errorf("failed to umount %s filesystem %v", shmPath, err)
12931297
}
12941298

12951299
mqueuePath, err := container.mqueuePath()
12961300
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+
}
12981308
}
12991309

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"))
13021312
}
13031313

13041314
return nil
@@ -1322,3 +1332,7 @@ func (container *Container) ipcMounts() []execdriver.Mount {
13221332
})
13231333
return mounts
13241334
}
1335+
1336+
func detachMounted(path string) error {
1337+
return syscall.Unmount(path, syscall.MNT_DETACH)
1338+
}

‎daemon/daemon.go‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,9 @@ func (daemon *Daemon) Register(container *Container) error {
209209
}
210210
daemon.execDriver.Terminate(cmd)
211211

212+
if err := container.unmountIpcMounts(); err != nil {
213+
logrus.Errorf("%s: Failed to umount ipc filesystems: %v", container.ID, err)
214+
}
212215
if err := container.Unmount(); err != nil {
213216
logrus.Debugf("unmount error %s", err)
214217
}
@@ -756,13 +759,14 @@ func NewDaemon(config *Config, registryService *registry.Service) (daemon *Daemo
756759
d.EventsService = eventsService
757760
d.volumes = volStore
758761
d.root = config.Root
759-
go d.execCommandGC()
760762

761-
if err := d.restore(); err != nil {
763+
if err := d.cleanupMounts(); err != nil {
762764
return nil, err
763765
}
764766

765-
if err := d.cleanupMounts(); err != nil {
767+
go d.execCommandGC()
768+
769+
if err := d.restore(); err != nil {
766770
return nil, err
767771
}
768772

‎daemon/daemon_linux.go‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,13 @@ package daemon
22

33
import (
44
"bufio"
5+
"fmt"
6+
"io"
57
"os"
68
"path/filepath"
79
"strings"
810

911
"github.com/Sirupsen/logrus"
10-
"github.com/docker/docker/pkg/mount"
1112
)
1213

1314
// cleanupMounts umounts shm/mqueue mounts for old containers
@@ -19,17 +20,24 @@ func (daemon *Daemon) cleanupMounts() error {
1920
}
2021
defer f.Close()
2122

22-
sc := bufio.NewScanner(f)
23+
return daemon.cleanupMountsFromReader(f, detachMounted)
24+
}
25+
26+
func (daemon *Daemon) cleanupMountsFromReader(reader io.Reader, unmount func(target string) error) error {
27+
sc := bufio.NewScanner(reader)
28+
var errors []string
2329
for sc.Scan() {
2430
line := sc.Text()
25-
fields := strings.Split(line, " ")
31+
fields := strings.Fields(line)
2632
if strings.HasPrefix(fields[4], daemon.repository) {
33+
logrus.Debugf("Mount base: %v, repository %s", fields[4], daemon.repository)
2734
mnt := fields[4]
2835
mountBase := filepath.Base(mnt)
2936
if mountBase == "mqueue" || mountBase == "shm" {
30-
logrus.Debugf("Unmounting %+v", mnt)
31-
if err := mount.Unmount(mnt); err != nil {
32-
return err
37+
logrus.Debugf("Unmounting %v", mnt)
38+
if err := unmount(mnt); err != nil {
39+
logrus.Error(err)
40+
errors = append(errors, err.Error())
3341
}
3442
}
3543
}
@@ -39,6 +47,10 @@ func (daemon *Daemon) cleanupMounts() error {
3947
return err
4048
}
4149

50+
if len(errors) > 0 {
51+
return fmt.Errorf("Error cleaningup mounts:\n%v", strings.Join(errors, "\n"))
52+
}
53+
4254
logrus.Debugf("Cleaning up old shm/mqueue mounts: done.")
4355
return nil
4456
}

‎daemon/daemon_linux_test.go‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
// +build linux
2+
3+
package daemon
4+
5+
import (
6+
"strings"
7+
"testing"
8+
)
9+
10+
func TestCleanupMounts(t *testing.T) {
11+
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
12+
231 230 0:56 / /proc rw,nosuid,nodev,noexec,relatime - proc proc rw
13+
232 230 0:57 / /dev rw,nosuid - tmpfs tmpfs rw,mode=755
14+
233 232 0:58 / /dev/pts rw,nosuid,noexec,relatime - devpts devpts rw,gid=5,mode=620,ptmxmode=666
15+
234 232 0:59 / /dev/shm rw,nosuid,nodev,noexec,relatime - tmpfs shm rw,size=65536k
16+
235 232 0:55 / /dev/mqueue rw,nosuid,nodev,noexec,relatime - mqueue mqueue rw
17+
236 230 0:61 / /sys rw,nosuid,nodev,noexec,relatime - sysfs sysfs rw
18+
237 236 0:62 / /sys/fs/cgroup rw,nosuid,nodev,noexec,relatime - tmpfs tmpfs rw
19+
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
20+
239 237 0:23 /docker/dfac036ce135a8914e292cb2f6fea114f7339983c186366aa26d0051e93162cb /sys/fs/cgroup/perf_event rw,nosuid,nodev,noexec,relatime - cgroup cgroup rw,perf_event
21+
240 237 0:24 /docker/dfac036ce135a8914e292cb2f6fea114f7339983c186366aa26d0051e93162cb /sys/fs/cgroup/cpuset rw,nosuid,nodev,noexec,relatime - cgroup cgroup rw,cpuset,clone_children
22+
241 237 0:25 /docker/dfac036ce135a8914e292cb2f6fea114f7339983c186366aa26d0051e93162cb /sys/fs/cgroup/devices rw,nosuid,nodev,noexec,relatime - cgroup cgroup rw,devices
23+
242 237 0:26 /docker/dfac036ce135a8914e292cb2f6fea114f7339983c186366aa26d0051e93162cb /sys/fs/cgroup/freezer rw,nosuid,nodev,noexec,relatime - cgroup cgroup rw,freezer
24+
243 237 0:27 /docker/dfac036ce135a8914e292cb2f6fea114f7339983c186366aa26d0051e93162cb /sys/fs/cgroup/cpu,cpuacct rw,nosuid,nodev,noexec,relatime - cgroup cgroup rw,cpu,cpuacct
25+
244 237 0:28 /docker/dfac036ce135a8914e292cb2f6fea114f7339983c186366aa26d0051e93162cb /sys/fs/cgroup/blkio rw,nosuid,nodev,noexec,relatime - cgroup cgroup rw,blkio
26+
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
27+
246 237 0:30 /docker/dfac036ce135a8914e292cb2f6fea114f7339983c186366aa26d0051e93162cb /sys/fs/cgroup/hugetlb rw,nosuid,nodev,noexec,relatime - cgroup cgroup rw,hugetlb
28+
247 237 0:31 /docker/dfac036ce135a8914e292cb2f6fea114f7339983c186366aa26d0051e93162cb /sys/fs/cgroup/memory rw,nosuid,nodev,noexec,relatime - cgroup cgroup rw,memory
29+
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
30+
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
31+
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
32+
252 232 0:13 /1 /dev/console rw,nosuid,noexec,relatime - devpts devpts rw,gid=5,mode=620,ptmxmode=000
33+
139 236 0:11 / /sys/kernel/security rw,relatime - securityfs none rw
34+
140 230 0:54 / /tmp rw,relatime - tmpfs none rw
35+
145 230 0:3 / /run/docker/netns/default rw - nsfs nsfs rw
36+
130 140 0:45 / /tmp/docker_recursive_mount_test312125472/tmpfs rw,relatime - tmpfs tmpfs rw
37+
131 230 0:3 / /run/docker/netns/47903e2e6701 rw - nsfs nsfs rw
38+
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`
39+
40+
d := &Daemon{
41+
repository: "/go/src/github.com/docker/docker/bundles/1.9.0-dev/test-integration-cli/d45526097/graph/containers/",
42+
}
43+
44+
expected := "/go/src/github.com/docker/docker/bundles/1.9.0-dev/test-integration-cli/d45526097/graph/containers/47903e2e67014246eba27607809d5f5c2437c3bf84c2986393448f84093cc40b/mqueue"
45+
var unmounted bool
46+
unmount := func(target string) error {
47+
if target == expected {
48+
unmounted = true
49+
}
50+
return nil
51+
}
52+
53+
d.cleanupMountsFromReader(strings.NewReader(fixture), unmount)
54+
55+
if !unmounted {
56+
t.Fatalf("Expected to unmount the mqueue")
57+
}
58+
}

‎integration-cli/docker_cli_daemon_test.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1474,9 +1474,11 @@ func (s *DockerDaemonSuite) TestCleanupMountsAfterCrash(c *check.C) {
14741474
id := strings.TrimSpace(out)
14751475
c.Assert(s.d.cmd.Process.Signal(os.Kill), check.IsNil)
14761476
c.Assert(s.d.Start(), check.IsNil)
1477-
mountOut, err := exec.Command("mount").CombinedOutput()
1477+
mountOut, err := ioutil.ReadFile("/proc/self/mountinfo")
14781478
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)
14801482
}
14811483

14821484
func (s *DockerDaemonSuite) TestRunContainerWithBridgeNone(c *check.C) {

0 commit comments

Comments
 (0)