From 46dba49ec8eff07a77f70479f4b32745e67a4e5d Mon Sep 17 00:00:00 2001 From: Austin Vazquez Date: Fri, 19 Dec 2025 10:55:02 -0600 Subject: [PATCH] Fix reclaimable image disk usage calculation for in-use images This change reworks to build up the reclaimable image disk uage instead of setting it to total size and subtracting active images. This change also includes the image index size as reclaimable if it is included with the image summary. Signed-off-by: Austin Vazquez --- client/system_disk_usage.go | 13 +- client/system_disk_usage_test.go | 120 ++++++++++++++++++ daemon/disk_usage.go | 18 ++- integration/system/disk_usage_test.go | 4 + .../moby/moby/client/system_disk_usage.go | 13 +- 5 files changed, 138 insertions(+), 30 deletions(-) diff --git a/client/system_disk_usage.go b/client/system_disk_usage.go index 1bb2d0d7ef..c5df1e1b19 100644 --- a/client/system_disk_usage.go +++ b/client/system_disk_usage.go @@ -244,22 +244,15 @@ func imageDiskUsageFromLegacyAPI(du *legacyDiskUsage) ImagesDiskUsage { Items: du.Images, } - var used int64 for _, i := range idu.Items { if i.Containers > 0 { idu.ActiveCount++ - - if i.Size == -1 || i.SharedSize == -1 { - continue - } - used += (i.Size - i.SharedSize) + } else if i.Size != -1 && i.SharedSize != -1 { + // Only count reclaimable size if we have size information + idu.Reclaimable += (i.Size - i.SharedSize) } } - if idu.TotalCount > 0 { - idu.Reclaimable = idu.TotalSize - used - } - return idu } diff --git a/client/system_disk_usage_test.go b/client/system_disk_usage_test.go index 5de5f47d52..a703bee3e7 100644 --- a/client/system_disk_usage_test.go +++ b/client/system_disk_usage_test.go @@ -8,6 +8,7 @@ import ( cerrdefs "github.com/containerd/errdefs" "github.com/moby/moby/api/types/image" "github.com/moby/moby/api/types/system" + ocispec "github.com/opencontainers/image-spec/specs-go/v1" "gotest.tools/v3/assert" is "gotest.tools/v3/assert/cmp" ) @@ -154,3 +155,122 @@ func TestLegacyDiskUsage(t *testing.T) { assert.Equal(t, du.Images.TotalSize, int64(4096)) assert.Equal(t, len(du.Images.Items), 0) } + +func TestImageDiskUsageFromLegacyAPI(t *testing.T) { + const legacyVersion = "1.51" + const expectedURL = "/system/df" + + tests := []struct { + name string + mockResponse *legacyDiskUsage + expectedActiveCount int64 + expectedTotalCount int64 + expectedReclaimable int64 + expectedTotalSize int64 + }{ + { + name: "no images", + mockResponse: &legacyDiskUsage{ + LayersSize: 0, + Images: []image.Summary{}, + }, + expectedActiveCount: 0, + expectedTotalCount: 0, + expectedReclaimable: 0, + expectedTotalSize: 0, + }, + { + name: "images with no containers", + mockResponse: &legacyDiskUsage{ + LayersSize: 8192, + Images: []image.Summary{ + {ID: "image1", Size: 4096, SharedSize: 0, Containers: 0}, + {ID: "image2", Size: 4096, SharedSize: 0, Containers: 0}, + }, + }, + expectedActiveCount: 0, + expectedTotalCount: 2, + expectedReclaimable: 8192, + expectedTotalSize: 8192, + }, + { + name: "images with containers", + mockResponse: &legacyDiskUsage{ + LayersSize: 12288, + Images: []image.Summary{ + {ID: "image1", Size: 4096, SharedSize: 0, Containers: 2}, + {ID: "image2", Size: 2048, SharedSize: 0, Containers: 0}, + {ID: "image3", Size: 6144, SharedSize: 0, Containers: 1}, + }, + }, + expectedActiveCount: 2, + expectedTotalCount: 3, + expectedReclaimable: 2048, + expectedTotalSize: 12288, + }, + { + name: "images with shared size", + mockResponse: &legacyDiskUsage{ + LayersSize: 15360, + Images: []image.Summary{ + {ID: "image1", Size: 4096, SharedSize: 1024, Containers: 1}, + {ID: "image2", Size: 8192, SharedSize: 2048, Containers: 0}, + {ID: "image3", Size: 3072, SharedSize: 1024, Containers: 0}, + }, + }, + expectedActiveCount: 1, + expectedTotalCount: 3, + expectedReclaimable: 8192, // (8192-2048) + (3072-1024) + expectedTotalSize: 15360, + }, + { + name: "multiplatform image with an image index", + mockResponse: &legacyDiskUsage{ + LayersSize: 4096, + Images: []image.Summary{ + {ID: "image1", Size: 4096, SharedSize: 0, Containers: 0, Descriptor: &ocispec.Descriptor{MediaType: ocispec.MediaTypeImageIndex, Size: 512}}, + }, + }, + expectedActiveCount: 0, + expectedTotalCount: 1, + expectedReclaimable: 4096, // (4096 - 0) + expectedTotalSize: 4096, + }, + { + name: "multiplatform image with a Docker distribution manifest", + mockResponse: &legacyDiskUsage{ + LayersSize: 4096, + Images: []image.Summary{ + {ID: "image1", Size: 4096, SharedSize: 0, Containers: 0, Descriptor: &ocispec.Descriptor{MediaType: "application/vnd.docker.distribution.manifest.v2+json", Size: 427}}, + }, + }, + expectedActiveCount: 0, + expectedTotalCount: 1, + expectedReclaimable: 4096, // (4096 - 0) + expectedTotalSize: 4096, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + client, err := New( + WithAPIVersion(legacyVersion), + WithMockClient(func(req *http.Request) (*http.Response, error) { + if err := assertRequest(req, http.MethodGet, "/v"+legacyVersion+expectedURL); err != nil { + return nil, err + } + + return mockJSONResponse(http.StatusOK, nil, tt.mockResponse)(req) + })) + assert.NilError(t, err) + + du, err := client.DiskUsage(t.Context(), DiskUsageOptions{Images: true}) + assert.NilError(t, err) + assert.Equal(t, du.Images.ActiveCount, tt.expectedActiveCount, tt.name) + assert.Equal(t, du.Images.TotalCount, tt.expectedTotalCount, tt.name) + assert.Equal(t, du.Images.Reclaimable, tt.expectedReclaimable, tt.name) + assert.Equal(t, du.Images.TotalSize, tt.expectedTotalSize, tt.name) + assert.Equal(t, len(du.Images.Items), len(tt.mockResponse.Images), tt.name) + }) + } +} diff --git a/daemon/disk_usage.go b/daemon/disk_usage.go index 7d88429b91..8acf1be42e 100644 --- a/daemon/disk_usage.go +++ b/daemon/disk_usage.go @@ -74,18 +74,16 @@ func (daemon *Daemon) imageDiskUsage(ctx context.Context, verbose bool) (*backen } du := &backend.ImageDiskUsage{ - ActiveCount: int64(len(images)), - Reclaimable: totalSize, - TotalCount: int64(len(images)), - TotalSize: totalSize, + TotalCount: int64(len(images)), + TotalSize: totalSize, } + for _, i := range images { - if i.Containers == 0 { - du.ActiveCount-- - if i.Size == -1 || i.SharedSize == -1 { - continue - } - du.Reclaimable -= i.Size - i.SharedSize + if i.Containers > 0 { + du.ActiveCount++ + } else if i.Size != -1 && i.SharedSize != -1 { + // Only count reclaimable size if we have size information + du.Reclaimable += (i.Size - i.SharedSize) } } diff --git a/integration/system/disk_usage_test.go b/integration/system/disk_usage_test.go index 6543648f64..8c74311683 100644 --- a/integration/system/disk_usage_test.go +++ b/integration/system/disk_usage_test.go @@ -76,6 +76,9 @@ func TestDiskUsage(t *testing.T) { }) assert.NilError(t, err) + assert.Equal(t, du.Images.ActiveCount, int64(0)) + assert.Equal(t, du.Images.TotalCount, int64(1)) + assert.Equal(t, du.Images.Reclaimable, du.Images.TotalSize) assert.Assert(t, du.Images.TotalSize > 0) assert.Equal(t, len(du.Images.Items), 1) assert.Equal(t, len(du.Images.Items[0].RepoTags), 1) @@ -113,6 +116,7 @@ func TestDiskUsage(t *testing.T) { assert.Equal(t, du.Images.ActiveCount, int64(1)) assert.Equal(t, du.Images.TotalCount, int64(1)) + assert.Equal(t, du.Images.Reclaimable, int64(0)) assert.Equal(t, len(du.Images.Items), 1) assert.Equal(t, du.Images.Items[0].Containers, prev.Images.Items[0].Containers+1) diff --git a/vendor/github.com/moby/moby/client/system_disk_usage.go b/vendor/github.com/moby/moby/client/system_disk_usage.go index 1bb2d0d7ef..c5df1e1b19 100644 --- a/vendor/github.com/moby/moby/client/system_disk_usage.go +++ b/vendor/github.com/moby/moby/client/system_disk_usage.go @@ -244,22 +244,15 @@ func imageDiskUsageFromLegacyAPI(du *legacyDiskUsage) ImagesDiskUsage { Items: du.Images, } - var used int64 for _, i := range idu.Items { if i.Containers > 0 { idu.ActiveCount++ - - if i.Size == -1 || i.SharedSize == -1 { - continue - } - used += (i.Size - i.SharedSize) + } else if i.Size != -1 && i.SharedSize != -1 { + // Only count reclaimable size if we have size information + idu.Reclaimable += (i.Size - i.SharedSize) } } - if idu.TotalCount > 0 { - idu.Reclaimable = idu.TotalSize - used - } - return idu }