diff --git a/container/container.go b/container/container.go index 16cdbf0f1c..92161e708d 100644 --- a/container/container.go +++ b/container/container.go @@ -400,21 +400,20 @@ func (container *Container) AddMountPointWithVolume(destination string, vol volu func (container *Container) UnmountVolumes(volumeEventLog func(name, action string, attributes map[string]string)) error { var errors []string for _, volumeMount := range container.MountPoints { - // Check if the mountpoint has an ID, this is currently the best way to tell if it's actually mounted - // TODO(cpuguyh83): there should be a better way to handle this - if volumeMount.Volume != nil && volumeMount.ID != "" { - if err := volumeMount.Volume.Unmount(volumeMount.ID); err != nil { - errors = append(errors, err.Error()) - continue - } - volumeMount.ID = "" - - attributes := map[string]string{ - "driver": volumeMount.Volume.DriverName(), - "container": container.ID, - } - volumeEventLog(volumeMount.Volume.Name(), "unmount", attributes) + if volumeMount.Volume == nil { + continue } + + if err := volumeMount.Cleanup(); err != nil { + errors = append(errors, err.Error()) + continue + } + + attributes := map[string]string{ + "driver": volumeMount.Volume.DriverName(), + "container": container.ID, + } + volumeEventLog(volumeMount.Volume.Name(), "unmount", attributes) } if len(errors) > 0 { return fmt.Errorf("error while unmounting volumes for container %s: %s", container.ID, strings.Join(errors, "; ")) diff --git a/integration-cli/docker_cli_external_volume_driver_unix_test.go b/integration-cli/docker_cli_external_volume_driver_unix_test.go index d6d1999c23..5fe417c2c8 100644 --- a/integration-cli/docker_cli_external_volume_driver_unix_test.go +++ b/integration-cli/docker_cli_external_volume_driver_unix_test.go @@ -616,3 +616,18 @@ func (s *DockerExternalVolumeSuite) TestExternalVolumeDriverUnmountOnMountFail(c out, _ = s.d.Cmd("run", "-w", "/foo", "-v", "testumount:/foo", "busybox", "true") c.Assert(s.ec.unmounts, checker.Equals, 0, check.Commentf(out)) } + +func (s *DockerExternalVolumeSuite) TestExternalVolumeDriverUnmountOnCp(c *check.C) { + s.d.StartWithBusybox(c) + s.d.Cmd("volume", "create", "-d", "test-external-volume-driver", "--name=test") + + out, _ := s.d.Cmd("run", "-d", "--name=test", "-v", "test:/foo", "busybox", "/bin/sh", "-c", "touch /test && top") + c.Assert(s.ec.mounts, checker.Equals, 1, check.Commentf(out)) + + out, _ = s.d.Cmd("cp", "test:/test", "/tmp/test") + c.Assert(s.ec.mounts, checker.Equals, 2, check.Commentf(out)) + c.Assert(s.ec.unmounts, checker.Equals, 1, check.Commentf(out)) + + out, _ = s.d.Cmd("kill", "test") + c.Assert(s.ec.unmounts, checker.Equals, 2, check.Commentf(out)) +} diff --git a/volume/volume.go b/volume/volume.go index a23d993830..5135605281 100644 --- a/volume/volume.go +++ b/volume/volume.go @@ -120,6 +120,28 @@ type MountPoint struct { // Sepc is a copy of the API request that created this mount. Spec mounttypes.Mount + + // Track usage of this mountpoint + // Specicially needed for containers which are running and calls to `docker cp` + // because both these actions require mounting the volumes. + active int +} + +// Cleanup frees resources used by the mountpoint +func (m *MountPoint) Cleanup() error { + if m.Volume == nil || m.ID == "" { + return nil + } + + if err := m.Volume.Unmount(m.ID); err != nil { + return errors.Wrapf(err, "error unmounting volume %s", m.Volume.Name()) + } + + m.active-- + if m.active == 0 { + m.ID = "" + } + return nil } // Setup sets up a mount point by either mounting the volume if it is @@ -147,12 +169,16 @@ func (m *MountPoint) Setup(mountLabel string, rootUID, rootGID int) (path string if err != nil { return "", errors.Wrapf(err, "error while mounting volume '%s'", m.Source) } + m.ID = id + m.active++ return path, nil } + if len(m.Source) == 0 { return "", fmt.Errorf("Unable to setup mount point, neither source nor volume defined") } + // system.MkdirAll() produces an error if m.Source exists and is a file (not a directory), if m.Type == mounttypes.TypeBind { // idtools.MkdirAllNewAs() produces an error if m.Source exists and is a file (not a directory)