Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Allow to delete images when names of images are short digest ids… #3519

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
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
49 changes: 49 additions & 0 deletions cmd/nerdctl/image/image_remove_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,11 @@ package image

import (
"errors"
"strings"
"testing"

"gotest.tools/v3/assert"

"github.com/containerd/nerdctl/v2/pkg/imgutil"
"github.com/containerd/nerdctl/v2/pkg/testutil"
"github.com/containerd/nerdctl/v2/pkg/testutil/nerdtest"
Expand Down Expand Up @@ -302,3 +305,49 @@ func TestRemove(t *testing.T) {

testCase.Run(t)
}

// TestIssue3016 tests https://github.com/containerd/nerdctl/issues/3016
func TestIssue3016(t *testing.T) {
testCase := nerdtest.Setup()

const (
tagIDKey = "tagID"
)

testCase.SubTests = []*test.Case{
{
Description: "Issue #3016 - Tags created using the short digest ids of container images cannot be deleted using the nerdctl rmi command.",
Setup: func(data test.Data, helpers test.Helpers) {
helpers.Ensure("pull", testutil.CommonImage)
helpers.Ensure("pull", testutil.NginxAlpineImage)

img := nerdtest.InspectImage(helpers, testutil.NginxAlpineImage)
repoName, _ := imgutil.ParseRepoTag(testutil.NginxAlpineImage)
tagID := strings.TrimPrefix(img.RepoDigests[0], repoName+"@sha256:")[0:8]

helpers.Ensure("tag", testutil.CommonImage, tagID)

data.Set(tagIDKey, tagID)
},
Command: func(data test.Data, helpers test.Helpers) test.TestableCommand {
return helpers.Command("rmi", data.Get(tagIDKey))
},
Expected: func(data test.Data, helpers test.Helpers) *test.Expected {
return &test.Expected{
ExitCode: 0,
Errors: []error{},
Output: func(stdout string, info string, t *testing.T) {
helpers.Command("images", data.Get(tagIDKey)).Run(&test.Expected{
ExitCode: 0,
Output: func(stdout string, info string, t *testing.T) {
assert.Equal(t, len(strings.Split(stdout, "\n")), 2)
},
})
},
}
},
},
}

testCase.Run(t)
}
15 changes: 11 additions & 4 deletions pkg/cmd/image/remove.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,18 @@ func Remove(ctx context.Context, client *containerd.Client, args []string, optio
walker := &imagewalker.ImageWalker{
Client: client,
OnFound: func(ctx context.Context, found imagewalker.Found) error {
// if found multiple images, return error unless in force-mode and
// there is only 1 unique image.
if found.MatchCount > 1 && !(options.Force && found.UniqueImages == 1) {
return fmt.Errorf("multiple IDs found with provided prefix: %s", found.Req)
if found.NameMatchIndex == -1 {
// if found multiple images, return error unless in force-mode and
// there is only 1 unique image.
if found.MatchCount > 1 && !(options.Force && found.UniqueImages == 1) {
return fmt.Errorf("multiple IDs found with provided prefix: %s", found.Req)
}
} else if found.NameMatchIndex != found.MatchIndex {
// when there is an image with a name matching the argument but the argument is a digest short id,
// the deletion process is not performed.
return nil
}

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)
}
Expand Down
37 changes: 24 additions & 13 deletions pkg/idutil/imagewalker/imagewalker.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,12 @@ import (
)

type Found struct {
Image images.Image
Req string // The raw request string. name, short ID, or long ID.
MatchIndex int // Begins with 0, up to MatchCount - 1.
MatchCount int // 1 on exact match. > 1 on ambiguous match. Never be <= 0.
UniqueImages int // Number of unique images in all found images.
Image images.Image
Req string // The raw request string. name, short ID, or long ID.
MatchIndex int // Begins with 0, up to MatchCount - 1.
MatchCount int // 1 on exact match. > 1 on ambiguous match. Never be <= 0.
UniqueImages int // Number of unique images in all found images.
NameMatchIndex int // Image index with a name matching the argument for `nerdctl rmi`.
}

type OnFound func(ctx context.Context, found Found) error
Expand All @@ -50,8 +51,12 @@ type ImageWalker struct {
// Returns the number of the found entries.
func (w *ImageWalker) Walk(ctx context.Context, req string) (int, error) {
var filters []string
if parsedReference, err := referenceutil.Parse(req); err == nil {
filters = append(filters, fmt.Sprintf("name==%s", parsedReference.String()))
var parsedReferenceStr string

parsedReference, err := referenceutil.Parse(req)
if err == nil {
parsedReferenceStr = parsedReference.String()
filters = append(filters, fmt.Sprintf("name==%s", parsedReferenceStr))
}
filters = append(filters,
fmt.Sprintf("name==%s", req),
Expand All @@ -68,17 +73,23 @@ func (w *ImageWalker) Walk(ctx context.Context, req string) (int, error) {
// to handle the `rmi -f` case where returned images are different but
// have the same short prefix.
uniqueImages := make(map[digest.Digest]bool)
for _, image := range images {
nameMatchIndex := -1
for i, image := range images {
uniqueImages[image.Target.Digest] = true
// to get target image index for `nerdctl rmi <short digest ids of another images>`.
if (parsedReferenceStr != "" && image.Name == parsedReferenceStr) || image.Name == req {
nameMatchIndex = i
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it possible name match happens on multiple images?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your feedback!

Yes, name matching can occur for multiple images if they share a similar prefix.
For example, when there are several alpine images with different tags, running nerdctl rmi alpine will specifically remove alpine:latest by default:

> nerdctl images | grep alpine
alpine                              latest      beefdbd8a1da    About a minute ago    linux/arm64    10.46MB    4.09MB
alpine                              20190408    8b6b8c0f71e8    16 minutes ago        linux/arm64    6.844MB    2.708MB
alpine                              20190228    6199d795f07e    21 minutes ago        linux/arm64    6.844MB    2.706MB
ghcr.io/stargz-containers/alpine    3.13-org    ec14c7992a97    19 hours ago          linux/arm64    6.865MB    2.714MB

> nerdctl rmi alpine
Untagged: docker.io/library/alpine:latest@sha256:beefdbd8a1da6d2915566fde36db9db0b524eb737fc57cd1367effd16dc0d06d
Deleted: sha256:16113d51b7181f20135f51e8ffbaead20a7671cd783017601f198748ce8a8ebf

> nerdctl images | grep alpine
alpine                              20190408    8b6b8c0f71e8    22 minutes ago    linux/arm64    6.844MB    2.708MB
alpine                              20190228    6199d795f07e    27 minutes ago    linux/arm64    6.844MB    2.706MB
ghcr.io/stargz-containers/alpine    3.13-org    ec14c7992a97    19 hours ago      linux/arm64    6.865MB    2.714MB

However, if an image matches the given prefix, such as alpine:custom, the image must be removed by specifying alpine:<tag name>.

Does this example and explanation address your concern?

}
}

for i, img := range images {
f := Found{
Image: img,
Req: req,
MatchIndex: i,
MatchCount: matchCount,
UniqueImages: len(uniqueImages),
Image: img,
Req: req,
MatchIndex: i,
MatchCount: matchCount,
UniqueImages: len(uniqueImages),
NameMatchIndex: nameMatchIndex,
}
if e := w.OnFound(ctx, f); e != nil {
return -1, e
Expand Down
Loading