Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions image/copy/copy.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

encconfig "github.com/containers/ocicrypt/config"
digest "github.com/opencontainers/go-digest"
imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/sirupsen/logrus"
"go.podman.io/image/v5/docker/reference"
internalblobinfocache "go.podman.io/image/v5/internal/blobinfocache"
Expand Down Expand Up @@ -93,8 +94,9 @@ type Options struct {
PreserveDigests bool
// manifest MIME type of image set by user. "" is default and means use the autodetection to the manifest MIME type
ForceManifestMIMEType string
ImageListSelection ImageListSelection // set to either CopySystemImage (the default), CopyAllImages, or CopySpecificImages to control which instances we copy when the source reference is a list; ignored if the source reference is not a list
Instances []digest.Digest // if ImageListSelection is CopySpecificImages, copy only these instances and the list itself
ImageListSelection ImageListSelection // set to either CopySystemImage (the default), CopyAllImages, or CopySpecificImages to control which instances we copy when the source reference is a list; ignored if the source reference is not a list
Instances []digest.Digest // if ImageListSelection is CopySpecificImages, copy only these instances, instances matching the InstancePlatforms list, and the list itself
InstancePlatforms []imgspecv1.Platform // if ImageListSelection is CopySpecificImages, copy only matching instances, instances listed in the Instances list, and the list itself
// Give priority to pulling gzip images if multiple images are present when configured to OptionalBoolTrue,
// prefers the best compression if this is configured as OptionalBoolFalse. Choose automatically (and the choice may change over time)
// if this is set to OptionalBoolUndefined (which is the default behavior, and recommended for most callers).
Expand Down
44 changes: 42 additions & 2 deletions image/copy/multiple.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"go.podman.io/image/v5/internal/set"
"go.podman.io/image/v5/manifest"
"go.podman.io/image/v5/pkg/compression"
"go.podman.io/image/v5/types"
)

