Skip to content

Commit 46dba49

Browse files
austinvazquezaustinvazquez
committed
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 <[email protected]>
1 parent 1fbaf34 commit 46dba49

5 files changed

Lines changed: 138 additions & 30 deletions

File tree

‎client/system_disk_usage.go‎

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -244,22 +244,15 @@ func imageDiskUsageFromLegacyAPI(du *legacyDiskUsage) ImagesDiskUsage {
244244
Items: du.Images,
245245
}
246246

247-
var used int64
248247
for _, i := range idu.Items {
249248
if i.Containers > 0 {
250249
idu.ActiveCount++
251-
252-
if i.Size == -1 || i.SharedSize == -1 {
253-
continue
254-
}
255-
used += (i.Size - i.SharedSize)
250+
} else if i.Size != -1 && i.SharedSize != -1 {
251+
// Only count reclaimable size if we have size information
252+
idu.Reclaimable += (i.Size - i.SharedSize)
256253
}
257254
}
258255

259-
if idu.TotalCount > 0 {
260-
idu.Reclaimable = idu.TotalSize - used
261-
}
262-
263256
return idu
264257
}
265258

‎client/system_disk_usage_test.go‎

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
cerrdefs "github.com/containerd/errdefs"
99
"github.com/moby/moby/api/types/image"
1010
"github.com/moby/moby/api/types/system"
11+
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
1112
"gotest.tools/v3/assert"
1213
is "gotest.tools/v3/assert/cmp"
1314
)
@@ -154,3 +155,122 @@ func TestLegacyDiskUsage(t *testing.T) {
154155
assert.Equal(t, du.Images.TotalSize, int64(4096))
155156
assert.Equal(t, len(du.Images.Items), 0)
156157
}
158+
159+
func TestImageDiskUsageFromLegacyAPI(t *testing.T) {
160+
const legacyVersion = "1.51"
161+
const expectedURL = "/system/df"
162+
163+
tests := []struct {
164+
name string
165+
mockResponse *legacyDiskUsage
166+
expectedActiveCount int64
167+
expectedTotalCount int64
168+
expectedReclaimable int64
169+
expectedTotalSize int64
170+
}{
171+
{
172+
name: "no images",
173+
mockResponse: &legacyDiskUsage{
174+
LayersSize: 0,
175+
Images: []image.Summary{},
176+
},
177+
expectedActiveCount: 0,
178+
expectedTotalCount: 0,
179+
expectedReclaimable: 0,
180+
expectedTotalSize: 0,
181+
},
182+
{
183+
name: "images with no containers",
184+
mockResponse: &legacyDiskUsage{
185+
LayersSize: 8192,
186+
Images: []image.Summary{
187+
{ID: "image1", Size: 4096, SharedSize: 0, Containers: 0},
188+
{ID: "image2", Size: 4096, SharedSize: 0, Containers: 0},
189+
},
190+
},
191+
expectedActiveCount: 0,
192+
expectedTotalCount: 2,
193+
expectedReclaimable: 8192,
194+
expectedTotalSize: 8192,
195+
},
196+
{
197+
name: "images with containers",
198+
mockResponse: &legacyDiskUsage{
199+
LayersSize: 12288,
200+
Images: []image.Summary{
201+
{ID: "image1", Size: 4096, SharedSize: 0, Containers: 2},
202+
{ID: "image2", Size: 2048, SharedSize: 0, Containers: 0},
203+
{ID: "image3", Size: 6144, SharedSize: 0, Containers: 1},
204+
},
205+
},
206+
expectedActiveCount: 2,
207+
expectedTotalCount: 3,
208+
expectedReclaimable: 2048,
209+
expectedTotalSize: 12288,
210+
},
211+
{
212+
name: "images with shared size",
213+
mockResponse: &legacyDiskUsage{
214+
LayersSize: 15360,
215+
Images: []image.Summary{
216+
{ID: "image1", Size: 4096, SharedSize: 1024, Containers: 1},
217+
{ID: "image2", Size: 8192, SharedSize: 2048, Containers: 0},
218+
{ID: "image3", Size: 3072, SharedSize: 1024, Containers: 0},
219+
},
220+
},
221+
expectedActiveCount: 1,
222+
expectedTotalCount: 3,
223+
expectedReclaimable: 8192, // (8192-2048) + (3072-1024)
224+
expectedTotalSize: 15360,
225+
},
226+
{
227+
name: "multiplatform image with an image index",
228+
mockResponse: &legacyDiskUsage{
229+
LayersSize: 4096,
230+
Images: []image.Summary{
231+
{ID: "image1", Size: 4096, SharedSize: 0, Containers: 0, Descriptor: &ocispec.Descriptor{MediaType: ocispec.MediaTypeImageIndex, Size: 512}},
232+
},
233+
},
234+
expectedActiveCount: 0,
235+
expectedTotalCount: 1,
236+
expectedReclaimable: 4096, // (4096 - 0)
237+
expectedTotalSize: 4096,
238+
},
239+
{
240+
name: "multiplatform image with a Docker distribution manifest",
241+
mockResponse: &legacyDiskUsage{
242+
LayersSize: 4096,
243+
Images: []image.Summary{
244+
{ID: "image1", Size: 4096, SharedSize: 0, Containers: 0, Descriptor: &ocispec.Descriptor{MediaType: "application/vnd.docker.distribution.manifest.v2+json", Size: 427}},
245+
},
246+
},
247+
expectedActiveCount: 0,
248+
expectedTotalCount: 1,
249+
expectedReclaimable: 4096, // (4096 - 0)
250+
expectedTotalSize: 4096,
251+
},
252+
}
253+
254+
for _, tt := range tests {
255+
t.Run(tt.name, func(t *testing.T) {
256+
client, err := New(
257+
WithAPIVersion(legacyVersion),
258+
WithMockClient(func(req *http.Request) (*http.Response, error) {
259+
if err := assertRequest(req, http.MethodGet, "/v"+legacyVersion+expectedURL); err != nil {
260+
return nil, err
261+
}
262+
263+
return mockJSONResponse(http.StatusOK, nil, tt.mockResponse)(req)
264+
}))
265+
assert.NilError(t, err)
266+
267+
du, err := client.DiskUsage(t.Context(), DiskUsageOptions{Images: true})
268+
assert.NilError(t, err)
269+
assert.Equal(t, du.Images.ActiveCount, tt.expectedActiveCount, tt.name)
270+
assert.Equal(t, du.Images.TotalCount, tt.expectedTotalCount, tt.name)
271+
assert.Equal(t, du.Images.Reclaimable, tt.expectedReclaimable, tt.name)
272+
assert.Equal(t, du.Images.TotalSize, tt.expectedTotalSize, tt.name)
273+
assert.Equal(t, len(du.Images.Items), len(tt.mockResponse.Images), tt.name)
274+
})
275+
}
276+
}

