Skip to content

Commit 3a458da

Browse files
committed
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. This also matters for the ACTIVE and RECLAIMABLE columns of `nerdctl system df`, which build on the same lookup. Signed-off-by: Eugene Kalinin <e.v.kalinin@gmail.com>
1 parent fcaa66c commit 3a458da

5 files changed

Lines changed: 152 additions & 4 deletions

File tree

cmd/nerdctl/image/image_list_test.go

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,56 @@ func TestImages(t *testing.T) {
207207
}
208208
},
209209
},
210+
{
211+
Description: "In use survives a retag",
212+
Setup: func(data test.Data, helpers test.Helpers) {
213+
// Run a container off a private tag, then move that tag onto another image.
214+
// The container still runs the original image, so that is the one that must
215+
// stay marked as in use.
216+
helpers.Ensure("tag", commonImage.String(), data.Identifier()+":moving")
217+
helpers.Ensure("run", "-d", "--quiet", "--name", data.Identifier(),
218+
data.Identifier()+":moving", "sleep", nerdtest.Infinity)
219+
helpers.Ensure("tag", testutil.NginxAlpineImage, data.Identifier()+":moving")
220+
221+
nginx, _ := referenceutil.Parse(testutil.NginxAlpineImage)
222+
data.Labels().Set("retaggedTo", nginx.FamiliarName()+":"+nginx.Tag)
223+
},
224+
Cleanup: func(data test.Data, helpers test.Helpers) {
225+
helpers.Anyhow("rm", "-f", data.Identifier())
226+
helpers.Anyhow("rmi", "-f", data.Identifier()+":moving")
227+
},
228+
Command: test.Command("images"),
229+
Expected: func(data test.Data, helpers test.Helpers) *test.Expected {
230+
return &test.Expected{
231+
Output: func(stdout string, t tig.T) {
232+
lines := strings.Split(strings.TrimSpace(stdout), "\n")
233+
assert.Assert(t, len(lines) >= 2, "there should be at least two lines\n")
234+
tab := tabutil.NewReader("IMAGE\tID\tDISK USAGE\tCONTENT SIZE\tEXTRA")
235+
err := tab.ParseHeader(lines[0])
236+
assert.NilError(t, err, "ParseHeader should not fail\n")
237+
238+
original := commonImage.FamiliarName() + ":" + commonImage.Tag
239+
retagged := data.Labels().Get("retaggedTo")
240+
seen := 0
241+
for _, line := range lines[1:] {
242+
image, _ := tab.ReadRow(line, "IMAGE")
243+
extra, _ := tab.ReadRow(line, "EXTRA")
244+
switch image {
245+
case original:
246+
assert.Equal(t, extra, "U",
247+
"the image the container runs must stay in use: "+image)
248+
seen++
249+
case retagged:
250+
assert.Equal(t, extra, "",
251+
"the image the tag now points at is not in use: "+image)
252+
seen++
253+
}
254+
}
255+
assert.Equal(t, seen, 2, "both images should be listed\n")
256+
},
257+
}
258+
},
259+
},
210260
},
211261
}
212262

pkg/cmd/container/create.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,12 @@ func Create(ctx context.Context, client *containerd.Client, args []string, netMa
215215
internalLabels.user = ensuredImage.ImageConfig.User
216216
}
217217

