Skip to content

Commit

Permalink
Add variant to image.Image and legacy builder
Browse files Browse the repository at this point in the history
This commit adds the image variant to the image.(Image) type and
updates related functionality. Images built from another will
inherit the OS, architecture and variant.

Note that if a base image does not specify an architecture, the
local machine's architecture is used for inherited images. On the
other hand, the variant is set equal to the parent image's variant,
even when the parent image's variant is unset.

The legacy builder is also updated to allow the user to specify
a '--platform' argument on the command line when creating an image
FROM scratch. A complete platform specification, including variant,
is supported. The built image will include the variant, as will any
derived images.

Signed-off-by: Chris Price <[email protected]>
  • Loading branch information
Chris Price authored and Tibor Vass committed Sep 24, 2019
1 parent 30c5ec4 commit c21a3cf
Show file tree
Hide file tree
Showing 9 changed files with 194 additions and 12 deletions.
1 change: 1 addition & 0 deletions api/types/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 16 additions & 5 deletions builder/dockerfile/imagecontext.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}

Expand All @@ -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
}
Expand Down
106 changes: 106 additions & 0 deletions builder/dockerfile/imagecontext_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
17 changes: 12 additions & 5 deletions builder/dockerfile/internals.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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,
Expand All @@ -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
}

Expand Down
46 changes: 46 additions & 0 deletions builder/dockerfile/internals_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
2 changes: 1 addition & 1 deletion builder/dockerfile/mockbackend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions daemon/images/image_inspect.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion distribution/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
10 changes: 10 additions & 0 deletions image/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit c21a3cf

Please sign in to comment.