From 9717369913214e9fbf1d656af24092d65a1e0102 Mon Sep 17 00:00:00 2001 From: Laura Brehm Date: Thu, 13 Apr 2023 13:19:51 +0100 Subject: c8d: implement classic builder Co-authored-by: Djordje Lukic Signed-off-by: Laura Brehm (cherry picked from commit e46674b6a70dac8f64aed1ab16ccc5e61c335ef5) Signed-off-by: Laura Brehm --- builder/builder.go | 4 +- builder/dockerfile/internals.go | 14 +- builder/dockerfile/internals_test.go | 4 +- builder/dockerfile/mockbackend_test.go | 7 +- daemon/containerd/cache.go | 73 ++++- daemon/containerd/image_builder.go | 475 ++++++++++++++++++++++++++++++++- daemon/containerd/image_commit.go | 12 +- daemon/containerd/service.go | 3 + daemon/image_service.go | 3 +- daemon/images/image_builder.go | 7 +- 10 files changed, 586 insertions(+), 16 deletions(-) diff --git a/builder/builder.go b/builder/builder.go index 8f33485250..d3521ddfbb 100644 --- a/builder/builder.go +++ b/builder/builder.go @@ -14,6 +14,7 @@ import ( containerpkg "github.com/docker/docker/container" "github.com/docker/docker/image" "github.com/docker/docker/layer" + "github.com/opencontainers/go-digest" ) const ( @@ -45,7 +46,7 @@ type Backend interface { // ContainerCreateWorkdir creates the workdir ContainerCreateWorkdir(containerID string) error - CreateImage(config []byte, parent string) (Image, error) + CreateImage(ctx context.Context, config []byte, parent string, contentStoreDigest digest.Digest) (Image, error) ImageCacheBuilder } @@ -104,6 +105,7 @@ type ROLayer interface { Release() error NewRWLayer() (RWLayer, error) DiffID() layer.DiffID + ContentStoreDigest() digest.Digest } // RWLayer is active layer that can be read/modified diff --git a/builder/dockerfile/internals.go b/builder/dockerfile/internals.go index 66d873e2e5..6aa96a972c 100644 --- a/builder/dockerfile/internals.go +++ b/builder/dockerfile/internals.go @@ -63,7 +63,7 @@ func (b *Builder) commitContainer(ctx context.Context, dispatchState *dispatchSt return err } -func (b *Builder) exportImage(state *dispatchState, layer builder.RWLayer, parent builder.Image, runConfig *container.Config) error { +func (b *Builder) exportImage(ctx context.Context, state *dispatchState, layer builder.RWLayer, parent builder.Image, runConfig *container.Config) error { newLayer, err := layer.Commit() if err != nil { return err @@ -98,7 +98,15 @@ func (b *Builder) exportImage(state *dispatchState, layer builder.RWLayer, paren return errors.Wrap(err, "failed to encode image config") } - exportedImage, err := b.docker.CreateImage(config, state.imageID) + // when writing the new image's manifest, we now need to pass in the new layer's digest. + // before the containerd store work this was unnecessary since we get the layer id + // from the image's RootFS ChainID -- see: + // https://github.com/moby/moby/blob/8cf66ed7322fa885ef99c4c044fa23e1727301dc/image/store.go#L162 + // however, with the containerd store we can't do this. An alternative implementation here + // without changing the signature would be to get the layer digest by walking the content store + // and filtering the objects to find the layer with the DiffID we want, but that has performance + // implications that should be called out/investigated + exportedImage, err := b.docker.CreateImage(ctx, config, state.imageID, newLayer.ContentStoreDigest()) if err != nil { return errors.Wrapf(err, "failed to export image") } @@ -170,7 +178,7 @@ func (b *Builder) performCopy(ctx context.Context, req dispatchRequest, inst cop return errors.Wrapf(err, "failed to copy files") } } - return b.exportImage(state, rwLayer, imageMount.Image(), runConfigWithCommentCmd) + return b.exportImage(ctx, state, rwLayer, imageMount.Image(), runConfigWithCommentCmd) } func createDestInfo(workingDir string, inst copyInstruction, rwLayer builder.RWLayer, platform string) (copyInfo, error) { diff --git a/builder/dockerfile/internals_test.go b/builder/dockerfile/internals_test.go index 8145fac90d..f56c44b751 100644 --- a/builder/dockerfile/internals_test.go +++ b/builder/dockerfile/internals_test.go @@ -1,6 +1,7 @@ package dockerfile // import "github.com/docker/docker/builder/dockerfile" import ( + "context" "fmt" "os" "runtime" @@ -193,6 +194,7 @@ type MockROLayer struct { diffID layer.DiffID } +func (l *MockROLayer) ContentStoreDigest() digest.Digest { return "" } func (l *MockROLayer) Release() error { return nil } func (l *MockROLayer) NewRWLayer() (builder.RWLayer, error) { return nil, nil } func (l *MockROLayer) DiffID() layer.DiffID { return l.diffID } @@ -217,6 +219,6 @@ func TestExportImage(t *testing.T) { imageSources: getMockImageSource(nil, nil, nil), docker: getMockBuildBackend(), } - err := b.exportImage(ds, layer, parentImage, runConfig) + err := b.exportImage(context.TODO(), ds, layer, parentImage, runConfig) assert.NilError(t, err) } diff --git a/builder/dockerfile/mockbackend_test.go b/builder/dockerfile/mockbackend_test.go index 5b5419cfd6..a9e43e9bd1 100644 --- a/builder/dockerfile/mockbackend_test.go +++ b/builder/dockerfile/mockbackend_test.go @@ -13,6 +13,7 @@ import ( containerpkg "github.com/docker/docker/container" "github.com/docker/docker/image" "github.com/docker/docker/layer" + "github.com/opencontainers/go-digest" ) // MockBackend implements the builder.Backend interface for unit testing @@ -80,7 +81,7 @@ func (m *MockBackend) MakeImageCache(ctx context.Context, cacheFrom []string) (b return nil, nil } -func (m *MockBackend) CreateImage(config []byte, parent string) (builder.Image, error) { +func (m *MockBackend) CreateImage(ctx context.Context, config []byte, parent string, layerDigest digest.Digest) (builder.Image, error) { return &mockImage{id: "test"}, nil } @@ -119,6 +120,10 @@ func (mic *mockImageCache) GetCache(parentID string, cfg *container.Config) (str type mockLayer struct{} +func (l *mockLayer) ContentStoreDigest() digest.Digest { + return "" +} + func (l *mockLayer) Release() error { return nil } diff --git a/daemon/containerd/cache.go b/daemon/containerd/cache.go index e066e8ae87..e9fa697932 100644 --- a/daemon/containerd/cache.go +++ b/daemon/containerd/cache.go @@ -2,11 +2,82 @@ package containerd import ( "context" + "reflect" + "github.com/docker/docker/api/types/container" + imagetype "github.com/docker/docker/api/types/image" "github.com/docker/docker/builder" + "github.com/docker/docker/image" ) // MakeImageCache creates a stateful image cache. func (i *ImageService) MakeImageCache(ctx context.Context, cacheFrom []string) (builder.ImageCache, error) { - panic("not implemented") + images := []*image.Image{} + for _, c := range cacheFrom { + im, err := i.GetImage(ctx, c, imagetype.GetImageOpts{}) + if err != nil { + return nil, err + } + images = append(images, im) + } + return &imageCache{images: images, c: i}, nil +} + +type imageCache struct { + images []*image.Image + c *ImageService +} + +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{}) + if err != nil { + for _, ii := range ic.images { + if ii.ID().String() == parentID { + if compare(ii.RunConfig(), &cfgCpy) { + return ii.ID().String(), nil + } + } + } + } else { + children, err := ic.c.Children(ctx, i.ID()) + 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 + } + } + } + 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 { + return false + } + + a.Image = "" + a.Hostname = "" + b.Image = "" + b.Hostname = "" + return reflect.DeepEqual(a, b) } diff --git a/daemon/containerd/image_builder.go b/daemon/containerd/image_builder.go index e70ec9d6c5..53adde38ad 100644 --- a/daemon/containerd/image_builder.go +++ b/daemon/containerd/image_builder.go @@ -2,23 +2,490 @@ package containerd import ( "context" - "errors" + "fmt" + "io" + "os" + "runtime" + "time" + "github.com/containerd/containerd" + cerrdefs "github.com/containerd/containerd/errdefs" + "github.com/containerd/containerd/leases" + "github.com/containerd/containerd/mount" + "github.com/containerd/containerd/platforms" + "github.com/containerd/containerd/rootfs" + "github.com/docker/distribution/reference" "github.com/docker/docker/api/types/backend" + imagetypes "github.com/docker/docker/api/types/image" + "github.com/docker/docker/api/types/registry" + registrypkg "github.com/docker/docker/registry" + + // "github.com/docker/docker/api/types/container" + containerdimages "github.com/containerd/containerd/images" + "github.com/docker/docker/api/types/image" "github.com/docker/docker/builder" "github.com/docker/docker/errdefs" + dimage "github.com/docker/docker/image" + "github.com/docker/docker/layer" + "github.com/docker/docker/pkg/progress" + "github.com/docker/docker/pkg/streamformatter" + "github.com/docker/docker/pkg/stringid" + "github.com/docker/docker/pkg/system" + "github.com/opencontainers/go-digest" + "github.com/opencontainers/image-spec/identity" + ocispec "github.com/opencontainers/image-spec/specs-go/v1" + "github.com/sirupsen/logrus" ) // GetImageAndReleasableLayer returns an image and releaseable layer for a // reference or ID. Every call to GetImageAndReleasableLayer MUST call // releasableLayer.Release() to prevent leaking of layers. func (i *ImageService) GetImageAndReleasableLayer(ctx context.Context, refOrID string, opts backend.GetImageAndLayerOptions) (builder.Image, builder.ROLayer, error) { - return nil, nil, errdefs.NotImplemented(errors.New("not implemented")) + if refOrID == "" { // from SCRATCH + os := runtime.GOOS + if runtime.GOOS == "windows" { + os = "linux" + } + if opts.Platform != nil { + os = opts.Platform.OS + } + if !system.IsOSSupported(os) { + return nil, nil, system.ErrNotSupportedOperatingSystem + } + return nil, &rolayer{ + key: "", + c: i.client, + snapshotter: i.snapshotter, + diffID: "", + root: "", + }, nil + } + + if opts.PullOption != backend.PullOptionForcePull { + // TODO(laurazard): same as below + img, err := i.GetImage(ctx, refOrID, image.GetImageOpts{Platform: opts.Platform}) + if err != nil && opts.PullOption == backend.PullOptionNoPull { + return nil, nil, err + } + imgDesc, err := i.resolveDescriptor(ctx, refOrID) + if err != nil && !errdefs.IsNotFound(err) { + return nil, nil, err + } + if img != nil { + if !system.IsOSSupported(img.OperatingSystem()) { + return nil, nil, system.ErrNotSupportedOperatingSystem + } + + layer, err := newROLayerForImage(ctx, &imgDesc, i, opts, refOrID, opts.Platform) + if err != nil { + return nil, nil, err + } + + return img, layer, nil + } + } + + ctx, _, err := i.client.WithLease(ctx, leases.WithRandomID(), leases.WithExpiration(1*time.Hour)) + if err != nil { + return nil, nil, fmt.Errorf("failed to create lease for commit: %w", err) + } + + // TODO(laurazard): do we really need a new method here to pull the image? + imgDesc, err := i.pullForBuilder(ctx, refOrID, opts.AuthConfig, opts.Output, opts.Platform) + if err != nil { + return nil, nil, err + } + + // TODO(laurazard): pullForBuilder should return whatever we + // need here instead of having to go and get it again + img, err := i.GetImage(ctx, refOrID, imagetypes.GetImageOpts{ + Platform: opts.Platform, + }) + if err != nil { + return nil, nil, err + } + + layer, err := newROLayerForImage(ctx, imgDesc, i, opts, refOrID, opts.Platform) + if err != nil { + return nil, nil, err + } + + return img, layer, nil +} + +func (i *ImageService) pullForBuilder(ctx context.Context, name string, authConfigs map[string]registry.AuthConfig, output io.Writer, platform *ocispec.Platform) (*ocispec.Descriptor, error) { + ref, err := reference.ParseNormalizedNamed(name) + if err != nil { + return nil, err + } + taggedRef := reference.TagNameOnly(ref) + + pullRegistryAuth := ®istry.AuthConfig{} + if len(authConfigs) > 0 { + // The request came with a full auth config, use it + repoInfo, err := i.registryService.ResolveRepository(ref) + if err != nil { + return nil, err + } + + resolvedConfig := registrypkg.ResolveAuthConfig(authConfigs, repoInfo.Index) + pullRegistryAuth = &resolvedConfig + } + + if err := i.PullImage(ctx, ref.Name(), taggedRef.(reference.NamedTagged).Tag(), platform, nil, pullRegistryAuth, output); err != nil { + return nil, err + } + + img, err := i.GetImage(ctx, name, imagetypes.GetImageOpts{Platform: platform}) + if err != nil { + if errdefs.IsNotFound(err) && img != nil && platform != nil { + imgPlat := ocispec.Platform{ + OS: img.OS, + Architecture: img.BaseImgArch(), + Variant: img.BaseImgVariant(), + } + + p := *platform + if !platforms.Only(p).Match(imgPlat) { + po := streamformatter.NewJSONProgressOutput(output, false) + progress.Messagef(po, "", ` +WARNING: Pulled image with specified platform (%s), but the resulting image's configured platform (%s) does not match. +This is most likely caused by a bug in the build system that created the fetched image (%s). +Please notify the image author to correct the configuration.`, + platforms.Format(p), platforms.Format(imgPlat), name, + ) + logrus.WithError(err).WithField("image", name).Warn("Ignoring error about platform mismatch where the manifest list points to an image whose configuration does not match the platform in the manifest.") + } + } else { + return nil, err + } + } + + if !system.IsOSSupported(img.OperatingSystem()) { + return nil, system.ErrNotSupportedOperatingSystem + } + + imgDesc, err := i.resolveDescriptor(ctx, name) + if err != nil { + return nil, err + } + + return &imgDesc, err +} + +func newROLayerForImage(ctx context.Context, imgDesc *ocispec.Descriptor, i *ImageService, opts backend.GetImageAndLayerOptions, refOrID string, platform *ocispec.Platform) (builder.ROLayer, error) { + if imgDesc == nil { + return nil, fmt.Errorf("can't make an RO layer for a nil image :'(") + } + + platMatcher := platforms.Default() + if platform != nil { + platMatcher = platforms.Only(*platform) + } + + // this needs it's own context + lease so that it doesn't get cleaned before we're ready + confDesc, err := containerdimages.Config(ctx, i.client.ContentStore(), *imgDesc, platMatcher) + if err != nil { + return nil, err + } + + diffIDs, err := containerdimages.RootFS(ctx, i.client.ContentStore(), confDesc) + if err != nil { + return nil, err + } + parent := identity.ChainID(diffIDs).String() + + s := i.client.SnapshotService(i.snapshotter) + key := stringid.GenerateRandomID() + ctx, _, err = i.client.WithLease(ctx, leases.WithRandomID(), leases.WithExpiration(1*time.Hour)) + if err != nil { + return nil, fmt.Errorf("failed to create lease for commit: %w", err) + } + mounts, err := s.View(ctx, key, parent) + if err != nil { + return nil, err + } + + tempMountLocation := os.TempDir() + root, err := os.MkdirTemp(tempMountLocation, "rootfs-mount") + if err != nil { + return nil, err + } + + if err := mount.All(mounts, root); err != nil { + return nil, err + } + + return &rolayer{ + key: key, + c: i.client, + snapshotter: i.snapshotter, + diffID: digest.Digest(parent), + root: root, + contentStoreDigest: "", + }, nil +} + +type rolayer struct { + key string + c *containerd.Client + snapshotter string + diffID digest.Digest + root string + contentStoreDigest digest.Digest +} + +func (rl *rolayer) ContentStoreDigest() digest.Digest { + return rl.contentStoreDigest +} + +func (rl *rolayer) DiffID() layer.DiffID { + if rl.diffID == "" { + return layer.DigestSHA256EmptyTar + } + return layer.DiffID(rl.diffID) +} + +func (rl *rolayer) Release() error { + snapshotter := rl.c.SnapshotService(rl.snapshotter) + err := snapshotter.Remove(context.TODO(), rl.key) + if err != nil && !cerrdefs.IsNotFound(err) { + return err + } + + if rl.root == "" { // nothing to release + return nil + } + if err := mount.UnmountAll(rl.root, 0); err != nil { + logrus.WithError(err).WithField("root", rl.root).Error("failed to unmount ROLayer") + return err + } + if err := os.Remove(rl.root); err != nil { + logrus.WithError(err).WithField("dir", rl.root).Error("failed to remove mount temp dir") + return err + } + rl.root = "" + return nil +} + +// NewRWLayer creates a new read-write layer for the builder +func (rl *rolayer) NewRWLayer() (builder.RWLayer, error) { + snapshotter := rl.c.SnapshotService(rl.snapshotter) + + // we need this here for the prepared snapshots or + // we'll have racy behaviour where sometimes they + // will get GC'd before we commit/use them + ctx, _, err := rl.c.WithLease(context.TODO(), leases.WithRandomID(), leases.WithExpiration(1*time.Hour)) + if err != nil { + return nil, fmt.Errorf("failed to create lease for commit: %w", err) + } + + key := stringid.GenerateRandomID() + mounts, err := snapshotter.Prepare(ctx, key, rl.diffID.String()) + if err != nil { + return nil, err + } + + root, err := os.MkdirTemp(os.TempDir(), "rootfs-mount") + if err != nil { + return nil, err + } + if err := mount.All(mounts, root); err != nil { + return nil, err + } + + return &rwlayer{ + key: key, + parent: rl.key, + c: rl.c, + snapshotter: rl.snapshotter, + root: root, + }, nil +} + +type rwlayer struct { + key string + parent string + c *containerd.Client + snapshotter string + root string +} + +func (rw *rwlayer) Root() string { + return rw.root +} + +func (rw *rwlayer) Commit() (builder.ROLayer, error) { + // we need this here for the prepared snapshots or + // we'll have racy behaviour where sometimes they + // will get GC'd before we commit/use them + ctx, _, err := rw.c.WithLease(context.TODO(), leases.WithRandomID(), leases.WithExpiration(1*time.Hour)) + if err != nil { + return nil, fmt.Errorf("failed to create lease for commit: %w", err) + } + snapshotter := rw.c.SnapshotService(rw.snapshotter) + + key := stringid.GenerateRandomID() + err = snapshotter.Commit(ctx, key, rw.key) + if err != nil && !cerrdefs.IsAlreadyExists(err) { + return nil, err + } + + differ := rw.c.DiffService() + desc, err := rootfs.CreateDiff(ctx, key, snapshotter, differ) + if err != nil { + return nil, err + } + info, err := rw.c.ContentStore().Info(ctx, desc.Digest) + if err != nil { + return nil, err + } + diffIDStr, ok := info.Labels["containerd.io/uncompressed"] + if !ok { + return nil, fmt.Errorf("invalid differ response with no diffID") + } + diffID, err := digest.Parse(diffIDStr) + if err != nil { + return nil, err + } + + return &rolayer{ + key: key, + c: rw.c, + snapshotter: rw.snapshotter, + diffID: diffID, + root: "", + contentStoreDigest: desc.Digest, + }, nil +} + +func (rw *rwlayer) Release() error { + snapshotter := rw.c.SnapshotService(rw.snapshotter) + err := snapshotter.Remove(context.TODO(), rw.key) + if err != nil && !cerrdefs.IsNotFound(err) { + return err + } + + if rw.root == "" { // nothing to release + return nil + } + if err := mount.UnmountAll(rw.root, 0); err != nil { + logrus.WithError(err).WithField("root", rw.root).Error("failed to unmount ROLayer") + return err + } + if err := os.Remove(rw.root); err != nil { + logrus.WithError(err).WithField("dir", rw.root).Error("failed to remove mount temp dir") + return err + } + rw.root = "" + return nil } // CreateImage creates a new image by adding a config and ID to the image store. // This is similar to LoadImage() except that it receives JSON encoded bytes of // an image instead of a tar archive. -func (i *ImageService) CreateImage(config []byte, parent string) (builder.Image, error) { - return nil, errdefs.NotImplemented(errors.New("not implemented")) +func (i *ImageService) CreateImage(ctx context.Context, config []byte, parent string, layerDigest digest.Digest) (builder.Image, error) { + imgToCreate, err := dimage.NewFromJSON(config) + if err != nil { + return nil, err + } + + rootfs := ocispec.RootFS{ + Type: imgToCreate.RootFS.Type, + DiffIDs: []digest.Digest{}, + } + for _, diffId := range imgToCreate.RootFS.DiffIDs { + rootfs.DiffIDs = append(rootfs.DiffIDs, digest.Digest(diffId)) + } + exposedPorts := make(map[string]struct{}, len(imgToCreate.Config.ExposedPorts)) + for k, v := range imgToCreate.Config.ExposedPorts { + exposedPorts[string(k)] = v + } + + // make an ocispec.Image from the docker/image.Image + ociImgToCreate := ocispec.Image{ + Created: &imgToCreate.Created, + Author: imgToCreate.Author, + Architecture: imgToCreate.Architecture, + Variant: imgToCreate.Variant, + OS: imgToCreate.OS, + OSVersion: imgToCreate.OSVersion, + OSFeatures: imgToCreate.OSFeatures, + Config: ocispec.ImageConfig{ + User: imgToCreate.Config.User, + ExposedPorts: exposedPorts, + Env: imgToCreate.Config.Env, + Entrypoint: imgToCreate.Config.Entrypoint, + Cmd: imgToCreate.Config.Cmd, + Volumes: imgToCreate.Config.Volumes, + WorkingDir: imgToCreate.Config.WorkingDir, + Labels: imgToCreate.Config.Labels, + StopSignal: imgToCreate.Config.StopSignal, + }, + RootFS: rootfs, + // TODO(laurazard) + History: []ocispec.History{}, + } + + var layers []ocispec.Descriptor + // if the image has a parent, we need to start with the parents layers descriptors + if parent != "" { + parentDesc, err := i.resolveDescriptor(ctx, parent) + if err != nil { + return nil, err + } + parentImageManifest, err := containerdimages.Manifest(ctx, i.client.ContentStore(), parentDesc, platforms.Default()) + if err != nil { + return nil, err + } + + layers = parentImageManifest.Layers + } + + // get the info for the new layers + info, err := i.client.ContentStore().Info(ctx, layerDigest) + if err != nil { + return nil, err + } + + // append the new layer descriptor + layers = append(layers, + ocispec.Descriptor{ + MediaType: containerdimages.MediaTypeDockerSchema2LayerGzip, + Digest: layerDigest, + Size: info.Size, + }, + ) + + commitManifestDesc, err := writeContentsForImage(ctx, i.snapshotter, i.client.ContentStore(), ociImgToCreate, layers) + if err != nil { + return nil, err + } + + // image create + img := containerdimages.Image{ + Name: danglingImageName(commitManifestDesc.Digest), + Target: commitManifestDesc, + CreatedAt: time.Now(), + } + + createdImage, err := i.client.ImageService().Update(ctx, img) + if err != nil { + if !cerrdefs.IsNotFound(err) { + return nil, err + } + + if createdImage, err = i.client.ImageService().Create(ctx, img); err != nil { + return nil, fmt.Errorf("failed to create new image: %w", err) + } + } + + if err := i.unpackImage(ctx, createdImage, platforms.DefaultSpec()); err != nil { + return nil, err + } + + newImage := dimage.NewImage(dimage.ID(createdImage.Target.Digest)) + newImage.V1Image = imgToCreate.V1Image + newImage.V1Image.ID = string(createdImage.Target.Digest) + return newImage, nil } diff --git a/daemon/containerd/image_commit.go b/daemon/containerd/image_commit.go index bbb4df0ddc..a721c41970 100644 --- a/daemon/containerd/image_commit.go +++ b/daemon/containerd/image_commit.go @@ -6,7 +6,6 @@ import ( "crypto/rand" "encoding/base64" "encoding/json" - "errors" "fmt" "runtime" "strings" @@ -20,7 +19,6 @@ import ( "github.com/containerd/containerd/rootfs" "github.com/containerd/containerd/snapshots" "github.com/docker/docker/api/types/backend" - "github.com/docker/docker/errdefs" "github.com/docker/docker/image" "github.com/opencontainers/go-digest" "github.com/opencontainers/image-spec/identity" @@ -298,5 +296,13 @@ func uniquePart() string { // // This is a temporary shim. Should be removed when builder stops using commit. func (i *ImageService) CommitBuildStep(ctx context.Context, c backend.CommitConfig) (image.ID, error) { - return "", errdefs.NotImplemented(errors.New("not implemented")) + ctr := i.containers.Get(c.ContainerID) + if ctr == nil { + // TODO: use typed error + return "", fmt.Errorf("container not found: %s", c.ContainerID) + } + c.ContainerMountLabel = ctr.MountLabel + c.ContainerOS = ctr.OS + c.ParentImageID = string(ctr.ImageID) + return i.CommitImage(ctx, c) } diff --git a/daemon/containerd/service.go b/daemon/containerd/service.go index d46bbd8c53..0aa0c39146 100644 --- a/daemon/containerd/service.go +++ b/daemon/containerd/service.go @@ -10,6 +10,7 @@ import ( "github.com/containerd/containerd/plugin" "github.com/containerd/containerd/remotes/docker" "github.com/containerd/containerd/snapshots" + "github.com/docker/distribution/reference" imagetypes "github.com/docker/docker/api/types/image" "github.com/docker/docker/container" daemonevents "github.com/docker/docker/daemon/events" @@ -17,6 +18,7 @@ import ( "github.com/docker/docker/errdefs" "github.com/docker/docker/image" "github.com/docker/docker/layer" + "github.com/docker/docker/registry" "github.com/opencontainers/go-digest" "github.com/opencontainers/image-spec/identity" ocispec "github.com/opencontainers/image-spec/specs-go/v1" @@ -41,6 +43,7 @@ type RegistryHostsProvider interface { type RegistryConfigProvider interface { IsInsecureRegistry(host string) bool + ResolveRepository(name reference.Named) (*registry.RepositoryInfo, error) } type ImageServiceConfig struct { diff --git a/daemon/image_service.go b/daemon/image_service.go index b47575b6db..8470d18e02 100644 --- a/daemon/image_service.go +++ b/daemon/image_service.go @@ -16,6 +16,7 @@ import ( "github.com/docker/docker/image" "github.com/docker/docker/layer" "github.com/docker/docker/pkg/archive" + "github.com/opencontainers/go-digest" v1 "github.com/opencontainers/image-spec/specs-go/v1" ) @@ -27,7 +28,7 @@ type ImageService interface { PullImage(ctx context.Context, name, tag string, platform *v1.Platform, metaHeaders map[string][]string, authConfig *registry.AuthConfig, outStream io.Writer) error PushImage(ctx context.Context, ref reference.Named, metaHeaders map[string][]string, authConfig *registry.AuthConfig, outStream io.Writer) error - CreateImage(config []byte, parent string) (builder.Image, error) + CreateImage(ctx context.Context, config []byte, parent string, contentStoreDigest digest.Digest) (builder.Image, error) ImageDelete(ctx context.Context, imageRef string, force, prune bool) ([]types.ImageDeleteResponseItem, error) ExportImage(ctx context.Context, names []string, outStream io.Writer) error PerformWithBaseFS(ctx context.Context, c *container.Container, fn func(string) error) error diff --git a/daemon/images/image_builder.go b/daemon/images/image_builder.go index af2d4073cc..b10878f9f1 100644 --- a/daemon/images/image_builder.go +++ b/daemon/images/image_builder.go @@ -19,6 +19,7 @@ import ( "github.com/docker/docker/pkg/stringid" "github.com/docker/docker/pkg/system" registrypkg "github.com/docker/docker/registry" + "github.com/opencontainers/go-digest" specs "github.com/opencontainers/image-spec/specs-go/v1" "github.com/pkg/errors" "github.com/sirupsen/logrus" @@ -30,6 +31,10 @@ type roLayer struct { roLayer layer.Layer } +func (l *roLayer) ContentStoreDigest() digest.Digest { + return "" +} + func (l *roLayer) DiffID() layer.DiffID { if l.roLayer == nil { return layer.DigestSHA256EmptyTar @@ -241,7 +246,7 @@ func (i *ImageService) GetImageAndReleasableLayer(ctx context.Context, refOrID s // CreateImage creates a new image by adding a config and ID to the image store. // This is similar to LoadImage() except that it receives JSON encoded bytes of // an image instead of a tar archive. -func (i *ImageService) CreateImage(config []byte, parent string) (builder.Image, error) { +func (i *ImageService) CreateImage(ctx context.Context, config []byte, parent string, _ digest.Digest) (builder.Image, error) { id, err := i.imageStore.Create(config) if err != nil { return nil, errors.Wrapf(err, "failed to create image") -- cgit v1.2.1 From 8587a1c617dae69474e8e3847d5fd799de156da6 Mon Sep 17 00:00:00 2001 From: Laura Brehm Date: Mon, 8 May 2023 23:51:40 +0100 Subject: c8d/builder: implement cache Signed-off-by: Laura Brehm (cherry picked from commit bd6868557d7ceb58d1d4717737e9da755cad87e5) Signed-off-by: Laura Brehm --- daemon/containerd/cache.go | 78 ++++++++++++++++++++------------------ daemon/containerd/image_builder.go | 26 +++++++++++-- daemon/containerd/image_commit.go | 9 +++-- 3 files changed, 69 insertions(+), 44 deletions(-) diff --git a/daemon/containerd/cache.go b/daemon/containerd/cache.go index e9fa697932..8bd01768f8 100644 --- a/daemon/containerd/cache.go +++ b/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, " ") } diff --git a/daemon/containerd/image_builder.go b/daemon/containerd/image_builder.go index 53adde38ad..c92bd8bf3e 100644 --- a/daemon/containerd/image_builder.go +++ b/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 } diff --git a/daemon/containerd/image_commit.go b/daemon/containerd/image_commit.go index a721c41970..632f28bc6b 100644 --- a/daemon/containerd/image_commit.go +++ b/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 == "", }), } -- cgit v1.2.1