Browse code
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 <austin.vazquez@docker.com>
Austin Vazquez authored on 2025/12/20 01:55:02Showing 5 changed files
- client/system_disk_usage.go
- client/system_disk_usage_test.go
- daemon/disk_usage.go
- integration/system/disk_usage_test.go
- vendor/github.com/moby/moby/client/system_disk_usage.go
| ... | ... |
@@ -244,22 +244,15 @@ func imageDiskUsageFromLegacyAPI(du *legacyDiskUsage) ImagesDiskUsage {
|
| 244 | 244 |
Items: du.Images, |
| 245 | 245 |
} |
| 246 | 246 |
|
| 247 |
- var used int64 |
|
| 248 | 247 |
for _, i := range idu.Items {
|
| 249 | 248 |
if i.Containers > 0 {
|
| 250 | 249 |
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) |
|
| 256 | 253 |
} |
| 257 | 254 |
} |
| 258 | 255 |
|
| 259 |
- if idu.TotalCount > 0 {
|
|
| 260 |
- idu.Reclaimable = idu.TotalSize - used |
|
| 261 |
- } |
|
| 262 |
- |
|
| 263 | 256 |
return idu |
| 264 | 257 |
} |
| 265 | 258 |
|
| ... | ... |
@@ -8,6 +8,7 @@ import ( |
| 8 | 8 |
cerrdefs "github.com/containerd/errdefs" |
| 9 | 9 |
"github.com/moby/moby/api/types/image" |
| 10 | 10 |
"github.com/moby/moby/api/types/system" |
| 11 |
+ ocispec "github.com/opencontainers/image-spec/specs-go/v1" |
|
| 11 | 12 |
"gotest.tools/v3/assert" |
| 12 | 13 |
is "gotest.tools/v3/assert/cmp" |
| 13 | 14 |
) |
| ... | ... |
@@ -154,3 +155,122 @@ func TestLegacyDiskUsage(t *testing.T) {
|
| 154 | 154 |
assert.Equal(t, du.Images.TotalSize, int64(4096)) |
| 155 | 155 |
assert.Equal(t, len(du.Images.Items), 0) |
| 156 | 156 |
} |
| 157 |
+ |
|
| 158 |
+func TestImageDiskUsageFromLegacyAPI(t *testing.T) {
|
|
| 159 |
+ const legacyVersion = "1.51" |
|
| 160 |
+ const expectedURL = "/system/df" |
|
| 161 |
+ |
|
| 162 |
+ tests := []struct {
|
|
| 163 |
+ name string |
|
| 164 |
+ mockResponse *legacyDiskUsage |
|
| 165 |
+ expectedActiveCount int64 |
|
| 166 |
+ expectedTotalCount int64 |
|
| 167 |
+ expectedReclaimable int64 |
|
| 168 |
+ expectedTotalSize int64 |
|
| 169 |
+ }{
|
|
| 170 |
+ {
|
|
| 171 |
+ name: "no images", |
|
| 172 |
+ mockResponse: &legacyDiskUsage{
|
|
| 173 |
+ LayersSize: 0, |
|
| 174 |
+ Images: []image.Summary{},
|
|
| 175 |
+ }, |
|
| 176 |
+ expectedActiveCount: 0, |
|
| 177 |
+ expectedTotalCount: 0, |
|
| 178 |
+ expectedReclaimable: 0, |
|
| 179 |
+ expectedTotalSize: 0, |
|
| 180 |
+ }, |
|
| 181 |
+ {
|
|
| 182 |
+ name: "images with no containers", |
|
| 183 |
+ mockResponse: &legacyDiskUsage{
|
|
| 184 |
+ LayersSize: 8192, |
|
| 185 |
+ Images: []image.Summary{
|
|
| 186 |
+ {ID: "image1", Size: 4096, SharedSize: 0, Containers: 0},
|
|
| 187 |
+ {ID: "image2", Size: 4096, SharedSize: 0, Containers: 0},
|
|
| 188 |
+ }, |
|
| 189 |
+ }, |
|
| 190 |
+ expectedActiveCount: 0, |
|
| 191 |
+ expectedTotalCount: 2, |
|
| 192 |
+ expectedReclaimable: 8192, |
|
| 193 |
+ expectedTotalSize: 8192, |
|
| 194 |
+ }, |
|
| 195 |
+ {
|
|
| 196 |
+ name: "images with containers", |
|
| 197 |
+ mockResponse: &legacyDiskUsage{
|
|
| 198 |
+ LayersSize: 12288, |
|
| 199 |
+ Images: []image.Summary{
|
|
| 200 |
+ {ID: "image1", Size: 4096, SharedSize: 0, Containers: 2},
|
|
| 201 |
+ {ID: "image2", Size: 2048, SharedSize: 0, Containers: 0},
|
|
| 202 |
+ {ID: "image3", Size: 6144, SharedSize: 0, Containers: 1},
|
|
| 203 |
+ }, |
|
| 204 |
+ }, |
|
| 205 |
+ expectedActiveCount: 2, |
|
| 206 |
+ expectedTotalCount: 3, |
|
| 207 |
+ expectedReclaimable: 2048, |
|
| 208 |
+ expectedTotalSize: 12288, |
|
| 209 |
+ }, |
|
| 210 |
+ {
|
|
| 211 |
+ name: "images with shared size", |
|
| 212 |
+ mockResponse: &legacyDiskUsage{
|
|
| 213 |
+ LayersSize: 15360, |
|
| 214 |
+ Images: []image.Summary{
|
|
| 215 |
+ {ID: "image1", Size: 4096, SharedSize: 1024, Containers: 1},
|
|
| 216 |
+ {ID: "image2", Size: 8192, SharedSize: 2048, Containers: 0},
|
|
| 217 |
+ {ID: "image3", Size: 3072, SharedSize: 1024, Containers: 0},
|
|
| 218 |
+ }, |
|
| 219 |
+ }, |
|
| 220 |
+ expectedActiveCount: 1, |
|
| 221 |
+ expectedTotalCount: 3, |
|
| 222 |
+ expectedReclaimable: 8192, // (8192-2048) + (3072-1024) |
|
| 223 |
+ expectedTotalSize: 15360, |
|
| 224 |
+ }, |
|
| 225 |
+ {
|
|
| 226 |
+ name: "multiplatform image with an image index", |
|
| 227 |
+ mockResponse: &legacyDiskUsage{
|
|
| 228 |
+ LayersSize: 4096, |
|
| 229 |
+ Images: []image.Summary{
|
|
| 230 |
+ {ID: "image1", Size: 4096, SharedSize: 0, Containers: 0, Descriptor: &ocispec.Descriptor{MediaType: ocispec.MediaTypeImageIndex, Size: 512}},
|
|
| 231 |
+ }, |
|
| 232 |
+ }, |
|
| 233 |
+ expectedActiveCount: 0, |
|
| 234 |
+ expectedTotalCount: 1, |
|
| 235 |
+ expectedReclaimable: 4096, // (4096 - 0) |
|
| 236 |
+ expectedTotalSize: 4096, |
|
| 237 |
+ }, |
|
| 238 |
+ {
|
|
| 239 |
+ name: "multiplatform image with a Docker distribution manifest", |
|
| 240 |
+ mockResponse: &legacyDiskUsage{
|
|
| 241 |
+ LayersSize: 4096, |
|
| 242 |
+ Images: []image.Summary{
|
|
| 243 |
+ {ID: "image1", Size: 4096, SharedSize: 0, Containers: 0, Descriptor: &ocispec.Descriptor{MediaType: "application/vnd.docker.distribution.manifest.v2+json", Size: 427}},
|
|
| 244 |
+ }, |
|
| 245 |
+ }, |
|
| 246 |
+ expectedActiveCount: 0, |
|
| 247 |
+ expectedTotalCount: 1, |
|
| 248 |
+ expectedReclaimable: 4096, // (4096 - 0) |
|
| 249 |
+ expectedTotalSize: 4096, |
|
| 250 |
+ }, |
|
| 251 |
+ } |
|
| 252 |
+ |
|
| 253 |
+ for _, tt := range tests {
|
|
| 254 |
+ t.Run(tt.name, func(t *testing.T) {
|
|
| 255 |
+ client, err := New( |
|
| 256 |
+ WithAPIVersion(legacyVersion), |
|
| 257 |
+ WithMockClient(func(req *http.Request) (*http.Response, error) {
|
|
| 258 |
+ if err := assertRequest(req, http.MethodGet, "/v"+legacyVersion+expectedURL); err != nil {
|
|
| 259 |
+ return nil, err |
|
| 260 |
+ } |
|
| 261 |
+ |
|
| 262 |
+ return mockJSONResponse(http.StatusOK, nil, tt.mockResponse)(req) |
|
| 263 |
+ })) |
|
| 264 |
+ assert.NilError(t, err) |
|
| 265 |
+ |
|
| 266 |
+ du, err := client.DiskUsage(t.Context(), DiskUsageOptions{Images: true})
|
|
| 267 |
+ assert.NilError(t, err) |
|
| 268 |
+ assert.Equal(t, du.Images.ActiveCount, tt.expectedActiveCount, tt.name) |
|
| 269 |
+ assert.Equal(t, du.Images.TotalCount, tt.expectedTotalCount, tt.name) |
|
| 270 |
+ assert.Equal(t, du.Images.Reclaimable, tt.expectedReclaimable, tt.name) |
|
| 271 |
+ assert.Equal(t, du.Images.TotalSize, tt.expectedTotalSize, tt.name) |
|
| 272 |
+ assert.Equal(t, len(du.Images.Items), len(tt.mockResponse.Images), tt.name) |
|
| 273 |
+ }) |
|
| 274 |
+ } |
|
| 275 |
+} |
| ... | ... |
@@ -74,18 +74,16 @@ func (daemon *Daemon) imageDiskUsage(ctx context.Context, verbose bool) (*backen |
| 74 | 74 |
} |
| 75 | 75 |
|
| 76 | 76 |
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, |
|
| 81 | 79 |
} |
| 80 |
+ |
|
| 82 | 81 |
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) |
|
| 89 | 87 |
} |
| 90 | 88 |
} |
| 91 | 89 |
|
| ... | ... |
@@ -76,6 +76,9 @@ func TestDiskUsage(t *testing.T) {
|
| 76 | 76 |
}) |
| 77 | 77 |
assert.NilError(t, err) |
| 78 | 78 |
|
| 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) |
|
| 79 | 82 |
assert.Assert(t, du.Images.TotalSize > 0) |
| 80 | 83 |
assert.Equal(t, len(du.Images.Items), 1) |
| 81 | 84 |
assert.Equal(t, len(du.Images.Items[0].RepoTags), 1) |
| ... | ... |
@@ -113,6 +116,7 @@ func TestDiskUsage(t *testing.T) {
|
| 113 | 113 |
|
| 114 | 114 |
assert.Equal(t, du.Images.ActiveCount, int64(1)) |
| 115 | 115 |
assert.Equal(t, du.Images.TotalCount, int64(1)) |
| 116 |
+ assert.Equal(t, du.Images.Reclaimable, int64(0)) |
|
| 116 | 117 |
assert.Equal(t, len(du.Images.Items), 1) |
| 117 | 118 |
assert.Equal(t, du.Images.Items[0].Containers, prev.Images.Items[0].Containers+1) |
| 118 | 119 |
|
| ... | ... |
@@ -244,22 +244,15 @@ func imageDiskUsageFromLegacyAPI(du *legacyDiskUsage) ImagesDiskUsage {
|
| 244 | 244 |
Items: du.Images, |
| 245 | 245 |
} |
| 246 | 246 |
|
| 247 |
- var used int64 |
|
| 248 | 247 |
for _, i := range idu.Items {
|
| 249 | 248 |
if i.Containers > 0 {
|
| 250 | 249 |
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) |
|
| 256 | 253 |
} |
| 257 | 254 |
} |
| 258 | 255 |
|
| 259 |
- if idu.TotalCount > 0 {
|
|
| 260 |
- idu.Reclaimable = idu.TotalSize - used |
|
| 261 |
- } |
|
| 262 |
- |
|
| 263 | 256 |
return idu |
| 264 | 257 |
} |
| 265 | 258 |
|