Merge pull request #19568 from cpuguy83/17907_fix_rmv

On container rm, don't remove named mountpoints

Author: tiborvass

daemon/mounts.go

@@ -25,6 +25,11 @@ func (daemon *Daemon) removeMountPoints(container *container.Container, rm bool)
 		}
 		daemon.volumes.Dereference(m.Volume, container.ID)
 		if rm {
+			// Do not remove named mountpoints
+			// these are mountpoints specified like `docker run -v <name>:/foo`
+			if m.Named {
+				continue
+			}
 			err := daemon.volumes.Remove(m.Volume)
 			// Ignore volume in use errors because having this
 			// volume being referenced by other container is

daemon/volumes.go

@@ -90,6 +90,7 @@ func (daemon *Daemon) registerMountPoints(container *container.Container, hostCo
 				Driver:      m.Driver,
 				Destination: m.Destination,
 				Propagation: m.Propagation,
+				Named:       m.Named,
 			}
 
 			if len(cp.Source) == 0 {
@@ -126,6 +127,7 @@ func (daemon *Daemon) registerMountPoints(container *container.Container, hostCo
 			bind.Source = v.Path()
 			// bind.Name is an already existing volume, we need to use that here
 			bind.Driver = v.DriverName()
+			bind.Named = true
 			bind = setBindModeIfNull(bind)
 		}
 		if label.RelabelNeeded(bind.Mode) {

docs/reference/commandline/rm.md

@@ -45,3 +45,17 @@ This command will delete all stopped containers. The command
 `docker ps -a -q` will return all existing container IDs and pass them to
 the `rm` command which will delete them. Any running containers will not be
 deleted.
+
+  $ docker rm -v redis
+  redis
+
+This command will remove the container and any volumes associated with it.
+Note that if a volume was specified with a name, it will not be removed.
+
+  $ docker create -v awesome:/foo -v /bar --name hello redis
+  hello
+  $ docker rm -v hello
+
+In this example, the volume for `/foo` will remain intact, but the volume for
+`/bar` will be removed. The same behavior holds for volumes inherited with
+`--volumes-from`.

docs/reference/run.md

@@ -590,7 +590,11 @@ the container exits**, you can add the `--rm` flag:
 
 > **Note**: When you set the `--rm` flag, Docker also removes the volumes
 associated with the container when the container is removed. This is similar
-to running `docker rm -v my-container`.
+to running `docker rm -v my-container`. Only volumes that are specified without a
+name are removed. For example, with
+`docker run --rm -v /foo -v awesome:/bar busybox top`, the volume for `/foo` will be removed,
+but the volume for `/bar` will not. Volumes inheritted via `--volumes-from` will be removed
+with the same logic -- if the original volume was specified with a name it will **not** be removed.
 
 ## Security configuration
     --security-opt="label:user:USER"   : Set the label user for the container

integration-cli/docker_cli_run_test.go

@@ -4137,3 +4137,42 @@ func (s *DockerSuite) TestRunNamedVolumeCopyImageData(c *check.C) {
 	out, _ := dockerCmd(c, "run", "-v", "foo:/foo", "busybox", "cat", "/foo/hello")
 	c.Assert(strings.TrimSpace(out), check.Equals, "hello")
 }
+
+func (s *DockerSuite) TestRunNamedVolumeNotRemoved(c *check.C) {
+	prefix := ""
+	if daemonPlatform == "windows" {
+		prefix = "c:"
+	}
+
+	dockerCmd(c, "volume", "create", "--name", "test")
+
+	dockerCmd(c, "run", "--rm", "-v", "test:"+prefix+"/foo", "-v", prefix+"/bar", "busybox", "true")
+	dockerCmd(c, "volume", "inspect", "test")
+	out, _ := dockerCmd(c, "volume", "ls", "-q")
+	c.Assert(strings.TrimSpace(out), checker.Equals, "test")
+
+	dockerCmd(c, "run", "--name=test", "-v", "test:"+prefix+"/foo", "-v", prefix+"/bar", "busybox", "true")
+	dockerCmd(c, "rm", "-fv", "test")
+	dockerCmd(c, "volume", "inspect", "test")
+	out, _ = dockerCmd(c, "volume", "ls", "-q")
+	c.Assert(strings.TrimSpace(out), checker.Equals, "test")
+}
+
+func (s *DockerSuite) TestRunNamedVolumesFromNotRemoved(c *check.C) {
+	prefix := ""
+	if daemonPlatform == "windows" {
+		prefix = "c:"
+	}
+
+	dockerCmd(c, "volume", "create", "--name", "test")
+	dockerCmd(c, "run", "--name=parent", "-v", "test:"+prefix+"/foo", "-v", prefix+"/bar", "busybox", "true")
+	dockerCmd(c, "run", "--name=child", "--volumes-from=parent", "busybox", "true")
+
+	// Remove the parent so there are not other references to the volumes
+	dockerCmd(c, "rm", "-f", "parent")
+	// now remove the child and ensure the named volume (and only the named volume) still exists
+	dockerCmd(c, "rm", "-fv", "child")
+	dockerCmd(c, "volume", "inspect", "test")
+	out, _ := dockerCmd(c, "volume", "ls", "-q")
+	c.Assert(strings.TrimSpace(out), checker.Equals, "test")
+}

integration-cli/docker_cli_start_volume_driver_unix_test.go

@@ -228,6 +228,9 @@ func (s *DockerExternalVolumeSuite) TestExternalVolumeDriverNamed(c *check.C) {
 	c.Assert(err, checker.IsNil, check.Commentf(out))
 	c.Assert(out, checker.Contains, s.server.URL)
 
+	_, err = s.d.Cmd("volume", "rm", "external-volume-test")
+	c.Assert(err, checker.IsNil)
+
 	p := hostVolumePath("external-volume-test")
 	_, err = os.Lstat(p)
 	c.Assert(err, checker.NotNil)
@@ -362,6 +365,9 @@ func (s *DockerExternalVolumeSuite) TestExternalVolumeDriverRetryNotImmediatelyE
 		c.Fatal("volume creates fail when plugin not immediately available")
 	}
 
+	_, err = s.d.Cmd("volume", "rm", "external-volume-test")
+	c.Assert(err, checker.IsNil)
+
 	c.Assert(s.ec.activations, checker.Equals, 1)
 	c.Assert(s.ec.creations, checker.Equals, 1)
 	c.Assert(s.ec.removals, checker.Equals, 1)
@@ -385,7 +391,7 @@ func (s *DockerExternalVolumeSuite) TestExternalVolumeDriverBindExternalVolume(c
 	c.Assert(mounts[0].Driver, checker.Equals, "test-external-volume-driver")
 }
 
-func (s *DockerExternalVolumeSuite) TestStartExternalVolumeDriverList(c *check.C) {
+func (s *DockerExternalVolumeSuite) TesttExternalVolumeDriverList(c *check.C) {
 	dockerCmd(c, "volume", "create", "-d", "test-external-volume-driver", "--name", "abc")
 	out, _ := dockerCmd(c, "volume", "ls")
 	ls := strings.Split(strings.TrimSpace(out), "\n")
@@ -399,7 +405,7 @@ func (s *DockerExternalVolumeSuite) TestStartExternalVolumeDriverList(c *check.C
 	c.Assert(s.ec.lists, check.Equals, 1)
 }
 
-func (s *DockerExternalVolumeSuite) TestStartExternalVolumeDriverGet(c *check.C) {
+func (s *DockerExternalVolumeSuite) TestExternalVolumeDriverGet(c *check.C) {
 	out, _, err := dockerCmdWithError("volume", "inspect", "dummy")
 	c.Assert(err, check.NotNil, check.Commentf(out))
 	c.Assert(s.ec.gets, check.Equals, 1)

man/docker-rm.1.md

@@ -48,6 +48,22 @@ command. The use that name as follows:
 
     docker rm hopeful_morse
 
+## Removing a container and all associated volumes
+
+  $ docker rm -v redis
+  redis
+
+This command will remove the container and any volumes associated with it.
+Note that if a volume was specified with a name, it will not be removed.
+
+  $ docker create -v awesome:/foo -v /bar --name hello redis
+  hello
+  $ docker rm -v hello
+
+In this example, the volume for `/foo` will remain in tact, but the volume for
+`/bar` will be removed. The same behavior holds for volumes inherited with
+`--volumes-from`.
+
 # HISTORY
 April 2014, Originally compiled by William Henry (whenry at redhat dot com)
 based on docker.com source material and internal work.

volume/volume.go

@@ -59,6 +59,7 @@ type MountPoint struct {
 
 	// Note Propagation is not used on Windows
 	Propagation string // Mount propagation string
+	Named       bool   // specifies if the mountpoint was specified by name
 }
 
 // Setup sets up a mount point by either mounting the volume if it is