Fix rmi err checking containers with deleted image

Signed-off-by: Jin Dong <[email protected]>

cmd/nerdctl/image_remove_linux_test.go

@@ -27,27 +27,66 @@ func TestRemoveImage(t *testing.T) {
 	tID := testutil.Identifier(t)
 	base.Cmd("image", "prune", "--force", "--all").AssertOK()
 
-	// ignore error
-	base.Cmd("rmi", "-f", tID).AssertOK()
-
 	base.Cmd("run", "--name", tID, testutil.CommonImage).AssertOK()
-	defer base.Cmd("rm", "-f", tID).Run()
+	defer base.Cmd("rm", "-f", tID).AssertOK()
 
 	base.Cmd("rmi", testutil.CommonImage).AssertFail()
-	defer base.Cmd("rmi", "-f", testutil.CommonImage).Run()
 	base.Cmd("rmi", "-f", testutil.CommonImage).AssertOK()
-
-	base.Cmd("images").AssertNoOut(testutil.CommonImage)
+	base.Cmd("images", "--names").AssertNoOut(testutil.CommonImage)
 }
 
 func TestRemoveRunningImage(t *testing.T) {
 	base := testutil.NewBase(t)
 	tID := testutil.Identifier(t)
+
+	base.Cmd("run", "--name", tID, "-d", testutil.CommonImage, "sleep", "infinity").AssertOK()
+	defer base.Cmd("rm", "-f", tID).AssertOK()
+
+	base.Cmd("rmi", testutil.CommonImage).AssertFail()
+
+	// `rmi -f` only logs err, not return err. need to check `images` output
+	base.Cmd("kill", tID).AssertOK()
+	base.Cmd("rmi", testutil.CommonImage).AssertFail()
+	base.Cmd("rmi", "-f", testutil.CommonImage).AssertOK()
+	base.Cmd("images", "--names").AssertNoOut(testutil.CommonImage)
+}
+
+func TestRemovePausedImage(t *testing.T) {
+	base := testutil.NewBase(t)
+	tID := testutil.Identifier(t)
+
 	base.Cmd("run", "--name", tID, "-d", testutil.CommonImage, "sleep", "infinity").AssertOK()
-	defer base.Cmd("rm", "-f", tID).Run()
+	base.Cmd("pause", tID).AssertOK()
+	defer base.Cmd("rm", "-f", tID).AssertOK()
+
+	// `rmi -f` cannot remove image (`-f` only logs err, not return err)
 	base.Cmd("rmi", testutil.CommonImage).AssertFail()
+	base.Cmd("rmi", "-f", testutil.CommonImage).AssertOK()
+	base.Cmd("images", "--names").AssertOutContains(testutil.CommonImage)
+
+	// `rmi -f` can remove image after container killed
 	base.Cmd("kill", tID).AssertOK()
 	base.Cmd("rmi", testutil.CommonImage).AssertFail()
 	base.Cmd("rmi", "-f", testutil.CommonImage).AssertOK()
-	base.Cmd("images").AssertNoOut(testutil.CommonImage)
+	base.Cmd("images", "--names").AssertNoOut(testutil.CommonImage)
+}
+
+func TestRemoveImageWithCreatedContainer(t *testing.T) {
+	base := testutil.NewBase(t)
+	tID := testutil.Identifier(t)
+
+	base.Cmd("pull", testutil.AlpineImage).AssertOK()
+	base.Cmd("pull", testutil.NginxAlpineImage).AssertOK()
+
+	base.Cmd("create", "--name", tID, testutil.AlpineImage, "sleep", "infinity").AssertOK()
+	defer base.Cmd("rm", "-f", tID).AssertOK()
+
+	// `rmi -f` can remove image associated with created container (`-f` only logs err, not return err)
+	base.Cmd("rmi", testutil.AlpineImage).AssertFail()
+	base.Cmd("rmi", "-f", testutil.AlpineImage).AssertOK()
+	base.Cmd("images", "--names").AssertNoOut(testutil.AlpineImage)
+
+	// a created container with removed image doesn't impact other `rmi` command
+	base.Cmd("rmi", "-f", testutil.NginxAlpineImage).AssertOK()
+	base.Cmd("images", "--names").AssertNoOut(testutil.NginxAlpineImage)
 }

pkg/cmd/image/remove.go

@@ -19,13 +19,12 @@ package image
 import (
 	"context"
 	"fmt"
-	"strings"
 
 	"github.com/containerd/containerd"
 	"github.com/containerd/containerd/images"
 	"github.com/containerd/containerd/platforms"
 	"github.com/containerd/nerdctl/pkg/api/types"
-	"github.com/containerd/nerdctl/pkg/formatter"
+	"github.com/containerd/nerdctl/pkg/containerutil"
 	"github.com/containerd/nerdctl/pkg/idutil/imagewalker"
 	"github.com/sirupsen/logrus"
 )
@@ -48,12 +47,16 @@ func Remove(ctx context.Context, client *containerd.Client, args []string, optio
 	for _, container := range containerList {
 		image, err := container.Image(ctx)
 		if err != nil {
-			return err
+			continue
 		}
-		cStatus := formatter.ContainerStatus(ctx, container)
-		if strings.HasPrefix(cStatus, "Up") {
+
+		// if err != nil, simply go to `default`
+		cStatus, _ := containerutil.ContainerStatus(ctx, container)
+		fmt.Println(cStatus)
+		switch cStatus.Status {
+		case containerd.Running, containerd.Pausing, containerd.Paused:
 			runningImages[image.Name()] = struct{}{}
-		} else {
+		default:
 			usedImages[image.Name()] = struct{}{}
 		}
 	}