Browse code

Fix volume Create to check against canonical driver name

Previously, it was comparing against the driver name passed in by the
caller. This could lead to subtle issues when using plugins, like
"plugin" vs. "plugin:latest".

Also, remove "conflict:" prefix to improve the error message.

Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>

Aaron Lehmann authored on 2016/12/14 10:20:52
Showing 4 changed files
... ...
@@ -284,6 +284,22 @@ func (s *DockerExternalVolumeSuite) TearDownSuite(c *check.C) {
284 284
 	c.Assert(err, checker.IsNil)
285 285
 }
286 286
 
287
+func (s *DockerExternalVolumeSuite) TestVolumeCLICreateOptionConflict(c *check.C) {
288
+	dockerCmd(c, "volume", "create", "test")
289
+
290
+	out, _, err := dockerCmdWithError("volume", "create", "test", "--driver", volumePluginName)
291
+	c.Assert(err, check.NotNil, check.Commentf("volume create exception name already in use with another driver"))
292
+	c.Assert(out, checker.Contains, "A volume named test already exists")
293
+
294
+	out, _ = dockerCmd(c, "volume", "inspect", "--format={{ .Driver }}", "test")
295
+	_, _, err = dockerCmdWithError("volume", "create", "test", "--driver", strings.TrimSpace(out))
296
+	c.Assert(err, check.IsNil)
297
+
298
+	// make sure hidden --name option conflicts with positional arg name
299
+	out, _, err = dockerCmdWithError("volume", "create", "--name", "test2", "test2")
300
+	c.Assert(err, check.NotNil, check.Commentf("Conflicting options: either specify --name or provide positional arg, not both"))
301
+}
302
+
287 303
 func (s *DockerExternalVolumeSuite) TestExternalVolumeDriverNamed(c *check.C) {
288 304
 	s.d.StartWithBusybox(c)
289 305
 
... ...
@@ -29,21 +29,6 @@ func (s *DockerSuite) TestVolumeCLICreate(c *check.C) {
29 29
 	c.Assert(name, check.Equals, "test2")
30 30
 }
31 31
 
32
-func (s *DockerSuite) TestVolumeCLICreateOptionConflict(c *check.C) {
33
-	dockerCmd(c, "volume", "create", "test")
34
-	out, _, err := dockerCmdWithError("volume", "create", "test", "--driver", "nosuchdriver")
35
-	c.Assert(err, check.NotNil, check.Commentf("volume create exception name already in use with another driver"))
36
-	c.Assert(out, checker.Contains, "A volume named test already exists")
37
-
38
-	out, _ = dockerCmd(c, "volume", "inspect", "--format={{ .Driver }}", "test")
39
-	_, _, err = dockerCmdWithError("volume", "create", "test", "--driver", strings.TrimSpace(out))
40
-	c.Assert(err, check.IsNil)
41
-
42
-	// make sure hidden --name option conflicts with positional arg name
43
-	out, _, err = dockerCmdWithError("volume", "create", "--name", "test2", "test2")
44
-	c.Assert(err, check.NotNil, check.Commentf("Conflicting options: either specify --name or provide positional arg, not both"))
45
-}
46
-
47 32
 func (s *DockerSuite) TestVolumeCLIInspect(c *check.C) {
48 33
 	c.Assert(
49 34
 		exec.Command(dockerBinary, "volume", "inspect", "doesntexist").Run(),
... ...
@@ -14,7 +14,7 @@ var (
14 14
 	// errInvalidName is a typed error returned when creating a volume with a name that is not valid on the platform
15 15
 	errInvalidName = errors.New("volume name is not valid on this platform")
16 16
 	// errNameConflict is a typed error returned on create when a volume exists with the given name, but for a different driver
17
-	errNameConflict = errors.New("conflict: volume name must be unique")
17
+	errNameConflict = errors.New("volume name must be unique")
18 18
 )
19 19
 
20 20
 // OpErr is the error type returned by functions in the store package. It describes
... ...
@@ -309,8 +309,17 @@ func (s *VolumeStore) checkConflict(name, driverName string) (volume.Volume, err
309 309
 
310 310
 	vDriverName := v.DriverName()
311 311
 	var conflict bool
312
-	if driverName != "" && vDriverName != driverName {
313
-		conflict = true
312
+	if driverName != "" {
313
+		// Retrieve canonical driver name to avoid inconsistencies (for example
314
+		// "plugin" vs. "plugin:latest")
315
+		vd, err := volumedrivers.GetDriver(driverName)
316
+		if err != nil {
317
+			return nil, err
318
+		}
319
+
320
+		if vDriverName != vd.Name() {
321
+			conflict = true
322
+		}
314 323
 	}
315 324
 
316 325
 	// let's check if the found volume ref