Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 24 additions & 8 deletions pkg/cmd/image/remove.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -32,6 +35,15 @@ import (
"github.com/containerd/nerdctl/v2/pkg/platformutil"
)

// danglingImageName returns the synthetic name given to an image that is force-removed while
// still referenced by a container: it must be unique per digest so that force-removing several
// such images in the same invocation does not collide on a shared name in the image store (see
// https://github.com/containerd/nerdctl/issues/4109). Consumers that need to recognize dangling
// images (e.g. pkg/imgutil filtering) match on the leading ":".
func danglingImageName(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
Expand Down Expand Up @@ -83,11 +95,13 @@ 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
// 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 dangling image, named uniquely per digest so that force-removing
// several running images in a row does not collide on a shared name (see
// https://github.com/containerd/nerdctl/issues/4109).
originalName := found.Image.Name
found.Image.Name = ":"
if _, err = is.Create(ctx, found.Image); err != nil {
found.Image.Name = danglingImageName(found.Image.Target.Digest)
if _, err = is.Create(ctx, found.Image); err != nil && !errdefs.IsAlreadyExists(err) {
return err
}

Expand Down Expand Up @@ -136,11 +150,13 @@ 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
// 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 dangling image, named uniquely per digest so that force-removing
// several running images in a row does not collide on a shared name (see
// https://github.com/containerd/nerdctl/issues/4109).
originalName := found.Image.Name
found.Image.Name = ":"
if _, err = is.Create(ctx, found.Image); err != nil {
found.Image.Name = danglingImageName(found.Image.Target.Digest)
if _, err = is.Create(ctx, found.Image); err != nil && !errdefs.IsAlreadyExists(err) {
return false, err
}

Expand Down
77 changes: 77 additions & 0 deletions pkg/cmd/image/remove_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
/*
Copyright The containerd Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package image

import (
"context"
"testing"

"github.com/opencontainers/go-digest"
"gotest.tools/v3/assert"

"github.com/containerd/containerd/v2/core/images"
"github.com/containerd/errdefs"
)

// fakeImageStore is a minimal images.Store that reproduces containerd's real constraint that
// Create fails with errdefs.ErrAlreadyExists when the image name already exists.
type fakeImageStore struct {
images.Store
byName map[string]images.Image
}

func newFakeImageStore() *fakeImageStore {
return &fakeImageStore{byName: map[string]images.Image{}}
}

func (f *fakeImageStore) Create(_ context.Context, image images.Image) (images.Image, error) {
if _, ok := f.byName[image.Name]; ok {
return images.Image{}, errdefs.ErrAlreadyExists
}
f.byName[image.Name] = image
return image, nil
}

// TestDanglingImageNameIsUniquePerDigest reproduces
// https://github.com/containerd/nerdctl/issues/4109: force-removing several images that are each
// in use by a running container used to create every kept-alive dangling ref under the exact same
// literal name (":"), so the second `is.Create` call in the same run failed with
// "image \":\": already exists". Naming the dangling ref after its digest (danglingImageName)
// keeps names unique across images, so both creations succeed.
func TestDanglingImageNameIsUniquePerDigest(t *testing.T) {
store := newFakeImageStore()
ctx := context.Background()

digestA := digest.FromString("image-a")
digestB := digest.FromString("image-b")

// Old, buggy behavior: every dangling ref reused the same literal ":" name.
const buggyName = ":"
_, err := store.Create(ctx, images.Image{Name: buggyName})
assert.NilError(t, err)
_, err = store.Create(ctx, images.Image{Name: buggyName})
assert.Assert(t, errdefs.IsAlreadyExists(err), "expected the second create with a shared name to collide, got %v", err)

// Fixed behavior: naming the dangling ref after its digest avoids the collision.
store = newFakeImageStore()
_, err = store.Create(ctx, images.Image{Name: danglingImageName(digestA)})
assert.NilError(t, err)
_, err = store.Create(ctx, images.Image{Name: danglingImageName(digestB)})
assert.NilError(t, err, "force-removing a second running image must not collide on the dangling ref name")

assert.Assert(t, danglingImageName(digestA) != danglingImageName(digestB))
}
5 changes: 3 additions & 2 deletions pkg/imgutil/filtering.go
Original file line number Diff line number Diff line change
Expand Up @@ -323,8 +323,9 @@ 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 ":<digest>" - see
// https://github.com/containerd/nerdctl/issues/3852 and https://github.com/containerd/nerdctl/issues/4109
if strings.HasPrefix(image.Name, ":") {
return false, nil
}

Expand Down
Loading