Merge pull request #39177 from pricec/image-variant

Add variant to image.Image and legacy builder

api/types/types.go

@@ -39,6 +39,7 @@ type ImageInspect struct {
 	Author          string
 	Config          *container.Config
 	Architecture    string
+	Variant         string `json:",omitempty"`
 	Os              string
 	OsVersion       string `json:",omitempty"`
 	Size            int64

builder/dockerfile/imagecontext.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"runtime"
 
+	"github.com/containerd/containerd/platforms"
 	"github.com/docker/docker/api/types/backend"
 	"github.com/docker/docker/builder"
 	dockerimage "github.com/docker/docker/image"
@@ -56,7 +57,7 @@ func (m *imageSources) Get(idOrRef string, localOnly bool, platform *specs.Platf
 		return nil, err
 	}
 	im := newImageMount(image, layer)
-	m.Add(im)
+	m.Add(im, platform)
 	return im, nil
 }
 
@@ -70,16 +71,26 @@ func (m *imageSources) Unmount() (retErr error) {
 	return
 }
 
-func (m *imageSources) Add(im *imageMount) {
+func (m *imageSources) Add(im *imageMount, platform *specs.Platform) {
 	switch im.image {
 	case nil:
-		// set the OS for scratch images
-		os := runtime.GOOS
+		// Set the platform for scratch images
+		if platform == nil {
+			p := platforms.DefaultSpec()
+			platform = &p
+		}
+
 		// Windows does not support scratch except for LCOW
+		os := platform.OS
 		if runtime.GOOS == "windows" {
 			os = "linux"
 		}
-		im.image = &dockerimage.Image{V1Image: dockerimage.V1Image{OS: os}}
+
+		im.image = &dockerimage.Image{V1Image: dockerimage.V1Image{
+			OS:           os,
+			Architecture: platform.Architecture,
+			Variant:      platform.Variant,
+		}}
 	default:
 		m.byImageID[im.image.ImageID()] = im
 	}

builder/dockerfile/imagecontext_test.go

@@ -0,0 +1,106 @@
+package dockerfile // import "github.com/docker/docker/builder/dockerfile"
+
+import (
+	"fmt"
+	"runtime"
+	"testing"
+
+	"github.com/containerd/containerd/platforms"
+	"github.com/docker/docker/builder"
+	"github.com/docker/docker/image"
+	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
+	"gotest.tools/assert"
+)
+
+func getMockImageSource(getImageImage builder.Image, getImageLayer builder.ROLayer, getImageError error) *imageSources {
+	return &imageSources{
+		byImageID: make(map[string]*imageMount),
+		mounts:    []*imageMount{},
+		getImage: func(name string, localOnly bool, platform *ocispec.Platform) (builder.Image, builder.ROLayer, error) {
+			return getImageImage, getImageLayer, getImageError
+		},
+	}
+}
+
+func getMockImageMount() *imageMount {
+	return &imageMount{
+		image: nil,
+		layer: nil,
+	}
+}
+
+func TestAddScratchImageAddsToMounts(t *testing.T) {
+	is := getMockImageSource(nil, nil, fmt.Errorf("getImage is not implemented"))
+	im := getMockImageMount()
+
+	// We are testing whether the imageMount is added to is.mounts
+	assert.Equal(t, len(is.mounts), 0)
+	is.Add(im, nil)
+	assert.Equal(t, len(is.mounts), 1)
+}
+
+func TestAddFromScratchPopulatesPlatform(t *testing.T) {
+	is := getMockImageSource(nil, nil, fmt.Errorf("getImage is not implemented"))
+
+	platforms := []*ocispec.Platform{
+		{
+			OS:           "linux",
+			Architecture: "amd64",
+		},
+		{
+			OS:           "linux",
+			Architecture: "arm64",
+			Variant:      "v8",
+		},
+	}
+
+	for i, platform := range platforms {
+		im := getMockImageMount()
+		assert.Equal(t, len(is.mounts), i)
+		is.Add(im, platform)
+		image, ok := im.image.(*image.Image)
+		assert.Assert(t, ok)
+		assert.Equal(t, image.OS, platform.OS)
+		assert.Equal(t, image.Architecture, platform.Architecture)
+		assert.Equal(t, image.Variant, platform.Variant)
+	}
+}
+
+func TestAddFromScratchDoesNotModifyArgPlatform(t *testing.T) {
+	is := getMockImageSource(nil, nil, fmt.Errorf("getImage is not implemented"))
+	im := getMockImageMount()
+
+	platform := &ocispec.Platform{
+		OS:           "windows",
+		Architecture: "amd64",
+	}
+	argPlatform := *platform
+
+	is.Add(im, &argPlatform)
+	// The way the code is written right now, this test
+	// really doesn't do much except on Windows.
+	assert.DeepEqual(t, *platform, argPlatform)
+}
+
+func TestAddFromScratchPopulatesPlatformIfNil(t *testing.T) {
+	is := getMockImageSource(nil, nil, fmt.Errorf("getImage is not implemented"))
+	im := getMockImageMount()
+	is.Add(im, nil)
+	image, ok := im.image.(*image.Image)
+	assert.Assert(t, ok)
+
+	expectedPlatform := platforms.DefaultSpec()
+	if runtime.GOOS == "windows" {
+		expectedPlatform.OS = "linux"
+	}
+	assert.Equal(t, expectedPlatform.OS, image.OS)
+	assert.Equal(t, expectedPlatform.Architecture, image.Architecture)
+	assert.Equal(t, expectedPlatform.Variant, image.Variant)
+}
+
+func TestImageSourceGetAddsToMounts(t *testing.T) {
+	is := getMockImageSource(nil, nil, nil)
+	_, err := is.Get("test", false, nil)
+	assert.NilError(t, err)
+	assert.Equal(t, len(is.mounts), 1)
+}

builder/dockerfile/internals.go

@@ -26,6 +26,7 @@ import (
 	"github.com/docker/docker/pkg/stringid"
 	"github.com/docker/docker/pkg/system"
 	"github.com/docker/go-connections/nat"
+	specs "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 )
@@ -117,15 +118,21 @@ func (b *Builder) exportImage(state *dispatchState, layer builder.RWLayer, paren
 		return err
 	}
 
-	// add an image mount without an image so the layer is properly unmounted
-	// if there is an error before we can add the full mount with image
-	b.imageSources.Add(newImageMount(nil, newLayer))
-
 	parentImage, ok := parent.(*image.Image)
 	if !ok {
 		return errors.Errorf("unexpected image type")
 	}
 
+	platform := &specs.Platform{
+		OS:           parentImage.OS,
+		Architecture: parentImage.Architecture,
+		Variant:      parentImage.Variant,
+	}
+
+	// add an image mount without an image so the layer is properly unmounted
+	// if there is an error before we can add the full mount with image
+	b.imageSources.Add(newImageMount(nil, newLayer), platform)
+
 	newImage := image.NewChildImage(parentImage, image.ChildConfig{
 		Author:          state.maintainer,
 		ContainerConfig: runConfig,
@@ -146,7 +153,7 @@ func (b *Builder) exportImage(state *dispatchState, layer builder.RWLayer, paren
 	}
 
 	state.imageID = exportedImage.ImageID()
-	b.imageSources.Add(newImageMount(exportedImage, newLayer))
+	b.imageSources.Add(newImageMount(exportedImage, newLayer), platform)
 	return nil
 }
 

builder/dockerfile/internals_test.go

@@ -11,8 +11,12 @@ import (
 	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/builder"
 	"github.com/docker/docker/builder/remotecontext"
+	"github.com/docker/docker/image"
+	"github.com/docker/docker/layer"
 	"github.com/docker/docker/pkg/archive"
+	"github.com/docker/docker/pkg/containerfs"
 	"github.com/docker/go-connections/nat"
+	"github.com/opencontainers/go-digest"
 	"gotest.tools/assert"
 	is "gotest.tools/assert/cmp"
 	"gotest.tools/skip"
@@ -176,3 +180,45 @@ func TestDeepCopyRunConfig(t *testing.T) {
 	copy.Shell[0] = "sh"
 	assert.Check(t, is.DeepEqual(fullMutableRunConfig(), runConfig))
 }
+
+type MockRWLayer struct{}
+
+func (l *MockRWLayer) Release() error                { return nil }
+func (l *MockRWLayer) Root() containerfs.ContainerFS { return nil }
+func (l *MockRWLayer) Commit() (builder.ROLayer, error) {
+	return &MockROLayer{
+		diffID: layer.DiffID(digest.Digest("sha256:1234")),
+	}, nil
+}
+
+type MockROLayer struct {
+	diffID layer.DiffID
+}
+
+func (l *MockROLayer) Release() error                       { return nil }
+func (l *MockROLayer) NewRWLayer() (builder.RWLayer, error) { return nil, nil }
+func (l *MockROLayer) DiffID() layer.DiffID                 { return l.diffID }
+
+func getMockBuildBackend() builder.Backend {
+	return &MockBackend{}
+}
+
+func TestExportImage(t *testing.T) {
+	ds := newDispatchState(NewBuildArgs(map[string]*string{}))
+	layer := &MockRWLayer{}
+	parentImage := &image.Image{
+		V1Image: image.V1Image{
+			OS:           "linux",
+			Architecture: "arm64",
+			Variant:      "v8",
+		},
+	}
+	runConfig := &container.Config{}
+
+	b := &Builder{
+		imageSources: getMockImageSource(nil, nil, nil),
+		docker:       getMockBuildBackend(),
+	}
+	err := b.exportImage(ds, layer, parentImage, runConfig)
+	assert.NilError(t, err)
+}

builder/dockerfile/mockbackend_test.go

@@ -82,7 +82,7 @@ func (m *MockBackend) MakeImageCache(cacheFrom []string) builder.ImageCache {
 }
 
 func (m *MockBackend) CreateImage(config []byte, parent string) (builder.Image, error) {
-	return nil, nil
+	return &mockImage{id: "test"}, nil
 }
 
 type mockImage struct {

daemon/images/image_inspect.go

@@ -76,6 +76,7 @@ func (i *ImageService) LookupImage(name string) (*types.ImageInspect, error) {
 		Author:          img.Author,
 		Config:          img.Config,
 		Architecture:    img.Architecture,
+		Variant:         img.Variant,
 		Os:              img.OperatingSystem(),
 		OsVersion:       img.OSVersion,
 		Size:            size,

distribution/config.go

@@ -170,7 +170,7 @@ func (s *imageConfigStore) PlatformFromConfig(c []byte) (*specs.Platform, error)
 	if !system.IsOSSupported(os) {
 		return nil, system.ErrNotSupportedOperatingSystem
 	}
-	return &specs.Platform{OS: os, Architecture: unmarshalledConfig.Architecture, OSVersion: unmarshalledConfig.OSVersion}, nil
+	return &specs.Platform{OS: os, Architecture: unmarshalledConfig.Architecture, Variant: unmarshalledConfig.Variant, OSVersion: unmarshalledConfig.OSVersion}, nil
 }
 
 type storeLayerProvider struct {

image/image.go

@@ -53,6 +53,8 @@ type V1Image struct {
 	Config *container.Config `json:"config,omitempty"`
 	// Architecture is the hardware that the image is built and runs on
 	Architecture string `json:"architecture,omitempty"`
+	// Variant is the CPU architecture variant (presently ARM-only)
+	Variant string `json:"variant,omitempty"`
 	// OS is the operating system used to build and run the image
 	OS string `json:"os,omitempty"`
 	// Size is the total size of the image including all layers it is composed of
@@ -105,6 +107,13 @@ func (img *Image) BaseImgArch() string {
 	return arch
 }
 
+// BaseImgVariant returns the image's variant, whether populated or not.
+// This avoids creating an inconsistency where the stored image variant
+// is "greater than" (i.e. v8 vs v6) the actual image variant.
+func (img *Image) BaseImgVariant() string {
+	return img.Variant
+}
+
 // OperatingSystem returns the image's operating system. If not populated, defaults to the host runtime OS.
 func (img *Image) OperatingSystem() string {
 	os := img.OS
@@ -167,6 +176,7 @@ func NewChildImage(img *Image, child ChildConfig, os string) *Image {
 			DockerVersion:   dockerversion.Version,
 			Config:          child.Config,
 			Architecture:    img.BaseImgArch(),
+			Variant:         img.BaseImgVariant(),
 			OS:              os,
 			Container:       child.ContainerID,
 			ContainerConfig: *child.ContainerConfig,