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>
Upstream-commit: 90ce6de50b3e5e39f8417e9804970c0fd1377062
Component: engine

components/engine/integration-cli/docker_cli_run_test.go

@@ -4567,3 +4567,24 @@ func (s *DockerSuite) TestRunServicingContainer(c *check.C) {
 	c.Assert(out2, checker.Contains, `Windows Container (Servicing)`, check.Commentf("Didn't find 'Windows Container (Servicing): %s", out2))
 	c.Assert(out2, checker.Contains, containerID+"_servicing", check.Commentf("Didn't find '%s_servicing': %s", containerID+"_servicing", out2))
 }
+
+func (s *DockerSuite) TestRunDuplicateMount(c *check.C) {
+	testRequires(c, DaemonIsLinux)
+
+	tmpFile, err := ioutil.TempFile("", "touch-me")
+	c.Assert(err, checker.IsNil)
+	defer tmpFile.Close()
+
+	data := "touch-me-foo-bar\n"
+	if _, err := tmpFile.Write([]byte(data)); err != nil {
+		c.Fatal(err)
+	}
+
+	name := "test"
+	out, _ := dockerCmd(c, "run", "--name", name, "-v", "/tmp:/tmp", "-v", "/tmp:/tmp", "busybox", "sh", "-c", "cat "+tmpFile.Name()+" && ls /")
+	c.Assert(out, checker.Not(checker.Contains), "tmp:")
+	c.Assert(out, checker.Contains, data)
+
+	out = inspectFieldJSON(c, name, "Config.Volumes")
+	c.Assert(out, checker.Contains, "null")
+}

components/engine/runconfig/opts/parse.go

@@ -348,13 +348,16 @@ func Parse(flags *pflag.FlagSet, copts *ContainerOptions) (*container.Config, *c
 	}
 
 	var binds []string
+	volumes := copts.volumes.GetMap()
 	// add any bind targets to the list of container volumes
 	for bind := range copts.volumes.GetMap() {
 		if arr := volumeSplitN(bind, 2); len(arr) > 1 {
 			// after creating the bind mount we want to delete it from the copts.volumes values because
 			// we do not want bind mounts being committed to image configs
 			binds = append(binds, bind)
-			copts.volumes.Delete(bind)
+			// We should delete from the map (`volumes`) here, as deleting from copts.volumes will not work if
+			// there are duplicates entries.
+			delete(volumes, bind)
 		}
 	}
 
@@ -556,7 +559,7 @@ func Parse(flags *pflag.FlagSet, copts *ContainerOptions) (*container.Config, *c
 		Env:             envVariables,
 		Cmd:             runCmd,
 		Image:           copts.Image,
-		Volumes:         copts.volumes.GetMap(),
+		Volumes:         volumes,
 		MacAddress:      copts.macAddress,
 		Entrypoint:      entrypoint,
 		WorkingDir:      copts.workingDir,