Merge pull request #51436 from thaJeztah/backend_buildcache

daemon: refactor disk-usage endpoint

daemon/disk_usage.go

@@ -26,30 +26,27 @@ func (daemon *Daemon) containerDiskUsage(ctx context.Context, verbose bool) (*ba
 			return nil, fmt.Errorf("failed to retrieve container list: %v", err)
 		}
 
-		// Remove image manifest descriptor from the result as it should not be included.
-		// https://github.com/moby/moby/pull/49407#discussion_r1954396666
-		for _, c := range containers {
-			c.ImageManifestDescriptor = nil
-		}
-
 		isActive := func(ctr *container.Summary) bool {
 			return ctr.State == container.StateRunning ||
 				ctr.State == container.StatePaused ||
 				ctr.State == container.StateRestarting
 		}
 
-		activeCount := int64(len(containers))
-
-		du := &backend.ContainerDiskUsage{TotalCount: activeCount}
+		du := &backend.ContainerDiskUsage{
+			ActiveCount: int64(len(containers)),
+			TotalCount:  int64(len(containers)),
+		}
 		for _, ctr := range containers {
 			du.TotalSize += ctr.SizeRw
 			if !isActive(ctr) {
 				du.Reclaimable += ctr.SizeRw
-				activeCount--
+				du.ActiveCount--
 			}
-		}
 
