Merge pull request #26074 from allencloud/return-err-when-remove-container-in-progress

make client side know container removal in progress

api/server/router/container/container_routes.go

@@ -6,7 +6,6 @@ import (
 	"io"
 	"net/http"
 	"strconv"
-	"strings"
 	"syscall"
 	"time"
 
@@ -386,10 +385,6 @@ func (s *containerRouter) deleteContainers(ctx context.Context, w http.ResponseW
 	}
 
 	if err := s.backend.ContainerRm(name, config); err != nil {
-		// Force a 404 for the empty string
-		if strings.Contains(strings.ToLower(err.Error()), "prefix can't be empty") {
-			return fmt.Errorf("no such container: \"\"")
-		}
 		return err
 	}
 

daemon/delete.go

@@ -26,7 +26,8 @@ func (daemon *Daemon) ContainerRm(name string, config *types.ContainerRmConfig)
 
 	// Container state RemovalInProgress should be used to avoid races.
 	if inProgress := container.SetRemovalInProgress(); inProgress {
-		return nil
+		err := fmt.Errorf("removal of container %s is already in progress", name)
+		return errors.NewBadRequestError(err)
 	}
 	defer container.ResetRemovalInProgress()
 

daemon/delete_test.go

@@ -1,6 +1,7 @@
 package daemon
 
 import (
+	"fmt"
 	"io/ioutil"
 	"os"
 	"testing"
@@ -34,9 +35,9 @@ func TestContainerDoubleDelete(t *testing.T) {
 	// Mark the container as having a delete in progress
 	container.SetRemovalInProgress()
 
-	// Try to remove the container when its start is removalInProgress.
-	// It should ignore the container and not return an error.
-	if err := daemon.ContainerRm(container.ID, &types.ContainerRmConfig{ForceRemove: true}); err != nil {
-		t.Fatal(err)
+	// Try to remove the container when its state is removalInProgress.
+	// It should return an error indicating it is under removal progress.
+	if err := daemon.ContainerRm(container.ID, &types.ContainerRmConfig{ForceRemove: true}); err == nil {
+		t.Fatalf("expected err: %v, got nil", fmt.Sprintf("removal of container %s is already in progress", container.ID))
 	}
 }

docs/reference/api/docker_remote_api.md

@@ -129,6 +129,7 @@ This section lists each version from latest to oldest.  Each listing includes a
 * `GET /networks/` endpoint now correctly returns a list of *all* networks,
   instead of the default network if a trailing slash is provided, but no `name`
   or `id`.
+* `DELETE /containers/(name)` endpoint now returns an error of `removal of container name is already in progress` with status code of 400, when container name is in a state of removal in progress.
 
 ### v1.24 API changes