Browse code

Merge pull request #35316 from kolyshkin/facepalm

Fix honoring tmpfs-size for user /dev/shm mount

Vincent Demeester authored on 2017/11/14 19:13:59
Showing 2 changed files
... ...
@@ -5,6 +5,13 @@ package daemon
5 5
 import (
6 6
 	"strings"
7 7
 	"testing"
8
+
9
+	containertypes "github.com/docker/docker/api/types/container"
10
+	"github.com/docker/docker/container"
11
+	"github.com/docker/docker/oci"
12
+	"github.com/docker/docker/pkg/idtools"
13
+
14
+	"github.com/stretchr/testify/assert"
8 15
 )
9 16
 
10 17
 const mountsFixture = `142 78 0:38 / / rw,relatime - aufs none rw,si=573b861da0b3a05b,dio
... ...
@@ -102,3 +109,51 @@ func TestNotCleanupMounts(t *testing.T) {
102 102
 		t.Fatal("Expected not to clean up /dev/shm")
103 103
 	}
104 104
 }
105
+
106
+// TestTmpfsDevShmSizeOverride checks that user-specified /dev/tmpfs mount
107
+// size is not overriden by the default shmsize (that should only be used
108
+// for default /dev/shm (as in "shareable" and "private" ipc modes).
109
+// https://github.com/moby/moby/issues/35271
110
+func TestTmpfsDevShmSizeOverride(t *testing.T) {
111
+	size := "777m"
112
+	mnt := "/dev/shm"
113
+
114
+	d := Daemon{
115
+		idMappings: &idtools.IDMappings{},
116
+	}
117
+	c := &container.Container{
118
+		HostConfig: &containertypes.HostConfig{
119
+			ShmSize: 48 * 1024, // size we should NOT end up with
120
+		},
121
+	}
122
+	ms := []container.Mount{
123
+		{
124
+			Source:      "tmpfs",
125
+			Destination: mnt,
126
+			Data:        "size=" + size,
127
+		},
128
+	}
129
+
130
+	// convert ms to spec
131
+	spec := oci.DefaultSpec()
132
+	err := setMounts(&d, &spec, c, ms)
133
+	assert.NoError(t, err)
134
+
135
+	// Check the resulting spec for the correct size
136
+	found := false
137
+	for _, m := range spec.Mounts {
138
+		if m.Destination == mnt {
139
+			for _, o := range m.Options {
140
+				if !strings.HasPrefix(o, "size=") {
141
+					continue
142
+				}
143
+				t.Logf("%+v\n", m.Options)
144
+				assert.Equal(t, "size="+size, o)
145
+				found = true
146
+			}
147
+		}
148
+	}
149
+	if !found {
150
+		t.Fatal("/dev/shm not found in spec, or size option missing")
151
+	}
152
+}
... ...
@@ -538,23 +538,35 @@ func setMounts(daemon *Daemon, s *specs.Spec, c *container.Container, mounts []c
538 538
 		userMounts[m.Destination] = struct{}{}
539 539
 	}
540 540
 
541
-	// Filter out mounts from spec
542
-	noIpc := c.HostConfig.IpcMode.IsNone()
543
-	// Filter out mounts that are overridden by user supplied mounts
541
+	// Copy all mounts from spec to defaultMounts, except for
542
+	//  - mounts overriden by a user supplied mount;
543
+	//  - all mounts under /dev if a user supplied /dev is present;
544
+	//  - /dev/shm, in case IpcMode is none.
545
+	// While at it, also
546
+	//  - set size for /dev/shm from shmsize.
544 547
 	var defaultMounts []specs.Mount
545 548
 	_, mountDev := userMounts["/dev"]
546 549
 	for _, m := range s.Mounts {
547
-		// filter out /dev/shm mount if case IpcMode is none
548
-		if noIpc && m.Destination == "/dev/shm" {
550
+		if _, ok := userMounts[m.Destination]; ok {
551
+			// filter out mount overridden by a user supplied mount
549 552
 			continue
550 553
 		}
551
-		// filter out mount overridden by a user supplied mount
552
-		if _, ok := userMounts[m.Destination]; !ok {
553
-			if mountDev && strings.HasPrefix(m.Destination, "/dev/") {
554
+		if mountDev && strings.HasPrefix(m.Destination, "/dev/") {
555
+			// filter out everything under /dev if /dev is user-mounted
556
+			continue
557
+		}
558
+
559
+		if m.Destination == "/dev/shm" {
560
+			if c.HostConfig.IpcMode.IsNone() {
561
+				// filter out /dev/shm for "none" IpcMode
554 562
 				continue
555 563
 			}
556
-			defaultMounts = append(defaultMounts, m)
564
+			// set size for /dev/shm mount from spec
565
+			sizeOpt := "size=" + strconv.FormatInt(c.HostConfig.ShmSize, 10)
566
+			m.Options = append(m.Options, sizeOpt)
557 567
 		}
568
+
569
+		defaultMounts = append(defaultMounts, m)
558 570
 	}
559 571
 
560 572
 	s.Mounts = defaultMounts
... ...
@@ -662,14 +674,6 @@ func setMounts(daemon *Daemon, s *specs.Spec, c *container.Container, mounts []c
662 662
 		s.Linux.MaskedPaths = nil
663 663
 	}
664 664
 
665
-	// Set size for /dev/shm mount that comes from spec (IpcMode: private only)
666
-	for i, m := range s.Mounts {
667
-		if m.Destination == "/dev/shm" {
668
-			sizeOpt := "size=" + strconv.FormatInt(c.HostConfig.ShmSize, 10)
669
-			s.Mounts[i].Options = append(s.Mounts[i].Options, sizeOpt)
670
-		}
671
-	}
672
-
673 665
 	// TODO: until a kernel/mount solution exists for handling remount in a user namespace,
674 666
 	// we must clear the readonly flag for the cgroups mount (@mrunalp concurs)
675 667
 	if uidMap := daemon.idMappings.UIDs(); uidMap != nil || c.HostConfig.Privileged {