type instanceCopyKind int
Expand Down Expand Up @@ -104,7 +105,7 @@ func prepareInstanceCopies(list internalManifest.List, instanceDigests []digest.
res := []instanceCopy{}
if options.ImageListSelection == CopySpecificImages && len(options.EnsureCompressionVariantsExist) > 0 {
// List can already contain compressed instance for a compression selected in `EnsureCompressionVariantsExist`
// Its unclear what it means when `CopySpecificImages` includes an instance in options.Instances,
// It's unclear what it means when `CopySpecificImages` includes an instance in options.Instances,
// EnsureCompressionVariantsExist asks for an instance with some compression,
// an instance with that compression already exists, but is not included in options.Instances.
// We might define the semantics and implement this in the future.
Expand All @@ -118,9 +119,19 @@ func prepareInstanceCopies(list internalManifest.List, instanceDigests []digest.
if err != nil {
return nil, err
}

// Determine which specific images to copy (combining digest-based and platform-based selection)
var specificImages *set.Set[digest.Digest]
if options.ImageListSelection == CopySpecificImages {
specificImages, err = determineSpecificImages(options, list)
if err != nil {
return nil, err
}
}

for i, instanceDigest := range instanceDigests {
if options.ImageListSelection == CopySpecificImages &&
!slices.Contains(options.Instances, instanceDigest) {
!specificImages.Contains(instanceDigest) {
logrus.Debugf("Skipping instance %s (%d/%d)", instanceDigest, i+1, len(instanceDigests))
continue
}
Expand Down Expand Up @@ -157,6 +168,35 @@ func prepareInstanceCopies(list internalManifest.List, instanceDigests []digest.
return res, nil
}

// determineSpecificImages returns a set of images to copy based on the
// Instances and InstancePlatforms fields of the passed-in options structure
func determineSpecificImages(options *Options, updatedList internalManifest.List) (*set.Set[digest.Digest], error) {
// Start with the instances that were listed by digest.
specificImages := set.New[digest.Digest]()
for _, instanceDigest := range options.Instances {
specificImages.Add(instanceDigest)
}

if len(options.InstancePlatforms) > 0 {
// Choose the best match for each platform we were asked to
// also copy, and add it to the set of instances to copy.
for _, platform := range options.InstancePlatforms {
platformContext := types.SystemContext{
OSChoice: platform.OS,
ArchitectureChoice: platform.Architecture,
VariantChoice: platform.Variant,
}
instanceDigest, err := updatedList.ChooseInstanceByCompression(&platformContext, options.PreferGzipInstances)
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

if err != nil {
return nil, fmt.Errorf("choosing instance for platform spec %v: %w", platform, err)
}
specificImages.Add(instanceDigest)
}
}

return specificImages, nil
}

// copyMultipleImages copies some or all of an image list's instances, using
// c.policyContext to validate source image admissibility.
func (c *copier) copyMultipleImages(ctx context.Context) (copiedManifest []byte, retErr error) {
Expand Down
75 changes: 75 additions & 0 deletions image/copy/multiple_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"testing"

digest "github.com/opencontainers/go-digest"
imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
internalManifest "go.podman.io/image/v5/internal/manifest"
Expand Down Expand Up @@ -170,3 +171,77 @@ func convertInstanceCopyToSimplerInstanceCopy(copies []instanceCopy) []simplerIn
}
return res
}

// TestDetermineSpecificImages tests the platform-based and digest-based instance selection
func TestDetermineSpecificImages(t *testing.T) {
validManifest, err := os.ReadFile(filepath.Join("..", "internal", "manifest", "testdata", "ociv1.image.index.json"))
require.NoError(t, err)
list, err := internalManifest.ListFromBlob(validManifest, internalManifest.GuessMIMEType(validManifest))
require.NoError(t, err)

// Digests from ociv1.image.index.json
ppc64leDigest := digest.Digest("sha256:e692418e4cbaf90ca69d05a66403747baa33ee08806650b51fab815ad7fc331f")
amd64Digest := digest.Digest("sha256:5b0bcabd1ed22e9fb1310cf6c2dec7cdef19f0ad69efa1f392e94a4333501270")

tests := []struct {
name string
instances []digest.Digest
instancePlatforms []imgspecv1.Platform
expectedSize int
expectedDigests []digest.Digest
expectError bool
}{
{
name: "PlatformOnly",
instancePlatforms: []imgspecv1.Platform{
{OS: "linux", Architecture: "ppc64le"},
},
expectedSize: 1,
expectedDigests: []digest.Digest{ppc64leDigest},
},
{
name: "DigestOnly",
instances: []digest.Digest{amd64Digest},
expectedSize: 1,
expectedDigests: []digest.Digest{amd64Digest},
},
{
name: "Combined",
instances: []digest.Digest{ppc64leDigest},
instancePlatforms: []imgspecv1.Platform{
{OS: "linux", Architecture: "amd64"},
},
expectedSize: 2,
expectedDigests: []digest.Digest{ppc64leDigest, amd64Digest},
},
{
name: "NonExistentPlatform",
instancePlatforms: []imgspecv1.Platform{
{OS: "linux", Architecture: "arm64"},
},
expectError: true,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
options := &Options{
Instances: tt.instances,
InstancePlatforms: tt.instancePlatforms,
}
specificImages, err := determineSpecificImages(options, list)

if tt.expectError {
assert.Error(t, err)
assert.Contains(t, err.Error(), "choosing instance for platform")
return
}

require.NoError(t, err)
assert.Equal(t, tt.expectedSize, specificImages.Size())
Copy link
Contributor

Choose a reason for hiding this comment

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

This can probably use assert.ElementsMatch, for a better error output on failure and perhaps smaller test code.

for _, expectedDigest := range tt.expectedDigests {
assert.True(t, specificImages.Contains(expectedDigest))
}
})
}
}
4 changes: 4 additions & 0 deletions image/internal/set/set.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,7 @@ func (s *Set[E]) Empty() bool {
func (s *Set[E]) All() iter.Seq[E] {
return maps.Keys(s.m)
}

func (s *Set[E]) Size() int {
return len(s.m)
}