Browse code

Cleanup: applyVolumesFrom

Docker-DCO-1.1-Signed-off-by: Brian Goff <cpuguy83@gmail.com> (github: cpuguy83)

Brian Goff authored on 2014/08/22 06:57:46
Showing 3 changed files
... ...
@@ -1145,3 +1145,47 @@ func (container *Container) getNetworkedContainer() (*Container, error) {
1145 1145
 		return nil, fmt.Errorf("network mode not set to container")
1146 1146
 	}
1147 1147
 }
1148
+
1149
+func (container *Container) GetVolumes() (map[string]*Volume, error) {
1150
+	// Get all the bind-mounts
1151
+	volumes, err := container.getBindMap()
1152
+	if err != nil {
1153
+		return nil, err
1154
+	}
1155
+
1156
+	// Get all the normal volumes
1157
+	for volPath, hostPath := range container.Volumes {
1158
+		if _, exists := volumes[volPath]; exists {
1159
+			continue
1160
+		}
1161
+		volumes[volPath] = &Volume{VolPath: volPath, HostPath: hostPath, isReadWrite: container.VolumesRW[volPath]}
1162
+	}
1163
+
1164
+	return volumes, nil
1165
+}
1166
+
1167
+func (container *Container) getBindMap() (map[string]*Volume, error) {
1168
+	var (
1169
+		// Create the requested bind mounts
1170
+		volumes = map[string]*Volume{}
1171
+		// Define illegal container destinations
1172
+		illegalDsts = []string{"/", "."}
1173
+	)
1174
+
1175
+	for _, bind := range container.hostConfig.Binds {
1176
+		vol, err := parseBindVolumeSpec(bind)
1177
+		if err != nil {
1178
+			return nil, err
1179
+		}
1180
+		vol.isBindMount = true
1181
+		// Bail if trying to mount to an illegal destination
1182
+		for _, illegal := range illegalDsts {
1183
+			if vol.VolPath == illegal {
1184
+				return nil, fmt.Errorf("Illegal bind destination: %s", vol.VolPath)
1185
+			}
1186
+		}
1187
+
1188
+		volumes[filepath.Clean(vol.VolPath)] = &vol
1189
+	}
1190
+	return volumes, nil
1191
+}
... ...
@@ -16,14 +16,10 @@ import (
16 16
 type Volume struct {
17 17
 	HostPath    string
18 18
 	VolPath     string
19
-	Mode        string
19
+	isReadWrite bool
20 20
 	isBindMount bool
21 21
 }
22 22
 
23
-func (v *Volume) isRw() bool {
24
-	return v.Mode == "" || strings.ToLower(v.Mode) == "rw"
25
-}
26
-
27 23
 func (v *Volume) isDir() (bool, error) {
28 24
 	stat, err := os.Stat(v.HostPath)
29 25
 	if err != nil {
... ...
@@ -42,10 +38,7 @@ func prepareVolumesForContainer(container *Container) error {
42 42
 		}
43 43
 	}
44 44
 
45
-	if err := createVolumes(container); err != nil {
46
-		return err
47
-	}
48
-	return nil
45
+	return createVolumes(container)
49 46
 }
50 47
 
51 48
 func setupMountsForContainer(container *Container) error {
... ...
@@ -74,87 +67,82 @@ func setupMountsForContainer(container *Container) error {
74 74
 	return nil
75 75
 }
76 76
 
77
+func parseVolumesFromSpec(container *Container, spec string) (map[string]*Volume, error) {
78
+	specParts := strings.SplitN(spec, ":", 2)
79
+	if len(specParts) == 0 {
80
+		return nil, fmt.Errorf("Malformed volumes-from specification: %s", spec)
81
+	}
82
+
83
+	c := container.daemon.Get(specParts[0])
84
+	if c == nil {
85
+		return nil, fmt.Errorf("Container %s not found. Impossible to mount its volumes", specParts[0])
86
+	}
87
+
88
+	volumes, err := c.GetVolumes()
89
+	if err != nil {
90
+		return nil, err
91
+	}
92
+
93
+	if len(specParts) == 2 {
94
+		mode := specParts[1]
95
+		if validVolumeMode(mode) {
96
+			return nil, fmt.Errorf("Invalid mode for volumes-from: %s", mode)
97
+		}
98
+
99
+		// Set the mode for the inheritted volume
100
+		for _, v := range volumes {
101
+			v.isReadWrite = mode != "ro"
102
+		}
103
+	}
104
+
105
+	return volumes, nil
106
+}
107
+
77 108
 func applyVolumesFrom(container *Container) error {
78 109
 	volumesFrom := container.hostConfig.VolumesFrom
79
-	if len(volumesFrom) > 0 {
80
-		for _, containerSpec := range volumesFrom {
81
-			var (
82
-				mountRW   = true
83
-				specParts = strings.SplitN(containerSpec, ":", 2)
84
-			)
85
-
86
-			switch len(specParts) {
87
-			case 0:
88
-				return fmt.Errorf("Malformed volumes-from specification: %s", containerSpec)
89
-			case 2:
90
-				switch specParts[1] {
91
-				case "ro":
92
-					mountRW = false
93
-				case "rw": // mountRW is already true
94
-				default:
95
-					return fmt.Errorf("Malformed volumes-from specification: %s", containerSpec)
96
-				}
97
-			}
98 110
 
99
-			c := container.daemon.Get(specParts[0])
100
-			if c == nil {
101
-				return fmt.Errorf("Container %s not found. Impossible to mount its volumes", specParts[0])
102
-			}
111
+	for _, spec := range volumesFrom {
112
+		volumes, err := parseVolumesFromSpec(container, spec)
113
+		if err != nil {
114
+			return err
115
+		}
103 116
 
104
-			if err := c.Mount(); err != nil {
105
-				return fmt.Errorf("Container %s failed to mount. Impossible to mount its volumes", specParts[0])
106
-			}
107
-			defer c.Unmount()
108
-
109
-			for volPath, id := range c.Volumes {
110
-				if _, exists := container.Volumes[volPath]; exists {
111
-					continue
112
-				}
113
-
114
-				pth, err := c.getResourcePath(volPath)
115
-				if err != nil {
116
-					return err
117
-				}
118
-
119
-				stat, err := os.Stat(pth)
120
-				if err != nil {
121
-					return err
122
-				}
123
-
124
-				if err := createIfNotExists(pth, stat.IsDir()); err != nil {
125
-					return err
126
-				}
127
-
128
-				container.Volumes[volPath] = id
129
-				if isRW, exists := c.VolumesRW[volPath]; exists {
130
-					container.VolumesRW[volPath] = isRW && mountRW
131
-				}
117
+		for _, v := range volumes {
118
+			if err = v.initialize(container); err != nil {
119
+				return err
132 120
 			}
133
-
134 121
 		}
135 122
 	}
136 123
 	return nil
137 124
 }
138 125
 
126
+func validVolumeMode(mode string) bool {
127
+	validModes := map[string]bool{
128
+		"rw": true,
129
+		"ro": true,
130
+	}
131
+
132
+	return validModes[mode]
133
+}
134
+
139 135
 func parseBindVolumeSpec(spec string) (Volume, error) {
140 136
 	var (
141 137
 		arr = strings.Split(spec, ":")
142 138
 		vol Volume
143 139
 	)
144 140
 
145
-	vol.isBindMount = true
146 141
 	switch len(arr) {
147 142
 	case 1:
148 143
 		vol.VolPath = spec
149
-		vol.Mode = "rw"
144
+		vol.isReadWrite = true
150 145
 	case 2:
151 146
 		vol.HostPath = arr[0]
152 147
 		vol.VolPath = arr[1]
153
-		vol.Mode = "rw"
148
+		vol.isReadWrite = true
154 149
 	case 3:
155 150
 		vol.HostPath = arr[0]
156 151
 		vol.VolPath = arr[1]
157
-		vol.Mode = arr[2]
152
+		vol.isReadWrite = validVolumeMode(arr[2]) && arr[2] == "rw"
158 153
 	default:
159 154
 		return vol, fmt.Errorf("Invalid volume specification: %s", spec)
160 155
 	}
... ...
@@ -166,34 +154,9 @@ func parseBindVolumeSpec(spec string) (Volume, error) {
166 166
 	return vol, nil
167 167
 }
168 168
 
169
-func getBindMap(container *Container) (map[string]Volume, error) {
170
-	var (
171
-		// Create the requested bind mounts
172
-		volumes = map[string]Volume{}
173
-		// Define illegal container destinations
174
-		illegalDsts = []string{"/", "."}
175
-	)
176
-
177
-	for _, bind := range container.hostConfig.Binds {
178
-		vol, err := parseBindVolumeSpec(bind)
179
-		if err != nil {
180
-			return volumes, err
181
-		}
182
-		// Bail if trying to mount to an illegal destination
183
-		for _, illegal := range illegalDsts {
184
-			if vol.VolPath == illegal {
185
-				return nil, fmt.Errorf("Illegal bind destination: %s", vol.VolPath)
186
-			}
187
-		}
188
-
189
-		volumes[filepath.Clean(vol.VolPath)] = vol
190
-	}
191
-	return volumes, nil
192
-}
193
-
194 169
 func createVolumes(container *Container) error {
195 170
 	// Get all the bindmounts
196
-	volumes, err := getBindMap(container)
171
+	volumes, err := container.GetVolumes()
197 172
 	if err != nil {
198 173
 		return err
199 174
 	}
... ...
@@ -202,9 +165,9 @@ func createVolumes(container *Container) error {
202 202
 	for volPath := range container.Config.Volumes {
203 203
 		// Make sure the the volume isn't already specified as a bindmount
204 204
 		if _, exists := volumes[volPath]; !exists {
205
-			volumes[volPath] = Volume{
205
+			volumes[volPath] = &Volume{
206 206
 				VolPath:     volPath,
207
-				Mode:        "rw",
207
+				isReadWrite: true,
208 208
 				isBindMount: false,
209 209
 			}
210 210
 		}
... ...
@@ -215,8 +178,8 @@ func createVolumes(container *Container) error {
215 215
 			return err
216 216
 		}
217 217
 	}
218
-	return nil
219 218
 
219
+	return nil
220 220
 }
221 221
 
222 222
 func createVolumeHostPath(container *Container) (string, error) {
... ...
@@ -247,7 +210,7 @@ func (v *Volume) initialize(container *Container) error {
247 247
 	}
248 248
 
249 249
 	// If it's not a bindmount we need to create the dir on the host
250
-	if !v.isBindMount {
250
+	if !v.isBindMount && v.HostPath == "" {
251 251
 		v.HostPath, err = createVolumeHostPath(container)
252 252
 		if err != nil {
253 253
 			return err
... ...
@@ -269,7 +232,7 @@ func (v *Volume) initialize(container *Container) error {
269 269
 	}
270 270
 
271 271
 	container.Volumes[v.VolPath] = hostPath
272
-	container.VolumesRW[v.VolPath] = v.isRw()
272
+	container.VolumesRW[v.VolPath] = v.isReadWrite
273 273
 
274 274
 	volIsDir, err := v.isDir()
275 275
 	if err != nil {
... ...
@@ -280,7 +243,7 @@ func (v *Volume) initialize(container *Container) error {
280 280
 	}
281 281
 
282 282
 	// Do not copy or change permissions if we are mounting from the host
283
-	if v.isRw() && !v.isBindMount {
283
+	if v.isReadWrite && !v.isBindMount {
284 284
 		return copyExistingContents(fullVolPath, hostPath)
285 285
 	}
286 286
 	return nil
... ...
@@ -374,6 +374,23 @@ func TestVolumesFromInReadWriteMode(t *testing.T) {
374 374
 	logDone("run - volumes from as read write mount")
375 375
 }
376 376
 
377
+func TestVolumesFromInheritsReadOnly(t *testing.T) {
378
+	cmd := exec.Command(dockerBinary, "run", "--name", "parent", "-v", "/test:/test:ro", "busybox", "true")
379
+	if _, err := runCommand(cmd); err != nil {
380
+		t.Fatal(err)
381
+	}
382
+
383
+	// Expect this "rw" mode to be be ignored since the inheritted volume is "ro"
384
+	cmd = exec.Command(dockerBinary, "run", "--volumes-from", "parent:rw", "busybox", "touch", "/test/file")
385
+	if _, err := runCommand(cmd); err == nil {
386
+		t.Fatal("Expected to inherit read-only volume even when passing in `rw`")
387
+	}
388
+
389
+	deleteAllContainers()
390
+
391
+	logDone("run - volumes from ignores `rw` if inherrited volume is `ro`")
392
+}
393
+
377 394
 // Test for #1351
378 395
 func TestApplyVolumesFromBeforeVolumes(t *testing.T) {
379 396
 	cmd := exec.Command(dockerBinary, "run", "--name", "parent", "-v", "/test", "busybox", "touch", "/test/foo")
... ...
@@ -1181,7 +1198,7 @@ func TestDockerRunWithVolumesIsRecursive(t *testing.T) {
1181 1181
 
1182 1182
 	deleteAllContainers()
1183 1183
 
1184
-	logDone("run - volumes are bind mounted recuursively")
1184
+	logDone("run - volumes are bind mounted recursively")
1185 1185
 }
1186 1186
 
1187 1187
 func TestDnsDefaultOptions(t *testing.T) {