Fix duplicate image entries in k8s.io namespaces

The same imageId underk8s.io is showing multiple results: repo:tag, repo:digest, configID. We expect to display only repo:tag, consistent with other namespaces and CRI. e.g. nerdctl -n k8s.io images REPOSITORY TAG IMAGE ID CREATED PLATFORM SIZE BLOB SIZE centos 7 be65f488b776 3 hours ago linux/amd64 211.5 MiB 72.6 MiB centos <none> be65f488b776 3 hours ago linux/amd64 211.5 MiB 72.6 MiB <none> <none> be65f488b776 3 hours ago linux/amd64 211.5 MiB 72.6 MiB expect: nerdctl --kube_hide_dupe -n k8s.io images REPOSITORY TAG IMAGE ID CREATED PLATFORM SIZE BLOB SIZE centos 7 be65f488b776 3 hours ago linux/amd64 211.5 MiB 72.6 MiB Of course, even after deduplicating the images displayed, there are still issues with deleting the images. It is necessary to distinguish between repo:tag and configId, as well as repoDigest. Considering the situation with tags, we need to ensure that all repo:tags under the same imageId are cleaned up before proceeding to clean up the configId and repoDigest. see: #3702 Signed-off-by: fengwei0328 <[email protected]>
containerd · Dec 17, 2024 · bf3dfaa · bf3dfaa
1 parent 1f81225
commit bf3dfaa
Show file tree

Hide file tree

Showing 8 changed files with 382 additions and 4 deletions.
diff --git a/cmd/nerdctl/helpers/flagutil.go b/cmd/nerdctl/helpers/flagutil.go
@@ -103,6 +103,10 @@ func ProcessRootCmdFlags(cmd *cobra.Command) (types.GlobalCommandOptions, error)
 	if err != nil {
 		return types.GlobalCommandOptions{}, err
 	}
+	kubeHideDupe, err := cmd.Flags().GetBool("kube-hide-dupe")
+	if err != nil {
+		return types.GlobalCommandOptions{}, err
+	}
 	return types.GlobalCommandOptions{
 		Debug:            debug,
 		DebugFull:        debugFull,
@@ -118,6 +122,7 @@ func ProcessRootCmdFlags(cmd *cobra.Command) (types.GlobalCommandOptions, error)
 		Experimental:     experimental,
 		HostGatewayIP:    hostGatewayIP,
 		BridgeIP:         bridgeIP,
+		KubeHideDupe:     kubeHideDupe,
 	}, nil
 }
 

diff --git a/cmd/nerdctl/image/image_list_test.go b/cmd/nerdctl/image/image_list_test.go
@@ -317,3 +317,54 @@ CMD ["echo", "nerdctl-build-notag-string"]
 
 	testCase.Run(t)
 }
