diff --git a/cmd/nerdctl/image_encrypt_linux_test.go b/cmd/nerdctl/image_encrypt_linux_test.go index 29096674bad..67ed3b04eed 100644 --- a/cmd/nerdctl/image_encrypt_linux_test.go +++ b/cmd/nerdctl/image_encrypt_linux_test.go @@ -68,7 +68,8 @@ func newJWEKeyPair(t testing.TB) *jweKeyPair { func rmiAll(base *testutil.Base) { base.T.Logf("Pruning images") imageIDs := base.Cmd("images", "--no-trunc", "-a", "-q").OutLines() - base.Cmd(append([]string{"rmi", "-f"}, imageIDs...)...).AssertOK() + imageIDs = imageIDs[:len(imageIDs)-1] // remove empty output line at the end + base.Cmd(append([]string{"rmi", "-f"}, imageIDs...)...).Run() base.T.Logf("Pruning build caches") if _, err := buildkitutil.GetBuildkitHost(testutil.Namespace); err == nil { @@ -97,7 +98,7 @@ func rmiAll(base *testutil.Base) { base.T.Logf("Pruning all images (again?)") imageIDs = base.Cmd("images", "--no-trunc", "-a", "-q").OutLines() - base.Cmd(append([]string{"rmi", "-f"}, imageIDs...)...).AssertOK() + base.Cmd(append([]string{"rmi", "-f"}, imageIDs...)...).Run() } } diff --git a/cmd/nerdctl/image_remove_linux_test.go b/cmd/nerdctl/image_remove_linux_test.go index efb43adb046..b772c90e908 100644 --- a/cmd/nerdctl/image_remove_linux_test.go +++ b/cmd/nerdctl/image_remove_linux_test.go @@ -31,7 +31,7 @@ func TestRemoveImage(t *testing.T) { 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() @@ -41,13 +41,63 @@ func TestRemoveImage(t *testing.T) { } func TestRemoveRunningImage(t *testing.T) { + // If an image is associated with a running/paused containers, `docker rmi -f imageName` + // untags `imageName` (left a `` image) without deletion; `docker rmi -rf imageID` fails. + // In both cases, `nerdctl rmi -f` will fail. + testutil.DockerIncompatible(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() + base.Cmd("rmi", "-f", testutil.CommonImage).AssertFail() + base.Cmd("images").AssertOutContains(testutil.ImageRepo(testutil.CommonImage)) + + base.Cmd("kill", tID).AssertOK() + base.Cmd("rmi", testutil.CommonImage).AssertFail() + base.Cmd("rmi", "-f", testutil.CommonImage).AssertOK() + base.Cmd("images").AssertNoOut(testutil.ImageRepo(testutil.CommonImage)) +} + +func TestRemovePausedImage(t *testing.T) { + // If an image is associated with a running/paused containers, `docker rmi -f imageName` + // untags `imageName` (left a `` image) without deletion; `docker rmi -rf imageID` fails. + // In both cases, `nerdctl rmi -f` will fail. + testutil.DockerIncompatible(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() + base.Cmd("rmi", testutil.CommonImage).AssertFail() + base.Cmd("rmi", "-f", testutil.CommonImage).AssertFail() + base.Cmd("images").AssertOutContains(testutil.ImageRepo(testutil.CommonImage)) + base.Cmd("kill", tID).AssertOK() base.Cmd("rmi", testutil.CommonImage).AssertFail() base.Cmd("rmi", "-f", testutil.CommonImage).AssertOK() base.Cmd("images").AssertNoOut(testutil.ImageRepo(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() + + base.Cmd("rmi", testutil.AlpineImage).AssertFail() + base.Cmd("rmi", "-f", testutil.AlpineImage).AssertOK() + base.Cmd("images").AssertNoOut(testutil.ImageRepo(testutil.AlpineImage)) + + // a created container with removed image doesn't impact other `rmi` command + base.Cmd("rmi", "-f", testutil.NginxAlpineImage).AssertOK() + base.Cmd("images").AssertNoOut(testutil.ImageRepo(testutil.NginxAlpineImage)) +} diff --git a/pkg/cmd/image/remove.go b/pkg/cmd/image/remove.go index 563022e245a..f2aab2058e4 100644 --- a/pkg/cmd/image/remove.go +++ b/pkg/cmd/image/remove.go @@ -18,6 +18,7 @@ package image import ( "context" + "errors" "fmt" "strings" @@ -25,7 +26,7 @@ import ( "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" ) @@ -43,18 +44,20 @@ func Remove(ctx context.Context, client *containerd.Client, args []string, optio if err != nil { return err } - usedImages := make(map[string]struct{}) - runningImages := make(map[string]struct{}) + usedImages := make(map[string]string) + runningImages := make(map[string]string) 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") { - runningImages[image.Name()] = struct{}{} - } else { - usedImages[image.Name()] = struct{}{} + + // if err != nil, simply go to `default` + switch cStatus, _ := containerutil.ContainerStatus(ctx, container); cStatus.Status { + case containerd.Running, containerd.Pausing, containerd.Paused: + runningImages[image.Name()] = container.ID() + default: + usedImages[image.Name()] = container.ID() } } @@ -66,11 +69,11 @@ func Remove(ctx context.Context, client *containerd.Client, args []string, optio if found.MatchCount > 1 && !(options.Force && found.UniqueImages == 1) { return fmt.Errorf("multiple IDs found with provided prefix: %s", found.Req) } - if _, ok := runningImages[found.Image.Name]; ok { - return fmt.Errorf("image %s is running, can't be forced removed", found.Image.Name) + if cid, ok := runningImages[found.Image.Name]; ok { + return fmt.Errorf("conflict: unable to delete %s (cannot be forced) - image is being used by running container %s", found.Req, cid) } - if _, ok := usedImages[found.Image.Name]; ok && !options.Force { - return fmt.Errorf("conflict: unable to remove repository reference %q (must force)", found.Req) + if cid, ok := usedImages[found.Image.Name]; ok && !options.Force { + return fmt.Errorf("conflict: unable to delete %s (must be forced) - image is being used by stopped container %s", found.Req, cid) } // digests is used only for emulating human-readable output of `docker rmi` digests, err := found.Image.RootFS(ctx, cs, platforms.DefaultStrict()) @@ -89,10 +92,27 @@ func Remove(ctx context.Context, client *containerd.Client, args []string, optio }, } - err = walker.WalkAll(ctx, args, true) - if err != nil && options.Force { - logrus.Error(err) - return nil + var errs []string + var fatalErr bool + for _, req := range args { + n, err := walker.Walk(ctx, req) + if err != nil { + fatalErr = true + } + if err == nil && n == 0 { + err = fmt.Errorf("no such image: %s", req) + } + if err != nil { + errs = append(errs, err.Error()) + } + } + + if len(errs) > 0 { + msg := fmt.Sprintf("%d errors:\n%s", len(errs), strings.Join(errs, "\n")) + if !options.Force || fatalErr { + return errors.New(msg) + } + logrus.Error(msg) } - return err + return nil }