From 9d1ff9247125ad52e15a84b8e5e4e0865d632d83 Mon Sep 17 00:00:00 2001 From: Eugene Kalinin Date: Wed, 5 Aug 2026 21:23:48 +0300 Subject: [PATCH] fix(image): pin the image a container runs to its digest `nerdctl images` marks an image as in use by resolving the image name stored on the container, which follows the tag wherever it points now. After `nerdctl tag` moves a tag onto another image, the container gets attributed to an image it never ran: the U indicator lands on the wrong row. Record the image target digest on the container at creation time, in a new nerdctl/image-digest label, and use it for the in-use lookup. Containers created before this label existed, or created outside nerdctl, are still resolved by name; an unparsable value falls back the same way rather than dropping the container from the set. That digest is also what `nerdctl inspect` now reports as Image, where Docker reports the image ID: with the containerd image store that ID is the digest of the image target (moby daemon/containerd/image.go, image.ID(img.Target.Digest)), pinned on the container when it is created. nerdctl used to report the image name there, which a retag moves just the same. The reference the user asked for stays in Config.Image, as it does in Docker. This also matters for the ACTIVE and RECLAIMABLE columns of `nerdctl system df`, which build on the same lookup. Signed-off-by: Eugene Kalinin --- .../container/container_inspect_linux_test.go | 8 ++- cmd/nerdctl/image/image_list_test.go | 62 +++++++++++++++++++ pkg/cmd/container/create.go | 12 ++++ pkg/cmd/image/list.go | 44 +++++++++++-- pkg/cmd/image/list_test.go | 45 ++++++++++++++ pkg/inspecttypes/dockercompat/dockercompat.go | 20 +++++- .../dockercompat/dockercompat_test.go | 30 +++++++++ pkg/labels/labels.go | 5 ++ 8 files changed, 219 insertions(+), 7 deletions(-) diff --git a/cmd/nerdctl/container/container_inspect_linux_test.go b/cmd/nerdctl/container/container_inspect_linux_test.go index 6be9a2fd510..b80387ee07c 100644 --- a/cmd/nerdctl/container/container_inspect_linux_test.go +++ b/cmd/nerdctl/container/container_inspect_linux_test.go @@ -278,7 +278,7 @@ func TestContainerInspectConfigImage(t *testing.T) { nerdtest.Setup() testCase := &test.Case{ - Description: "Container inspect contains Config.Image field", + Description: "Container inspect names the image by digest, and by reference in Config.Image", Setup: func(data test.Data, helpers test.Helpers) { helpers.Ensure("run", "-d", "--name", data.Identifier(), testutil.AlpineImage, "sleep", nerdtest.Infinity) }, @@ -297,6 +297,12 @@ func TestContainerInspectConfigImage(t *testing.T) { container := containers[0] assert.Assert(tt, container.Config != nil, "container Config should not be nil") assert.Assert(tt, container.Config.Image != "", "Config.Image should not be empty") + // Docker identifies the image a container runs by digest, pinned at creation, and + // keeps the reference the user asked for in Config.Image. + assert.Assert(tt, strings.HasPrefix(container.Image, "sha256:"), + "Image should be a digest, got %q", container.Image) + assert.Assert(tt, !strings.HasPrefix(container.Config.Image, "sha256:"), + "Config.Image should be a reference, got %q", container.Config.Image) }), } diff --git a/cmd/nerdctl/image/image_list_test.go b/cmd/nerdctl/image/image_list_test.go index f9f5ad3a594..4281b0339a1 100644 --- a/cmd/nerdctl/image/image_list_test.go +++ b/cmd/nerdctl/image/image_list_test.go @@ -38,6 +38,17 @@ import ( "github.com/containerd/nerdctl/v2/pkg/testutil/nerdtest" ) +// padRow widens a row of a table back to the width of its header, so that its last column can be +// read. tabutil indexes the columns by byte offset and slices without checking the bounds, and a +// row can be shorter than the header in two ways: the trailing column is empty, and the padding of +// the very last line is gone once the output has been trimmed. +func padRow(header, row string) string { + if pad := len(header) - len(row); pad > 0 { + return row + strings.Repeat(" ", pad) + } + return row +} + // TestNameFilterFor is a regression test for // https://github.com/containerd/nerdctl/issues/5113: `nerdctl image ls // myapp`, where myapp is a bare repository name, returned nothing unless @@ -207,6 +218,57 @@ func TestImages(t *testing.T) { } }, }, + { + Description: "In use survives a retag", + Setup: func(data test.Data, helpers test.Helpers) { + // Run a container off a private tag, then move that tag onto another image. + // The container still runs the original image, so that is the one that must + // stay marked as in use. + helpers.Ensure("tag", commonImage.String(), data.Identifier()+":moving") + helpers.Ensure("run", "-d", "--quiet", "--name", data.Identifier(), + data.Identifier()+":moving", "sleep", nerdtest.Infinity) + helpers.Ensure("tag", testutil.NginxAlpineImage, data.Identifier()+":moving") + + nginx, _ := referenceutil.Parse(testutil.NginxAlpineImage) + data.Labels().Set("retaggedTo", nginx.FamiliarName()+":"+nginx.Tag) + }, + Cleanup: func(data test.Data, helpers test.Helpers) { + helpers.Anyhow("rm", "-f", data.Identifier()) + helpers.Anyhow("rmi", "-f", data.Identifier()+":moving") + }, + Command: test.Command("images"), + Expected: func(data test.Data, helpers test.Helpers) *test.Expected { + return &test.Expected{ + Output: func(stdout string, t tig.T) { + lines := strings.Split(strings.TrimSpace(stdout), "\n") + assert.Assert(t, len(lines) >= 2, "there should be at least two lines\n") + tab := tabutil.NewReader("IMAGE\tID\tDISK USAGE\tCONTENT SIZE\tEXTRA") + err := tab.ParseHeader(lines[0]) + assert.NilError(t, err, "ParseHeader should not fail\n") + + original := commonImage.FamiliarName() + ":" + commonImage.Tag + retagged := data.Labels().Get("retaggedTo") + seen := 0 + for _, line := range lines[1:] { + line = padRow(lines[0], line) + image, _ := tab.ReadRow(line, "IMAGE") + extra, _ := tab.ReadRow(line, "EXTRA") + switch image { + case original: + assert.Equal(t, extra, "U", + "the image the container runs must stay in use: "+image) + seen++ + case retagged: + assert.Equal(t, extra, "", + "the image the tag now points at is not in use: "+image) + seen++ + } + } + assert.Equal(t, seen, 2, "both images should be listed\n") + }, + } + }, + }, }, } diff --git a/pkg/cmd/container/create.go b/pkg/cmd/container/create.go index 83abc56ad0b..e2f89159d95 100644 --- a/pkg/cmd/container/create.go +++ b/pkg/cmd/container/create.go @@ -215,6 +215,12 @@ func Create(ctx context.Context, client *containerd.Client, args []string, netMa internalLabels.user = ensuredImage.ImageConfig.User } + // Pin the image the container is created from. containerd only records the image name, and a + // name can later be retagged onto a different image. + if ensuredImage != nil && ensuredImage.Image != nil { + internalLabels.imageDigest = ensuredImage.Image.Target().Digest.String() + } + // Override it if User is passed if options.User != "" { internalLabels.user = options.User @@ -811,6 +817,8 @@ type internalLabels struct { domainname string // automatically generated stateDir string + // the digest of the image target the container was created from + imageDigest string // network networks []string ipAddress string @@ -919,6 +927,10 @@ func withInternalLabels(internalLabels internalLabels) (containerd.NewContainerO return nil, err } + if internalLabels.imageDigest != "" { + m[labels.ImageDigest] = internalLabels.imageDigest + } + if len(internalLabels.mountPoints) > 0 { mounts := dockercompatMounts(internalLabels.mountPoints) jsonMountBytes, err := json.Marshal(mounts) diff --git a/pkg/cmd/image/list.go b/pkg/cmd/image/list.go index f87fce0a272..703baa6679a 100644 --- a/pkg/cmd/image/list.go +++ b/pkg/cmd/image/list.go @@ -48,6 +48,7 @@ import ( "github.com/containerd/nerdctl/v2/pkg/containerdutil" "github.com/containerd/nerdctl/v2/pkg/formatter" "github.com/containerd/nerdctl/v2/pkg/imgutil" + "github.com/containerd/nerdctl/v2/pkg/labels" "github.com/containerd/nerdctl/v2/pkg/referenceutil" ) @@ -575,15 +576,50 @@ func imagesInUse(ctx context.Context, client *containerd.Client) map[digest.Dige return inUse } for _, container := range containerList { - image, err := container.Image(ctx) - if err != nil { - continue + if dgst, ok := containerImageDigest(ctx, container); ok { + inUse[dgst] = true } - inUse[image.Target().Digest] = true } return inUse } +// containerImageDigest returns the image target a container was created from. +// +// The digest is read from the label nerdctl records at creation time. Resolving the image name +// instead would follow the tag wherever it points now: after `nerdctl tag` moves a tag onto another +// image, the container would be attributed to an image it never ran. Containers created before this +// label existed, or outside nerdctl, still have to be resolved by name. +func containerImageDigest(ctx context.Context, container containerd.Container) (digest.Digest, bool) { + // The already-loaded metadata carries the labels, so this costs no extra round trip. + if info, err := container.Info(ctx, containerd.WithoutRefreshedMetadata); err == nil { + if dgst, ok := pinnedImageDigest(info.Labels); ok { + return dgst, true + } + } + + image, err := container.Image(ctx) + if err != nil { + return "", false + } + return image.Target().Digest, true +} + +// pinnedImageDigest returns the image target digest a container pinned at creation time. An +// unparsable value is treated as absent, so that a hand-edited label degrades to resolving the +// image by name rather than dropping the container from the in-use set. +func pinnedImageDigest(containerLabels map[string]string) (digest.Digest, bool) { + value := containerLabels[labels.ImageDigest] + if value == "" { + return "", false + } + dgst, err := digest.Parse(value) + if err != nil { + log.L.Debugf("ignoring invalid %s label value %q", labels.ImageDigest, value) + return "", false + } + return dgst, true +} + func isAttestationManifestDescriptor(desc ocispec.Descriptor) bool { const manifestReferenceType = "vnd.docker.reference.type" const attestationManifest = "attestation-manifest" diff --git a/pkg/cmd/image/list_test.go b/pkg/cmd/image/list_test.go index da83f8fc769..ccfa2a1338a 100644 --- a/pkg/cmd/image/list_test.go +++ b/pkg/cmd/image/list_test.go @@ -22,6 +22,8 @@ import ( "gotest.tools/v3/assert" "github.com/containerd/containerd/v2/core/images" + + "github.com/containerd/nerdctl/v2/pkg/labels" ) func TestNewViewImageRef(t *testing.T) { @@ -75,3 +77,46 @@ func TestSortByImageRef(t *testing.T) { assert.Equal(t, img.Name, expected[i]) } } + +func TestPinnedImageDigest(t *testing.T) { + t.Parallel() + + const pinned = "sha256:09538a1f51d3ec5af0449a1640937dfdf79b0e9b8c4da5b8a883086d5c1492ef" + + testCases := []struct { + name string + containerLabels map[string]string + expected string + }{ + { + name: "pinned at creation", + containerLabels: map[string]string{labels.ImageDigest: pinned}, + expected: pinned, + }, + { + // Containers created before the label existed, or outside nerdctl, have to be resolved + // by image name instead. + name: "no label", + containerLabels: map[string]string{labels.Platform: "linux/amd64"}, + }, + { + name: "empty label", + containerLabels: map[string]string{labels.ImageDigest: ""}, + }, + { + // Falling back to the name is better than dropping the container from the in-use set. + name: "unparsable label", + containerLabels: map[string]string{labels.ImageDigest: "not-a-digest"}, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + dgst, ok := pinnedImageDigest(tc.containerLabels) + assert.Equal(t, ok, tc.expected != "") + assert.Equal(t, string(dgst), tc.expected) + }) + } +} diff --git a/pkg/inspecttypes/dockercompat/dockercompat.go b/pkg/inspecttypes/dockercompat/dockercompat.go index f05f091def0..befcf4818d3 100644 --- a/pkg/inspecttypes/dockercompat/dockercompat.go +++ b/pkg/inspecttypes/dockercompat/dockercompat.go @@ -340,13 +340,28 @@ var defaultCaps = map[string]struct{}{ "CAP_AUDIT_WRITE": {}, } +// containerImage is the image the container was created from, the way Docker identifies it: by +// digest, pinned when the container was created. containerd only records the image name, and a name +// can later be retagged onto another image, so it is not an answer. The name is still the fallback +// for the containers created before that digest was recorded, or created outside nerdctl. +// +// With the containerd image store, the image ID Docker reports here is the digest of the image +// target (moby daemon/containerd/image.go, image.ID(img.Target.Digest)), which is what nerdctl +// pins. +func containerImage(n *native.Container) string { + if dgst := n.Labels[labels.ImageDigest]; dgst != "" { + return dgst + } + return n.Image +} + // ContainerFromNative instantiates a Docker-compatible Container from containerd-native Container. func ContainerFromNative(n *native.Container) (*Container, error) { var hostname string c := &Container{ ID: n.ID, Created: n.CreatedAt.Format(time.RFC3339Nano), - Image: n.Image, + Image: containerImage(n), Name: n.Labels[labels.Name], Driver: n.Snapshotter, // XXX is this always right? what if the container OS is NOT the same as the host OS? @@ -576,7 +591,8 @@ func ContainerFromNative(n *native.Container) (*Container, error) { c.State = cs c.Config = &Config{ Labels: n.Labels, - Image: c.Image, + // Docker keeps the reference the user asked for here, and the digest in Image above. + Image: n.Image, } if exposedPortsJSON := n.Labels[labels.ExposedPorts]; exposedPortsJSON != "" { var exposedPorts nat.PortSet diff --git a/pkg/inspecttypes/dockercompat/dockercompat_test.go b/pkg/inspecttypes/dockercompat/dockercompat_test.go index 17c21f6155c..ea326e7fadf 100644 --- a/pkg/inspecttypes/dockercompat/dockercompat_test.go +++ b/pkg/inspecttypes/dockercompat/dockercompat_test.go @@ -418,6 +418,36 @@ func TestContainerFromNative(t *testing.T) { } } +func TestContainerFromNativeImage(t *testing.T) { + const ( + ref = "example.com/foo:latest" + digest = "sha256:0168606be2318a4b6a9ad9e5a6d9dbf1b0a6d7e2c8c4a1b0e5d3f2a1c0b9e8d7" + ) + + // Docker names the image a container was created from by digest, and keeps the reference the + // user asked for in Config.Image. + pinned, err := ContainerFromNative(&native.Container{ + Container: containers.Container{ + Image: ref, + Labels: map[string]string{labels.ImageDigest: digest}, + }, + Spec: &specs.Spec{}, + }) + assert.NilError(t, err) + assert.Equal(t, pinned.Image, digest) + assert.Equal(t, pinned.Config.Image, ref) + + // A container created before that digest was recorded, or created outside nerdctl, is left + // with the name it has. + unpinned, err := ContainerFromNative(&native.Container{ + Container: containers.Container{Image: ref}, + Spec: &specs.Spec{}, + }) + assert.NilError(t, err) + assert.Equal(t, unpinned.Image, ref) + assert.Equal(t, unpinned.Config.Image, ref) +} + func TestGetCapabilitiesFromNative(t *testing.T) { // Build the full default bounding set for test fixtures. allDefaults := []string{ diff --git a/pkg/labels/labels.go b/pkg/labels/labels.go index dd32cca0616..9f689ff7cff 100644 --- a/pkg/labels/labels.go +++ b/pkg/labels/labels.go @@ -87,6 +87,11 @@ const ( // Platform is the normalized platform string like "linux/ppc64le". Platform = Prefix + "platform" + // ImageDigest is the digest of the image target the container was created from. The image name + // stored by containerd can be retagged to point at something else, so it is not enough to tell + // which image a container actually uses. + ImageDigest = Prefix + "image-digest" + // Mounts is the mount points for the container. Mounts = Prefix + "mounts"