+
+func TestImagesCri(t *testing.T) {
+	testCase := nerdtest.Setup()
+
+	testCase.SubTests = []*test.Case{
+		{
+			Description: "the same imageId will not be printed no-repo:tag in k8s.io",
+			Require: test.Require(
+				test.Not(nerdtest.Docker),
+			),
+			NoParallel: true,
+			Setup: func(data test.Data, helpers test.Helpers) {
+				helpers.Ensure("--namespace=k8s.io", "pull", testutil.BusyboxImage)
+				helpers.Ensure("--namespace=k8s.io", "pull", testutil.NginxAlpineImage)
+			},
+			Cleanup: func(data test.Data, helpers test.Helpers) {
+				helpers.Anyhow("--namespace=k8s.io", "rmi", testutil.BusyboxImage)
+				helpers.Anyhow("--namespace=k8s.io", "rmi", testutil.NginxAlpineImage)
+			},
+			Command: test.Command("--namespace=k8s.io", "--kube-hide-dupe", "images"),
+			Expected: func(data test.Data, helpers test.Helpers) *test.Expected {
+				return &test.Expected{
+					Output: func(stdout string, info string, t *testing.T) {
+						lines := strings.Split(strings.TrimSpace(stdout), "\n")
+						assert.Assert(t, len(lines) == 3, info)
+						header := "REPOSITORY\tTAG\tIMAGE ID\tCREATED\tPLATFORM\tSIZE\tBLOB SIZE"
+						if nerdtest.IsDocker() {
+							header = "REPOSITORY\tTAG\tIMAGE ID\tCREATED\tSIZE"
+						}
+						tab := tabutil.NewReader(header)
+						err := tab.ParseHeader(lines[0])
+						assert.NilError(t, err, info)
+						found := true
+						for _, line := range lines[1:] {
+							repo, _ := tab.ReadRow(line, "REPOSITORY")
+							tag, _ := tab.ReadRow(line, "TAG")
+							imageTag := repo + ":" + tag
+							if imageTag == testutil.BusyboxImage || imageTag == testutil.NginxAlpineImage {
+								continue
+							}
+							found = false
+							break
+						}
+						assert.Assert(t, found, info)
+					},
+				}
+			},
+		},
+	}
+	testCase.Run(t)
+}
diff --git a/cmd/nerdctl/image/image_remove_test.go b/cmd/nerdctl/image/image_remove_test.go
@@ -24,6 +24,7 @@ import (
 	"gotest.tools/v3/assert"
 
 	"github.com/containerd/nerdctl/v2/pkg/imgutil"
+	"github.com/containerd/nerdctl/v2/pkg/tabutil"
 	"github.com/containerd/nerdctl/v2/pkg/testutil"
 	"github.com/containerd/nerdctl/v2/pkg/testutil/nerdtest"
 	"github.com/containerd/nerdctl/v2/pkg/testutil/test"
@@ -351,3 +352,117 @@ func TestIssue3016(t *testing.T) {
 
 	testCase.Run(t)
 }
+
+func TestRemoveCri(t *testing.T) {
+	testCase := nerdtest.Setup()
+	header := "REPOSITORY\tTAG\tIMAGE ID\tCREATED\tPLATFORM\tSIZE\tBLOB SIZE"
+	if nerdtest.IsDocker() {
+		header = "REPOSITORY\tTAG\tIMAGE ID\tCREATED\tSIZE"
+	}
+
+	testCase.SubTests = []*test.Case{
+		{
+			Require:     test.Not(nerdtest.Docker),
+			Description: "After removing the tag, repodigest cleans it up together",
+			NoParallel:  true,
+			Setup: func(data test.Data, helpers test.Helpers) {
+				helpers.Ensure("--namespace=k8s.io", "pull", testutil.CommonImage)
+			},
+			Cleanup: func(data test.Data, helpers test.Helpers) {
+				helpers.Anyhow("--namespace=k8s.io", "--kube-hide-dupe", "rmi", "-f", testutil.CommonImage)
+			},
+			Command: test.Command("--namespace=k8s.io", "--kube-hide-dupe", "rmi", "-f", testutil.CommonImage),
+			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("--namespace=k8s.io", "--kube-hide-dupe", "images").Run(&test.Expected{
+							Output: func(stdout string, info string, t *testing.T) {
+								lines := strings.Split(strings.TrimSpace(stdout), "\n")
+								assert.Assert(t, len(lines) == 1, info)
+							},
+						})
+						helpers.Command("--namespace=k8s.io", "images").Run(&test.Expected{
+							Output: test.Contains("<none>"),
+						})
+					},
+				}
+			},
+		},
+		{
+			Require:     test.Not(nerdtest.Docker),
+			Description: "If there are other tags, the Repodigest will not be deleted",
+			NoParallel:  true,
+			Setup: func(data test.Data, helpers test.Helpers) {
+				helpers.Ensure("--namespace=k8s.io", "pull", testutil.NginxAlpineImage)
+				helpers.Ensure("--namespace=k8s.io", "tag", testutil.NginxAlpineImage, data.Identifier())
+			},
+			Cleanup: func(data test.Data, helpers test.Helpers) {
+				helpers.Anyhow("--namespace=k8s.io", "--kube-hide-dupe", "rmi", "-f", data.Identifier())
+				helpers.Anyhow("--namespace=k8s.io", "--kube-hide-dupe", "rmi", "-f", testutil.NginxAlpineImage)
+			},
+			Command: test.Command("--namespace=k8s.io", "--kube-hide-dupe", "rmi", testutil.NginxAlpineImage),
+			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("--namespace=k8s.io", "--kube-hide-dupe", "images").Run(&test.Expected{
+							Output: func(stdout string, info string, t *testing.T) {
+								lines := strings.Split(strings.TrimSpace(stdout), "\n")
+								assert.Assert(t, len(lines) == 2, info)
+							},
+						})
+						helpers.Command("--namespace=k8s.io", "images").Run(&test.Expected{
+							Output: test.Contains("<none>"),
+						})
+					},
+				}
+			},
+		},
+		{
+			Require:     test.Not(nerdtest.Docker),
+			Description: "After deleting a repo:tag, repodigest does not clean up when there are other repo:tag with the same imageID",
+			NoParallel:  true,
+			Setup: func(data test.Data, helpers test.Helpers) {
+				helpers.Ensure("--namespace=k8s.io", "pull", testutil.NginxAlpineImage)
+				helpers.Ensure("--namespace=k8s.io", "tag", testutil.NginxAlpineImage, data.Identifier())
+			},
+			Cleanup: func(data test.Data, helpers test.Helpers) {
+				helpers.Anyhow("--namespace=k8s.io", "--kube-hide-dupe", "rmi", "-f", data.Identifier())
+				helpers.Anyhow("--namespace=k8s.io", "--kube-hide-dupe", "rmi", "-f", testutil.NginxAlpineImage)
+			},
+			Command: test.Command("--namespace=k8s.io", "--kube-hide-dupe", "rmi", testutil.NginxAlpineImage),
+			Expected: func(data test.Data, helpers test.Helpers) *test.Expected {
+				return &test.Expected{
+					Output: func(stdout string, info string, t *testing.T) {
+						helpers.Command("--namespace=k8s.io", "--kube-hide-dupe", "images").Run(&test.Expected{
+							Output: func(stdout string, info string, t *testing.T) {
+								lines := strings.Split(strings.TrimSpace(stdout), "\n")
+								assert.Assert(t, len(lines) == 2, info)
+								tab := tabutil.NewReader(header)
+								err := tab.ParseHeader(lines[0])
+								assert.NilError(t, err, info)
+								found := true
+								for _, line := range lines[1:] {
+									repo, _ := tab.ReadRow(line, "REPOSITORY")
+									tag, _ := tab.ReadRow(line, "TAG")
+									if repo+":"+tag != data.Identifier()+":"+"latest" {
+										found = false
+										break
+									}
+								}
+								assert.Assert(t, found, info)
+							},
+						})
+						helpers.Command("--namespace=k8s.io", "images").Run(&test.Expected{
+							Output: test.Contains("<none>"),
+						})
+					},
+				}
+			},
+		},
+	}
+	testCase.Run(t)
+}
diff --git a/cmd/nerdctl/main.go b/cmd/nerdctl/main.go
@@ -184,6 +184,7 @@ func initRootCmdFlags(rootCmd *cobra.Command, tomlPath string) (*pflag.FlagSet,
 	helpers.AddPersistentBoolFlag(rootCmd, "experimental", nil, nil, cfg.Experimental, "NERDCTL_EXPERIMENTAL", "Control experimental: https://github.com/containerd/nerdctl/blob/main/docs/experimental.md")
 	helpers.AddPersistentStringFlag(rootCmd, "host-gateway-ip", nil, nil, nil, aliasToBeInherited, cfg.HostGatewayIP, "NERDCTL_HOST_GATEWAY_IP", "IP address that the special 'host-gateway' string in --add-host resolves to. Defaults to the IP address of the host. It has no effect without setting --add-host")
 	helpers.AddPersistentStringFlag(rootCmd, "bridge-ip", nil, nil, nil, aliasToBeInherited, cfg.BridgeIP, "NERDCTL_BRIDGE_IP", "IP address for the default nerdctl bridge network")
+	rootCmd.PersistentFlags().Bool("kube-hide-dupe", cfg.KubeHideDupe, "Deduplicate images for Kubernetes with namespace k8s.io")
 	return aliasToBeInherited, nil
 }
 

