1. docker
  2. Commit 1861abdc4a31efad202a5c3d89a895bb7a62799a
Browse code

Fix "duplicate mount point" when --tmpfs /dev/shm is used

This is a fix to the following issue:

$ docker run --tmpfs /dev/shm busybox sh
docker: Error response from daemon: linux mounts: Duplicate mount point '/dev/shm'.

In current code (daemon.createSpec()), tmpfs mount from --tmpfs is added
to list of mounts (`ms`), when the mount from IpcMounts() is added.
While IpcMounts() is checking for existing mounts first, it does that
by using container.HasMountFor() function which only checks container.Mounts
but not container.Tmpfs.

Ultimately, the solution is to get rid of container.Tmpfs (moving its
data to container.Mounts). Current workaround is to add checking
of container.Tmpfs into container.HasMountFor().

A unit test case is included.

Unfortunately we can't call daemon.createSpec() from a unit test,
as the code relies a lot on various daemon structures to be initialized
properly, and it is hard to achieve. Therefore, we minimally mimick
the code flow of daemon.createSpec() -- barely enough to reproduce
the issue.

https://github.com/moby/moby/issues/35455

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>

Kir Kolyshkin authored on 2017/11/11 08:43:57
Showing 2 changed files
container/container_unix.go
History View file @ 1861abd
... ... 
@@ -157,7 +157,18 @@ func (container *Container) ShmResourcePath() (string, error) {
157 157 
 // HasMountFor checks if path is a mountpoint
158 158 
 func (container *Container) HasMountFor(path string) bool {
159 159 
 	_, exists := container.MountPoints[path]
160 
-	return exists
160 
+	if exists {
161 
+		return true
162 
+	}
163 
+
164 
+	// Also search among the tmpfs mounts
165 
+	for dest := range container.HostConfig.Tmpfs {
166 
+		if dest == path {
167 
+			return true
168 
+		}
169 
+	}
170 
+
171 
+	return false
161 172 
 }
162 173
163 174 
 // UnmountIpcMount uses the provided unmount function to unmount shm if it was mounted
daemon/oci_linux_test.go
History View file @ 1861abd
164 175 
new file mode 100644
... ... 
@@ -0,0 +1,50 @@
0 
+package daemon
1 
+
2 
+import (
3 
+	"testing"
4 
+
5 
+	containertypes "github.com/docker/docker/api/types/container"
6 
+	"github.com/docker/docker/container"
7 
+	"github.com/docker/docker/daemon/config"
8 
+	"github.com/docker/docker/oci"
9 
+	"github.com/docker/docker/pkg/idtools"
10 
+
11 
+	"github.com/stretchr/testify/assert"
12 
+)
13 
+
14 
+// TestTmpfsDevShmNoDupMount checks that a user-specified /dev/shm tmpfs
15 
+// mount (as in "docker run --tmpfs /dev/shm:rw,size=NNN") does not result
16 
+// in "Duplicate mount point" error from the engine.
17 
+// https://github.com/moby/moby/issues/35455
18 
+func TestTmpfsDevShmNoDupMount(t *testing.T) {
19 
+	d := Daemon{
20 
+		// some empty structs to avoid getting a panic
21 
+		// caused by a null pointer dereference
22 
+		idMappings:  &idtools.IDMappings{},
23 
+		configStore: &config.Config{},
24 
+	}
25 
+	c := &container.Container{
26 
+		ShmPath: "foobar", // non-empty, for c.IpcMounts() to work
27 
+		HostConfig: &containertypes.HostConfig{
28 
+			IpcMode: containertypes.IpcMode("shareable"), // default mode
29 
+			// --tmpfs /dev/shm:rw,exec,size=NNN
30 
+			Tmpfs: map[string]string{
31 
+				"/dev/shm": "rw,exec,size=1g",
32 
+			},
33 
+		},
34 
+	}
35 
+
36 
+	// Mimick the code flow of daemon.createSpec(), enough to reproduce the issue
37 
+	ms, err := d.setupMounts(c)
38 
+	assert.NoError(t, err)
39 
+
40 
+	ms = append(ms, c.IpcMounts()...)
41 
+
42 
+	tmpfsMounts, err := c.TmpfsMounts()
43 
+	assert.NoError(t, err)
44 
+	ms = append(ms, tmpfsMounts...)
45 
+
46 
+	s := oci.DefaultSpec()
47 
+	err = setMounts(&d, &s, c, ms)
48 
+	assert.NoError(t, err)
49 
+}