Use content digest for pulled service images - #13998
Open
benjick wants to merge 2 commits into
Open
Conversation
pullServiceImage returned the pulled image's raw inspect ID, while getImageSummaries resolves already-local images through contentDigest (the platform image-manifest digest). Both values feed the com.docker.compose.image label that mustRecreate compares to detect image changes, so the two paths disagreeing made the first 'up' after the pulling 'up' see a phantom image change and recreate every container once, with no change anywhere. Under the containerd image store a tag@digest reference triggers this: the raw inspect ID is the index digest, while contentDigest picks the platform manifest digest. Resolve the pulled image through the same manifests-aware inspect and contentDigest call getImageSummaries uses, so both sides of the staleness comparison speak the same scheme. Verified against a fresh docker:dind (29.7.0, containerd store) with a tag@digest service: unpatched v5.4.0 recreates the container on the second 'up'; with this fix the container survives repeated 'up' runs. Existing behavior is preserved for engines without manifest support (contentDigest falls back to the plain ID). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Max Malm <benjick@dumfan.net>
Extract the inspect-with-manifests + contentDigest sequence from pullServiceImage into inspectContentDigest, next to the contentDigest and manifestsSupported helpers it belongs with. pullServiceImage was over the gocyclo limit of 16 with the resolve block inlined, and getImageSummaries already ran the same sequence, so this names it once. Rename the image parameter of the serviceWithHook test helper to img: it shadowed the moby image package, newly imported by pull_test.go. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Max Malm <benjick@dumfan.net>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What I did
pullServiceImagereturned the pulled image's raw inspect ID, whilegetImageSummariesresolves already-local images throughcontentDigest(the platform image-manifest digest, introduced for #13636). Both values feed thecom.docker.compose.imagelabel thatmustRecreatecompares to detect image changes - so the two paths disagreeing made the firstupafter the pullingupsee a phantom image change and recreate every container once, with no change to the project, the image, or the compose file.Under the containerd image store a
tag@digestimage reference triggers this reliably: the pull path records the index digest (the raw inspect ID) on the container, and the nextupresolves the same image to its platform manifest digest viacontentDigest. The strings differ,mustRecreatefires, and the replacement container gets the platform digest - so it happens exactly once per container, which makes it easy to miss but breaks any workflow that relies onupbeing idempotent (we hit it because our orchestrator asserts unchanged services are never recreated across reconciles).The fix makes
pullServiceImageresolve the pulled image through the same manifests-aware inspect +contentDigestcallgetImageSummariesuses, so both sides of the staleness comparison speak the same scheme. Engines without manifest support (API < 1.48) keep the previous behavior viacontentDigest's plain-ID fallback.Repro
The two containers'
com.docker.compose.imagelabels show the mismatch: container A carriessha256:6baf...(the pinned index digest), its replacement carries the platform manifest digest. v5.3.1 does not exhibit this; it appeared when the local-inspect side moved tocontentDigest.Verification
TestPullServiceImageUsesContentDigestfails against the old return value and passes with the fix.go build ./...,go vet ./pkg/compose/,gofmt -s -lclean;go test ./pkg/compose/passes.docker:dind(29.7.0): with this fix the container from the pullingupsurvives repeatedupruns (same container ID); unpatched v5.4.0 recreates it on the secondup.Related: #13636 (the fix that introduced
contentDigestfor the local-inspect path; this aligns the pull path with it).Disclosure: this PR was prepared by Claude Code (Anthropic) - the diagnosis, fix, tests, and the verification runs above - in an agentic session I directed. I'm the submitter and point of contact.
🤖 Generated with Claude Code