diff --git a/pkg/cmd/image/list.go b/pkg/cmd/image/list.go
@@ -29,6 +29,7 @@ import (
 	"text/template"
 	"time"
 
+	"github.com/distribution/reference"
 	"github.com/docker/go-units"
 	"github.com/opencontainers/image-spec/identity"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
@@ -128,6 +129,35 @@ type imagePrintable struct {
 
 func printImages(ctx context.Context, client *containerd.Client, imageList []images.Image, options *types.ImageListOptions) error {
 	w := options.Stdout
+	var ImageList []images.Image
+	/*
+		the same imageId underk8s.io is showing multiple results: repo:tag, repo:digest, configID.
+		We expect to display only repo:tag, consistent with other namespaces and CRI.
+		e.g.
+		nerdctl -n k8s.io images
+		REPOSITORY    TAG       IMAGE ID        CREATED        PLATFORM       SIZE         BLOB SIZE
+		centos        7         be65f488b776    3 hours ago    linux/amd64    211.5 MiB    72.6 MiB
+		centos        <none>    be65f488b776    3 hours ago    linux/amd64    211.5 MiB    72.6 MiB
+		<none>        <none>    be65f488b776    3 hours ago    linux/amd64    211.5 MiB    72.6 MiB
+		expect:
+		nerdctl --kube_hide_dupe -n k8s.io images
+		REPOSITORY    TAG       IMAGE ID        CREATED        PLATFORM       SIZE         BLOB SIZE
+		centos        7         be65f488b776    3 hours ago    linux/amd64    211.5 MiB    72.6 MiB
+	*/
+	if options.GOptions.KubeHideDupe && options.GOptions.Namespace == "k8s.io" {
+		for _, ima := range imageList {
+			parsed, err := reference.ParseAnyReference(ima.Name)
+			if err != nil {
+				continue
+			}
+			if _, ok := parsed.(reference.Tagged); !ok {
+				continue
+			}
+			ImageList = append(ImageList, ima)
+		}
+	} else {
+		ImageList = imageList
+	}
 	digestsFlag := options.Digests
 	if options.Format == "wide" {
 		digestsFlag = true
@@ -174,7 +204,7 @@ func printImages(ctx context.Context, client *containerd.Client, imageList []ima
 		snapshotter: containerdutil.SnapshotService(client, options.GOptions.Snapshotter),
 	}
 
-	for _, img := range imageList {
+	for _, img := range ImageList {
 		if err := printer.printImage(ctx, img); err != nil {
 			log.G(ctx).Warn(err)
 		}

diff --git a/pkg/cmd/image/remove.go b/pkg/cmd/image/remove.go
@@ -111,12 +111,64 @@ func Remove(ctx context.Context, client *containerd.Client, args []string, optio
 			}
 			return nil
 		},
+		OnFoundCriRm: func(ctx context.Context, found imagewalker.Found) (bool, error) {
+			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 false, 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 false, nil
+			}
+
+			if cid, ok := runningImages[found.Image.Name]; ok {
+				if options.Force {
+					if err = is.Delete(ctx, found.Image.Name); err != nil {
+						return false, err
+					}
+					fmt.Fprintf(options.Stdout, "Untagged: %s\n", found.Image.Name)
+					fmt.Fprintf(options.Stdout, "Untagged: %s\n", found.Image.Target.Digest.String())
+
+					found.Image.Name = ":"
+					if _, err = is.Create(ctx, found.Image); err != nil {
+						return false, err
+					}
+					return false, nil
+				}
+				return false, fmt.Errorf("conflict: unable to delete %s (cannot be forced) - image is being used by running container %s", found.Req, cid)
+			}
+			if cid, ok := usedImages[found.Image.Name]; ok && !options.Force {
+				return false, fmt.Errorf("conflict: unable to delete %s (must be forced) - image is being used by stopped container %s", found.Req, cid)
+			}
+			// digests is used only for emulating human-readable output of `docker rmi`
+			digests, err := found.Image.RootFS(ctx, cs, platforms.DefaultStrict())
+			if err != nil {
+				log.G(ctx).WithError(err).Warning("failed to enumerate rootfs")
+			}
+
+			if err := is.Delete(ctx, found.Image.Name, delOpts...); err != nil {
+				return false, err
+			}
+			fmt.Fprintf(options.Stdout, "Untagged: %s@%s\n", found.Image.Name, found.Image.Target.Digest)
+			for _, digest := range digests {
+				fmt.Fprintf(options.Stdout, "Deleted: %s\n", digest)
+			}
+			return true, nil
+		},
 	}
 
 	var errs []string
 	var fatalErr bool
 	for _, req := range args {
-		n, err := walker.Walk(ctx, req)
+		var n int
+		if options.GOptions.KubeHideDupe && options.GOptions.Namespace == "k8s.io" {
+			n, err = walker.WalkCriRm(ctx, req)
+		} else {
+			n, err = walker.Walk(ctx, req)
+		}
 		if err != nil {
 			fatalErr = true
 		}

diff --git a/pkg/config/config.go b/pkg/config/config.go
@@ -40,6 +40,7 @@ type Config struct {
 	Experimental     bool     `toml:"experimental"`
 	HostGatewayIP    string   `toml:"host_gateway_ip"`
 	BridgeIP         string   `toml:"bridge_ip, omitempty"`
+	KubeHideDupe     bool     `toml:"kube_hide_dupe"`
 }
 
 // New creates a default Config object statically,
@@ -59,5 +60,6 @@ func New() *Config {
 		HostsDir:         ncdefaults.HostsDirs(),
 		Experimental:     true,
 		HostGatewayIP:    ncdefaults.HostGatewayIP(),
+		KubeHideDupe:     false,
 	}
 }