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>
(cherry picked from commit a854cf262336e5625ec06e8e12e8ebc1500ce656)

Aaron Lehmann authored on 2016/12/14 10:20:52
Showing 4 changed files
... ...
@@ -279,6 +279,22 @@ func (s *DockerExternalVolumeSuite) TearDownSuite(c *check.C) {
279 279
 	c.Assert(err, checker.IsNil)
280 280
 }
281 281
 
282
+func (s *DockerExternalVolumeSuite) TestVolumeCLICreateOptionConflict(c *check.C) {
283
+	dockerCmd(c, "volume", "create", "test")
284
+
285
+	out, _, err := dockerCmdWithError("volume", "create", "test", "--driver", volumePluginName)
286
+	c.Assert(err, check.NotNil, check.Commentf("volume create exception name already in use with another driver"))
287
+	c.Assert(out, checker.Contains, "A volume named test already exists")
288
+
289
+	out, _ = dockerCmd(c, "volume", "inspect", "--format={{ .Driver }}", "test")
290
+	_, _, err = dockerCmdWithError("volume", "create", "test", "--driver", strings.TrimSpace(out))
291
+	c.Assert(err, check.IsNil)
292
+
293
+	// make sure hidden --name option conflicts with positional arg name
294
+	out, _, err = dockerCmdWithError("volume", "create", "--name", "test2", "test2")
295
+	c.Assert(err, check.NotNil, check.Commentf("Conflicting options: either specify --name or provide positional arg, not both"))
296
+}
297
+
282 298
 func (s *DockerExternalVolumeSuite) TestExternalVolumeDriverNamed(c *check.C) {
283 299
 	err := s.d.StartWithBusybox()
284 300
 	c.Assert(err, checker.IsNil)
... ...
@@ -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
... ...
@@ -290,8 +290,17 @@ func (s *VolumeStore) checkConflict(name, driverName string) (volume.Volume, err
290 290
 
291 291
 	vDriverName := v.DriverName()
292 292
 	var conflict bool
293
-	if driverName != "" && vDriverName != driverName {
294
-		conflict = true
293
+	if driverName != "" {
294
+		// Retrieve canonical driver name to avoid inconsistencies (for example
295
+		// "plugin" vs. "plugin:latest")
296
+		vd, err := volumedrivers.GetDriver(driverName)
297
+		if err != nil {
298
+			return nil, err
299
+		}
300
+
301
+		if vDriverName != vd.Name() {
302
+			conflict = true
303
+		}
295 304
 	}
296 305
 
297 306
 	// let's check if the found volume ref