Merge pull request #52773 from vvoland/c8d-amd64-variants

c8d: Use maximum native platform for matching

daemon/containerd/image_builder.go

@@ -200,7 +200,7 @@ func newROLayerForImage(ctx context.Context, imgDesc *ocispec.Descriptor, i *Ima
 		return nil, errors.New("can't make an RO layer for a nil image :'(")
 	}
 
-	platMatcher := platforms.Default()
+	platMatcher := i.hostPlatformMatcher()
 	if platform != nil {
 		platMatcher = platforms.Only(*platform)
 	}
@@ -449,7 +449,7 @@ func (i *ImageService) CreateImage(ctx context.Context, config []byte, parent st
 		if err != nil {
 			return nil, err
 		}
-		parentImageManifest, err := c8dimages.Manifest(ctx, i.content, parentDesc, platforms.Default())
+		parentImageManifest, err := c8dimages.Manifest(ctx, i.content, parentDesc, i.hostPlatformMatcher())
 		if err != nil {
 			return nil, err
 		}

daemon/containerd/image_identity.go

@@ -424,7 +424,7 @@ func (i *ImageService) refreshImageIdentityCacheKey(ctx context.Context, cacheKe
 		return nil
 	}
 
-	platformMatcher, err := imageIdentityPlatformMatcher(bestPlatform)
+	platformMatcher, err := i.imageIdentityPlatformMatcher(bestPlatform)
 	if err != nil {
 		return err
 	}
@@ -448,9 +448,9 @@ func (i *ImageService) refreshImageIdentityCacheKey(ctx context.Context, cacheKe
 	return nil
 }
 
-func imageIdentityPlatformMatcher(platform string) (platforms.MatchComparer, error) {
+func (i *ImageService) imageIdentityPlatformMatcher(platform string) (platforms.MatchComparer, error) {
 	if platform == "" {
-		return matchAnyWithPreference(platforms.Default(), nil), nil
+		return matchAnyWithPreference(i.hostPlatformMatcher(), nil), nil
 	}
 	parsed, err := platforms.Parse(platform)
 	if err != nil {
@@ -541,7 +541,7 @@ func (i *ImageService) warmImageIdentityCache(ctx context.Context, img c8dimages
 	go func() {
 		warmCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), imageIdentityWarmupTimeout)
 		defer cancel()
-		multi, err := i.multiPlatformSummary(warmCtx, img, matchAnyWithPreference(platforms.Default(), nil))
+		multi, err := i.multiPlatformSummary(warmCtx, img, matchAnyWithPreference(i.hostPlatformMatcher(), nil))
 		if err != nil {
 			log.G(warmCtx).WithError(err).WithField("image", img.Name).Debug("failed to build image identity cache in background")
 			return

daemon/containerd/image_list.go

@@ -107,7 +107,7 @@ func (i *ImageService) Images(ctx context.Context, opts imagebackend.ListOptions
 	}
 
 	// TODO: Allow platform override?
-	platformMatcher := matchAnyWithPreference(platforms.Default(), nil)
+	platformMatcher := matchAnyWithPreference(i.hostPlatformMatcher(), nil)
 
 	for _, img := range imgs {
 		isDangling := isDanglingImage(img)

daemon/containerd/image_load_test.go

@@ -37,7 +37,7 @@ func TestImageLoad(t *testing.T) {
 
 	imgSvc := fakeImageService(t, ctx, store)
 	// Mock the daemon platform.
-	imgSvc.defaultPlatformOverride = platforms.Only(linuxAmd64)
+	imgSvc.defaultPlatformOverride = &linuxAmd64
 
 	tryLoad := func(ctx context.Context, t *testing.T, dir string, platformList []ocispec.Platform) error {
 		tarRc, err := archive.Tar(dir, compression.None)

daemon/containerd/image_pull.go

@@ -104,10 +104,11 @@ func (i *ImageService) pullTag(ctx context.Context, ref reference.Named, platfor
 	ctx = remotes.WithMediaTypeKeyPrefix(ctx, policyimage.ArtifactTypeCosignSignature, "cosign-signature")
 	ctx = remotes.WithMediaTypeKeyPrefix(ctx, policyimage.ArtifactTypeSigstoreBundle, "sigstore-bundle")
 
-	var opts []containerd.RemoteOpt
+	pullPlatform := i.hostPlatformSpec()
 	if platform != nil {
-		opts = append(opts, containerd.WithPlatform(platforms.FormatAll(*platform)))
+		pullPlatform = *platform
 	}
+	opts := []containerd.RemoteOpt{containerd.WithPlatform(platforms.FormatAll(pullPlatform))}
 
 	resolver, _ := i.newResolverFromAuthConfig(ctx, authConfig, ref, metaHeaders)
 	opts = append(opts, containerd.WithResolver(resolver))
@@ -138,10 +139,7 @@ func (i *ImageService) pullTag(ctx context.Context, ref reference.Named, platfor
 		}()
 	}
 
-	p := platforms.Default()
-	if platform != nil {
-		p = platforms.Only(*platform)
-	}
+	p := platforms.Only(pullPlatform)
 
 	pullJobs := newJobs()
 	opts = append(opts, containerd.WithImageHandler(c8dimages.HandlerFunc(func(ctx context.Context, desc ocispec.Descriptor) ([]ocispec.Descriptor, error) {
@@ -252,10 +250,7 @@ func (i *ImageService) pullTag(ctx context.Context, ref reference.Named, platfor
 			// the same message as the graphdrivers backend.
 			// The one returned by containerd doesn't contain the platform and is much less informative.
 			if strings.Contains(err.Error(), "platform") {
-				platformStr := platforms.DefaultString()
-				if platform != nil {
-					platformStr = platforms.FormatAll(*platform)
-				}
+				platformStr := platforms.FormatAll(pullPlatform)
 				return errdefs.NotFound(fmt.Errorf("no matching manifest for %s in the manifest list entries: %w", platformStr, err))
 			}
 		}

daemon/containerd/image_push_test.go

@@ -195,11 +195,11 @@ func TestImagePushIndex(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			imgSvc := fakeImageService(t, ctx, store)
 			// Mock the daemon platform.
+			daemonPlatform := defaultDaemonPlatform
 			if tc.daemonPlatform != nil {
-				imgSvc.defaultPlatformOverride = platforms.Only(*tc.daemonPlatform)
-			} else {
-				imgSvc.defaultPlatformOverride = platforms.Only(defaultDaemonPlatform)
+				daemonPlatform = *tc.daemonPlatform
 			}
+			imgSvc.defaultPlatformOverride = &daemonPlatform
 
 			idx, _, err := specialimage.MultiPlatform(csDir, "multiplatform:latest", tc.indexPlatforms)
 			assert.NilError(t, err)

daemon/containerd/image_save_test.go

@@ -37,7 +37,7 @@ func TestImageMultiplatformSaveShallowWithNative(t *testing.T) {
 
 	imgSvc := fakeImageService(t, ctx, store)
 	// Mock the native platform.
-	imgSvc.defaultPlatformOverride = platforms.Only(native)
+	imgSvc.defaultPlatformOverride = &native
 
 	idx, _, err := specialimage.PartialMultiPlatform(contentDir, "partial-with-native:latest", specialimage.PartialOpts{
 		Stored:  []ocispec.Platform{native, riscv64},
@@ -98,7 +98,7 @@ func TestImageMultiplatformSaveShallowWithoutNative(t *testing.T) {
 
 	imgSvc := fakeImageService(t, ctx, store)
 	// Mock the native platform.
-	imgSvc.defaultPlatformOverride = platforms.Only(native)
+	imgSvc.defaultPlatformOverride = &native
 
 	idx, _, err := specialimage.PartialMultiPlatform(contentDir, "partial-without-native:latest", specialimage.PartialOpts{
 		Stored:  []ocispec.Platform{arm64, riscv64},

daemon/containerd/platform_matchers.go

@@ -3,6 +3,7 @@ package containerd
 import (
 	"github.com/containerd/platforms"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
+	archvariant "github.com/tonistiigi/go-archvariant"
 )
 
 // platformsWithPreferenceMatcher is a platform matcher that matches any of the
@@ -65,9 +66,18 @@ func (i *ImageService) matchRequestedOrDefault(
 
 // hostPlatformMatcher returns a platform match comparer that matches the host platform.
 func (i *ImageService) hostPlatformMatcher() platforms.MatchComparer {
-	// Allow to override the host platform for testing purposes.
-	if i.defaultPlatformOverride != nil {
-		return i.defaultPlatformOverride
-	}
-	return platforms.Default()
+	return platforms.Only(i.hostPlatformSpec())
+}
+
+// hostPlatformSpec returns the host platform specification.
+func (i *ImageService) hostPlatformSpec() ocispec.Platform {
+	// Allow tests to override the host platform before constructing matchers.
+	if i.defaultPlatformOverride != nil {
+		return *i.defaultPlatformOverride
+	}
+	p := platforms.DefaultSpec()
+	if p.Architecture == "amd64" {
+		p.Variant = archvariant.AMD64Variant()
+	}
+	return p
 }

daemon/containerd/platform_matchers_test.go

@@ -3,6 +3,7 @@ package containerd
 import (
 	"reflect"
 	"runtime"
+	"strings"
 	"testing"
 
 	"github.com/containerd/platforms"
@@ -18,6 +19,24 @@ var (
 		Architecture: "amd64",
 	}
 
+	pLinuxAmd64v2 = ocispec.Platform{
+		OS:           "linux",
+		Architecture: "amd64",
+		Variant:      "v2",
+	}
+
+	pLinuxAmd64v3 = ocispec.Platform{
+		OS:           "linux",
+		Architecture: "amd64",
+		Variant:      "v3",
+	}
+
+	pLinuxAmd64v4 = ocispec.Platform{
+		OS:           "linux",
+		Architecture: "amd64",
+		Variant:      "v4",
+	}
+
 	pLinuxArmv5 = ocispec.Platform{
 		OS:           "linux",
 		Architecture: "arm",
@@ -43,6 +62,55 @@ var (
 	}
 )
 
+func TestHostPlatformSpecSetsAMD64Variant(t *testing.T) {
+	imgSvc := ImageService{defaultPlatformOverride: &pLinuxAmd64}
+	p := imgSvc.hostPlatformSpec()
+	assert.Check(t, is.DeepEqual(p, pLinuxAmd64))
+
+	imgSvc = ImageService{}
+	p = imgSvc.hostPlatformSpec()
+	if p.Architecture == "amd64" {
+		assert.Assert(t, strings.HasPrefix(p.Variant, "v"))
+	}
+}
+
+func TestMatcherOnLinuxAmd64v4(t *testing.T) {
+	yes := true
+	no := false
+
+	for _, indexTc := range []indexTestCase{
+		{
+			name:  "linux_amd64_linux_amd64_v3",
+			index: []ocispec.Platform{pLinuxAmd64, pLinuxAmd64v3},
+			tc: []requestedAndFirst{
+				{requested: nil, first: &pLinuxAmd64v3},
+				{requested: &ocispec.Platform{OS: "linux", Architecture: "amd64"}, first: &pLinuxAmd64},
+				{requested: &ocispec.Platform{OS: "linux", Architecture: "amd64", Variant: "v3"}, first: &pLinuxAmd64v3},
+			},
+		},
+		{
+			name:  "linux_amd64_v3_only",
+			index: []ocispec.Platform{pLinuxAmd64v3},
+			tc: []requestedAndFirst{
+				{requested: nil, first: &pLinuxAmd64v3},
+				{strict: &yes, requested: &ocispec.Platform{OS: "linux", Architecture: "amd64"}, first: nil},
+				{strict: &no, requested: &ocispec.Platform{OS: "linux", Architecture: "amd64"}, first: nil},
+			},
+		},
+		{
+			name:  "linux_amd64_v2_v3",
+			index: []ocispec.Platform{pLinuxAmd64v2, pLinuxAmd64v3},
+			tc: []requestedAndFirst{
+				{requested: nil, first: &pLinuxAmd64v3},
+				{strict: &yes, requested: &ocispec.Platform{OS: "linux", Architecture: "amd64", Variant: "v4"}, first: nil},
+				{strict: &no, requested: &ocispec.Platform{OS: "linux", Architecture: "amd64", Variant: "v4"}, first: &pLinuxAmd64v3},
+			},
+		},
+	} {
+		testOnlyAndOnlyStrict(t, pLinuxAmd64v4, indexTc)
+	}
+}
+
 type requestedAndFirst struct {
 	// Whether platforms.Only or OnlyStrict should be used
 	// Nil means both should be the same
@@ -58,11 +126,11 @@ type indexTestCase struct {
 }
 
 func TestMatcherOnLinuxArm64v8(t *testing.T) {
-	daemonPlatform := platforms.Only(ocispec.Platform{
+	daemonPlatform := ocispec.Platform{
 		OS:           "linux",
 		Architecture: "arm64",
 		Variant:      "v8",
-	})
+	}
 
 	yes := true
 	no := false
@@ -96,11 +164,11 @@ func TestMatcherOnLinuxArm64v8(t *testing.T) {
 
 func TestMatcherOnWindowsAmd64(t *testing.T) {
 	skip.If(t, runtime.GOOS != "windows", "TODO: containerd matcher only matches OSVersion when on Windows")
-	daemonPlatform := platforms.Only(ocispec.Platform{
+	daemonPlatform := ocispec.Platform{
 		OS:           "windows",
 		Architecture: "amd64",
 		OSVersion:    "10.0.18362",
-	})
+	}
 
 	for _, indexTc := range []indexTestCase{
 		{
@@ -121,9 +189,9 @@ func TestMatcherOnWindowsAmd64(t *testing.T) {
 	}
 }
 
-func testOnlyAndOnlyStrict(t *testing.T, daemonPlatform platforms.MatchComparer, indexTc indexTestCase) {
+func testOnlyAndOnlyStrict(t *testing.T, daemonPlatform ocispec.Platform, indexTc indexTestCase) {
 	imgSvc := ImageService{}
-	imgSvc.defaultPlatformOverride = daemonPlatform
+	imgSvc.defaultPlatformOverride = &daemonPlatform
 
 	t.Run(indexTc.name, func(t *testing.T) {
 		indexTc := indexTc

daemon/containerd/service.go

@@ -13,7 +13,6 @@ import (
 	"github.com/containerd/containerd/v2/plugins"
 	cerrdefs "github.com/containerd/errdefs"
 	"github.com/containerd/log"
-	"github.com/containerd/platforms"
 	"github.com/moby/moby/v2/daemon/container"
 	"github.com/moby/moby/v2/daemon/containerd/identitycache"
 	daemonevents "github.com/moby/moby/v2/daemon/events"
@@ -47,7 +46,7 @@ type ImageService struct {
 	identity            imageIdentityState
 
 	// defaultPlatformOverride is used in tests to override the host platform.
-	defaultPlatformOverride platforms.MatchComparer
+	defaultPlatformOverride *ocispec.Platform
 }
 
 type ImageServiceConfig struct {