Skip to content

fix(compose): report pinned image IDs after retagging - #5251

Merged
ChengyuZhu6 merged 1 commit into
containerd:mainfrom
git-jxj:fix/compose-images-pinned-digest-2198
Oct 15, 2026
Merged

ChengyuZhu6 merged 1 commit into
containerd:mainfrom
git-jxj:fix/compose-images-pinned-digest-2198

Conversation

@git-jxj

@git-jxj git-jxj commented Oct 8, 2026 •

Copy link
Copy Markdown

compose images resolves containers through mutable image names, so retagging can change their reported ID or combine the original ID with another image's size. Resolve the pinned local content and container platform for table/JSON output, and reuse the shared pinned-digest helper for quiet output. Preserve name-based resolution for missing or invalid digest labels.

Fixes #2198.

Validation: the private-namespace Alpine-to-BusyBox compose regression fails on the old head and passes after the fix for table/JSON size consistency and quiet IDs. Local image-store/CLI checks cover removed image names, fallback labels, platform selection, and unavailable content/snapshots. make test, builds, changed-code Go lint on all four supported OSes, YAML, module, confinement, license, and commit checks pass. Full lint reproduces the existing Go formatting and shellcheck failures on the clean upstream baseline.

AI assistance: Codex assisted with the implementation, regression tests, and PR description.


func TestComposeImagesAfterRetag(t *testing.T) {
testCase := nerdtest.Setup()
testCase.Require = require.All(nerdtest.Private, require.Not(nerdtest.Docker))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a comment to explain why require.Not(nerdtest.Docker)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the comment above the requirement. This regression uses nerdctl-specific image convert to retag the image and image inspect --mode native to check its manifest digest, so it cannot run against Docker.

@AkihiroSuda

Copy link
Copy Markdown
Member

Please squash the commits

@git-jxj

git-jxj commented Oct 10, 2026

Copy link
Copy Markdown
Author

Squashed the two commits into aa86f31. The code and regression test are unchanged, including the comment explaining why the test excludes Docker.

Comment on lines 151 to 162

size, err := imgutil.UnpackedImageSize(ctx, sn, image)
if err != nil {
return err
}

metaImage := image.Metadata()
repository, tag := imgutil.ParseRepoTag(metaImage.Name)
imageID := metaImage.Target.Digest.String()
imageID := composeImageDigest(info.Labels, metaImage.Target.Digest).String()
if repository == "" {
repository = "<none>"
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ImageID uses the pinned digest, but Size comes from
imgutil.UnpackedImageSize where image is resolved by the current tag.
After retagging, the row is internally inconsistent: I get alpine's ID with busybox's size

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 5a360b6. Table and JSON now resolve the pinned digest from the local content store and select the manifest using the container's platform label, so the ID and unpacked size come from the same image. The repository/tag still comes from the container's original image reference. The regression now retags Alpine to BusyBox and compares the full table and JSON output: it failed with the old head (5.9 MiB became 4.8 MiB) and passes with the fix. Local image-store/CLI checks also pass when the original image record or container image name is removed; unavailable pinned content or snapshots return an error instead of reporting another image's size.

Comment thread cmd/nerdctl/compose/compose_images.go Outdated

// Image tags can change after a container is created. Prefer the digest pinned
// at creation, falling back to the current image for containers without it.
func composeImageDigest(containerLabels map[string]string, fallback digest.Digest) digest.Digest {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pinnedImageDigest in pkg/cmd/image/list.go already reads
labels.ImageDigest with identical fallback semantics. Please reuse pinnedImageDigest instead of adding composeImageDigest.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 5a360b6. Exported the existing helper as image.PinnedImageDigest and reused it in both compose output paths. Removed composeImageDigest; missing/empty/invalid labels keep the existing name-based fallback and debug logging. The existing four helper cases and make test pass.

testCase.Run(t)
}

func TestComposeImageDigestFallback(t *testing.T) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It duplicates the existing TestPinnedImageDigest in pkg/cmd/image/list_test.go case for case.
TestComposeImagesAfterRetag already covers the real behavior

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the duplicate TestComposeImageDigestFallback in 5a360b6 and kept the existing TestPinnedImageDigest coverage. TestComposeImagesAfterRetag now uses Alpine and BusyBox with different rootfs sizes and compares the full table and JSON output, while still checking the quiet short ID. The old head fails the table/JSON regression and the updated head passes all three modes in a private containerd namespace.

@AkihiroSuda AkihiroSuda added this to the v2.4.2 milestone Oct 13, 2026
Resolve the image content recorded at container creation so compose images
reports the original target digest and unpacked size together. Use the
container's platform when selecting an indexed manifest and preserve
name-based resolution when the digest label is missing or invalid.

Reuse the existing pinned image digest helper and cover retagging between
images with different rootfs sizes in table, JSON, and quiet output.
Explain that native image inspection makes this regression nerdctl-only.

Fixes containerd#2198.

Assisted-by: Codex
Signed-off-by: xinjun.jiang <xinjun.jiang@daocloud.io>
@git-jxj

git-jxj commented Oct 14, 2026

Copy link
Copy Markdown
Author

The remaining failure in the current [flaky, see #3988] run is the FreeBSD job: after reporting Nerdctl is up and running, the Run: test-integration action times out at its 3-minute limit without a test assertion in the log. The other jobs in that run passed.

I attempted to rerun only the FreeBSD job, but GitHub returned 403 (Must have admin rights to Repository). Could a maintainer rerun it? The four code-review items remain implemented in the current head for reviewer confirmation.

@git-jxj

git-jxj commented Oct 14, 2026

Copy link
Copy Markdown
Author

The attempt 3 FreeBSD job repeats the same timeout after printing Nerdctl is up and running; its build and package unit tests pass. I checked the FreeBSD job on unchanged current main 805511ad: it also prints that message and then times out at the same 3-minute integration step. The workflow is unchanged between this PR's base and head. The timeout is therefore also present upstream; the FreeBSD harness needs separate investigation from the Compose image fix.

@ChengyuZhu6 ChengyuZhu6 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ChengyuZhu6
ChengyuZhu6 merged commit 477a85e into containerd:main Oct 15, 2026
58 of 64 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

compose images returns wrong image ID

3 participants