diff --git a/daemon/delete.go b/daemon/delete.go index 6b4b7642a9..d922fd6f90 100644 --- a/daemon/delete.go +++ b/daemon/delete.go @@ -37,8 +37,11 @@ func (daemon *Daemon) containerRm(cfg *config.Config, name string, opts *backend // Container state RemovalInProgress should be used to avoid races. if inProgress := ctr.State.SetRemovalInProgress(); inProgress { - err := fmt.Errorf("removal of container %s is already in progress", name) - return errdefs.Conflict(err) + // TODO(thaJeztah): Consider waiting for an in-progress removal instead of returning a conflict. + // Concurrent removal is not itself a conflicting operation, and using Conflict makes it + // indistinguishable from actual state conflicts, such as attempting to remove a running + // or paused container without force. + return errdefs.Conflict(fmt.Errorf("removal of container %s is already in progress", name)) } defer ctr.State.ResetRemovalInProgress() diff --git a/daemon/delete_test.go b/daemon/delete_test.go index 21974a0544..3dc8bf0470 100644 --- a/daemon/delete_test.go +++ b/daemon/delete_test.go @@ -80,11 +80,12 @@ func TestContainerDelete(t *testing.T) { } } -func TestContainerDoubleDelete(t *testing.T) { - c := newContainerWithState(&container.State{}) - - // Mark the container as having a delete in progress - c.State.SetRemovalInProgress() +// TestContainerDeleteRemovalInProgress verifies that removing a container +// that is already being removed returns a conflict, even when forced. +func TestContainerDeleteRemovalInProgress(t *testing.T) { + c := newContainerWithState(&container.State{ + RemovalInProgress: true, + }) d, cleanup := newDaemonWithTmpRoot(t) defer cleanup() @@ -93,5 +94,6 @@ func TestContainerDoubleDelete(t *testing.T) { // Try to remove the container when its state is removalInProgress. // It should return an error indicating it is under removal progress. err := d.ContainerRm(c.ID, &backend.ContainerRmConfig{ForceRemove: true}) + assert.Check(t, is.ErrorType(err, cerrdefs.IsConflict)) assert.Check(t, is.ErrorContains(err, fmt.Sprintf("removal of container %s is already in progress", c.ID))) } diff --git a/internal/testutil/environment/clean.go b/internal/testutil/environment/clean.go index 695b474e90..017f8136d7 100644 --- a/internal/testutil/environment/clean.go +++ b/internal/testutil/environment/clean.go @@ -76,7 +76,7 @@ func deleteAllContainers(ctx context.Context, t testing.TB, apiclient client.Con }) // Ignore if container is already gone, or removal of container is already in progress. - if err == nil || cerrdefs.IsNotFound(err) || strings.Contains(err.Error(), "is already in progress") { + if err == nil || cerrdefs.IsNotFound(err) || (cerrdefs.IsConflict(err) && strings.Contains(err.Error(), "is already in progress")) { continue } assert.Check(t, err, "failed to remove %s", ctr.ID)