From 530b764b27288470af0039d55c2e9a36440e8eb9 Mon Sep 17 00:00:00 2001 From: Hayato Kiwata Date: Tue, 8 Oct 2024 23:30:01 +0900 Subject: [PATCH] fix: Allow to delete images when names of images are short digest ids of another images. "nerdctl rmi " can be run to delete the target images. However, at the current implementation, the deletion fails when images names are the short digest ids of another images. The specific behavior is described below, which is how it works in the current implementation. First, suppose there are alpine and busybox images. ``` > nerdctl images REPOSITORY TAG IMAGE ID CREATED PLATFORM SIZE BLOB SIZE busybox latest 768e5c6f5cb6 3 seconds ago linux/arm64 4.092MB 1.845MB alpine latest beefdbd8a1da 11 seconds ago linux/arm64 10.46MB 4.09MB ``` Then, we tag the alpine image using digest id of the busybox image. ``` > nerdctl tag alpine $(dn inspect busybox | jq -rc .[0].RepoDigests[0] | awk -F':' '{print substr($2, 1, 8)}') > nerdctl images REPOSITORY TAG IMAGE ID CREATED PLATFORM SIZE BLOB SIZE 768e5c6f latest beefdbd8a1da 4 seconds ago linux/arm64 10.46MB 4.09MB busybox latest 768e5c6f5cb6 22 hours ago linux/arm64 4.092MB 1.845MB alpine latest beefdbd8a1da 22 hours ago linux/arm64 10.46MB 4.09MB ``` In this situation, running 'nerdctl rmi "$(dn inspect busybox | jq -rc .[0].RepoDigests[0] | awk -F':' '{print substr($2, 1, 8)}')"' will fail to remove the image. The details of the error are as follows. ``` > nerdctl rmi "$(dn inspect busybox | jq -rc .[0].RepoDigests[0] | awk -F':' '{print substr($2, 1, 8)}')" FATA[0000] 1 errors: multiple IDs found with provided prefix: 768e5c6f ``` This issue is reported in the following issue. - https://github.com/containerd/nerdctl/issues/3016 Therefore, this pull request modifies this so that images can be deleted with "nerdctl rmi " when images names are the short digest ids of another images. Signed-off-by: Hayato Kiwata --- cmd/nerdctl/image/image_remove_test.go | 53 ++++++++++++++++++++++++++ pkg/cmd/image/remove.go | 15 ++++++-- pkg/idutil/imagewalker/imagewalker.go | 37 +++++++++++------- 3 files changed, 88 insertions(+), 17 deletions(-) diff --git a/cmd/nerdctl/image/image_remove_test.go b/cmd/nerdctl/image/image_remove_test.go index 85b665f350f..eb064caade7 100644 --- a/cmd/nerdctl/image/image_remove_test.go +++ b/cmd/nerdctl/image/image_remove_test.go @@ -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" @@ -302,3 +305,53 @@ 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) + }, + Cleanup: func(data test.Data, helpers test.Helpers) { + helpers.Anyhow("rmi", "-f", testutil.CommonImage) + helpers.Anyhow("rmi", "-f", testutil.NginxAlpineImage) + }, + 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) +} diff --git a/pkg/cmd/image/remove.go b/pkg/cmd/image/remove.go index 09d76b90e69..3afa2bff80b 100644 --- a/pkg/cmd/image/remove.go +++ b/pkg/cmd/image/remove.go @@ -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) } diff --git a/pkg/idutil/imagewalker/imagewalker.go b/pkg/idutil/imagewalker/imagewalker.go index df948a48f01..4103711ed91 100644 --- a/pkg/idutil/imagewalker/imagewalker.go +++ b/pkg/idutil/imagewalker/imagewalker.go @@ -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 @@ -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), @@ -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 `. + 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