diff --git a/builder/builder.go b/builder/builder.go index f01563812f..e74fc54a49 100644 --- a/builder/builder.go +++ b/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. diff --git a/builder/dockerfile/copy.go b/builder/dockerfile/copy.go index 2c413ca047..b052527a1f 100644 --- a/builder/dockerfile/copy.go +++ b/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, diff --git a/builder/dockerfile/imageprobe.go b/builder/dockerfile/imageprobe.go index 6960bf8897..c2a8d116b1 100644 --- a/builder/dockerfile/imageprobe.go +++ b/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 } diff --git a/builder/dockerfile/internals.go b/builder/dockerfile/internals.go index 1bc445d383..c8ece04ca0 100644 --- a/builder/dockerfile/internals.go +++ b/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 +} diff --git a/builder/dockerfile/mockbackend_test.go b/builder/dockerfile/mockbackend_test.go index 0310374a69..5cd91a5374 100644 --- a/builder/dockerfile/mockbackend_test.go +++ b/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) } diff --git a/image/cache/cache.go b/image/cache/cache.go index 77cb9d1706..ee89a171e2 100644 --- a/image/cache/cache.go +++ b/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) { diff --git a/image/cache/compare.go b/image/cache/compare.go index ec1971a207..d438b65be7 100644 --- a/image/cache/compare.go +++ b/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 }