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 <djordje.lukic@docker.com>
moby · Feb 6, 2024 · acd023d · acd023d
1 parent 7a075ca
commit acd023d
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 62 deletions.
diff --git a/daemon/containerd/image.go b/daemon/containerd/image.go
@@ -42,44 +42,10 @@ func (i *ImageService) GetImage(ctx context.Context, refOrID string, options ima
 		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 ima
 	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 imagetype.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
@@ -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) {