From fa18f83f8337a8accd0587f8901e8eebbdd538af Mon Sep 17 00:00:00 2001 From: Ogulcan Aydogan Date: Sat, 3 Oct 2026 09:39:20 +0300 Subject: [PATCH] fix(image): avoid dangling-ref name collision on `rmi -f` for running images nerdctl rmi -f on an image still used by a running container renames it before deleting the original, so containerd keeps the layers alive instead of garbage-collecting them. That dangling ref was always renamed to the literal string ":". Since containerd's image store requires unique names, force-removing a second running image's image in a separate invocation made its own rename fail with "image \":\": already exists", aborting the command. Name the dangling ref after its content digest (":") instead, so each kept-alive ref gets a distinct name. Tolerate AlreadyExists on the create call: if two different tags share the same digest and are both force-removed as running images, the second create legitimately no-ops (the digest is already pinned). The one other consumer that special-cased the exact ":" name (pkg/imgutil filtering, used by --filter reference=... to skip unparsable dangling names without erroring) is updated to match on the ":" prefix instead. Fixes #4109 Signed-off-by: Ogulcan Aydogan --- cmd/nerdctl/image/image_remove_test.go | 34 ++++++++++++++++++++++++++ pkg/cmd/image/remove.go | 28 ++++++++++++++++----- pkg/imgutil/filtering.go | 7 ++++-- pkg/imgutil/filtering_test.go | 23 +++++++++++++++++ 4 files changed, 84 insertions(+), 8 deletions(-) diff --git a/cmd/nerdctl/image/image_remove_test.go b/cmd/nerdctl/image/image_remove_test.go index 1ad1cee6a30..f33c7236118 100644 --- a/cmd/nerdctl/image/image_remove_test.go +++ b/cmd/nerdctl/image/image_remove_test.go @@ -149,6 +149,40 @@ func TestRemove(t *testing.T) { } }, }, + { + Description: "Issue #4109 - force-removing an in-use image does not collide with a dangling ref left by an earlier force-remove", + NoParallel: true, + Require: require.All( + // Dangling-ref naming on force-remove of an in-use image is a nerdctl-specific + // implementation detail; Docker doesn't use this scheme, so the test doesn't apply. + require.Not(nerdtest.Docker), + ), + Setup: func(data test.Data, helpers test.Helpers) { + helpers.Ensure("run", "--quiet", "--pull", "always", "-d", "--name", data.Identifier()+"-1", testutil.CommonImage, "sleep", nerdtest.Infinity) + helpers.Ensure("run", "--quiet", "--pull", "always", "-d", "--name", data.Identifier()+"-2", testutil.BusyboxImage, "sleep", nerdtest.Infinity) + // Force-remove the first in-use image now: this creates a dangling ref to keep + // its layers alive. Before the fix, that ref was unconditionally named ":", so + // the second force-remove below (the command under test) would fail creating + // its own dangling ref with "image \":\": already exists". + helpers.Ensure("rmi", "-f", testutil.CommonImage) + }, + Cleanup: func(data test.Data, helpers test.Helpers) { + helpers.Anyhow("rm", "-f", data.Identifier()+"-1") + helpers.Anyhow("rm", "-f", data.Identifier()+"-2") + }, + Command: test.Command("rmi", "-f", testutil.BusyboxImage), + Expected: func(data test.Data, helpers test.Helpers) *test.Expected { + return &test.Expected{ + ExitCode: 0, + Errors: []error{}, + Output: func(stdout string, t tig.T) { + helpers.Command("images").Run(&test.Expected{ + Output: expect.Contains(""), + }) + }, + } + }, + }, { Description: "Remove image with created container - without -f", NoParallel: true, diff --git a/pkg/cmd/image/remove.go b/pkg/cmd/image/remove.go index 5e5c9ccd9fd..5ac57620565 100644 --- a/pkg/cmd/image/remove.go +++ b/pkg/cmd/image/remove.go @@ -22,8 +22,11 @@ import ( "fmt" "strings" + "github.com/opencontainers/go-digest" + containerd "github.com/containerd/containerd/v2/client" "github.com/containerd/containerd/v2/core/images" + "github.com/containerd/errdefs" "github.com/containerd/log" "github.com/containerd/nerdctl/v2/pkg/api/types" @@ -32,6 +35,15 @@ import ( "github.com/containerd/nerdctl/v2/pkg/platformutil" ) +// danglingRefName builds a dangling-image name unique to the given digest, so that +// force-removing more than one running image's image in the same invocation does not +// collide on image creation: containerd's image store requires unique names, and a +// fixed ":" name is shared by every dangling ref. See: +// https://github.com/containerd/nerdctl/issues/4109 +func danglingRefName(dgst digest.Digest) string { + return ":" + dgst.String() +} + // Remove removes a list of `images`. func Remove(ctx context.Context, client *containerd.Client, args []string, options types.ImageRemoveOptions) error { var delOpts []images.DeleteOpt @@ -84,10 +96,12 @@ func Remove(ctx context.Context, client *containerd.Client, args []string, optio if cid, ok := runningImages[found.Image.Name]; ok { if options.Force { // This is a running image, so, we need to keep a ref on it so that containerd does not GC the layers - // First create the new image with an empty name + // First create the new image with a dangling name unique to its digest: a fixed ":" name + // collides ("image \":\": already exists") when force-removing more than one running + // image's image in the same invocation. originalName := found.Image.Name - found.Image.Name = ":" - if _, err = is.Create(ctx, found.Image); err != nil { + found.Image.Name = danglingRefName(found.Image.Target.Digest) + if _, err = is.Create(ctx, found.Image); err != nil && !errdefs.IsAlreadyExists(err) { return err } @@ -137,10 +151,12 @@ func Remove(ctx context.Context, client *containerd.Client, args []string, optio if cid, ok := runningImages[found.Image.Name]; ok { if options.Force { // This is a running image, so, we need to keep a ref on it so that containerd does not GC the layers - // First create the new image with an empty name + // First create the new image with a dangling name unique to its digest: a fixed ":" name + // collides ("image \":\": already exists") when force-removing more than one running + // image's image in the same invocation. originalName := found.Image.Name - found.Image.Name = ":" - if _, err = is.Create(ctx, found.Image); err != nil { + found.Image.Name = danglingRefName(found.Image.Target.Digest) + if _, err = is.Create(ctx, found.Image); err != nil && !errdefs.IsAlreadyExists(err) { return false, err } diff --git a/pkg/imgutil/filtering.go b/pkg/imgutil/filtering.go index c7ffeb3f49e..f7efd6c739c 100644 --- a/pkg/imgutil/filtering.go +++ b/pkg/imgutil/filtering.go @@ -323,8 +323,11 @@ func matchesAllLabels(imageCfgLabels map[string]string, filterLabels map[string] func matchesReferences(image images.Image, referencePatterns []string) (bool, error) { var matches int - // Containerd returns ":" for dangling untagged images - see https://github.com/containerd/nerdctl/issues/3852 - if image.Name == ":" { + // Dangling untagged images are named ":" or ":" (see + // https://github.com/containerd/nerdctl/issues/3852 and + // https://github.com/containerd/nerdctl/issues/4109), neither of which is a + // parsable reference. + if strings.HasPrefix(image.Name, ":") { return false, nil } diff --git a/pkg/imgutil/filtering_test.go b/pkg/imgutil/filtering_test.go index 7b0b3995f74..c28d2d10ff7 100644 --- a/pkg/imgutil/filtering_test.go +++ b/pkg/imgutil/filtering_test.go @@ -271,6 +271,29 @@ func TestFilterByReference(t *testing.T) { referencePatterns: []string{"foobar"}, expectedImages: []images.Image{}, }, + { + // Dangling refs kept alive by `rmi -f` on a running image are named ":" or, since + // #4109, ":". Neither is a parsable reference, so they must be skipped + // rather than erroring out the whole filter. See issues #3852 and #4109. + name: "SkipsDanglingRefsWithoutErroring", + images: []images.Image{ + { + Name: "foo:latest", + }, + { + Name: ":", + }, + { + Name: ":sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", + }, + }, + referencePatterns: []string{"foo"}, + expectedImages: []images.Image{ + { + Name: "foo:latest", + }, + }, + }, } for _, test := range tests {