‎daemon/disk_usage.go‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -74,18 +74,16 @@ func (daemon *Daemon) imageDiskUsage(ctx context.Context, verbose bool) (*backen
7474
}
7575

7676
du := &backend.ImageDiskUsage{
77-
ActiveCount: int64(len(images)),
78-
Reclaimable: totalSize,
79-
TotalCount: int64(len(images)),
80-
TotalSize: totalSize,
77+
TotalCount: int64(len(images)),
78+
TotalSize: totalSize,
8179
}
80+
8281
for _, i := range images {
83-
if i.Containers == 0 {
84-
du.ActiveCount--
85-
if i.Size == -1 || i.SharedSize == -1 {
86-
continue
87-
}
88-
du.Reclaimable -= i.Size - i.SharedSize
82+
if i.Containers > 0 {
83+
du.ActiveCount++
84+
} else if i.Size != -1 && i.SharedSize != -1 {
85+
// Only count reclaimable size if we have size information
86+
du.Reclaimable += (i.Size - i.SharedSize)
8987
}
9088
}
9189

‎integration/system/disk_usage_test.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,9 @@ func TestDiskUsage(t *testing.T) {
7676
})
7777
assert.NilError(t, err)
7878

79+
assert.Equal(t, du.Images.ActiveCount, int64(0))
80+
assert.Equal(t, du.Images.TotalCount, int64(1))
81+
assert.Equal(t, du.Images.Reclaimable, du.Images.TotalSize)
7982
assert.Assert(t, du.Images.TotalSize > 0)
8083
assert.Equal(t, len(du.Images.Items), 1)
8184
assert.Equal(t, len(du.Images.Items[0].RepoTags), 1)
@@ -113,6 +116,7 @@ func TestDiskUsage(t *testing.T) {
113116

114117
assert.Equal(t, du.Images.ActiveCount, int64(1))
115118
assert.Equal(t, du.Images.TotalCount, int64(1))
119+
assert.Equal(t, du.Images.Reclaimable, int64(0))
116120
assert.Equal(t, len(du.Images.Items), 1)
117121
assert.Equal(t, du.Images.Items[0].Containers, prev.Images.Items[0].Containers+1)
118122

‎vendor/github.com/moby/moby/client/system_disk_usage.go‎

Lines changed: 3 additions & 10 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)