218+
// Pin the image the container is created from. containerd only records the image name, and a
219+
// name can later be retagged onto a different image.
220+
if ensuredImage != nil && ensuredImage.Image != nil {
221+
internalLabels.imageDigest = ensuredImage.Image.Target().Digest.String()
222+
}
223+
218224
// Override it if User is passed
219225
if options.User != "" {
220226
internalLabels.user = options.User
@@ -811,6 +817,8 @@ type internalLabels struct {
811817
domainname string
812818
// automatically generated
813819
stateDir string
820+
// the digest of the image target the container was created from
821+
imageDigest string
814822
// network
815823
networks []string
816824
ipAddress string
@@ -919,6 +927,10 @@ func withInternalLabels(internalLabels internalLabels) (containerd.NewContainerO
919927
return nil, err
920928
}
921929

930+
if internalLabels.imageDigest != "" {
931+
m[labels.ImageDigest] = internalLabels.imageDigest
932+
}
933+
922934
if len(internalLabels.mountPoints) > 0 {
923935
mounts := dockercompatMounts(internalLabels.mountPoints)
924936
jsonMountBytes, err := json.Marshal(mounts)

pkg/cmd/image/list.go

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ import (
4848
"github.com/containerd/nerdctl/v2/pkg/containerdutil"
4949
"github.com/containerd/nerdctl/v2/pkg/formatter"
5050
"github.com/containerd/nerdctl/v2/pkg/imgutil"
51+
"github.com/containerd/nerdctl/v2/pkg/labels"
5152
"github.com/containerd/nerdctl/v2/pkg/referenceutil"
5253
)
5354

@@ -575,15 +576,50 @@ func imagesInUse(ctx context.Context, client *containerd.Client) map[digest.Dige
575576
return inUse
576577
}
577578
for _, container := range containerList {
578-
image, err := container.Image(ctx)
579-
if err != nil {
580-
continue
579+
if dgst, ok := containerImageDigest(ctx, container); ok {
580+
inUse[dgst] = true
581581
}
582-
inUse[image.Target().Digest] = true
583582
}
584583
return inUse
585584
}
586585

586+
// containerImageDigest returns the image target a container was created from.
587+
//
588+
// The digest is read from the label nerdctl records at creation time. Resolving the image name
589+
// instead would follow the tag wherever it points now: after `nerdctl tag` moves a tag onto another
590+
// image, the container would be attributed to an image it never ran. Containers created before this
591+
// label existed, or outside nerdctl, still have to be resolved by name.
592+
func containerImageDigest(ctx context.Context, container containerd.Container) (digest.Digest, bool) {
593+
// The already-loaded metadata carries the labels, so this costs no extra round trip.
594+
if info, err := container.Info(ctx, containerd.WithoutRefreshedMetadata); err == nil {
595+
if dgst, ok := pinnedImageDigest(info.Labels); ok {
596+
return dgst, true
597+
}
598+
}
599+
600+
image, err := container.Image(ctx)
601+
if err != nil {
602+
return "", false
603+
}
604+
return image.Target().Digest, true
605+
}
606+
607+
// pinnedImageDigest returns the image target digest a container pinned at creation time. An
608+
// unparsable value is treated as absent, so that a hand-edited label degrades to resolving the
609+
// image by name rather than dropping the container from the in-use set.
610+
func pinnedImageDigest(containerLabels map[string]string) (digest.Digest, bool) {
611+
value := containerLabels[labels.ImageDigest]
612+
if value == "" {
613+
return "", false
614+
}
615+
dgst, err := digest.Parse(value)
616+
if err != nil {
617+
log.L.Debugf("ignoring invalid %s label value %q", labels.ImageDigest, value)
618+
return "", false
619+
}
620+
return dgst, true
621+
}
622+
587623
func isAttestationManifestDescriptor(desc ocispec.Descriptor) bool {
588624
const manifestReferenceType = "vnd.docker.reference.type"
589625
const attestationManifest = "attestation-manifest"

pkg/cmd/image/list_test.go

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ import (
2222
"gotest.tools/v3/assert"
2323

2424
"github.com/containerd/containerd/v2/core/images"
25+
26+
"github.com/containerd/nerdctl/v2/pkg/labels"
2527
)
2628

2729
func TestNewViewImageRef(t *testing.T) {
@@ -75,3 +77,46 @@ func TestSortByImageRef(t *testing.T) {
7577
assert.Equal(t, img.Name, expected[i])
7678
}
7779
}
80+
81+
func TestPinnedImageDigest(t *testing.T) {
82+
t.Parallel()
83+
84+
const pinned = "sha256:09538a1f51d3ec5af0449a1640937dfdf79b0e9b8c4da5b8a883086d5c1492ef"
85+
86+
testCases := []struct {
87+
name string
88+
containerLabels map[string]string
89+
expected string
90+
}{
91+
{
92+
name: "pinned at creation",
93+
containerLabels: map[string]string{labels.ImageDigest: pinned},
94+
expected: pinned,
95+
},
96+
{
97+
// Containers created before the label existed, or outside nerdctl, have to be resolved
98+
// by image name instead.
99+
name: "no label",
100+
containerLabels: map[string]string{labels.Platform: "linux/amd64"},
101+
},
102+
{
103+
name: "empty label",
104+
containerLabels: map[string]string{labels.ImageDigest: ""},
105+
},
106+
{
107+
// Falling back to the name is better than dropping the container from the in-use set.
108+
name: "unparsable label",
109+
containerLabels: map[string]string{labels.ImageDigest: "not-a-digest"},
110+
},
111+
}
112+
113+
for _, tc := range testCases {
114+
t.Run(tc.name, func(t *testing.T) {
115+
t.Parallel()
116+
117+
dgst, ok := pinnedImageDigest(tc.containerLabels)
118+
assert.Equal(t, ok, tc.expected != "")
119+
assert.Equal(t, string(dgst), tc.expected)
120+
})
121+
}
122+
}

pkg/labels/labels.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,11 @@ const (
8787
// Platform is the normalized platform string like "linux/ppc64le".
8888
Platform = Prefix + "platform"
8989

90+
// ImageDigest is the digest of the image target the container was created from. The image name
91+
// stored by containerd can be retagged to point at something else, so it is not enough to tell
92+
// which image a container actually uses.
93+
ImageDigest = Prefix + "image-digest"
94+
9095
// Mounts is the mount points for the container.
9196
Mounts = Prefix + "mounts"
9297

0 commit comments

Comments
 (0)