-		du.ActiveCount = activeCount
+			// Remove image manifest descriptor from the result as it should not be included.
+			// https://github.com/moby/moby/pull/49407#discussion_r1954396666
+			ctr.ImageManifestDescriptor = nil
+		}
 
 		if verbose {
 			du.Items = sliceutil.Deref(containers)
@@ -73,29 +70,29 @@ func (daemon *Daemon) imageDiskUsage(ctx context.Context, verbose bool) (*backen
 			return nil, errors.Wrap(err, "failed to retrieve image list")
 		}
 
-		reclaimable, _, err := daemon.usageLayer.Do(ctx, struct{}{}, func(ctx context.Context) (int64, error) {
+		totalSize, _, err := daemon.usageLayer.Do(ctx, struct{}{}, func(ctx context.Context) (int64, error) {
 			return daemon.imageService.ImageDiskUsage(ctx)
 		})
 		if err != nil {
 			return nil, errors.Wrap(err, "failed to calculate image disk usage")
 		}
 
-		activeCount := int64(len(images))
-
-		du := &backend.ImageDiskUsage{TotalCount: activeCount, TotalSize: reclaimable}
+		du := &backend.ImageDiskUsage{
+			ActiveCount: int64(len(images)),
+			Reclaimable: totalSize,
+			TotalCount:  int64(len(images)),
+			TotalSize:   totalSize,
+		}
 		for _, i := range images {
 			if i.Containers == 0 {
-				activeCount--
+				du.ActiveCount--
 				if i.Size == -1 || i.SharedSize == -1 {
 					continue
 				}
-				reclaimable -= i.Size - i.SharedSize
+				du.Reclaimable -= i.Size - i.SharedSize
 			}
 		}
 
-		du.Reclaimable = reclaimable
-		du.ActiveCount = activeCount
-
 		if verbose {
 			du.Items = sliceutil.Deref(images)
 		}
@@ -115,21 +112,20 @@ func (daemon *Daemon) localVolumesSize(ctx context.Context, verbose bool) (*back
 			return nil, err
 		}
 
-		activeCount := int64(len(volumes))
-
-		du := &backend.VolumeDiskUsage{TotalCount: activeCount}
+		du := &backend.VolumeDiskUsage{
+			ActiveCount: int64(len(volumes)),
+			TotalCount:  int64(len(volumes)),
+		}
 		for _, v := range volumes {
 			if v.UsageData.Size != -1 {
+				du.TotalSize += v.UsageData.Size
 				if v.UsageData.RefCount == 0 {
 					du.Reclaimable += v.UsageData.Size
-					activeCount--
+					du.ActiveCount--
 				}
-				du.TotalSize += v.UsageData.Size
 			}
 		}
 
-		du.ActiveCount = activeCount
-
 		if verbose {
 			du.Items = sliceutil.Deref(volumes)
 		}

daemon/internal/builder-next/builder.go

@@ -150,15 +150,26 @@ func (b *Builder) Cancel(ctx context.Context, id string) error {
 }
 
 // DiskUsage returns a report about space used by build cache
-func (b *Builder) DiskUsage(ctx context.Context) ([]build.CacheRecord, error) {
+func (b *Builder) DiskUsage(ctx context.Context, options buildbackend.DiskUsageOptions) (*buildbackend.DiskUsage, error) {
 	duResp, err := b.controller.DiskUsage(ctx, &controlapi.DiskUsageRequest{})
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("error getting build cache usage: %w", err)
 	}
 
-	var items []build.CacheRecord
+	var usage buildbackend.DiskUsage
 	for _, r := range duResp.Record {
-		items = append(items, build.CacheRecord{
+		usage.TotalCount++
+		usage.TotalSize += r.Size
+		if r.InUse {
+			usage.ActiveCount++
+		}
+		if !r.InUse && !r.Shared {
+			usage.Reclaimable += r.Size
+		}
+		if !options.Verbose {
+			continue
+		}
+		usage.Items = append(usage.Items, build.CacheRecord{
 			ID:          r.ID,
 			Parents:     r.Parents,
 			Type:        r.RecordType,
@@ -182,7 +193,7 @@ func (b *Builder) DiskUsage(ctx context.Context) ([]build.CacheRecord, error) {
 			UsageCount: int(r.UsageCount),
 		})
 	}
-	return items, nil
+	return &usage, nil
 }
 
 // Prune clears all reclaimable build cache.

daemon/server/backend/disk_usage.go

@@ -28,41 +28,14 @@ type DiskUsage struct {
 	Images     *ImageDiskUsage
 	Containers *ContainerDiskUsage
 	Volumes    *VolumeDiskUsage
-	BuildCache *BuildCacheDiskUsage
-}
-
-// BuildCacheDiskUsage contains disk usage for the build cache.
-type BuildCacheDiskUsage struct {
-	ActiveCount int64
-	TotalCount  int64
-	TotalSize   int64
-	Reclaimable int64
-	Items       []build.CacheRecord
+	BuildCache *build.DiskUsage
 }
 
 // ContainerDiskUsage contains disk usage for containers.
-type ContainerDiskUsage struct {
-	ActiveCount int64
-	TotalCount  int64
-	TotalSize   int64
-	Reclaimable int64
-	Items       []container.Summary
-}
+type ContainerDiskUsage = container.DiskUsage
 
 // ImageDiskUsage contains disk usage for images.
-type ImageDiskUsage struct {
-	ActiveCount int64
-	TotalCount  int64
-	TotalSize   int64
-	Reclaimable int64
-	Items       []image.Summary
-}
+type ImageDiskUsage = image.DiskUsage
 
 // VolumeDiskUsage contains disk usage for volumes.
-type VolumeDiskUsage struct {
-	ActiveCount int64
-	TotalCount  int64
-	TotalSize   int64
-	Reclaimable int64
-	Items       []volume.Volume
-}
+type VolumeDiskUsage = volume.DiskUsage

daemon/server/buildbackend/build.go

@@ -11,6 +11,13 @@ import (
 	"github.com/moby/moby/v2/daemon/internal/filters"
 )
 
+type DiskUsageOptions struct {
+	Verbose bool
+}
+
+// DiskUsage contains disk usage for the build cache.
+type DiskUsage = build.DiskUsage
+
 type CachePruneOptions struct {
 	All           bool
 	ReservedSpace int64

daemon/server/router/system/backend.go

@@ -4,13 +4,13 @@ import (
 	"context"
 	"time"
 
-	"github.com/moby/moby/api/types/build"
 	"github.com/moby/moby/api/types/events"
 	"github.com/moby/moby/api/types/registry"
 	"github.com/moby/moby/api/types/swarm"
 	"github.com/moby/moby/api/types/system"
 	"github.com/moby/moby/v2/daemon/internal/filters"
 	"github.com/moby/moby/v2/daemon/server/backend"
+	"github.com/moby/moby/v2/daemon/server/buildbackend"
 )
 
 // Backend is the methods that need to be implemented to provide
@@ -32,7 +32,7 @@ type ClusterBackend interface {
 
 // BuildBackend provides build specific system information.
 type BuildBackend interface {
-	DiskUsage(context.Context) ([]build.CacheRecord, error)
+	DiskUsage(context.Context, buildbackend.DiskUsageOptions) (*buildbackend.DiskUsage, error)
 }
 
 // StatusProvider provides methods to get the swarm status of the current node.

daemon/server/router/system/system_routes.go

@@ -5,26 +5,22 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
-	"strconv"
 	"time"
 
 	"github.com/containerd/log"
 	"github.com/golang/gddo/httputil"
 	"github.com/moby/moby/api/pkg/authconfig"
 	"github.com/moby/moby/api/types"
-	buildtypes "github.com/moby/moby/api/types/build"
-	"github.com/moby/moby/api/types/container"
 	"github.com/moby/moby/api/types/events"
-	"github.com/moby/moby/api/types/image"
 	"github.com/moby/moby/api/types/registry"
 	"github.com/moby/moby/api/types/swarm"
 	"github.com/moby/moby/api/types/system"
-	"github.com/moby/moby/api/types/volume"
 	"github.com/moby/moby/v2/daemon/internal/compat"
 	"github.com/moby/moby/v2/daemon/internal/filters"
 	"github.com/moby/moby/v2/daemon/internal/timestamp"
 	"github.com/moby/moby/v2/daemon/internal/versions"
 	"github.com/moby/moby/v2/daemon/server/backend"
+	"github.com/moby/moby/v2/daemon/server/buildbackend"
 	"github.com/moby/moby/v2/daemon/server/httputils"
 	"github.com/moby/moby/v2/daemon/server/router/build"
 	"github.com/moby/moby/v2/pkg/ioutils"
@@ -165,42 +161,49 @@ func (s *systemRouter) getDiskUsage(ctx context.Context, w http.ResponseWriter,
 		}
 	}
 
-	// To maintain backwards compatibility with older clients, when communicating with API versions prior to 1.52,
-	// verbose mode is always enabled. For API 1.52 and onwards, if the "verbose" query parameter is not set,
-	// assume legacy fields should be included.
 	var verbose, legacyFields bool
-	if v := r.Form.Get("verbose"); versions.GreaterThanOrEqualTo(version, "1.52") && v != "" {
-		var err error
-		verbose, err = strconv.ParseBool(v)
-		if err != nil {
-			return invalidRequestError{Err: fmt.Errorf("invalid value for verbose: %s", v)}
-		}
+	if versions.LessThan(version, "1.52") {
+		legacyFields = true
 	} else {
-		// In versions prior to 1.52, legacy fields were always included.
-		legacyFields, verbose = true, true
+		verbose = httputils.BoolValue(r, "verbose")
+
+		// For API 1.52, we include both legacy and current fields, as some
+		// integrations (such as "docker-py") currently use "latest", non-versioned
+		// API version.
+		//
+		// However, if the "verbose" query parameter is set, we can assume
+		// the client is "API 1.52 aware", and we'll omit the legacy fields.
+		//
+		// FIXME(thaJeztah): remove legacy fields entirely for API 1.53
+		legacyFields = !verbose
 	}
 
 	eg, ctx := errgroup.WithContext(ctx)
 
-	var systemDiskUsage *backend.DiskUsage
+	diskUsage := &backend.DiskUsage{}
 	if getContainers || getImages || getVolumes {
 		eg.Go(func() error {
-			var err error
-			systemDiskUsage, err = s.backend.SystemDiskUsage(ctx, backend.DiskUsageOptions{
+			du, err := s.backend.SystemDiskUsage(ctx, backend.DiskUsageOptions{
 				Containers: getContainers,
 				Images:     getImages,
 				Volumes:    getVolumes,
-				Verbose:    verbose,
+				Verbose:    verbose || legacyFields,
 			})
-			return err
+			if err != nil {
+				return err
+			}
+			diskUsage = du
+			return nil
 		})
 	}
 
-	var buildCache []buildtypes.CacheRecord
+	var buildCacheUsage *buildbackend.DiskUsage
 	if getBuildCache {
 		eg.Go(func() error {
 			var err error
-			buildCache, err = s.builder.DiskUsage(ctx)
+			buildCacheUsage, err = s.builder.DiskUsage(ctx, buildbackend.DiskUsageOptions{
+				Verbose: verbose || legacyFields,
+			})
 			if err != nil {
 				return errors.Wrap(err, "error getting build cache usage")
 			}
@@ -211,83 +214,35 @@ func (s *systemRouter) getDiskUsage(ctx context.Context, w http.ResponseWriter,
 	if err := eg.Wait(); err != nil {
 		return err
 	}
+	diskUsage.BuildCache = buildCacheUsage
 
-	var v system.DiskUsage
-	if systemDiskUsage != nil && systemDiskUsage.Images != nil {
-		v.ImageUsage = &image.DiskUsage{
-			ActiveCount: systemDiskUsage.Images.ActiveCount,
-			Reclaimable: systemDiskUsage.Images.Reclaimable,
-			TotalCount:  systemDiskUsage.Images.TotalCount,
-			TotalSize:   systemDiskUsage.Images.TotalSize,
+	var legacy system.LegacyDiskUsage
+	if legacyFields {
+		if diskUsage.Images != nil {
+			legacy.LayersSize = diskUsage.Images.TotalSize      //nolint: staticcheck,SA1019: kept to maintain backwards compatibility with API < v1.52.
+			legacy.Images = nonNilSlice(diskUsage.Images.Items) //nolint: staticcheck,SA1019: kept to maintain backwards compatibility with API < v1.52.
 		}
-
-		if legacyFields {
-			v.LayersSize = systemDiskUsage.Images.TotalSize      //nolint: staticcheck,SA1019: v.LayersSize is deprecated: kept to maintain backwards compatibility with API < v1.52, use [ImagesDiskUsage.TotalSize] instead.
-			v.Images = nonNilSlice(systemDiskUsage.Images.Items) //nolint: staticcheck,SA1019: v.Images is deprecated: kept to maintain backwards compatibility with API < v1.52, use [ImagesDiskUsage.Items] instead.
-		} else if verbose {
-			v.ImageUsage.Items = systemDiskUsage.Images.Items
+		if diskUsage.Containers != nil {
+			legacy.Containers = nonNilSlice(diskUsage.Containers.Items) //nolint: staticcheck,SA1019: kept to maintain backwards compatibility with API < v1.52.
+		}
+		if diskUsage.Volumes != nil {
+			legacy.Volumes = nonNilSlice(diskUsage.Volumes.Items) //nolint: staticcheck,SA1019: kept to maintain backwards compatibility with API < v1.52.
+		}
+		if diskUsage.BuildCache != nil {
+			legacy.BuildCache = nonNilSlice(diskUsage.BuildCache.Items) //nolint: staticcheck,SA1019: kept to maintain backwards compatibility with API < v1.52.
 		}
 	}
-	if systemDiskUsage != nil && systemDiskUsage.Containers != nil {
-		v.ContainerUsage = &container.DiskUsage{
-			ActiveCount: systemDiskUsage.Containers.ActiveCount,
-			Reclaimable: systemDiskUsage.Containers.Reclaimable,
-			TotalCount:  systemDiskUsage.Containers.TotalCount,
-			TotalSize:   systemDiskUsage.Containers.TotalSize,
-		}
-
-		if legacyFields {
-			v.Containers = nonNilSlice(systemDiskUsage.Containers.Items) //nolint: staticcheck,SA1019: v.Containers is deprecated: kept to maintain backwards compatibility with API < v1.52, use [ContainersDiskUsage.Items] instead.
-		} else if verbose {
-			v.ContainerUsage.Items = systemDiskUsage.Containers.Items
-		}
+	if versions.LessThan(version, "1.52") {
+		return httputils.WriteJSON(w, http.StatusOK, legacy)
 	}
-	if systemDiskUsage != nil && systemDiskUsage.Volumes != nil {
-		v.VolumeUsage = &volume.DiskUsage{
-			ActiveCount: systemDiskUsage.Volumes.ActiveCount,
-			TotalSize:   systemDiskUsage.Volumes.TotalSize,
-			Reclaimable: systemDiskUsage.Volumes.Reclaimable,
-			TotalCount:  systemDiskUsage.Volumes.TotalCount,
-		}
 
-		if legacyFields {
-			v.Volumes = nonNilSlice(systemDiskUsage.Volumes.Items) //nolint: staticcheck,SA1019: v.Volumes is deprecated: kept to maintain backwards compatibility with API < v1.52, use [VolumesDiskUsage.Items] instead.
-		} else if verbose {
-			v.VolumeUsage.Items = systemDiskUsage.Volumes.Items
-		}
-	}
-	if getBuildCache {
-		v.BuildCacheUsage = &buildtypes.DiskUsage{
-			TotalCount: int64(len(buildCache)),
-		}
-
-		activeCount := v.BuildCacheUsage.TotalCount
-		var totalSize, reclaimable int64
-
-		for _, b := range buildCache {
-			if versions.LessThan(version, "1.42") {
-				totalSize += b.Size
-			}
-
-			if !b.InUse {
-				activeCount--
-			}
-			if !b.InUse && !b.Shared {
-				reclaimable += b.Size
-			}
-		}
-
-		v.BuildCacheUsage.ActiveCount = activeCount
-		v.BuildCacheUsage.TotalSize = totalSize
-		v.BuildCacheUsage.Reclaimable = reclaimable
-
-		if legacyFields {
-			v.BuildCache = nonNilSlice(buildCache) //nolint: staticcheck,SA1019: v.BuildCache is deprecated: kept to maintain backwards compatibility with API < v1.52, use [BuildCacheDiskUsage.Items] instead.
-		} else if verbose {
-			v.BuildCacheUsage.Items = buildCache
-		}
-	}
-	return httputils.WriteJSON(w, http.StatusOK, v)
+	return httputils.WriteJSON(w, http.StatusOK, &system.DiskUsage{
+		LegacyDiskUsage: legacy,
+		ImageUsage:      diskUsage.Images,
+		ContainerUsage:  diskUsage.Containers,
+		VolumeUsage:     diskUsage.Volumes,
+		BuildCacheUsage: diskUsage.BuildCache,
+	})
 }
 
 // nonNilSlice is used for the legacy fields, which are either omitted