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 877ebbe038)
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
This commit is contained in:
Paweł Gronowski 2023-12-01 10:24:29 +01:00
parent 17af50f46b
commit 8a19bb7193
No known key found for this signature in database
GPG key ID: B85EFCFE26DEF92A
8 changed files with 62 additions and 36 deletions

View file

@ -15,6 +15,7 @@ import (
"github.com/docker/docker/image"
"github.com/docker/docker/layer"
"github.com/opencontainers/go-digest"
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.

View file

@ -8,7 +8,6 @@ import (
"net/url"
"os"
"path/filepath"
"runtime"
"sort"
"strings"
"time"
@ -74,7 +73,7 @@ type copier struct {
source builder.Source
pathCache pathCache
download sourceDownloader
platform *ocispec.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.
@ -83,19 +82,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 = &ocispec.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,

View file

@ -5,6 +5,7 @@ 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"
)
@ -12,7 +13,7 @@ import (
// cache.
type ImageProber interface {
Reset(ctx context.Context) error
Probe(parentID string, runConfig *container.Config) (string, error)
Probe(parentID string, runConfig *container.Config, platform ocispec.Platform) (string, error)
}
type resetFunc func(context.Context) (builder.ImageCache, error)
@ -51,11 +52,11 @@ func (c *imageProber) Reset(ctx context.Context) error {
// 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
}
@ -74,6 +75,6 @@ func (c *nopProber) Reset(ctx context.Context) error {
return nil
}
func (c *nopProber) Probe(_ string, _ *container.Config) (string, error) {
func (c *nopProber) Probe(_ string, _ *container.Config, _ ocispec.Platform) (string, error) {
return "", nil
}

View file

@ -10,6 +10,7 @@ import (
"fmt"
"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"
@ -328,7 +329,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
}
@ -388,3 +389,17 @@ func hostConfigFromOptions(options *types.ImageBuildOptions) *container.HostConf
}
return hc
}
func (b *Builder) getPlatform(state *dispatchState) ocispec.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
}

View file

@ -14,6 +14,7 @@ import (
"github.com/docker/docker/image"
"github.com/docker/docker/layer"
"github.com/opencontainers/go-digest"
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)
}

View file

@ -8,7 +8,9 @@ import (
"github.com/docker/docker/api/types/container"
imagetype "github.com/docker/docker/api/types/image"
"github.com/docker/docker/builder"
"github.com/docker/docker/errdefs"
"github.com/docker/docker/image"
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
)
// MakeImageCache creates a stateful image cache.
@ -29,7 +31,7 @@ type imageCache struct {
c *ImageService
}
func (ic *imageCache) GetCache(parentID string, cfg *container.Config) (imageID string, err error) {
func (ic *imageCache) GetCache(parentID string, cfg *container.Config, platform ocispec.Platform) (imageID string, err error) {
ctx := context.TODO()
if parentID == "" {
@ -37,8 +39,11 @@ func (ic *imageCache) GetCache(parentID string, cfg *container.Config) (imageID
return "", nil
}
parent, err := ic.c.GetImage(ctx, parentID, imagetype.GetImageOpts{})
parent, err := ic.c.GetImage(ctx, parentID, imagetype.GetImageOpts{Platform: &platform})
if err != nil {
if errdefs.IsNotFound(err) {
return "", nil
}
return "", err
}
@ -54,8 +59,11 @@ func (ic *imageCache) GetCache(parentID string, cfg *container.Config) (imageID
}
for _, children := range children {
childImage, err := ic.c.GetImage(ctx, children.String(), imagetype.GetImageOpts{})
childImage, err := ic.c.GetImage(ctx, children.String(), imagetype.GetImageOpts{Platform: &platform})
if err != nil {
if errdefs.IsNotFound(err) {
continue
}
return "", err
}

32
image/cache/cache.go vendored
View file

@ -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) {

View file

@ -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
}