c8d/builder: implement cache
Signed-off-by: Laura Brehm <laurabrehm@hey.com>
(cherry picked from commit bd6868557d7ceb58d1d4717737e9da755cad87e5)
Signed-off-by: Laura Brehm <laurabrehm@hey.com>
diff --git a/daemon/containerd/cache.go b/daemon/containerd/cache.go
index e9fa697..8bd0176 100644
--- a/daemon/containerd/cache.go
+++ b/daemon/containerd/cache.go
@@ -3,6 +3,7 @@
import (
"context"
"reflect"
+ "strings"
"github.com/docker/docker/api/types/container"
imagetype "github.com/docker/docker/api/types/image"
@@ -30,54 +31,57 @@
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
}
- a.Image = ""
- a.Hostname = ""
- b.Image = ""
- b.Hostname = ""
- return reflect.DeepEqual(a, b)
+ if len(target.History) != len(parent.History)+1 ||
+ len(target.RootFS.DiffIDs) != len(parent.RootFS.DiffIDs)+1 {
+ return false
+ }
+
+ 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, " ")
}
diff --git a/daemon/containerd/image_builder.go b/daemon/containerd/image_builder.go
index 53adde3..c92bd8b 100644
--- a/daemon/containerd/image_builder.go
+++ b/daemon/containerd/image_builder.go
@@ -402,6 +402,18 @@
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 @@
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 @@
},
)
+ // 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 @@
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
}
diff --git a/daemon/containerd/image_commit.go b/daemon/containerd/image_commit.go
index a721c41..632f28b 100644
--- a/daemon/containerd/image_commit.go
+++ b/daemon/containerd/image_commit.go
@@ -141,10 +141,11 @@
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 == "",
}),
}
