Merge pull request #46097 from vvoland/c8d-missing-config-24

[24.0 backport] c8d/container: Follow snapshot parents for size calculation

daemon/containerd/image_list.go

@@ -10,6 +10,7 @@ import (
 	cerrdefs "github.com/containerd/containerd/errdefs"
 	"github.com/containerd/containerd/images"
 	"github.com/containerd/containerd/labels"
+	"github.com/containerd/containerd/snapshots"
 	"github.com/docker/distribution/reference"
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/filters"
@@ -142,41 +143,32 @@ func (i *ImageService) Images(ctx context.Context, opts types.ImageListOptions)
 func (i *ImageService) singlePlatformImage(ctx context.Context, contentStore content.Store, image *ImageManifest) (*types.ImageSummary, []digest.Digest, error) {
 	diffIDs, err := image.RootFS(ctx)
 	if err != nil {
-		return nil, nil, err
-	}
-	chainIDs := identity.ChainIDs(diffIDs)
-
-	size, err := image.Size(ctx)
-	if err != nil {
-		return nil, nil, err
+		return nil, nil, errors.Wrapf(err, "failed to get rootfs of image %s", image.Name())
 	}
 
 	// TODO(thaJeztah): do we need to take multiple snapshotters into account? See https://github.com/moby/moby/issues/45273
 	snapshotter := i.client.SnapshotService(i.snapshotter)
-	sizeCache := make(map[digest.Digest]int64)
 
-	snapshotSizeFn := func(d digest.Digest) (int64, error) {
-		if s, ok := sizeCache[d]; ok {
-			return s, nil
-		}
-		usage, err := snapshotter.Usage(ctx, d.String())
-		if err != nil {
-			if cerrdefs.IsNotFound(err) {
-				return 0, nil
-			}
-			return 0, err
+	imageSnapshotID := identity.ChainID(diffIDs).String()
+	unpackedUsage, err := calculateSnapshotTotalUsage(ctx, snapshotter, imageSnapshotID)
+	if err != nil {
+		if !cerrdefs.IsNotFound(err) {
+			logrus.WithError(err).WithFields(logrus.Fields{
+				"image":      image.Name(),
+				"snapshotID": imageSnapshotID,
+			}).Warn("failed to calculate unpacked size of image")
 		}
-		sizeCache[d] = usage.Size
-		return usage.Size, nil
+		unpackedUsage = snapshots.Usage{Size: 0}
 	}
-	snapshotSize, err := computeSnapshotSize(chainIDs, snapshotSizeFn)
+
+	contentSize, err := image.Size(ctx)
 	if err != nil {
 		return nil, nil, err
 	}
 
 	// totalSize is the size of the image's packed layers and snapshots
 	// (unpacked layers) combined.
-	totalSize := size + snapshotSize
+	totalSize := contentSize + unpackedUsage.Size
 
 	var repoTags, repoDigests []string
 	rawImg := image.Metadata()
@@ -225,7 +217,7 @@ func (i *ImageService) singlePlatformImage(ctx context.Context, contentStore con
 		Containers: -1,
 	}
 
-	return summary, chainIDs, nil
+	return summary, identity.ChainIDs(diffIDs), nil
 }
 
 type imageFilterFunc func(image images.Image) bool
@@ -446,20 +438,6 @@ func setupLabelFilter(store content.Store, fltrs filters.Args) (func(image image
 	}, nil
 }
 
-// computeSnapshotSize calculates the total size consumed by the snapshots
-// for the given chainIDs.
-func computeSnapshotSize(chainIDs []digest.Digest, sizeFn func(d digest.Digest) (int64, error)) (int64, error) {
-	var totalSize int64
-	for _, chainID := range chainIDs {
-		size, err := sizeFn(chainID)
-		if err != nil {
-			return totalSize, err
-		}
-		totalSize += size
-	}
-	return totalSize, nil
-}
-
 func computeSharedSize(chainIDs []digest.Digest, layers map[digest.Digest]int, sizeFn func(d digest.Digest) (int64, error)) (int64, error) {
 	var sharedSize int64
 	for _, chainID := range chainIDs {

daemon/containerd/image_snapshot.go

@@ -2,13 +2,18 @@ package containerd
 
 import (
 	"context"
+	"fmt"
 
 	"github.com/containerd/containerd"
+	cerrdefs "github.com/containerd/containerd/errdefs"
 	containerdimages "github.com/containerd/containerd/images"
 	"github.com/containerd/containerd/leases"
 	"github.com/containerd/containerd/platforms"
+	"github.com/containerd/containerd/snapshots"
+	"github.com/docker/docker/errdefs"
 	"github.com/opencontainers/image-spec/identity"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
+	"github.com/pkg/errors"
 )
 
 // PrepareSnapshot prepares a snapshot from a parent image for a container
@@ -67,3 +72,46 @@ func (i *ImageService) PrepareSnapshot(ctx context.Context, id string, parentIma
 	_, err = s.Prepare(ctx, id, parent)
 	return err
 }
+
+// calculateSnapshotParentUsage returns the usage of all ancestors of the
+// provided snapshot. It doesn't include the size of the snapshot itself.
+func calculateSnapshotParentUsage(ctx context.Context, snapshotter snapshots.Snapshotter, snapshotID string) (snapshots.Usage, error) {
+	info, err := snapshotter.Stat(ctx, snapshotID)
+	if err != nil {
+		if cerrdefs.IsNotFound(err) {
+			return snapshots.Usage{}, errdefs.NotFound(err)
+		}
+		return snapshots.Usage{}, errdefs.System(errors.Wrapf(err, "snapshotter.Stat failed for %s", snapshotID))
+	}
+	if info.Parent == "" {
+		return snapshots.Usage{}, errdefs.NotFound(fmt.Errorf("snapshot %s has no parent", snapshotID))
+	}
+
+	return calculateSnapshotTotalUsage(ctx, snapshotter, info.Parent)
+}
+
+// calculateSnapshotTotalUsage returns the total usage of that snapshot
+// including all of its ancestors.
+func calculateSnapshotTotalUsage(ctx context.Context, snapshotter snapshots.Snapshotter, snapshotID string) (snapshots.Usage, error) {
+	var total snapshots.Usage
+	next := snapshotID
+
+	for next != "" {
+		usage, err := snapshotter.Usage(ctx, next)
+		if err != nil {
+			if cerrdefs.IsNotFound(err) {
+				return total, errdefs.NotFound(errors.Wrapf(err, "non-existing ancestor of %s", snapshotID))
+			}
+			return total, errdefs.System(errors.Wrapf(err, "snapshotter.Usage failed for %s", next))
+		}
+		total.Size += usage.Size
+		total.Inodes += usage.Inodes
+
+		info, err := snapshotter.Stat(ctx, next)
+		if err != nil {
+			return total, errdefs.System(errors.Wrapf(err, "snapshotter.Stat failed for %s", next))
+		}
+		next = info.Parent
+	}
+	return total, nil
+}

daemon/containerd/service.go

@@ -2,16 +2,15 @@ package containerd
 
 import (
 	"context"
-	"encoding/json"
+	"fmt"
 	"sync/atomic"
 
 	"github.com/containerd/containerd"
-	"github.com/containerd/containerd/content"
+	cerrdefs "github.com/containerd/containerd/errdefs"
 	"github.com/containerd/containerd/plugin"
 	"github.com/containerd/containerd/remotes/docker"
 	"github.com/containerd/containerd/snapshots"
 	"github.com/docker/distribution/reference"
-	imagetypes "github.com/docker/docker/api/types/image"
 	"github.com/docker/docker/container"
 	daemonevents "github.com/docker/docker/daemon/events"
 	"github.com/docker/docker/daemon/images"
@@ -20,8 +19,6 @@ import (
 	"github.com/docker/docker/image"
 	"github.com/docker/docker/layer"
 	"github.com/docker/docker/registry"
-	"github.com/opencontainers/go-digest"
-	"github.com/opencontainers/image-spec/identity"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
@@ -165,72 +162,29 @@ func (i *ImageService) GetContainerLayerSize(ctx context.Context, containerID st
 	}
 
 	snapshotter := i.client.SnapshotService(ctr.Driver)
-
-	usage, err := snapshotter.Usage(ctx, containerID)
+	rwLayerUsage, err := snapshotter.Usage(ctx, containerID)
 	if err != nil {
-		return 0, 0, err
-	}
-
-	imageManifest, err := getContainerImageManifest(ctr)
-	if err != nil {
-		// Best efforts attempt to pick an image.
-		// We don't have platform information at this point, so we can only
-		// assume that the platform matches host.
-		// Otherwise this will give a wrong base image size (different
-		// platform), but should be close enough.
-		mfst, err := i.GetImageManifest(ctx, ctr.Config.Image, imagetypes.GetImageOpts{})
-		if err != nil {
-			// Log error, don't error out whole operation.
-			logrus.WithFields(logrus.Fields{
-				logrus.ErrorKey: err,
-				"container":     containerID,
-			}).Warn("empty ImageManifest, can't calculate base image size")
-			return usage.Size, 0, nil
+		if cerrdefs.IsNotFound(err) {
+			return 0, 0, errdefs.NotFound(fmt.Errorf("rw layer snapshot not found for container %s", containerID))
 		}
-		imageManifest = *mfst
-	}
-	cs := i.client.ContentStore()
-
-	imageManifestBytes, err := content.ReadBlob(ctx, cs, imageManifest)
-	if err != nil {
-		return 0, 0, err
-	}
-
-	var manifest ocispec.Manifest
-	if err := json.Unmarshal(imageManifestBytes, &manifest); err != nil {
-		return 0, 0, err
+		return 0, 0, errdefs.System(errors.Wrapf(err, "snapshotter.Usage failed for %s", containerID))
 	}
 
-	imageConfigBytes, err := content.ReadBlob(ctx, cs, manifest.Config)
+	unpackedUsage, err := calculateSnapshotParentUsage(ctx, snapshotter, containerID)
 	if err != nil {
-		return 0, 0, err
-	}
-	var img ocispec.Image
-	if err := json.Unmarshal(imageConfigBytes, &img); err != nil {
-		return 0, 0, err
-	}
-
-	sizeCache := make(map[digest.Digest]int64)
-	snapshotSizeFn := func(d digest.Digest) (int64, error) {
-		if s, ok := sizeCache[d]; ok {
-			return s, nil
+		if cerrdefs.IsNotFound(err) {
+			logrus.WithField("ctr", containerID).Warn("parent of container snapshot no longer present")
+		} else {
+			logrus.WithError(err).WithField("ctr", containerID).Warn("unexpected error when calculating usage of the parent snapshots")
 		}
-		u, err := snapshotter.Usage(ctx, d.String())
-		if err != nil {
-			return 0, err
-		}
-		sizeCache[d] = u.Size
-		return u.Size, nil
-	}
-
-	chainIDs := identity.ChainIDs(img.RootFS.DiffIDs)
-	snapShotSize, err := computeSnapshotSize(chainIDs, snapshotSizeFn)
-	if err != nil {
-		return 0, 0, err
 	}
+	logrus.WithFields(logrus.Fields{
+		"rwLayerUsage": rwLayerUsage.Size,
+		"unpacked":     unpackedUsage.Size,
+	}).Debug("GetContainerLayerSize")
 
 	// TODO(thaJeztah): include content-store size for the image (similar to "GET /images/json")
-	return usage.Size, usage.Size + snapShotSize, nil
+	return rwLayerUsage.Size, rwLayerUsage.Size + unpackedUsage.Size, nil
 }
 
 // getContainerImageManifest safely dereferences ImageManifest.