Index images by RepoDigests in LocalImagesCache, not just RepoTags#11948
Open
itsmehotpants wants to merge 1 commit into
Open
Index images by RepoDigests in LocalImagesCache, not just RepoTags#11948itsmehotpants wants to merge 1 commit into
itsmehotpants wants to merge 1 commit into
Conversation
LocalImagesCache.populateFromList() only indexed local images by their RepoTags. An image referenced only by digest (e.g. after 'docker pull image@sha256:...', or when Docker reports RepoTags as null/<none> for an otherwise-tagged image, which happens after certain rebuild/retag sequences) was never added to the cache. Testcontainers would then treat the image as absent locally and re-pull it every time, even though it was already present, defeating both the image pull policy and Ryuk/reaper image lookups by digest. populateFromList() now also reads Image::getRepoDigests() and indexes those digest-qualified names into the same cache, alongside RepoTags. An image with neither populated is still skipped, as before. Added LocalImagesCacheTest covering: digest-only images, images with both a tag and a digest, and the neither-present case. Uses the same ObjectMapper#convertValue(Map, Class) pattern already used in ReusabilityUnitTests for constructing docker-java model objects in tests, and the existing (previously unused) LocalImagesCacheAccessor for cache isolation between tests. Fixes testcontainers#1406
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
1 participant
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.
Fixes #1406.
LocalImagesCache.populateFromList()only indexed local images by theirRepoTags. An image reachable only by digest (e.g.docker pull image@sha256:...) - or a tagged image where Docker reportsRepoTagsasnull/["<none>:<none>"]after certain rebuild/retag sequences, whileRepoDigestsremains populated (see moby/moby#29157 for a concrete real-world case of this) - was never added to the cache. Testcontainers would then treat the image as absent locally and re-pull it every time, even though it was already present, defeating both the configuredImagePullPolicyand any lookup of an image by digest.Change:
populateFromList()now also readsImage::getRepoDigests()and indexes those digest-qualified names into the same cache, alongsideRepoTags. An image with neither field populated is still skipped, as before (just with an updated log message reflecting the new condition).Tests: added
LocalImagesCacheTestcovering a digest-only image, an image with both a tag and a digest, and the neither-present case. Uses the sameObjectMapper#convertValue(Map, Class)pattern already used inReusabilityUnitTestsfor constructing docker-java model objects in tests, and the existing (previously unused)LocalImagesCacheAccessortest helper for cache isolation between tests.Scope note: I did not additionally add Image ID-based lookup (mentioned in the issue's 2020 edit) since
DockerImageNameparsing doesn't cleanly represent a bare image ID (e.g.sha256:abcdef...without a repository) today - that would need its own design discussion, so I've kept this PR focused on the RepoDigests fix. Happy to look at Image ID support separately if maintainers want it.