mirror of
https://github.com/Studio-Saelix/sencho.git
synced 2026-08-10 10:49:35 +00:00
fix(resources): use daemon-reported reclaimable image bytes (#1154)
* fix(resources): use daemon-reported reclaimable image bytes The Reclaim banner summed per-image `VirtualSize` (or `Size`) across every image with no running container. That counts shared base layers once per image, so an unused base layer of 1 GB shared across ten builds showed up as 10 GB of "prunable" space. The actual prune frees the layer once and reports a much smaller `SpaceReclaimed`, leaving the banner and the post-prune toast badly out of step. Prefer Docker's own `ImageUsage.Reclaimable` (API v1.44+), which is the exact value `docker system df` displays. Older daemons fall back to the Docker CLI's internal formula: `LayersSize - sum(Size - SharedSize)` for in-use images, clamped to 0, skipping any image Docker flags with the -1 unknown-size sentinel. Verified against a live daemon: the banner now matches `docker system df`'s IMAGES RECLAIMABLE byte-for-byte. * fix(resources): treat SharedSize=-1 as 0 in fallback, don't drop in-use bytes The fallback formula is `LayersSize - used`. The previous version skipped any active image whose VirtualSize or SharedSize was -1 (Docker's "unknown" sentinel). Skipping leaves the image's bytes out of `used`, which reads back as reclaimable -- the exact inflation the PR set out to fix, just on older daemons. Treat SharedSize=-1 (or absent) as 0 so the image's full size counts as in-use, and only skip when no usable size is available at all (VirtualSize and Size both unknown). Under-reporting reclaimable is the safe direction; over-reporting was the original bug. Add a test for the SharedSize=-1 case with Size known, and rename the existing test so it reflects what is actually being asserted now. Addresses Codex audit blocker on PR #1154.
This commit is contained in:
@@ -167,9 +167,10 @@ describe('DockerController - removeContainers', () => {
|
||||
describe('DockerController - getDiskUsage', () => {
|
||||
it('calculates reclaimable space correctly', async () => {
|
||||
mockDocker.df.mockResolvedValue({
|
||||
LayersSize: 800, // total image-layer bytes on disk
|
||||
Images: [
|
||||
{ Id: 'img1', Containers: 0, Size: 500 }, // reclaimable (unused)
|
||||
{ Id: 'img2', Containers: 1, Size: 300 }, // not reclaimable (in use)
|
||||
{ Id: 'img1', Containers: 0, Size: 500, SharedSize: 0 }, // reclaimable (unused)
|
||||
{ Id: 'img2', Containers: 1, Size: 300, SharedSize: 0 }, // not reclaimable (in use); used = 300
|
||||
],
|
||||
Containers: [
|
||||
{ State: 'running', SizeRw: 100 }, // not reclaimable (running)
|
||||
@@ -188,13 +189,108 @@ describe('DockerController - getDiskUsage', () => {
|
||||
const dc = DockerController.getInstance(1);
|
||||
const usage = await dc.getDiskUsage();
|
||||
|
||||
// 800 LayersSize - 300 used-by-in-use-image = 500 reclaimable
|
||||
expect(usage.reclaimableImages).toBe(500);
|
||||
expect(usage.reclaimableImageCount).toBe(1);
|
||||
expect(usage.reclaimableContainers).toBe(200);
|
||||
expect(usage.reclaimableVolumes).toBe(400);
|
||||
expect(usage.reclaimableBuildCache).toBe(600);
|
||||
expect(usage.reclaimableBuildCacheCount).toBe(1);
|
||||
});
|
||||
|
||||
it('uses daemon-reported ImageUsage.Reclaimable when present (API v1.44+)', async () => {
|
||||
// Modern Docker daemons compute Reclaimable server-side and ship the exact
|
||||
// value `docker system df` displays. Trust it over any client-side fallback.
|
||||
mockDocker.df.mockResolvedValue({
|
||||
LayersSize: 10_000_000_000,
|
||||
ImageUsage: { Reclaimable: 7_837_305_085, TotalSize: 10_000_000_000, ActiveCount: 11, TotalCount: 33 },
|
||||
Images: [
|
||||
{ Id: 'a', Containers: 0, Size: 5_000_000_000, SharedSize: 0 }, // a fallback formula would say 5G
|
||||
{ Id: 'b', Containers: 1, Size: 3_000_000_000, SharedSize: 0 },
|
||||
],
|
||||
Containers: [],
|
||||
Volumes: [],
|
||||
});
|
||||
|
||||
const dc = DockerController.getInstance(1);
|
||||
const usage = await dc.getDiskUsage();
|
||||
|
||||
expect(usage.reclaimableImages).toBe(7_837_305_085);
|
||||
expect(usage.reclaimableImageCount).toBe(1);
|
||||
});
|
||||
|
||||
it('does not double-count shared layers across unused images', async () => {
|
||||
// Three images sharing a 400MB base layer: two unused (800MB virtual each),
|
||||
// one in-use (600MB virtual). Total on-disk: 1GB. The in-use image holds
|
||||
// its unique 200MB; pruning the two unused frees the remaining 800MB.
|
||||
// The previous formula summed VirtualSize and would have returned 1.6GB
|
||||
// (impossibly larger than LayersSize).
|
||||
mockDocker.df.mockResolvedValue({
|
||||
LayersSize: 1_000_000_000,
|
||||
Images: [
|
||||
{ Id: 'a', Containers: 0, VirtualSize: 800_000_000, SharedSize: 400_000_000 },
|
||||
{ Id: 'b', Containers: 0, VirtualSize: 800_000_000, SharedSize: 400_000_000 },
|
||||
{ Id: 'c', Containers: 1, VirtualSize: 600_000_000, SharedSize: 400_000_000 },
|
||||
],
|
||||
Containers: [],
|
||||
Volumes: [],
|
||||
});
|
||||
|
||||
const dc = DockerController.getInstance(1);
|
||||
const usage = await dc.getDiskUsage();
|
||||
|
||||
expect(usage.reclaimableImages).toBe(800_000_000);
|
||||
expect(usage.reclaimableImageCount).toBe(2);
|
||||
});
|
||||
|
||||
it('skips active images only when no usable size is available', async () => {
|
||||
// Truly unaccountable image: both VirtualSize and Size are -1 / missing.
|
||||
// Skipping leaks at most one image's worth of bytes into the reclaim
|
||||
// total, but modern daemons never return this shape; the prior "always
|
||||
// skip on -1" path moved any image with SharedSize=-1 into the leak set.
|
||||
mockDocker.df.mockResolvedValue({
|
||||
LayersSize: 1000,
|
||||
Images: [
|
||||
{ Id: 'known', Containers: 1, VirtualSize: 400, SharedSize: 100 },
|
||||
{ Id: 'truly-lost', Containers: 1, VirtualSize: -1, Size: -1 },
|
||||
],
|
||||
Containers: [],
|
||||
Volumes: [],
|
||||
});
|
||||
|
||||
const dc = DockerController.getInstance(1);
|
||||
const usage = await dc.getDiskUsage();
|
||||
|
||||
// truly-lost is unaccountable and skipped; used = 400 - 100 = 300
|
||||
expect(usage.reclaimableImages).toBe(700);
|
||||
expect(usage.reclaimableImageCount).toBe(0);
|
||||
});
|
||||
|
||||
it('treats SharedSize=-1 conservatively (full Size counts as used)', async () => {
|
||||
// Older daemons may report SharedSize as -1 (unknown) while Size is
|
||||
// accurate. Treating SharedSize as 0 in that case under-reports
|
||||
// reclaimable, which is the safe direction; the prior skip-on-(-1)
|
||||
// path made the image's full Size look reclaimable.
|
||||
mockDocker.df.mockResolvedValue({
|
||||
LayersSize: 1000,
|
||||
Images: [
|
||||
{ Id: 'modern', Containers: 1, Size: 400, SharedSize: 100 },
|
||||
{ Id: 'no-shared-info', Containers: 1, Size: 300, SharedSize: -1 },
|
||||
],
|
||||
Containers: [],
|
||||
Volumes: [],
|
||||
});
|
||||
|
||||
const dc = DockerController.getInstance(1);
|
||||
const usage = await dc.getDiskUsage();
|
||||
|
||||
// modern: used += 400 - 100 = 300
|
||||
// no-shared-info: shared treated as 0, used += 300 - 0 = 300; total = 600
|
||||
// (the old buggy formula skipped no-shared-info, leaving used=300 and
|
||||
// reclaimable=700 — i.e. the in-use 300 bytes looked reclaimable)
|
||||
expect(usage.reclaimableImages).toBe(400);
|
||||
});
|
||||
|
||||
it('handles empty arrays gracefully', async () => {
|
||||
mockDocker.df.mockResolvedValue({
|
||||
Images: [],
|
||||
|
||||
@@ -145,16 +145,41 @@ class DockerController {
|
||||
return { bytes, count: reclaimable.length };
|
||||
};
|
||||
|
||||
const reclaimableImages = (items: any[]) => {
|
||||
// Prefer the daemon's own ImageUsage.Reclaimable (Docker API v1.44+); it is
|
||||
// the exact number `docker system df` displays and accounts for shared
|
||||
// layers correctly. On older daemons, fall back to LayersSize minus the
|
||||
// unique-to-in-use bytes, the same arithmetic the CLI uses internally.
|
||||
// Summing per-image Size/VirtualSize would over-report by the multiplicity
|
||||
// of every shared layer (the old bug that inflated the banner).
|
||||
const reclaimableImages = (items: any[], layersSize: number, serverReclaimable: number | undefined) => {
|
||||
if (!items || !Array.isArray(items)) return { bytes: 0, count: 0 };
|
||||
const reclaimable = items.filter(i => i.Containers === 0);
|
||||
const bytes = reclaimable.reduce((acc, item) => {
|
||||
let size = item.VirtualSize || item.Size || item.SharedSize || 0;
|
||||
if (item.UsageData && typeof item.UsageData.Size === 'number') {
|
||||
size = item.UsageData.Size;
|
||||
const reclaimable = items.filter(i => (i?.Containers ?? 0) === 0);
|
||||
if (serverReclaimable !== undefined && serverReclaimable >= 0) {
|
||||
return { bytes: serverReclaimable, count: reclaimable.length };
|
||||
}
|
||||
let used = 0;
|
||||
for (const item of items) {
|
||||
if (!item || (item.Containers ?? 0) <= 0) continue;
|
||||
// Choose the best non-negative size: prefer VirtualSize, fall back to
|
||||
// Size. If neither is known the image is truly unaccountable; skipping
|
||||
// it leaks at most one image's worth of bytes into the reclaim total.
|
||||
let virt = -1;
|
||||
if (typeof item.VirtualSize === 'number' && item.VirtualSize >= 0) {
|
||||
virt = item.VirtualSize;
|
||||
} else if (typeof item.Size === 'number' && item.Size >= 0) {
|
||||
virt = item.Size;
|
||||
}
|
||||
return acc + size;
|
||||
}, 0);
|
||||
if (virt < 0) continue;
|
||||
// SharedSize === -1 (or absent) means "unknown" on older daemons. Treat
|
||||
// it as 0 so the image's full size counts as in-use. Under-reporting
|
||||
// reclaimable is the safer direction; the previous skip-on-(-1) path
|
||||
// moved those bytes into the reclaimable total and re-inflated it.
|
||||
const shared = typeof item.SharedSize === 'number' && item.SharedSize >= 0
|
||||
? item.SharedSize
|
||||
: 0;
|
||||
used += Math.max(0, virt - shared);
|
||||
}
|
||||
const bytes = Math.max(0, (layersSize || 0) - used);
|
||||
return { bytes, count: reclaimable.length };
|
||||
};
|
||||
|
||||
@@ -175,7 +200,10 @@ class DockerController {
|
||||
return { bytes, count: reclaimable.length };
|
||||
};
|
||||
|
||||
const images = df.Images ? reclaimableImages(df.Images) : { bytes: 0, count: 0 };
|
||||
const imageUsage = (df as { ImageUsage?: { Reclaimable?: number } }).ImageUsage;
|
||||
const images = df.Images
|
||||
? reclaimableImages(df.Images, df.LayersSize ?? 0, imageUsage?.Reclaimable)
|
||||
: { bytes: 0, count: 0 };
|
||||
const containers = df.Containers ? reclaimableContainers(df.Containers) : { bytes: 0, count: 0 };
|
||||
const volumes = df.Volumes ? reclaimableVolumes(df.Volumes) : { bytes: 0, count: 0 };
|
||||
const buildCache = df.BuildCache ? reclaimableBuildCache(df.BuildCache) : { bytes: 0, count: 0 };
|
||||
|
||||
Reference in New Issue
Block a user