Merge pull request #47348 from rumpl/25.0_backport-history-config

[25.0 backport] c8d: Use the same logic to get the present images
moby · Feb 6, 2024 · f417435 · f417435
2 parents 7a075ca + acd023d
commit f417435
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) {