Merge pull request #457 from docker/feat-aci-force-rm

Add --force to rm on ACI

aci/backend.go

@@ -49,11 +49,9 @@ const (
 	composeContainerTag       = "docker-compose-application"
 	composeContainerSeparator = "_"
 	statusUnknown             = "Unknown"
+	statusRunning             = "Running"
 )
 
-// ErrNoSuchContainer is returned when the mentioned container does not exist
-var ErrNoSuchContainer = errors.New("no such container")
-
 // ContextParams options for creating ACI context
 type ContextParams struct {
 	Description    string
@@ -292,18 +290,46 @@ func (cs *aciContainerService) Logs(ctx context.Context, containerName string, r
 	return err
 }
 
-func (cs *aciContainerService) Delete(ctx context.Context, containerID string, _ bool) error {
+func (cs *aciContainerService) Delete(ctx context.Context, containerID string, request containers.DeleteRequest) error {
 	groupName, containerName := getGroupAndContainerName(containerID)
 	if groupName != containerID {
 		msg := "cannot delete service %q from compose application %q, you can delete the entire compose app with docker compose down --project-name %s"
 		return errors.New(fmt.Sprintf(msg, containerName, groupName, groupName))
 	}
-	cg, err := deleteACIContainerGroup(ctx, cs.ctx, groupName)
+
+	containerGroupsClient, err := getContainerGroupsClient(cs.ctx.SubscriptionID)
 	if err != nil {
 		return err
 	}
+
+	if !request.Force {
+		cg, err := containerGroupsClient.Get(ctx, cs.ctx.ResourceGroup, groupName)
+		if err != nil {
+			if cg.StatusCode == http.StatusNotFound {
+				return errdefs.ErrNotFound
+			}
+			return err
+		}
+
+		for _, container := range *cg.Containers {
+			status := statusUnknown
+			if container.InstanceView != nil && container.InstanceView.CurrentState != nil {
+				status = *container.InstanceView.CurrentState.State
+			}
+
+			if status == statusRunning {
+				return errdefs.ErrForbidden
+			}
+		}
+	}
+
+	cg, err := deleteACIContainerGroup(ctx, cs.ctx, groupName)
+	// Delete returns `StatusNoContent` if the group is not found
 	if cg.StatusCode == http.StatusNoContent {
-		return ErrNoSuchContainer
+		return errdefs.ErrNotFound
+	}
+	if err != nil {
+		return err
 	}
 
 	return err
@@ -317,7 +343,7 @@ func (cs *aciContainerService) Inspect(ctx context.Context, containerID string)
 		return containers.Container{}, err
 	}
 	if cg.StatusCode == http.StatusNoContent {
-		return containers.Container{}, ErrNoSuchContainer
+		return containers.Container{}, errdefs.ErrNotFound
 	}
 
 	var cc containerinstance.Container
@@ -330,7 +356,7 @@ func (cs *aciContainerService) Inspect(ctx context.Context, containerID string)
 		}
 	}
 	if !found {
-		return containers.Container{}, ErrNoSuchContainer
+		return containers.Container{}, errdefs.ErrNotFound
 	}
 
 	return convert.ContainerGroupToContainer(containerID, cg, cc)
@@ -374,7 +400,7 @@ func (cs *aciComposeService) Down(ctx context.Context, opts cli.ProjectOptions)
 		return err
 	}
 	if cg.StatusCode == http.StatusNoContent {
-		return ErrNoSuchContainer
+		return errdefs.ErrNotFound
 	}
 
 	return err

aci/backend_test.go

