Browse code

Fix issue related to duplicate identical bind mounts for `docker run`

This fix tries to address the issue raised in 27969 where
duplicate identical bind mounts for `docker run` caused additional volumes
to be created.

The reason was that in `runconfig`, if duplicate identical bind mounts
have been specified, the `copts.volumes.Delete(bind)` will not truly
delete the second entry from the slice. (Only the first entry is deleted).

This fix fixes the issue.

An integration test has been added to cover the changes

This fix fixes 27969.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>

Yong Tang authored on 2016/11/02 10:47:55
Showing 2 changed files
... ...
@@ -4567,3 +4567,24 @@ func (s *DockerSuite) TestRunServicingContainer(c *check.C) {
4567 4567
 	c.Assert(out2, checker.Contains, `Windows Container (Servicing)`, check.Commentf("Didn't find 'Windows Container (Servicing): %s", out2))
4568 4568
 	c.Assert(out2, checker.Contains, containerID+"_servicing", check.Commentf("Didn't find '%s_servicing': %s", containerID+"_servicing", out2))
4569 4569
 }
4570
+
4571
+func (s *DockerSuite) TestRunDuplicateMount(c *check.C) {
4572
+	testRequires(c, DaemonIsLinux)
4573
+
4574
+	tmpFile, err := ioutil.TempFile("", "touch-me")
4575
+	c.Assert(err, checker.IsNil)
4576
+	defer tmpFile.Close()
4577
+
4578
+	data := "touch-me-foo-bar\n"
4579
+	if _, err := tmpFile.Write([]byte(data)); err != nil {
4580
+		c.Fatal(err)
4581
+	}
4582
+
4583
+	name := "test"
4584
+	out, _ := dockerCmd(c, "run", "--name", name, "-v", "/tmp:/tmp", "-v", "/tmp:/tmp", "busybox", "sh", "-c", "cat "+tmpFile.Name()+" && ls /")
4585
+	c.Assert(out, checker.Not(checker.Contains), "tmp:")
4586
+	c.Assert(out, checker.Contains, data)
4587
+
4588
+	out = inspectFieldJSON(c, name, "Config.Volumes")
4589
+	c.Assert(out, checker.Contains, "null")
4590
+}
... ...
@@ -348,13 +348,16 @@ func Parse(flags *pflag.FlagSet, copts *ContainerOptions) (*container.Config, *c
348 348
 	}
349 349
 
350 350
 	var binds []string
351
+	volumes := copts.volumes.GetMap()
351 352
 	// add any bind targets to the list of container volumes
352 353
 	for bind := range copts.volumes.GetMap() {
353 354
 		if arr := volumeSplitN(bind, 2); len(arr) > 1 {
354 355
 			// after creating the bind mount we want to delete it from the copts.volumes values because
355 356
 			// we do not want bind mounts being committed to image configs
356 357
 			binds = append(binds, bind)
357
-			copts.volumes.Delete(bind)
358
+			// We should delete from the map (`volumes`) here, as deleting from copts.volumes will not work if
359
+			// there are duplicates entries.
360
+			delete(volumes, bind)
358 361
 		}
359 362
 	}
360 363
 
... ...
@@ -556,7 +559,7 @@ func Parse(flags *pflag.FlagSet, copts *ContainerOptions) (*container.Config, *c
556 556
 		Env:             envVariables,
557 557
 		Cmd:             runCmd,
558 558
 		Image:           copts.Image,
559
-		Volumes:         copts.volumes.GetMap(),
559
+		Volumes:         volumes,
560 560
 		MacAddress:      copts.macAddress,
561 561
 		Entrypoint:      entrypoint,
562 562
 		WorkingDir:      copts.workingDir,