c8d/builder: implement cache

Signed-off-by: Laura Brehm <laurabrehm@hey.com>

daemon/containerd/cache.go

@@ -3,6 +3,7 @@ package containerd
 import (
 	"context"
 	"reflect"
+	"strings"
 
 	"github.com/docker/docker/api/types/container"
 	imagetype "github.com/docker/docker/api/types/image"
@@ -30,54 +31,57 @@ type imageCache struct {
 
 func (ic *imageCache) GetCache(parentID string, cfg *container.Config) (imageID string, err error) {
 	ctx := context.TODO()
-	cfgCpy := *cfg
-	i, err := ic.c.GetImage(ctx, parentID, imagetype.GetImageOpts{})
+	parent, err := ic.c.GetImage(ctx, parentID, imagetype.GetImageOpts{})
 	if err != nil {
-		for _, ii := range ic.images {
-			if ii.ID().String() == parentID {
-				if compare(ii.RunConfig(), &cfgCpy) {
-					return ii.ID().String(), nil
-				}
-			}
+		return "", err
+	}
+
+	for _, localCachedImage := range ic.images {
+		if isMatch(localCachedImage, parent, cfg) {
+			return localCachedImage.ID().String(), nil
 		}
-	} else {
-		children, err := ic.c.Children(ctx, i.ID())
+	}
+
+	children, err := ic.c.Children(ctx, parent.ID())
+	if err != nil {
+		return "", err
+	}
+
+	for _, children := range children {
+		childImage, err := ic.c.GetImage(ctx, children.String(), imagetype.GetImageOpts{})
 		if err != nil {
 			return "", err
 		}
-		for _, ch := range children {
-			childImage, err := ic.c.GetImage(context.TODO(), ch.String(), imagetype.GetImageOpts{})
-			if err != nil {
-				return "", err
-			}
-			// this implementation looks correct but it's currently not working
-			// with the containerd store as we're not storing the image ContainerConfig
-			// and so intermediate images with ContainerConfigs such as
-			// #(nop) COPY file:c6ab44934e83eeb07289a211582c6faa25dea7d06dae077b6ef76029e92400ce in ...
-			// are not getting a hit
-			if compare(&childImage.ContainerConfig, &cfgCpy) {
-				return ch.String(), nil
-			}
+
+		if isMatch(childImage, parent, cfg) {
+			return children.String(), nil
 		}
 	}
+
 	return "", nil
 }
 
-// compare two Config structs. Do not consider the "Hostname" field as it
-// defaults to the randomly generated short container ID or "Image" as it
-// represents the name of the image as it was passed in (symbolic) and does
-// not provide any meaningful information about whether the image is usable
-// as cache.
-// If OpenStdin is set, then it differs
-func compare(a, b *container.Config) bool {
-	if a == nil || b == nil ||
-		a.OpenStdin || b.OpenStdin {
+// 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 {
+		return false
+	}
+
+	if len(target.History) != len(parent.History)+1 ||
+		len(target.RootFS.DiffIDs) != len(parent.RootFS.DiffIDs)+1 {
 		return false
 	}
 
-	a.Image = ""
-	a.Hostname = ""
-	b.Image = ""
-	b.Hostname = ""
-	return reflect.DeepEqual(a, b)
+	for i := range parent.History {
+		if !reflect.DeepEqual(parent.History[i], target.History[i]) {
+			return false
+		}
+	}
+
+	childCreatedBy := target.History[len(target.History)-1].CreatedBy
+	return childCreatedBy == strings.Join(cfg.Cmd, " ")
 }

daemon/containerd/image_builder.go

@@ -402,6 +402,18 @@ func (i *ImageService) CreateImage(ctx context.Context, config []byte, parent st
 		exposedPorts[string(k)] = v
 	}
 
+	var ociHistory []ocispec.History
+	for _, history := range imgToCreate.History {
+		created := history.Created
+		ociHistory = append(ociHistory, ocispec.History{
+			Created:    &created,
+			CreatedBy:  history.CreatedBy,
+			Author:     history.Author,
+			Comment:    history.Comment,
+			EmptyLayer: history.EmptyLayer,
+		})
+	}
+
 	// make an ocispec.Image from the docker/image.Image
 	ociImgToCreate := ocispec.Image{
 		Created:      &imgToCreate.Created,
@@ -422,9 +434,8 @@ func (i *ImageService) CreateImage(ctx context.Context, config []byte, parent st
 			Labels:       imgToCreate.Config.Labels,
 			StopSignal:   imgToCreate.Config.StopSignal,
 		},
-		RootFS: rootfs,
-		// TODO(laurazard)
-		History: []ocispec.History{},
+		RootFS:  rootfs,
+		History: ociHistory,
 	}
 
 	var layers []ocispec.Descriptor
@@ -457,6 +468,14 @@ func (i *ImageService) CreateImage(ctx context.Context, config []byte, parent st
 		},
 	)
 
+	// necessary to prevent the contents from being GC'd
+	// between writing them here and creating an image
+	ctx, done, err := i.client.WithLease(ctx, leases.WithRandomID(), leases.WithExpiration(1*time.Hour))
+	if err != nil {
+		return nil, err
+	}
+	defer done(ctx)
+
 	commitManifestDesc, err := writeContentsForImage(ctx, i.snapshotter, i.client.ContentStore(), ociImgToCreate, layers)
 	if err != nil {
 		return nil, err
@@ -487,5 +506,6 @@ func (i *ImageService) CreateImage(ctx context.Context, config []byte, parent st
 	newImage := dimage.NewImage(dimage.ID(createdImage.Target.Digest))
 	newImage.V1Image = imgToCreate.V1Image
 	newImage.V1Image.ID = string(createdImage.Target.Digest)
+	newImage.History = imgToCreate.History
 	return newImage, nil
 }

daemon/containerd/image_commit.go

@@ -141,10 +141,11 @@ func generateCommitImageConfig(baseConfig ocispec.Image, diffID digest.Digest, o
 			DiffIDs: append(baseConfig.RootFS.DiffIDs, diffID),
 		},
 		History: append(baseConfig.History, ocispec.History{
-			Created:    &createdTime,
-			CreatedBy:  strings.Join(opts.ContainerConfig.Cmd, " "),
-			Author:     opts.Author,
-			Comment:    opts.Comment,
+			Created:   &createdTime,
+			CreatedBy: strings.Join(opts.ContainerConfig.Cmd, " "),
+			Author:    opts.Author,
+			Comment:   opts.Comment,
+			// TODO(laurazard): this check might be incorrect
 			EmptyLayer: diffID == "",
 		}),
 	}