From a34060cdb42f6c4b1452cec27b715fa4c0a13e4f Mon Sep 17 00:00:00 2001 From: Laura Brehm Date: Sat, 25 Feb 2023 18:18:05 +0100 Subject: [PATCH] Resolve and store manifest when creating container This addresses the previous issue with the containerd store where, after a container is created, we can't deterministically resolve which image variant was used to run it (since we also don't store what platform the image was fetched for). This is required for things like `docker commit`, and computing the containers layer size later, since we need to resolve the specific image variant. Signed-off-by: Laura Brehm --- container/container.go | 2 ++ daemon/containerd/image.go | 38 +++++++++++++++++++++++++++++++ daemon/containerd/image_commit.go | 16 ++++++------- daemon/create.go | 22 ++++++++++++++---- daemon/image_service.go | 1 + daemon/images/image.go | 4 ++++ 6 files changed, 69 insertions(+), 14 deletions(-) diff --git a/container/container.go b/container/container.go index 37a6a74c1cf88..53e0cfe963e0b 100644 --- a/container/container.go +++ b/container/container.go @@ -39,6 +39,7 @@ import ( agentexec "github.com/moby/swarmkit/v2/agent/exec" "github.com/moby/sys/signal" "github.com/moby/sys/symlink" + ocispec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/pkg/errors" "github.com/sirupsen/logrus" ) @@ -72,6 +73,7 @@ type Container struct { Args []string Config *containertypes.Config ImageID image.ID `json:"Image"` + ImageManifest *ocispec.Descriptor NetworkSettings *network.Settings LogPath string Name string diff --git a/daemon/containerd/image.go b/daemon/containerd/image.go index 484640febe49e..e54013a5a387a 100644 --- a/daemon/containerd/image.go +++ b/daemon/containerd/image.go @@ -122,6 +122,44 @@ func (i *ImageService) GetImage(ctx context.Context, refOrID string, options ima return img, nil } +func (i *ImageService) GetImageManifest(ctx context.Context, refOrID string, options imagetype.GetImageOpts) (*ocispec.Descriptor, error) { + cs := i.client.ContentStore() + + desc, err := i.resolveDescriptor(ctx, refOrID) + if err != nil { + return nil, err + } + + if containerdimages.IsManifestType(desc.MediaType) { + return &desc, nil + } + + if containerdimages.IsIndexType(desc.MediaType) { + platform := platforms.AllPlatformsWithPreference(cplatforms.Default()) + if options.Platform != nil { + platform = cplatforms.Only(*options.Platform) + } + + childManifests, err := containerdimages.LimitManifests(containerdimages.ChildrenHandler(cs), platform, 1)(ctx, desc) + if err != nil { + if cerrdefs.IsNotFound(err) { + return nil, errdefs.NotFound(err) + } + return nil, errdefs.System(err) + } + + // len(childManifests) == 1 since we requested 1 and if none + // were found LimitManifests would have thrown an error + if !containerdimages.IsManifestType(childManifests[0].MediaType) { + return nil, errdefs.NotFound(fmt.Errorf("manifest has incorrect mediatype: %s", childManifests[0].MediaType)) + } + + return &childManifests[0], nil + } + + return nil, errdefs.NotFound(errors.New("failed to find manifest")) +} + // size returns the total size of the image's packed resources. func (i *ImageService) size(ctx context.Context, desc ocispec.Descriptor, platform cplatforms.MatchComparer) (int64, error) { var size int64 diff --git a/daemon/containerd/image_commit.go b/daemon/containerd/image_commit.go index 75bed98ac3878..2db67ec68b0df 100644 --- a/daemon/containerd/image_commit.go +++ b/daemon/containerd/image_commit.go @@ -16,7 +16,6 @@ import ( cerrdefs "github.com/containerd/containerd/errdefs" "github.com/containerd/containerd/images" "github.com/containerd/containerd/leases" - "github.com/containerd/containerd/platforms" "github.com/containerd/containerd/rootfs" "github.com/containerd/containerd/snapshots" "github.com/docker/docker/api/types/backend" @@ -39,20 +38,19 @@ with adaptations to match the Moby data model and services. // CommitImage creates a new image from a commit config. func (i *ImageService) CommitImage(ctx context.Context, cc backend.CommitConfig) (image.ID, error) { container := i.containers.Get(cc.ContainerID) + cs := i.client.ContentStore() - desc, err := i.resolveDescriptor(ctx, container.Config.Image) + imageManifestBytes, err := content.ReadBlob(ctx, cs, *container.ImageManifest) if err != nil { return "", err } - cs := i.client.ContentStore() - - ocimanifest, err := images.Manifest(ctx, cs, desc, platforms.DefaultStrict()) - if err != nil { + var manifest ocispec.Manifest + if err := json.Unmarshal(imageManifestBytes, &manifest); err != nil { return "", err } - imageConfigBytes, err := content.ReadBlob(ctx, cs, ocimanifest.Config) + imageConfigBytes, err := content.ReadBlob(ctx, cs, manifest.Config) if err != nil { return "", err } @@ -88,7 +86,7 @@ func (i *ImageService) CommitImage(ctx context.Context, cc backend.CommitConfig) return "", fmt.Errorf("failed to apply diff: %w", err) } - layers := append(ocimanifest.Layers, diffLayerDesc) + layers := append(manifest.Layers, diffLayerDesc) commitManifestDesc, configDigest, err := writeContentsForImage(ctx, i.snapshotter, cs, imageConfig, layers) if err != nil { return "", err @@ -96,7 +94,7 @@ func (i *ImageService) CommitImage(ctx context.Context, cc backend.CommitConfig) // image create img := images.Image{ - Name: configDigest.String(), + Name: danglingImageName(configDigest.Digest()), Target: commitManifestDesc, CreatedAt: time.Now(), } diff --git a/daemon/create.go b/daemon/create.go index d8581b0b88e71..cdd72381fe0fa 100644 --- a/daemon/create.go +++ b/daemon/create.go @@ -115,11 +115,12 @@ func (daemon *Daemon) containerCreate(ctx context.Context, opts createOpts) (con // Create creates a new container from the given configuration with a given name. func (daemon *Daemon) create(ctx context.Context, opts createOpts) (retC *container.Container, retErr error) { var ( - ctr *container.Container - img *image.Image - imgID image.ID - err error - os = runtime.GOOS + ctr *container.Container + img *image.Image + imgManifest *v1.Descriptor + imgID image.ID + err error + os = runtime.GOOS ) if opts.params.Config.Image != "" { @@ -127,6 +128,16 @@ func (daemon *Daemon) create(ctx context.Context, opts createOpts) (retC *contai if err != nil { return nil, err } + // when using the containerd store, we need to get the actual + // image manifest so we can store it and later deterministically + // resolve the specific image the container is running + if daemon.UsesSnapshotter() { + imgManifest, err = daemon.imageService.GetImageManifest(ctx, opts.params.Config.Image, imagetypes.GetImageOpts{Platform: opts.params.Platform}) + if err != nil { + logrus.WithError(err).Error("failed to find image manifest") + return nil, err + } + } os = img.OperatingSystem() imgID = img.ID() } else if isWindows { @@ -169,6 +180,7 @@ func (daemon *Daemon) create(ctx context.Context, opts createOpts) (retC *contai } ctr.HostConfig.StorageOpt = opts.params.HostConfig.StorageOpt + ctr.ImageManifest = imgManifest if daemon.UsesSnapshotter() { if err := daemon.imageService.PrepareSnapshot(ctx, ctr.ID, opts.params.Config.Image, opts.params.Platform); err != nil { diff --git a/daemon/image_service.go b/daemon/image_service.go index 5c7ab257617cd..23d50474c78d3 100644 --- a/daemon/image_service.go +++ b/daemon/image_service.go @@ -46,6 +46,7 @@ type ImageService interface { // Containerd related methods PrepareSnapshot(ctx context.Context, id string, image string, platform *v1.Platform) error + GetImageManifest(ctx context.Context, refOrID string, options imagetype.GetImageOpts) (*v1.Descriptor, error) // Layers diff --git a/daemon/images/image.go b/daemon/images/image.go index 3ee0a7dd66fe9..0e2cb0d0843b3 100644 --- a/daemon/images/image.go +++ b/daemon/images/image.go @@ -192,6 +192,10 @@ func (i *ImageService) GetImage(ctx context.Context, refOrID string, options ima return img, nil } +func (i *ImageService) GetImageManifest(ctx context.Context, refOrID string, options imagetypes.GetImageOpts) (*v1.Descriptor, error) { + panic("not implemented") +} + func (i *ImageService) getImage(ctx context.Context, refOrID string, options imagetypes.GetImageOpts) (retImg *image.Image, retErr error) { defer func() { if retErr != nil || retImg == nil || options.Platform == nil {