daemon: check for error-type for "removal already in progress"

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

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()
 

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)))
 }

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)