@@ -42,7 +42,7 @@ func TestGetContainerName(t *testing.T) {
 
 func TestErrorMessageDeletingContainerFromComposeApplication(t *testing.T) {
 	service := aciContainerService{}
-	err := service.Delete(context.TODO(), "compose-app_service1", false)
+	err := service.Delete(context.TODO(), "compose-app_service1", containers.DeleteRequest{Force: false})
 	assert.Error(t, err, "cannot delete service \"service1\" from compose application \"compose-app\", you can delete the entire compose app with docker compose down --project-name compose-app")
 }
 

cli/cmd/rm.go

@@ -24,6 +24,8 @@ import (
 	"github.com/spf13/cobra"
 
 	"github.com/docker/api/client"
+	"github.com/docker/api/containers"
+	"github.com/docker/api/errdefs"
 	"github.com/docker/api/multierror"
 )
 
@@ -57,11 +59,20 @@ func runRm(ctx context.Context, args []string, opts rmOpts) error {
 
 	var errs *multierror.Error
 	for _, id := range args {
-		err := c.ContainerService().Delete(ctx, id, opts.force)
+		err := c.ContainerService().Delete(ctx, id, containers.DeleteRequest{
+			Force: opts.force,
+		})
 		if err != nil {
-			errs = multierror.Append(errs, err)
+			if errdefs.IsForbiddenError(err) {
+				errs = multierror.Append(errs, fmt.Errorf("you cannot remove a running container %s. Stop the container before attempting removal or force remove", id))
+			} else if errdefs.IsNotFoundError(err) {
+				errs = multierror.Append(errs, fmt.Errorf("container %s not found", id))
+			} else {
+				errs = multierror.Append(errs, err)
+			}
 			continue
 		}
+
 		fmt.Println(id)
 	}
 

cli/main.go

@@ -179,10 +179,11 @@ func main() {
 	ctx = store.WithContextStore(ctx, s)
 
 	if err = root.ExecuteContext(ctx); err != nil {
-		//if user canceled request, simply exit without any error message
+		// if user canceled request, simply exit without any error message
 		if errors.Is(ctx.Err(), context.Canceled) {
 			os.Exit(130)
 		}
+
 		// Context should always be handled by new CLI
 		requiredCmd, _, _ := root.Find(os.Args[1:])
 		if requiredCmd != nil && isOwnCommand(requiredCmd) {
@@ -191,6 +192,7 @@ func main() {
 		mobycli.ExecIfDefaultCtxType(ctx)
 
 		checkIfUnknownCommandExistInDefaultContext(err, currentContext)
+
 		exit(err)
 	}
 }
@@ -203,6 +205,11 @@ func exit(err error) {
 	fatal(err)
 }
 
+func fatal(err error) {
+	fmt.Fprintln(os.Stderr, err)
+	os.Exit(1)
+}
+
 func checkIfUnknownCommandExistInDefaultContext(err error, currentContext string) {
 	submatch := unknownCommandRegexp.FindSubmatch([]byte(err.Error()))
 	if len(submatch) == 2 {
@@ -241,8 +248,3 @@ func determineCurrentContext(flag string, configDir string) string {
 	}
 	return res
 }
-
-func fatal(err error) {
-	fmt.Fprintln(os.Stderr, err)
-	os.Exit(1)
-}

containers/api.go

@@ -105,6 +105,11 @@ type LogsRequest struct {
 	Writer io.Writer
 }
 
+// DeleteRequest contains configuration about a delete request
+type DeleteRequest struct {
+	Force bool
+}
+
 // Service interacts with the underlying container backend
 type Service interface {
 	// List returns all the containers
@@ -118,7 +123,7 @@ type Service interface {
 	// Logs returns all the logs of a container
 	Logs(ctx context.Context, containerName string, request LogsRequest) error
 	// Delete removes containers
-	Delete(ctx context.Context, id string, force bool) error
+	Delete(ctx context.Context, containerID string, request DeleteRequest) error
 	// Inspect get a specific container
 	Inspect(ctx context.Context, id string) (Container, error)
 }

example/backend.go

@@ -24,15 +24,13 @@ import (
 	"fmt"
 
 	"github.com/compose-spec/compose-go/cli"
-
-	"github.com/docker/api/context/cloud"
-	"github.com/docker/api/errdefs"
-
 	ecstypes "github.com/docker/ecs-plugin/pkg/compose"
 
 	"github.com/docker/api/backend"
 	"github.com/docker/api/compose"
 	"github.com/docker/api/containers"
+	"github.com/docker/api/context/cloud"
+	"github.com/docker/api/errdefs"
 )
 
 type apiService struct {
@@ -108,8 +106,8 @@ func (cs *containerService) Logs(ctx context.Context, containerName string, requ
 	return nil
 }
 
-func (cs *containerService) Delete(ctx context.Context, id string, force bool) error {
-	fmt.Printf("Deleting container %q with force = %t\n", id, force)
+func (cs *containerService) Delete(ctx context.Context, id string, request containers.DeleteRequest) error {
+	fmt.Printf("Deleting container %q with force = %t\n", id, request.Force)
 	return nil
 }
 

local/backend.go

@@ -249,9 +249,9 @@ func (ms *local) Logs(ctx context.Context, containerName string, request contain
 	return err
 }
 
-func (ms *local) Delete(ctx context.Context, containerID string, force bool) error {
+func (ms *local) Delete(ctx context.Context, containerID string, request containers.DeleteRequest) error {
 	err := ms.apiClient.ContainerRemove(ctx, containerID, types.ContainerRemoveOptions{
-		Force: force,
+		Force: request.Force,
 	})
 	if client.IsErrNotFound(err) {
 		return errors.Wrapf(errdefs.ErrNotFound, "container %q", containerID)

server/proxy/containers.go

@@ -77,7 +77,9 @@ func (p *proxy) Inspect(ctx context.Context, request *containersv1.InspectReques
 }
 
 func (p *proxy) Delete(ctx context.Context, request *containersv1.DeleteRequest) (*containersv1.DeleteResponse, error) {
-	return &containersv1.DeleteResponse{}, Client(ctx).ContainerService().Delete(ctx, request.Id, request.Force)
+	return &containersv1.DeleteResponse{}, Client(ctx).ContainerService().Delete(ctx, request.Id, containers.DeleteRequest{
+		Force: request.Force,
+	})
 }
 
 func (p *proxy) Exec(ctx context.Context, request *containersv1.ExecRequest) (*containersv1.ExecResponse, error) {

tests/aci-e2e/e2e-aci_test.go

@@ -265,9 +265,21 @@ func TestContainerRun(t *testing.T) {
 		}
 	})
 
-	t.Run("rm", func(t *testing.T) {
+	t.Run("rm a running container", func(t *testing.T) {
 		res := c.RunDockerCmd("rm", container)
-		res.Assert(t, icmd.Expected{Out: container})
+		res.Assert(t, icmd.Expected{
+			Err:      fmt.Sprintf("Error: You cannot remove a running container %s. Stop the container before attempting removal or force remove\n", container),
+			ExitCode: 1,
+		})
+	})
+
+	t.Run("force rm", func(t *testing.T) {
+		res := c.RunDockerCmd("rm", "-f", container)
+		res.Assert(t, icmd.Expected{
+			Out:      container,
+			ExitCode: 0,
+		})
+
 		checkStopped := func(t poll.LogT) poll.Result {
 			res := c.RunDockerCmd("inspect", container)
 			if res.ExitCode == 1 {
@@ -349,7 +361,7 @@ func TestContainerRunAttached(t *testing.T) {
 	})
 
 	t.Run("rm attached", func(t *testing.T) {
-		res := c.RunDockerCmd("rm", container)
+		res := c.RunDockerCmd("rm", "-f", container)
 		res.Assert(t, icmd.Expected{Out: container})
 	})
 }