From 90ce6de50b3e5e39f8417e9804970c0fd1377062 Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Tue, 1 Nov 2016 18:47:55 -0700 Subject: [PATCH] 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 --- integration-cli/docker_cli_run_test.go | 21 +++++++++++++++++++++ runconfig/opts/parse.go | 7 +++++-- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/integration-cli/docker_cli_run_test.go b/integration-cli/docker_cli_run_test.go index d1aaeda58a..4ba33a5324 100644 --- a/integration-cli/docker_cli_run_test.go +++ b/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") +} diff --git a/runconfig/opts/parse.go b/runconfig/opts/parse.go index e5b8cb9bb3..8233bae287 100644 --- a/runconfig/opts/parse.go +++ b/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,