copy: add InstancePlatforms field for platform-based filtering #656
Conversation
Add the ability to select images by platform name instead of requiring digest hashes. This implements the functionality originally proposed in containers/image#1938. When copying specific images from a multi-architecture manifest list, users currently must specify exact digest hashes. This is cumbersome and error-prone, as users must manually look up digests and can easily confuse which digest corresponds to which platform. This commit adds a new InstancePlatforms field that allows users to specify platforms by human-readable names like "linux/amd64" or "linux/arm64". The field works alongside the existing Instances field, allowing users to combine both methods. The implementation includes: 1. New InstancePlatforms []imgspecv1.Platform field in Options 2. determineSpecificImages() function to resolve platforms to digests and combine with digest-based selection 3. Efficient Set-based filtering (O(1) lookup vs O(n) with slices) 4. Size() method added to internal/set for Set length queries 5. Table-driven tests covering all selection scenarios Based on original work by @nalind in containers/image#1938, adapted for the container-libs monorepo structure. Relates to containers#227 Signed-off-by: Alex Guidi <[email protected]>
|
Packit jobs failed. @containers/packit-build please check. |
1 similar comment
|
Packit jobs failed. @containers/packit-build please check. |
mtrmac
left a comment
mtrmac
left a comment
There was a problem hiding this comment.
Thanks!
An extremely brief look for now.
| ArchitectureChoice: platform.Architecture, | ||
| VariantChoice: platform.Variant, | ||
| } | ||
| instanceDigest, err := updatedList.ChooseInstanceByCompression(&platformContext, options.PreferGzipInstances) |
There was a problem hiding this comment.
Hum… we have multi-compression images (where there is a gzip instance and a zstd instance for the same platform). This would only copy one of the instances.
OTOH a trivial “does the instance match the required Platform value” check might copy too much, because a v1 variant requirement would match a v1,v2,v3 instances.
| } | ||
|
|
||
| require.NoError(t, err) | ||
| assert.Equal(t, tt.expectedSize, specificImages.Size()) |
There was a problem hiding this comment.
This can probably use assert.ElementsMatch, for a better error output on failure and perhaps smaller test code.
|
I've approved this and restarted a couple of failng tests. I'd be interested in seeing your reply to @mtrmac 's comment on the multi-compression images. Overall, a nice change, thanks! |
3 participants
Add platform-based filtering for copying specific images
This PR adds the ability to select images by platform name (e.g.,
linux/amd64) instead of requiring digest hashes, implementing the functionality proposed in containers/image#1938.Motivation
Relates to #227
When copying specific images from a multi-architecture manifest list, the current
CopySpecificImagesmode requires users to specify exact digest hashes in theInstancesfield. This is cumbersome because:This PR adds a new
InstancePlatformsfield that allows users to specify platforms by human-readable names.Changes
1. New
InstancePlatformsfield inimage/copyInstancePlatforms []imgspecv1.Platformfield toOptionsstruct{OS: "linux", Architecture: "amd64"}Instancesfield (both can be used simultaneously)2. Platform resolution in
determineSpecificImages()(image/copy/multiple.go)ChooseInstanceByCompression()to find best match for each platform3. Efficient Set-based filtering
slices.Contains()to Set-based lookupSize()method tointernal/set/set.go4. Comprehensive test coverage
TestDetermineSpecificImageswith table-driven testsUser Experience Improvement
Before (digest-based only):
After (platform-based):
Combined (both methods):
Testing
All existing tests pass. New tests verify:
Compatibility
Instancesfield continues to work unchangedCredits
This implementation is based on the original work by @nalind in containers/image#1938, adapted for the container-libs monorepo structure with the following changes: