image/cache: Check image platform

Make sure the cache candidate platform matches the requested.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
(cherry picked from commit 877ebbe038ee914e4243d5b7e198eda00494d757)
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
(cherry picked from commit 8a19bb719377a5d9e26bf63ae88e155f37a87701)
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>

builder/builder.go

@@ -15,6 +15,7 @@ import (
 	"github.com/docker/docker/image"
 	"github.com/docker/docker/layer"
 	"github.com/docker/docker/pkg/containerfs"
+	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 )
 
 const (
@@ -89,7 +90,7 @@ type ImageCacheBuilder interface {
 type ImageCache interface {
 	// GetCache returns a reference to a cached image whose parent equals `parent`
 	// and runconfig equals `cfg`. A cache miss is expected to return an empty ID and a nil error.
-	GetCache(parentID string, cfg *container.Config) (imageID string, err error)
+	GetCache(parentID string, cfg *container.Config, platform ocispec.Platform) (imageID string, err error)
 }
 
 // Image represents a Docker image used by the builder.

builder/dockerfile/copy.go

@@ -9,7 +9,6 @@ import (
 	"net/url"
 	"os"
 	"path/filepath"
-	"runtime"
 	"sort"
 	"strings"
 	"time"
@@ -25,7 +24,7 @@ import (
 	"github.com/docker/docker/pkg/streamformatter"
 	"github.com/docker/docker/pkg/system"
 	"github.com/moby/buildkit/frontend/dockerfile/instructions"
-	specs "github.com/opencontainers/image-spec/specs-go/v1"
+	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/pkg/errors"
 )
 
@@ -75,7 +74,7 @@ type copier struct {
 	source      builder.Source
 	pathCache   pathCache
 	download    sourceDownloader
-	platform    *specs.Platform
+	platform    ocispec.Platform
 	// for cleanup. TODO: having copier.cleanup() is error prone and hard to
 	// follow. Code calling performCopy should manage the lifecycle of its params.
 	// Copier should take override source as input, not imageMount.
@@ -84,19 +83,7 @@ type copier struct {
 }
 
 func copierFromDispatchRequest(req dispatchRequest, download sourceDownloader, imageSource *imageMount) copier {
-	platform := req.builder.platform
-	if platform == nil {
-		// May be nil if not explicitly set in API/dockerfile
-		platform = &specs.Platform{}
-	}
-	if platform.OS == "" {
-		// Default to the dispatch requests operating system if not explicit in API/dockerfile
-		platform.OS = req.state.operatingSystem
-	}
-	if platform.OS == "" {
-		// This is a failsafe just in case. Shouldn't be hit.
-		platform.OS = runtime.GOOS
-	}
+	platform := req.builder.getPlatform(req.state)
 
 	return copier{
 		source:      req.source,

builder/dockerfile/imageprobe.go

@@ -3,6 +3,7 @@ package dockerfile // import "github.com/docker/docker/builder/dockerfile"
 import (
 	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/builder"
+	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/sirupsen/logrus"
 )
 
@@ -10,7 +11,7 @@ import (
 // cache.
 type ImageProber interface {
 	Reset()
-	Probe(parentID string, runConfig *container.Config) (string, error)
+	Probe(parentID string, runConfig *container.Config, platform ocispec.Platform) (string, error)
 }
 
 type imageProber struct {
@@ -37,11 +38,11 @@ func (c *imageProber) Reset() {
 
 // Probe checks if cache match can be found for current build instruction.
 // It returns the cachedID if there is a hit, and the empty string on miss
-func (c *imageProber) Probe(parentID string, runConfig *container.Config) (string, error) {
+func (c *imageProber) Probe(parentID string, runConfig *container.Config, platform ocispec.Platform) (string, error) {
 	if c.cacheBusted {
 		return "", nil
 	}
-	cacheID, err := c.cache.GetCache(parentID, runConfig)
+	cacheID, err := c.cache.GetCache(parentID, runConfig, platform)
 	if err != nil {
 		return "", err
 	}
@@ -58,6 +59,6 @@ type nopProber struct{}
 
 func (c *nopProber) Reset() {}
 
-func (c *nopProber) Probe(_ string, _ *container.Config) (string, error) {
+func (c *nopProber) Probe(_ string, _ *container.Config, _ ocispec.Platform) (string, error) {
 	return "", nil
 }

builder/dockerfile/internals.go

@@ -10,6 +10,7 @@ import (
 	"io"
 	"strings"
 
+	"github.com/containerd/containerd/platforms"
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/backend"
 	"github.com/docker/docker/api/types/container"
@@ -364,7 +365,7 @@ func getShell(c *container.Config, os string) []string {
 }
 
 func (b *Builder) probeCache(dispatchState *dispatchState, runConfig *container.Config) (bool, error) {
-	cachedID, err := b.imageProber.Probe(dispatchState.imageID, runConfig)
+	cachedID, err := b.imageProber.Probe(dispatchState.imageID, runConfig, b.getPlatform(dispatchState))
 	if cachedID == "" || err != nil {
 		return false, err
 	}
@@ -424,3 +425,17 @@ func hostConfigFromOptions(options *types.ImageBuildOptions) *container.HostConf
 	}
 	return hc
 }
+
+func (b *Builder) getPlatform(state *dispatchState) specs.Platform {
+	// May be nil if not explicitly set in API/dockerfile
+	out := platforms.DefaultSpec()
+	if b.platform != nil {
+		out = *b.platform
+	}
+
+	if state.operatingSystem != "" {
+		out.OS = state.operatingSystem
+	}
+
+	return out
+}

builder/dockerfile/mockbackend_test.go

@@ -14,6 +14,7 @@ import (
 	"github.com/docker/docker/image"
 	"github.com/docker/docker/layer"
 	"github.com/docker/docker/pkg/containerfs"
+	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 )
 
 // MockBackend implements the builder.Backend interface for unit testing
@@ -111,7 +112,7 @@ type mockImageCache struct {
 	getCacheFunc func(parentID string, cfg *container.Config) (string, error)
 }
 
-func (mic *mockImageCache) GetCache(parentID string, cfg *container.Config) (string, error) {
+func (mic *mockImageCache) GetCache(parentID string, cfg *container.Config, _ ocispec.Platform) (string, error) {
 	if mic.getCacheFunc != nil {
 		return mic.getCacheFunc(parentID, cfg)
 	}

image/cache/cache.go

@@ -1,18 +1,19 @@
 package cache // import "github.com/docker/docker/image/cache"
 
 import (
-	"context"
 	"encoding/json"
 	"fmt"
 	"reflect"
 	"strings"
 
-	"github.com/containerd/log"
+	"github.com/containerd/containerd/platforms"
 	containertypes "github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/dockerversion"
 	"github.com/docker/docker/image"
 	"github.com/docker/docker/layer"
+	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
 )
 
 // NewLocal returns a local image cache, based on parent chain
@@ -28,8 +29,8 @@ type LocalImageCache struct {
 }
 
 // GetCache returns the image id found in the cache
-func (lic *LocalImageCache) GetCache(imgID string, config *containertypes.Config) (string, error) {
-	return getImageIDAndError(getLocalCachedImage(lic.store, image.ID(imgID), config))
+func (lic *LocalImageCache) GetCache(imgID string, config *containertypes.Config, platform ocispec.Platform) (string, error) {
+	return getImageIDAndError(getLocalCachedImage(lic.store, image.ID(imgID), config, platform))
 }
 
 // New returns an image cache, based on history objects
@@ -53,8 +54,8 @@ func (ic *ImageCache) Populate(image *image.Image) {
 }
 
 // GetCache returns the image id found in the cache
-func (ic *ImageCache) GetCache(parentID string, cfg *containertypes.Config) (string, error) {
-	imgID, err := ic.localImageCache.GetCache(parentID, cfg)
+func (ic *ImageCache) GetCache(parentID string, cfg *containertypes.Config, platform ocispec.Platform) (string, error) {
+	imgID, err := ic.localImageCache.GetCache(parentID, cfg, platform)
 	if err != nil {
 		return "", err
 	}
@@ -217,7 +218,7 @@ func getImageIDAndError(img *image.Image, err error) (string, error) {
 // of the image with imgID, that had the same config when it was
 // created. nil is returned if a child cannot be found. An error is
 // returned if the parent image cannot be found.
-func getLocalCachedImage(imageStore image.Store, imgID image.ID, config *containertypes.Config) (*image.Image, error) {
+func getLocalCachedImage(imageStore image.Store, imgID image.ID, config *containertypes.Config, platform ocispec.Platform) (*image.Image, error) {
 	if config == nil {
 		return nil, nil
 	}
@@ -225,7 +226,7 @@ func getLocalCachedImage(imageStore image.Store, imgID image.ID, config *contain
 	isBuiltLocally := func(id image.ID) bool {
 		builtLocally, err := imageStore.IsBuiltLocally(id)
 		if err != nil {
-			log.G(context.TODO()).WithFields(log.Fields{
+			logrus.WithFields(logrus.Fields{
 				"error": err,
 				"id":    id,
 			}).Warn("failed to check if image was built locally")
@@ -247,6 +248,21 @@ func getLocalCachedImage(imageStore image.Store, imgID image.ID, config *contain
 				continue
 			}
 
+			imgPlatform := ocispec.Platform{
+				Architecture: img.Architecture,
+				OS:           img.OS,
+				OSVersion:    img.OSVersion,
+				OSFeatures:   img.OSFeatures,
+				Variant:      img.Variant,
+			}
+			// Discard old linux/amd64 images with empty platform.
+			if imgPlatform.OS == "" && imgPlatform.Architecture == "" {
+				continue
+			}
+			if !platforms.OnlyStrict(platform).Match(imgPlatform) {
+				continue
+			}
+
 			if compare(&img.ContainerConfig, config) {
 				// check for the most up to date match
 				if match == nil || match.Created.Before(img.Created) {

image/cache/compare.go

@@ -135,9 +135,6 @@ func compare(a, b *container.Config) bool {
 		if a.Healthcheck.Interval != b.Healthcheck.Interval {
 			return false
 		}
-		if a.Healthcheck.StartInterval != b.Healthcheck.StartInterval {
-			return false
-		}
 		if a.Healthcheck.StartPeriod != b.Healthcheck.StartPeriod {
 			return false
 		}