Skip to content

Commit

Permalink
Merge pull request #3519 from haytok/issue_3016
Browse files Browse the repository at this point in the history
fix: Allow to delete images when names of images are short digest ids…
  • Loading branch information
AkihiroSuda authored Oct 20, 2024
2 parents fad0285 + c3627e1 commit 07cb00b
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 17 deletions.
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
}
}

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

0 comments on commit 07cb00b

Please sign in to comment.