From f1e69582951e28da635c55b68e20c1d57d7e513f Mon Sep 17 00:00:00 2001 From: Djordje Lukic Date: Tue, 6 Feb 2024 12:26:50 +0100 Subject: [PATCH] c8d: Use the same logic to get the present images Inspect and history used two different ways to find the present images. This made history fail in some cases where image inspect would work (if a configuration of a manifest wasn't found in the content store). With this change we now use the same logic for both inspect and history. Signed-off-by: Djordje Lukic --- daemon/containerd/image.go | 86 ++++++++++++++++++------------ daemon/containerd/image_history.go | 30 ++--------- 2 files changed, 54 insertions(+), 62 deletions(-) diff --git a/daemon/containerd/image.go b/daemon/containerd/image.go index 5950cd9576..9822d88cb1 100644 --- a/daemon/containerd/image.go +++ b/daemon/containerd/image.go @@ -42,44 +42,10 @@ func (i *ImageService) GetImage(ctx context.Context, refOrID string, options bac platform = platforms.OnlyStrict(*options.Platform) } - var presentImages []imagespec.DockerOCIImage - err = i.walkImageManifests(ctx, desc, func(img *ImageManifest) error { - conf, err := img.Config(ctx) - if err != nil { - if cerrdefs.IsNotFound(err) { - log.G(ctx).WithFields(log.Fields{ - "manifestDescriptor": img.Target(), - }).Debug("manifest was present, but accessing its config failed, ignoring") - return nil - } - return errdefs.System(fmt.Errorf("failed to get config descriptor: %w", err)) - } - - var ociimage imagespec.DockerOCIImage - if err := readConfig(ctx, i.content, conf, &ociimage); err != nil { - if cerrdefs.IsNotFound(err) { - log.G(ctx).WithFields(log.Fields{ - "manifestDescriptor": img.Target(), - "configDescriptor": conf, - }).Debug("manifest present, but its config is missing, ignoring") - return nil - } - return errdefs.System(fmt.Errorf("failed to read config of the manifest %v: %w", img.Target().Digest, err)) - } - presentImages = append(presentImages, ociimage) - return nil - }) + presentImages, err := i.presentImages(ctx, desc, refOrID, platform) if err != nil { return nil, err } - if len(presentImages) == 0 { - ref, _ := reference.ParseAnyReference(refOrID) - return nil, images.ErrImageDoesNotExist{Ref: ref} - } - - sort.SliceStable(presentImages, func(i, j int) bool { - return platform.Less(presentImages[i].Platform, presentImages[j].Platform) - }) ociImage := presentImages[0] img := dockerOciImageToDockerImagePartial(image.ID(desc.Target.Digest), ociImage) @@ -156,6 +122,56 @@ func (i *ImageService) GetImage(ctx context.Context, refOrID string, options bac return img, nil } +// presentImages returns the images that are present in the content store, +// manifests without a config are ignored. +// The images are filtered and sorted by platform preference. +func (i *ImageService) presentImages(ctx context.Context, desc containerdimages.Image, refOrID string, platform platforms.MatchComparer) ([]imagespec.DockerOCIImage, error) { + var presentImages []imagespec.DockerOCIImage + err := i.walkImageManifests(ctx, desc, func(img *ImageManifest) error { + conf, err := img.Config(ctx) + if err != nil { + if cerrdefs.IsNotFound(err) { + log.G(ctx).WithFields(log.Fields{ + "manifestDescriptor": img.Target(), + }).Debug("manifest was present, but accessing its config failed, ignoring") + return nil + } + return errdefs.System(fmt.Errorf("failed to get config descriptor: %w", err)) + } + + var ociimage imagespec.DockerOCIImage + if err := readConfig(ctx, i.content, conf, &ociimage); err != nil { + if errdefs.IsNotFound(err) { + log.G(ctx).WithFields(log.Fields{ + "manifestDescriptor": img.Target(), + "configDescriptor": conf, + }).Debug("manifest present, but its config is missing, ignoring") + return nil + } + return errdefs.System(fmt.Errorf("failed to read config of the manifest %v: %w", img.Target().Digest, err)) + } + + if platform.Match(ociimage.Platform) { + presentImages = append(presentImages, ociimage) + } + + return nil + }) + if err != nil { + return nil, err + } + if len(presentImages) == 0 { + ref, _ := reference.ParseAnyReference(refOrID) + return nil, images.ErrImageDoesNotExist{Ref: ref} + } + + sort.SliceStable(presentImages, func(i, j int) bool { + return platform.Less(presentImages[i].Platform, presentImages[j].Platform) + }) + + return presentImages, nil +} + func (i *ImageService) GetImageManifest(ctx context.Context, refOrID string, options backend.GetImageOpts) (*ocispec.Descriptor, error) { platform := matchAllWithPreference(platforms.Default()) if options.Platform != nil { diff --git a/daemon/containerd/image_history.go b/daemon/containerd/image_history.go index ba9df59be0..39f96d99ba 100644 --- a/daemon/containerd/image_history.go +++ b/daemon/containerd/image_history.go @@ -2,18 +2,14 @@ package containerd import ( "context" - "sort" - "github.com/containerd/containerd/images" containerdimages "github.com/containerd/containerd/images" "github.com/containerd/containerd/platforms" "github.com/containerd/log" "github.com/distribution/reference" imagetype "github.com/docker/docker/api/types/image" - "github.com/docker/docker/errdefs" "github.com/opencontainers/go-digest" "github.com/opencontainers/image-spec/identity" - ocispec "github.com/opencontainers/image-spec/specs-go/v1" "github.com/pkg/errors" ) @@ -25,33 +21,13 @@ func (i *ImageService) ImageHistory(ctx context.Context, name string) ([]*imaget return nil, err } - cs := i.client.ContentStore() // TODO: pass platform in from the CLI platform := matchAllWithPreference(platforms.Default()) - var presentImages []ocispec.Image - err = i.walkImageManifests(ctx, img, func(img *ImageManifest) error { - conf, err := img.Config(ctx) - if err != nil { - return err - } - var ociImage ocispec.Image - if err := readConfig(ctx, cs, conf, &ociImage); err != nil { - return err - } - presentImages = append(presentImages, ociImage) - return nil - }) + presentImages, err := i.presentImages(ctx, img, name, platform) if err != nil { return nil, err } - if len(presentImages) == 0 { - return nil, errdefs.NotFound(errors.New("failed to find image manifest")) - } - - sort.SliceStable(presentImages, func(i, j int) bool { - return platform.Less(presentImages[i].Platform, presentImages[j].Platform) - }) ociImage := presentImages[0] var ( @@ -96,7 +72,7 @@ func (i *ImageService) ImageHistory(ctx context.Context, name string) ([]*imaget }}, history...) } - findParents := func(img images.Image) []images.Image { + findParents := func(img containerdimages.Image) []containerdimages.Image { imgs, err := i.getParentsByBuilderLabel(ctx, img) if err != nil { log.G(ctx).WithFields(log.Fields{ @@ -141,7 +117,7 @@ func (i *ImageService) ImageHistory(ctx context.Context, name string) ([]*imaget return history, nil } -func getImageTags(ctx context.Context, imgs []images.Image) []string { +func getImageTags(ctx context.Context, imgs []containerdimages.Image) []string { var tags []string for _, img := range imgs { if isDanglingImage(img) {