Browse code

builder: Fix handling of VOLUME command where multiple volumes are specified in a space delimited list.

Docker-DCO-1.1-Signed-off-by: Erik Hollensbe <github@hollensbe.org> (github: erikh)

Erik Hollensbe authored on 2014/09/11 22:27:51
Showing 7 changed files
... ...
@@ -310,23 +310,20 @@ func user(b *Builder, args []string, attributes map[string]bool) error {
310 310
 
311 311
 // VOLUME /foo
312 312
 //
313
-// Expose the volume /foo for use. Will also accept the JSON form, but either
314
-// way requires exactly one argument.
313
+// Expose the volume /foo for use. Will also accept the JSON array form.
315 314
 //
316 315
 func volume(b *Builder, args []string, attributes map[string]bool) error {
317
-	if len(args) != 1 {
316
+	if len(args) == 0 {
318 317
 		return fmt.Errorf("Volume cannot be empty")
319 318
 	}
320 319
 
321
-	volume := args
322
-
323 320
 	if b.Config.Volumes == nil {
324 321
 		b.Config.Volumes = map[string]struct{}{}
325 322
 	}
326
-	for _, v := range volume {
323
+	for _, v := range args {
327 324
 		b.Config.Volumes[v] = struct{}{}
328 325
 	}
329
-	if err := b.commit("", b.Config.Cmd, fmt.Sprintf("VOLUME %s", args)); err != nil {
326
+	if err := b.commit("", b.Config.Cmd, fmt.Sprintf("VOLUME %v", args)); err != nil {
330 327
 		return err
331 328
 	}
332 329
 	return nil
... ...
@@ -135,3 +135,21 @@ func parseMaybeJSON(rest string) (*Node, map[string]bool, error) {
135 135
 	node.Value = rest
136 136
 	return node, nil, nil
137 137
 }
138
+
139
+// parseMaybeJSONToList determines if the argument appears to be a JSON array. If
140
+// so, passes to parseJSON; if not, attmpts to parse it as a whitespace
141
+// delimited string.
142
+func parseMaybeJSONToList(rest string) (*Node, map[string]bool, error) {
143
+	rest = strings.TrimSpace(rest)
144
+
145
+	node, attrs, err := parseJSON(rest)
146
+
147
+	if err == nil {
148
+		return node, attrs, nil
149
+	}
150
+	if err == errDockerfileJSONNesting {
151
+		return nil, nil, err
152
+	}
153
+
154
+	return parseStringsWhitespaceDelimited(rest)
155
+}
... ...
@@ -55,7 +55,7 @@ func init() {
55 55
 		"cmd":            parseMaybeJSON,
56 56
 		"entrypoint":     parseMaybeJSON,
57 57
 		"expose":         parseStringsWhitespaceDelimited,
58
-		"volume":         parseMaybeJSON,
58
+		"volume":         parseMaybeJSONToList,
59 59
 		"insert":         parseIgnore,
60 60
 	}
61 61
 }
62 62
new file mode 100644
... ...
@@ -0,0 +1,3 @@
0
+FROM foo
1
+
2
+VOLUME /opt/nagios/var /opt/nagios/etc /opt/nagios/libexec /var/log/apache2 /usr/share/snmp/mibs
0 3
new file mode 100644
... ...
@@ -0,0 +1,2 @@
0
+(from "foo")
1
+(volume "/opt/nagios/var" "/opt/nagios/etc" "/opt/nagios/libexec" "/var/log/apache2" "/usr/share/snmp/mibs")
... ...
@@ -445,9 +445,10 @@ optional but default, you could use a `CMD` instruction:
445 445
 The `VOLUME` instruction will create a mount point with the specified name
446 446
 and mark it as holding externally mounted volumes from native host or other
447 447
 containers. The value can be a JSON array, `VOLUME ["/var/log/"]`, or a plain
448
-string, `VOLUME /var/log`. For more information/examples and mounting
449
-instructions via the Docker client, refer to [*Share Directories via Volumes*](
450
-/userguide/dockervolumes/#volume-def) documentation.
448
+string with multiple arguments, such as `VOLUME /var/log` or `VOLUME /var/log
449
+/var/db`.  For more information/examples and mounting instructions via the
450
+Docker client, refer to [*Share Directories via Volumes*](/userguide/dockervolumes/#volume-def)
451
+documentation.
451 452
 
452 453
 ## USER
453 454
 
... ...
@@ -582,13 +582,26 @@ func TestBuildWithVolumes(t *testing.T) {
582 582
 		result   map[string]map[string]struct{}
583 583
 		name     = "testbuildvolumes"
584 584
 		emptyMap = make(map[string]struct{})
585
-		expected = map[string]map[string]struct{}{"/test1": emptyMap, "/test2": emptyMap}
585
+		expected = map[string]map[string]struct{}{
586
+			"/test1":  emptyMap,
587
+			"/test2":  emptyMap,
588
+			"/test3":  emptyMap,
589
+			"/test4":  emptyMap,
590
+			"/test5":  emptyMap,
591
+			"/test6":  emptyMap,
592
+			"[/test7": emptyMap,
593
+			"/test8]": emptyMap,
594
+		}
586 595
 	)
587 596
 	defer deleteImages(name)
588 597
 	_, err := buildImage(name,
589 598
 		`FROM scratch
590 599
 		VOLUME /test1
591
-		VOLUME /test2`,
600
+		VOLUME /test2
601
+    VOLUME /test3 /test4
602
+    VOLUME ["/test5", "/test6"]
603
+    VOLUME [/test7 /test8]
604
+    `,
592 605
 		true)
593 606
 	if err != nil {
594 607
 		t.Fatal(err)