Merge pull request #49533 from vvoland/c8d-inspectlist-indeximg

c8d/list&inspect: Better handle images without any platform blobs available locally

daemon/containerd/image.go

@@ -31,9 +31,11 @@ type errPlatformNotFound struct {
 
 func (e *errPlatformNotFound) NotFound() {}
 func (e *errPlatformNotFound) Error() string {
-	msg := "image with reference " + e.imageRef + " was found but does not provide the specified platform"
+	msg := "image with reference " + e.imageRef + " was found but does not provide "
 	if e.wanted.OS != "" {
-		msg += " (" + platforms.FormatAll(e.wanted) + ")"
+		msg += "the specified platform (" + platforms.FormatAll(e.wanted) + ")"
+	} else {
+		msg += "any platform"
 	}
 	return msg
 }

daemon/containerd/image_inspect.go

@@ -23,6 +23,9 @@ import (
 )
 
 func (i *ImageService) ImageInspect(ctx context.Context, refOrID string, opts backend.ImageInspectOpts) (*imagetypes.InspectResponse, error) {
+	// TODO: Pass in opts
+	var requestedPlatform *ocispec.Platform
+
 	c8dImg, err := i.resolveImage(ctx, refOrID)
 	if err != nil {
 		return nil, err
@@ -46,7 +49,7 @@ func (i *ImageService) ImageInspect(ctx context.Context, refOrID string, opts ba
 		}
 	}
 
-	platform := matchAllWithPreference(platforms.Default())
+	platform := i.matchRequestedOrDefault(platforms.OnlyStrict, requestedPlatform)
 	size, err := i.size(ctx, target, platform)
 	if err != nil {
 		return nil, err
@@ -57,32 +60,19 @@ func (i *ImageService) ImageInspect(ctx context.Context, refOrID string, opts ba
 		return nil, err
 	}
 
-	if multi.Best == nil {
+	//nolint:govet // TODO: requestedPlatform is always nil, but should be passed by the caller
+	if multi.Best == nil && requestedPlatform != nil {
 		return nil, &errPlatformNotFound{
-			wanted:   platforms.DefaultSpec(),
 			imageRef: refOrID,
+			wanted:   *requestedPlatform,
 		}
 	}
 
-	best := multi.Best
 	var img imagespec.DockerOCIImage
-	if err := best.ReadConfig(ctx, &img); err != nil {
-		return nil, err
-	}
-
-	var comment string
-	if len(comment) == 0 && len(img.History) > 0 {
-		comment = img.History[len(img.History)-1].Comment
-	}
-
-	var created string
-	if img.Created != nil {
-		created = img.Created.Format(time.RFC3339Nano)
-	}
-
-	var layers []string
-	for _, layer := range img.RootFS.DiffIDs {
-		layers = append(layers, layer.String())
+	if multi.Best != nil {
+		if err := multi.Best.ReadConfig(ctx, &img); err != nil {
+			return nil, err
+		}
 	}
 
 	parent, err := i.getImageLabelByDigest(ctx, target.Digest, imageLabelClassicBuilderParent)
@@ -97,35 +87,49 @@ func (i *ImageService) ImageInspect(ctx context.Context, refOrID string, opts ba
 
 	repoTags, repoDigests := collectRepoTagsAndDigests(ctx, tagged)
 
-	return &imagetypes.InspectResponse{
+	resp := &imagetypes.InspectResponse{
 		ID:            target.Digest.String(),
 		RepoTags:      repoTags,
 		Descriptor:    &target,
 		RepoDigests:   repoDigests,
 		Parent:        parent,
-		Comment:       comment,
-		Created:       created,
 		DockerVersion: "",
-		Author:        img.Author,
-		Config:        dockerOCIImageConfigToContainerConfig(img.Config),
-		Architecture:  img.Architecture,
-		Variant:       img.Variant,
-		Os:            img.OS,
-		OsVersion:     img.OSVersion,
 		Size:          size,
 		Manifests:     manifests,
 		GraphDriver: storage.DriverData{
 			Name: i.snapshotter,
 			Data: nil,
 		},
-		RootFS: imagetypes.RootFS{
-			Type:   img.RootFS.Type,
-			Layers: layers,
-		},
 		Metadata: imagetypes.Metadata{
 			LastTagTime: lastUpdated,
 		},
-	}, nil
+	}
+
+	if multi.Best != nil {
+		resp.Author = img.Author
+		resp.Config = dockerOCIImageConfigToContainerConfig(img.Config)
+		resp.Architecture = img.Architecture
+		resp.Variant = img.Variant
+		resp.Os = img.OS
+		resp.OsVersion = img.OSVersion
+
+		if len(img.History) > 0 {
+			resp.Comment = img.History[len(img.History)-1].Comment
+		}
+
+		if img.Created != nil {
+			resp.Created = img.Created.Format(time.RFC3339Nano)
+		}
+
+		resp.RootFS = imagetypes.RootFS{
+			Type: img.RootFS.Type,
+		}
+		for _, layer := range img.RootFS.DiffIDs {
+			resp.RootFS.Layers = append(resp.RootFS.Layers, layer.String())
+		}
+	}
+
+	return resp, nil
 }
 
 func collectRepoTagsAndDigests(ctx context.Context, tagged []c8dimages.Image) (repoTags []string, repoDigests []string) {

daemon/containerd/image_inspect_test.go

@@ -0,0 +1,64 @@
+package containerd
+
+import (
+	"context"
+	"fmt"
+	"path/filepath"
+	"testing"
+
+	c8dimages "github.com/containerd/containerd/v2/core/images"
+	"github.com/containerd/containerd/v2/pkg/namespaces"
+	"github.com/containerd/log/logtest"
+	"github.com/docker/docker/api/types/backend"
+	"github.com/docker/docker/internal/testutils/specialimage"
+	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
+	"gotest.tools/v3/assert"
+	is "gotest.tools/v3/assert/cmp"
+)
+
+func TestImageInspect(t *testing.T) {
+	ctx := namespaces.WithNamespace(context.TODO(), "testing")
+
+	blobsDir := t.TempDir()
+
+	toContainerdImage := func(t *testing.T, imageFunc specialimage.SpecialImageFunc) c8dimages.Image {
+		idx, err := imageFunc(blobsDir)
+		assert.NilError(t, err)
+
+		return imagesFromIndex(idx)[0]
+	}
+
+	missingMultiPlatform := toContainerdImage(t, func(dir string) (*ocispec.Index, error) {
+		idx, _, err := specialimage.PartialMultiPlatform(dir, "missingmp:latest", specialimage.PartialOpts{
+			Stored: nil,
+			Missing: []ocispec.Platform{
+				{OS: "linux", Architecture: "arm64"},
+				{OS: "linux", Architecture: "amd64"},
+			},
+		})
+		return idx, err
+	})
+
+	cs := &blobsDirContentStore{blobs: filepath.Join(blobsDir, "blobs/sha256")}
+
+	t.Run("inspect image with manifests but missing platform blobs", func(t *testing.T) {
+		ctx := logtest.WithT(ctx, t)
+		service := fakeImageService(t, ctx, cs)
+
+		_, err := service.images.Create(ctx, missingMultiPlatform)
+		assert.NilError(t, err)
+
+		for _, manifests := range []bool{true, false} {
+			t.Run(fmt.Sprintf("manifests=%t", manifests), func(t *testing.T) {
+				inspect, err := service.ImageInspect(ctx, missingMultiPlatform.Name, backend.ImageInspectOpts{Manifests: manifests})
+				assert.NilError(t, err)
+
+				if manifests {
+					assert.Check(t, is.Len(inspect.Manifests, 2))
+				} else {
+					assert.Check(t, is.Len(inspect.Manifests, 0))
+				}
+			})
+		}
+	})
+}

daemon/containerd/image_list.go

@@ -406,6 +406,7 @@ func (i *ImageService) imageSummary(ctx context.Context, img c8dimages.Image, pl
 			RepoDigests: []string{target.Digest.String()},
 			RepoTags:    tagsByDigest[target.Digest],
 			Size:        summary.TotalSize,
+			Manifests:   summary.Manifests,
 			// -1 indicates that the value has not been set (avoids ambiguity
 			// between 0 (default) and "not set". We cannot use a pointer (nil)
 			// for this, as the JSON representation uses "omitempty", which would

daemon/containerd/image_list_test.go

@@ -198,6 +198,16 @@ func TestImageList(t *testing.T) {
 	emptyIndex := toContainerdImage(t, specialimage.EmptyIndex)
 	configTarget := toContainerdImage(t, specialimage.ConfigTarget)
 	textplain := toContainerdImage(t, specialimage.TextPlain)
+	missingMultiPlatform := toContainerdImage(t, func(dir string) (*ocispec.Index, error) {
+		idx, _, err := specialimage.PartialMultiPlatform(dir, "missingmp:latest", specialimage.PartialOpts{
+			Stored: nil,
+			Missing: []ocispec.Platform{
+				{OS: "linux", Architecture: "arm64"},
+				{OS: "linux", Architecture: "amd64"},
+			},
+		})
+		return idx, err
+	})
 
 	cs := &blobsDirContentStore{blobs: filepath.Join(blobsDir, "blobs/sha256")}
 
@@ -320,7 +330,15 @@ func TestImageList(t *testing.T) {
 				}
 				assert.Check(t, is.Equal(all[0].ID, textplain.Target.Digest.String()))
 
-				assert.Assert(t, is.Len(all[0].Manifests, 0))
+				assert.Assert(t, is.Len(all[0].Manifests, 1))
+			},
+		},
+		{
+			name:   "multi-platform with no platforms available locally",
+			images: []c8dimages.Image{missingMultiPlatform},
+			check: func(t *testing.T, all []*imagetypes.Summary) {
+				assert.Assert(t, is.Len(all, 1))
+				assert.Check(t, is.Len(all[0].Manifests, 2))
 			},
 		},
 	} {