Merge pull request #31450 from thaJeztah/fix-volume-force-remove
do not ignore "volume in use" errors when force-delete
This commit is contained in:
commit
95cb74818a
|
@ -8,11 +8,12 @@ import (
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
"github.com/Sirupsen/logrus"
|
"github.com/Sirupsen/logrus"
|
||||||
"github.com/docker/docker/api/errors"
|
apierrors "github.com/docker/docker/api/errors"
|
||||||
"github.com/docker/docker/api/types"
|
"github.com/docker/docker/api/types"
|
||||||
"github.com/docker/docker/container"
|
"github.com/docker/docker/container"
|
||||||
"github.com/docker/docker/layer"
|
"github.com/docker/docker/layer"
|
||||||
volumestore "github.com/docker/docker/volume/store"
|
volumestore "github.com/docker/docker/volume/store"
|
||||||
|
"github.com/pkg/errors"
|
||||||
)
|
)
|
||||||
|
|
||||||
// ContainerRm removes the container id from the filesystem. An error
|
// ContainerRm removes the container id from the filesystem. An error
|
||||||
|
@ -29,7 +30,7 @@ func (daemon *Daemon) ContainerRm(name string, config *types.ContainerRmConfig)
|
||||||
// Container state RemovalInProgress should be used to avoid races.
|
// Container state RemovalInProgress should be used to avoid races.
|
||||||
if inProgress := container.SetRemovalInProgress(); inProgress {
|
if inProgress := container.SetRemovalInProgress(); inProgress {
|
||||||
err := fmt.Errorf("removal of container %s is already in progress", name)
|
err := fmt.Errorf("removal of container %s is already in progress", name)
|
||||||
return errors.NewBadRequestError(err)
|
return apierrors.NewBadRequestError(err)
|
||||||
}
|
}
|
||||||
defer container.ResetRemovalInProgress()
|
defer container.ResetRemovalInProgress()
|
||||||
|
|
||||||
|
@ -80,7 +81,7 @@ func (daemon *Daemon) cleanupContainer(container *container.Container, forceRemo
|
||||||
if container.IsRunning() {
|
if container.IsRunning() {
|
||||||
if !forceRemove {
|
if !forceRemove {
|
||||||
err := fmt.Errorf("You cannot remove a running container %s. Stop the container before attempting removal or use -f", container.ID)
|
err := fmt.Errorf("You cannot remove a running container %s. Stop the container before attempting removal or use -f", container.ID)
|
||||||
return errors.NewRequestConflictError(err)
|
return apierrors.NewRequestConflictError(err)
|
||||||
}
|
}
|
||||||
if err := daemon.Kill(container); err != nil {
|
if err := daemon.Kill(container); err != nil {
|
||||||
return fmt.Errorf("Could not kill running container %s, cannot remove - %v", container.ID, err)
|
return fmt.Errorf("Could not kill running container %s, cannot remove - %v", container.ID, err)
|
||||||
|
@ -143,6 +144,9 @@ func (daemon *Daemon) cleanupContainer(container *container.Container, forceRemo
|
||||||
// This is called directly from the Engine API
|
// This is called directly from the Engine API
|
||||||
func (daemon *Daemon) VolumeRm(name string, force bool) error {
|
func (daemon *Daemon) VolumeRm(name string, force bool) error {
|
||||||
err := daemon.volumeRm(name)
|
err := daemon.volumeRm(name)
|
||||||
|
if err != nil && volumestore.IsInUse(err) {
|
||||||
|
return apierrors.NewRequestConflictError(err)
|
||||||
|
}
|
||||||
if err == nil || force {
|
if err == nil || force {
|
||||||
daemon.volumes.Purge(name)
|
daemon.volumes.Purge(name)
|
||||||
return nil
|
return nil
|
||||||
|
@ -157,11 +161,7 @@ func (daemon *Daemon) volumeRm(name string) error {
|
||||||
}
|
}
|
||||||
|
|
||||||
if err := daemon.volumes.Remove(v); err != nil {
|
if err := daemon.volumes.Remove(v); err != nil {
|
||||||
if volumestore.IsInUse(err) {
|
return errors.Wrap(err, "unable to remove volume")
|
||||||
err := fmt.Errorf("Unable to remove volume, volume still in use: %v", err)
|
|
||||||
return errors.NewRequestConflictError(err)
|
|
||||||
}
|
|
||||||
return fmt.Errorf("Error while removing volume %s: %v", name, err)
|
|
||||||
}
|
}
|
||||||
daemon.LogVolumeEvent(v.Name(), "destroy", map[string]string{"driver": v.DriverName()})
|
daemon.LogVolumeEvent(v.Name(), "destroy", map[string]string{"driver": v.DriverName()})
|
||||||
return nil
|
return nil
|
||||||
|
|
|
@ -423,14 +423,54 @@ func (s *DockerSuite) TestVolumeCLIRmForce(c *check.C) {
|
||||||
path := strings.TrimSuffix(strings.TrimSpace(out), "/_data")
|
path := strings.TrimSuffix(strings.TrimSpace(out), "/_data")
|
||||||
icmd.RunCommand("rm", "-rf", path).Assert(c, icmd.Success)
|
icmd.RunCommand("rm", "-rf", path).Assert(c, icmd.Success)
|
||||||
|
|
||||||
dockerCmd(c, "volume", "rm", "-f", "test")
|
dockerCmd(c, "volume", "rm", "-f", name)
|
||||||
out, _ = dockerCmd(c, "volume", "ls")
|
out, _ = dockerCmd(c, "volume", "ls")
|
||||||
c.Assert(out, checker.Not(checker.Contains), name)
|
c.Assert(out, checker.Not(checker.Contains), name)
|
||||||
dockerCmd(c, "volume", "create", "test")
|
dockerCmd(c, "volume", "create", name)
|
||||||
out, _ = dockerCmd(c, "volume", "ls")
|
out, _ = dockerCmd(c, "volume", "ls")
|
||||||
c.Assert(out, checker.Contains, name)
|
c.Assert(out, checker.Contains, name)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestVolumeCLIRmForceInUse verifies that repeated `docker volume rm -f` calls does not remove a volume
|
||||||
|
// if it is in use. Test case for https://github.com/docker/docker/issues/31446
|
||||||
|
func (s *DockerSuite) TestVolumeCLIRmForceInUse(c *check.C) {
|
||||||
|
name := "testvolume"
|
||||||
|
out, _ := dockerCmd(c, "volume", "create", name)
|
||||||
|
id := strings.TrimSpace(out)
|
||||||
|
c.Assert(id, checker.Equals, name)
|
||||||
|
|
||||||
|
prefix, slash := getPrefixAndSlashFromDaemonPlatform()
|
||||||
|
out, e := dockerCmd(c, "create", "-v", "testvolume:"+prefix+slash+"foo", "busybox")
|
||||||
|
cid := strings.TrimSpace(out)
|
||||||
|
|
||||||
|
_, _, err := dockerCmdWithError("volume", "rm", "-f", name)
|
||||||
|
c.Assert(err, check.NotNil)
|
||||||
|
c.Assert(err.Error(), checker.Contains, "volume is in use")
|
||||||
|
out, _ = dockerCmd(c, "volume", "ls")
|
||||||
|
c.Assert(out, checker.Contains, name)
|
||||||
|
|
||||||
|
// The original issue did not _remove_ the volume from the list
|
||||||
|
// the first time. But a second call to `volume rm` removed it.
|
||||||
|
// Calling `volume rm` a second time to confirm it's not removed
|
||||||
|
// when calling twice.
|
||||||
|
_, _, err = dockerCmdWithError("volume", "rm", "-f", name)
|
||||||
|
c.Assert(err, check.NotNil)
|
||||||
|
c.Assert(err.Error(), checker.Contains, "volume is in use")
|
||||||
|
out, _ = dockerCmd(c, "volume", "ls")
|
||||||
|
c.Assert(out, checker.Contains, name)
|
||||||
|
|
||||||
|
// Verify removing the volume after the container is removed works
|
||||||
|
_, e = dockerCmd(c, "rm", cid)
|
||||||
|
c.Assert(e, check.Equals, 0)
|
||||||
|
|
||||||
|
_, e = dockerCmd(c, "volume", "rm", "-f", name)
|
||||||
|
c.Assert(e, check.Equals, 0)
|
||||||
|
|
||||||
|
out, e = dockerCmd(c, "volume", "ls")
|
||||||
|
c.Assert(e, check.Equals, 0)
|
||||||
|
c.Assert(out, checker.Not(checker.Contains), name)
|
||||||
|
}
|
||||||
|
|
||||||
func (s *DockerSuite) TestVolumeCliInspectWithVolumeOpts(c *check.C) {
|
func (s *DockerSuite) TestVolumeCliInspectWithVolumeOpts(c *check.C) {
|
||||||
testRequires(c, DaemonIsLinux)
|
testRequires(c, DaemonIsLinux)
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue