c8d: make the cache in classic builder work

In order for the cache in the classic builder to work we need to: - use the came comparison function as the graph drivers implementation - save the container config when commiting the image - use all images to search a 'FROM "scratch"' image - load all images if `cacheFrom` is empty Signed-off-by: Djordje Lukic <djordje.lukic@docker.com> Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
moby · Jan 17, 2024 · 71ebfc7 · 71ebfc7
1 parent 2c47a6d
commit 71ebfc7
Show file tree

Hide file tree

Showing 8 changed files with 298 additions and 218 deletions.
diff --git a/daemon/containerd/cache.go b/daemon/containerd/cache.go
@@ -5,6 +5,7 @@ import (
 	"reflect"
 	"strings"
 
+	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/container"
 	imagetype "github.com/docker/docker/api/types/image"
 	"github.com/docker/docker/builder"
@@ -14,80 +15,232 @@ import (
 // MakeImageCache creates a stateful image cache.
 func (i *ImageService) MakeImageCache(ctx context.Context, cacheFrom []string) (builder.ImageCache, error) {
 	images := []*image.Image{}
+	if len(cacheFrom) == 0 {
+		return &localCache{
+			imageService: i,
+		}, nil
+	}
+
 	for _, c := range cacheFrom {
-		im, err := i.GetImage(ctx, c, imagetype.GetImageOpts{})
+		h, err := i.ImageHistory(ctx, c)
 		if err != nil {
-			return nil, err
+			continue
+		}
+		for _, hi := range h {
+			if hi.ID != "<missing>" {
+				im, err := i.GetImage(ctx, hi.ID, imagetype.GetImageOpts{})
+				if err != nil {
+					return nil, err
+				}
+				images = append(images, im)
+			}
 		}
-		images = append(images, im)
 	}
-	return &imageCache{images: images, c: i}, nil
+
+	return &imageCache{
+		lc: &localCache{
+			imageService: i,
+		},
+		images:       images,
+		imageService: i,
+	}, nil
 }
 
-type imageCache struct {
-	images []*image.Image
-	c      *ImageService
+type localCache struct {
+	imageService *ImageService
 }
 
-func (ic *imageCache) GetCache(parentID string, cfg *container.Config) (imageID string, err error) {
+func (ic *localCache) GetCache(parentID string, cfg *container.Config) (imageID string, err error) {
 	ctx := context.TODO()
 
+	var children []image.ID
+
+	// FROM scratch
 	if parentID == "" {
-		// TODO handle "parentless" image cache lookups ("FROM scratch")
-		return "", nil
+		imgs, err := ic.imageService.Images(ctx, types.ImageListOptions{
+			All: true,
+		})
+		if err != nil {
+			return "", err
+		}
+		for _, img := range imgs {
+			if img.ParentID == parentID {
+				children = append(children, image.ID(img.ID))
+			}
+		}
+	} else {
+		c, err := ic.imageService.Children(ctx, image.ID(parentID))
+		if err != nil {
+			return "", err
+		}
+		children = c
 	}
 
-	parent, err := ic.c.GetImage(ctx, parentID, imagetype.GetImageOpts{})
-	if err != nil {
-		return "", err
-	}
+	var match *image.Image
+	for _, child := range children {
+		childImage, err := ic.imageService.GetImage(ctx, child.String(), imagetype.GetImageOpts{})
+		if err != nil {
+			return "", err
+		}
 
-	for _, localCachedImage := range ic.images {
-		if isMatch(localCachedImage, parent, cfg) {
-			return localCachedImage.ID().String(), nil
+		if isMatch(&childImage.ContainerConfig, cfg) {
+			if childImage.Created != nil && (match == nil || match.Created.Before(*childImage.Created)) {
+				match = childImage
+			}
 		}
 	}
 
-	children, err := ic.c.Children(ctx, parent.ID())
+	if match == nil {
+		return "", nil
+	}
+
+	return match.ID().String(), nil
+}
+
+type imageCache struct {
+	images       []*image.Image
+	imageService *ImageService
+	lc           *localCache
+}
+
+func (ic *imageCache) GetCache(parentID string, cfg *container.Config) (imageID string, err error) {
+	ctx := context.TODO()
+
+	imgID, err := ic.lc.GetCache(parentID, cfg)
 	if err != nil {
 		return "", err
 	}
+	if imgID != "" {
+		for _, s := range ic.images {
+			if ic.isParent(ctx, s, image.ID(imgID)) {
+				return imgID, nil
+			}
+		}
+	}
 
-	for _, children := range children {
-		childImage, err := ic.c.GetImage(ctx, children.String(), imagetype.GetImageOpts{})
+	var parent *image.Image
+	lenHistory := 0
+
+	if parentID != "" {
+		parent, err = ic.imageService.GetImage(ctx, parentID, imagetype.GetImageOpts{})
 		if err != nil {
 			return "", err
 		}
-
-		if isMatch(childImage, parent, cfg) {
-			return children.String(), nil
+		lenHistory = len(parent.History)
+	}
+	for _, target := range ic.images {
+		if !isValidParent(target, parent) || !isValidConfig(cfg, target.History[lenHistory]) {
+			continue
 		}
+		return target.ID().String(), nil
 	}
 
 	return "", nil
 }
 
-// isMatch checks whether a given target can be used as cache for the given
-// parent image/config combination.
-// A target can only be an immediate child of the given parent image. For
-// a parent image with `n` history entries, a valid target must have `n+1`
-// entries and the extra entry must match the provided config
-func isMatch(target, parent *image.Image, cfg *container.Config) bool {
-	if target == nil || parent == nil || cfg == nil {
+func isValidConfig(cfg *container.Config, h image.History) bool {
+	// todo: make this format better than join that loses data
+	return strings.Join(cfg.Cmd, " ") == h.CreatedBy
+}
+
+func isValidParent(img, parent *image.Image) bool {
+	if len(img.History) == 0 {
+		return false
+	}
+	if parent == nil || len(parent.History) == 0 && len(parent.RootFS.DiffIDs) == 0 {
+		return true
+	}
+	if len(parent.History) >= len(img.History) {
+		return false
+	}
+	if len(parent.RootFS.DiffIDs) > len(img.RootFS.DiffIDs) {
+		return false
+	}
+
+	for i, h := range parent.History {
+		if !reflect.DeepEqual(h, img.History[i]) {
+			return false
+		}
+	}
+	for i, d := range parent.RootFS.DiffIDs {
+		if d != img.RootFS.DiffIDs[i] {
+			return false
+		}
+	}
+	return true
+}
+
+func (ic *imageCache) isParent(ctx context.Context, img *image.Image, parentID image.ID) bool {
+	ii, err := ic.imageService.resolveImage(ctx, img.ImageID())
+	if err != nil {
 		return false
 	}
+	parent, ok := ii.Labels[imageLabelClassicBuilderParent]
+	if ok {
+		return parent == parentID.String()
+	}
 
-	if len(target.History) != len(parent.History)+1 ||
-		len(target.RootFS.DiffIDs) != len(parent.RootFS.DiffIDs)+1 {
+	p, err := ic.imageService.GetImage(ctx, parentID.String(), imagetype.GetImageOpts{})
+	if err != nil {
 		return false
 	}
+	return ic.isParent(ctx, p, parentID)
+}
 
-	for i := range parent.History {
-		if !reflect.DeepEqual(parent.History[i], target.History[i]) {
+// compare two Config struct. Do not compare the "Image" nor "Hostname" fields
+// If OpenStdin is set, then it differs
+func isMatch(a, b *container.Config) bool {
+	if a == nil || b == nil ||
+		a.OpenStdin || b.OpenStdin {
+		return false
+	}
+	if a.AttachStdout != b.AttachStdout ||
+		a.AttachStderr != b.AttachStderr ||
+		a.User != b.User ||
+		a.OpenStdin != b.OpenStdin ||
+		a.Tty != b.Tty {
+		return false
+	}
+
+	if len(a.Cmd) != len(b.Cmd) ||
+		len(a.Env) != len(b.Env) ||
+		len(a.Labels) != len(b.Labels) ||
+		len(a.ExposedPorts) != len(b.ExposedPorts) ||
+		len(a.Entrypoint) != len(b.Entrypoint) ||
+		len(a.Volumes) != len(b.Volumes) {
+		return false
+	}
+
+	for i := 0; i < len(a.Cmd); i++ {
+		if a.Cmd[i] != b.Cmd[i] {
+			return false
+		}
+	}
+	for i := 0; i < len(a.Env); i++ {
+		if a.Env[i] != b.Env[i] {
+			return false
+		}
+	}
+	for k, v := range a.Labels {
+		if v != b.Labels[k] {
+			return false
+		}
+	}
+	for k := range a.ExposedPorts {
+		if _, exists := b.ExposedPorts[k]; !exists {
 			return false
 		}
 	}
 
-	childCreatedBy := target.History[len(target.History)-1].CreatedBy
-	return childCreatedBy == strings.Join(cfg.Cmd, " ")
+	for i := 0; i < len(a.Entrypoint); i++ {
+		if a.Entrypoint[i] != b.Entrypoint[i] {
+			return false
+		}
+	}
+	for key := range a.Volumes {
+		if _, exists := b.Volumes[key]; !exists {
+			return false
+		}
+	}
+	return true
 }