Skip to content

Commit

Permalink
Fix duplicate image entries in k8s.io namespaces
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
fengwei0328 committed Dec 19, 2024
1 parent 1f81225 commit 8da215c
Show file tree
Hide file tree
Showing 9 changed files with 365 additions and 4 deletions.
5 changes: 5 additions & 0 deletions cmd/nerdctl/helpers/flagutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -118,6 +122,7 @@ func ProcessRootCmdFlags(cmd *cobra.Command) (types.GlobalCommandOptions, error)
Experimental: experimental,
HostGatewayIP: hostGatewayIP,
BridgeIP: bridgeIP,
KubeHideDupe: kubeHideDupe,
}, nil
}

Expand Down
36 changes: 36 additions & 0 deletions cmd/nerdctl/image/image_list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -317,3 +317,39 @@ CMD ["echo", "nerdctl-build-notag-string"]

testCase.Run(t)
}

func TestImagesKubeWithKubeHideDupe(t *testing.T) {
nerdtest.Setup()

testCase := &test.Case{
Require: test.Require(
nerdtest.OnlyKubernetes,
test.Not(nerdtest.Docker),
),
Setup: func(data test.Data, helpers test.Helpers) {
helpers.Ensure("pull", "--quiet", testutil.NginxAlpineImage)
},
SubTests: []*test.Case{
{
Description: "the same imageId will not print no-repo:tag in k8s.io with kube-hide-dupe",
Command: test.Command("--kube-hide-dupe", "images"),
Expected: func(data test.Data, helpers test.Helpers) *test.Expected {
return &test.Expected{
Output: test.DoesNotContain("<none>"),
}
},
},
{
Description: "the same imageId will print no-repo:tag in k8s.io without kube-hide-dupe",
Command: test.Command("images"),
Expected: func(data test.Data, helpers test.Helpers) *test.Expected {
return &test.Expected{
Output: test.Contains("<none>"),
}
},
},
},
}

testCase.Run(t)
}
107 changes: 107 additions & 0 deletions cmd/nerdctl/image/image_remove_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -351,3 +351,110 @@ func TestIssue3016(t *testing.T) {

testCase.Run(t)
}

func TestRemoveKubeWithKubeHideDupe(t *testing.T) {
var numTags, numNoTags int
testCase := nerdtest.Setup()
testCase.Cleanup = func(data test.Data, helpers test.Helpers) {
helpers.Anyhow("--kube-hide-dupe", "rmi", "-f", testutil.NginxAlpineImage)
numTags = len(strings.Split(strings.TrimSpace(helpers.Capture("--kube-hide-dupe", "images")), "\n"))
numNoTags = len(strings.Split(strings.TrimSpace(helpers.Capture("images")), "\n"))
}
testCase.Require = test.Require(
nerdtest.OnlyKubernetes,
test.Not(nerdtest.Docker),
)
testCase.SubTests = []*test.Case{
{
Description: "After removing the tag without kube-hide-dupe, repodigest is shown as <none>",
NoParallel: true,
Setup: func(data test.Data, helpers test.Helpers) {
helpers.Ensure("pull", testutil.NginxAlpineImage)
},
Command: test.Command("rmi", "-f", 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("--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) == numTags+1, info)
},
})
helpers.Command("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) == numNoTags+1, info)
},
})
},
}
},
},
{
Description: "If there are other tags, the Repodigest will not be deleted",
NoParallel: true,
Cleanup: func(data test.Data, helpers test.Helpers) {
helpers.Anyhow("--kube-hide-dupe", "rmi", data.Identifier())
},
Setup: func(data test.Data, helpers test.Helpers) {
helpers.Ensure("pull", testutil.NginxAlpineImage)
helpers.Ensure("tag", testutil.NginxAlpineImage, data.Identifier())
},
Command: test.Command("--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("--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) == numTags+1, info)
},
})
helpers.Command("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) == numNoTags+2, info)
},
})
},
}
},
},
{
Description: "After deleting all repo:tag entries, all repodigests will be cleaned up",
NoParallel: true,
Setup: func(data test.Data, helpers test.Helpers) {
helpers.Ensure("pull", testutil.NginxAlpineImage)
helpers.Ensure("tag", testutil.NginxAlpineImage, data.Identifier())
},
Command: func(data test.Data, helpers test.Helpers) test.TestableCommand {
helpers.Ensure("--kube-hide-dupe", "rmi", "-f", testutil.NginxAlpineImage)
return helpers.Command("--kube-hide-dupe", "rmi", "-f", data.Identifier())
},
Expected: func(data test.Data, helpers test.Helpers) *test.Expected {
return &test.Expected{
Output: func(stdout string, info string, t *testing.T) {
helpers.Command("--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) == numTags, info)
},
})
helpers.Command("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) == numNoTags, info)
},
})
},
}
},
},
}
testCase.Run(t)
}
1 change: 1 addition & 0 deletions cmd/nerdctl/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
1 change: 1 addition & 0 deletions docs/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ experimental = true
| `experimental` | `--experimental` | `NERDCTL_EXPERIMENTAL` | Enable [experimental features](experimental.md) | Since 0.22.3 |
| `host_gateway_ip` | `--host-gateway-ip` | `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 | Since 1.3.0 |
| `bridge_ip` | `--bridge-ip` | `NERDCTL_BRIDGE_IP` | IP address for the default nerdctl bridge network, e.g., 10.1.100.1/24 | Since 2.0.1 |
| `kube-hide-dupe` | `--kube-hide-dupe` | | Deduplicate images for Kubernetes with namespace k8s.io, no more redundant <none> ones are displayed | Since 2.0.3 |

The properties are parsed in the following precedence:
1. CLI flag
Expand Down
44 changes: 43 additions & 1 deletion pkg/cmd/image/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@ import (
"text/template"
"time"

"github.com/distribution/reference"
"github.com/docker/go-units"
"github.com/opencontainers/go-digest"
"github.com/opencontainers/image-spec/identity"
ocispec "github.com/opencontainers/image-spec/specs-go/v1"

Expand Down Expand Up @@ -128,6 +130,46 @@ 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 under k8s.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" {
imageDigest := make(map[digest.Digest]bool)
var ImageNoTag []images.Image
for _, img := range imageList {
parsed, err := reference.ParseAnyReference(img.Name)
if err != nil {
continue
}
if _, ok := parsed.(reference.Tagged); !ok {
ImageNoTag = append(ImageNoTag, img)
continue
}
ImageList = append(ImageList, img)
imageDigest[img.Target.Digest] = true
}
//Ensure that dangling images without a repo:tag are displayed correctly.
for _, ima := range ImageNoTag {
if !imageDigest[ima.Target.Digest] {
ImageList = append(ImageList, ima)
imageDigest[ima.Target.Digest] = true
}
}
} else {
ImageList = imageList
}
digestsFlag := options.Digests
if options.Format == "wide" {
digestsFlag = true
Expand Down Expand Up @@ -174,7 +216,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)
}
Expand Down
54 changes: 53 additions & 1 deletion pkg/cmd/image/remove.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 2 additions & 0 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -59,5 +60,6 @@ func New() *Config {
HostsDir: ncdefaults.HostsDirs(),
Experimental: true,
HostGatewayIP: ncdefaults.HostGatewayIP(),
KubeHideDupe: false,
}
}
Loading

0 comments on commit 8da215c

Please sign in to comment.