mirror of
https://github.com/Studio-Saelix/sencho.git
synced 2026-08-28 03:06:57 +00:00
fix(resources): subtract shared layers when accounting managed prune bytes (#1155)
* fix(resources): subtract shared layers when accounting managed prune bytes Both `pruneManagedOnly` and `estimateManagedReclaim` walked the Sencho-managed prunable image set and summed `img.Size` per image. That counts shared base layers once per image, so a 1 GB base layer shared across N managed images was reported as N GB freed — the same shape of inflation we just fixed for the system-scope banner. Introduce `getImageSharedSizeMap()` which reads `df.Images[].SharedSize` once and lets both code paths subtract `SharedSize` per image when totalling: `+= max(0, Size - shared)`. If `df` fails, the helper returns an empty map and the accounting degrades to the prior sum-of-Size behavior rather than failing the prune. Verified against a live daemon: `/api/system/prune/estimate` with `scope: managed, target: images` now returns the layer-aware number; the older per-image-Size sum was roughly 1.8× larger on the same set. * fix(resources): use df-delta for destructive managed prune; label estimate as lower bound The first attempt at this PR subtracted SharedSize per prunable image on both paths. That formula undercounts when prunable images share a layer exclusively with each other: Docker frees the layer once, but the per-image subtraction removes it from every referrer. The reported total is then strictly less than the truth. Split the two paths: - pruneManagedOnly (destructive) now snapshots `docker df` before and after the parallel removes and reports `max(0, before.LayersSize - after.LayersSize)`. That is the honest measurement of bytes freed. Concurrent pulls during the prune can grow the after value; the clamp treats that as 0 reclaimed for the affected delta rather than attributing the new bytes to us. - estimateManagedReclaim keeps the per-image Σ(Size - SharedSize) formula but the JSDoc now calls it a "conservative lower bound" and documents the under-report mechanism. There is no cheap way to exactly price an arbitrary prune subset without per-layer enumeration. Fallback chain when df fails on the destructive path: - before-snapshot succeeded, after failed → per-image lower bound from before-snapshot (safe; SharedSize was known at start). - before-snapshot failed → report 0 with a warn log (after-only would build a SharedSize map missing the just-pruned images, which would over-report by treating them as having no sharing). Replaces the prior `getImageSharedSizeMap()` helper with two pieces: `safeDfSnapshot()` (I/O) and a private static `mapSharedSizesFromDf()` (pure parse), reused by both code paths. New invariant test asserts `prune.reclaimedBytes >= estimate.reclaimableBytes` on the same inputs so future changes to either formula cannot flip the direction. Addresses Codex audit blocker on PR #1155.
This commit is contained in:
@@ -431,6 +431,210 @@ describe('DockerController - getClassifiedResources', () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// ── pruneManagedOnly / estimateManagedReclaim (images) ─────────────────
|
||||||
|
|
||||||
|
describe('DockerController - managed image prune accounting', () => {
|
||||||
|
// Two unused images each report a 1000-byte rolled-up Size, but 400 of those
|
||||||
|
// bytes live in a layer shared with a third (in-use) image, so removing
|
||||||
|
// the unused pair only frees the 600 unique bytes from each. The buggy
|
||||||
|
// sum-of-Size accounting would have reported 2000; the fix should report
|
||||||
|
// 1200 (600 × 2).
|
||||||
|
const sharedLayerDfMock = {
|
||||||
|
Images: [
|
||||||
|
{ Id: 'img-a', SharedSize: 400 },
|
||||||
|
{ Id: 'img-b', SharedSize: 400 },
|
||||||
|
{ Id: 'img-c', SharedSize: 400 },
|
||||||
|
],
|
||||||
|
};
|
||||||
|
const unusedManagedListMock = [
|
||||||
|
{ Id: 'img-a', Containers: 0, Size: 1000 },
|
||||||
|
{ Id: 'img-b', Containers: 0, Size: 1000 },
|
||||||
|
{ Id: 'img-c', Containers: 1, Size: 800 }, // in-use; filtered by Containers===0
|
||||||
|
];
|
||||||
|
|
||||||
|
it('estimateManagedReclaim subtracts SharedSize per prunable image', async () => {
|
||||||
|
mockDocker.df.mockResolvedValue(sharedLayerDfMock);
|
||||||
|
mockDocker.listImages.mockResolvedValue(unusedManagedListMock);
|
||||||
|
mockDocker.listContainers.mockResolvedValue([]);
|
||||||
|
|
||||||
|
const dc = DockerController.getInstance(1);
|
||||||
|
const result = await dc.estimateManagedReclaim('images', ['any-stack']);
|
||||||
|
|
||||||
|
expect(result.reclaimableBytes).toBe(1200);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('pruneManagedOnly reports the LayersSize delta as the reclaimed total', async () => {
|
||||||
|
// df-before reports 5000 bytes on disk; df-after reports 3400. The
|
||||||
|
// honest reclaim is 1600, independent of per-image Size/SharedSize.
|
||||||
|
// Two prunable images that share a layer exclusively would be
|
||||||
|
// undercounted by the per-image formula (1200) but Docker actually
|
||||||
|
// frees the shared layer too.
|
||||||
|
mockDocker.df
|
||||||
|
.mockResolvedValueOnce({
|
||||||
|
LayersSize: 5000,
|
||||||
|
Images: [
|
||||||
|
{ Id: 'img-a', SharedSize: 400 },
|
||||||
|
{ Id: 'img-b', SharedSize: 400 },
|
||||||
|
],
|
||||||
|
})
|
||||||
|
.mockResolvedValueOnce({ LayersSize: 3400, Images: [] });
|
||||||
|
mockDocker.listImages.mockResolvedValue([
|
||||||
|
{ Id: 'img-a', Containers: 0, Size: 1000 },
|
||||||
|
{ Id: 'img-b', Containers: 0, Size: 1000 },
|
||||||
|
]);
|
||||||
|
mockDocker.listContainers.mockResolvedValue([]);
|
||||||
|
const removeFn = vi.fn().mockResolvedValue(undefined);
|
||||||
|
mockDocker.getImage.mockReturnValue({ remove: removeFn });
|
||||||
|
|
||||||
|
const dc = DockerController.getInstance(1);
|
||||||
|
const result = await dc.pruneManagedOnly('images', ['any-stack']);
|
||||||
|
|
||||||
|
expect(result).toEqual({ success: true, reclaimedBytes: 1600 });
|
||||||
|
expect(removeFn).toHaveBeenCalledTimes(2);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('pruneManagedOnly falls back to per-image lower bound using before-snapshot when after-df fails', async () => {
|
||||||
|
// Before-snapshot succeeded (SharedSize known); after-snapshot failed.
|
||||||
|
// Fall back to Σ(Size - SharedSize) over the prunable set: a
|
||||||
|
// conservative lower bound, but defensible because each pruned image's
|
||||||
|
// SharedSize was known at the start.
|
||||||
|
mockDocker.df
|
||||||
|
.mockResolvedValueOnce({
|
||||||
|
Images: [
|
||||||
|
{ Id: 'img-a', SharedSize: 400 },
|
||||||
|
{ Id: 'img-b', SharedSize: 400 },
|
||||||
|
],
|
||||||
|
})
|
||||||
|
.mockRejectedValueOnce(new Error('df unavailable after prune'));
|
||||||
|
mockDocker.listImages.mockResolvedValue([
|
||||||
|
{ Id: 'img-a', Containers: 0, Size: 1000 },
|
||||||
|
{ Id: 'img-b', Containers: 0, Size: 1000 },
|
||||||
|
]);
|
||||||
|
mockDocker.listContainers.mockResolvedValue([]);
|
||||||
|
mockDocker.getImage.mockReturnValue({ remove: vi.fn().mockResolvedValue(undefined) });
|
||||||
|
|
||||||
|
const dc = DockerController.getInstance(1);
|
||||||
|
const result = await dc.pruneManagedOnly('images', ['any-stack']);
|
||||||
|
|
||||||
|
// (1000 - 400) + (1000 - 400) = 1200
|
||||||
|
expect(result).toEqual({ success: true, reclaimedBytes: 1200 });
|
||||||
|
});
|
||||||
|
|
||||||
|
it('pruneManagedOnly reports 0 reclaimed when both df snapshots fail', async () => {
|
||||||
|
// Without the before-snapshot we cannot build a meaningful per-image
|
||||||
|
// bound (after-only has stale image IDs missing from its view), so the
|
||||||
|
// destructive path completes the removes and reports 0 rather than a
|
||||||
|
// misleading approximation.
|
||||||
|
mockDocker.df
|
||||||
|
.mockRejectedValueOnce(new Error('df unavailable before'))
|
||||||
|
.mockRejectedValueOnce(new Error('df unavailable after'));
|
||||||
|
mockDocker.listImages.mockResolvedValue([
|
||||||
|
{ Id: 'img-a', Containers: 0, Size: 1000 },
|
||||||
|
]);
|
||||||
|
mockDocker.listContainers.mockResolvedValue([]);
|
||||||
|
const removeFn = vi.fn().mockResolvedValue(undefined);
|
||||||
|
mockDocker.getImage.mockReturnValue({ remove: removeFn });
|
||||||
|
|
||||||
|
const dc = DockerController.getInstance(1);
|
||||||
|
const result = await dc.pruneManagedOnly('images', ['any-stack']);
|
||||||
|
|
||||||
|
expect(result).toEqual({ success: true, reclaimedBytes: 0 });
|
||||||
|
expect(removeFn).toHaveBeenCalledTimes(1);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('estimateManagedReclaim under-reports when layers are shared exclusively between prunable images', async () => {
|
||||||
|
// Two prunable images that share a 400-byte layer with no retained
|
||||||
|
// image. Docker would free that layer once on prune (1600 bytes total:
|
||||||
|
// each image's unique 600 plus the shared 400). The estimate formula
|
||||||
|
// subtracts 400 from each image and reports 1200, a conservative
|
||||||
|
// lower bound documented in the JSDoc.
|
||||||
|
mockDocker.df.mockResolvedValue({
|
||||||
|
Images: [
|
||||||
|
{ Id: 'img-a', SharedSize: 400 },
|
||||||
|
{ Id: 'img-b', SharedSize: 400 },
|
||||||
|
],
|
||||||
|
});
|
||||||
|
mockDocker.listImages.mockResolvedValue([
|
||||||
|
{ Id: 'img-a', Containers: 0, Size: 1000 },
|
||||||
|
{ Id: 'img-b', Containers: 0, Size: 1000 },
|
||||||
|
]);
|
||||||
|
mockDocker.listContainers.mockResolvedValue([]);
|
||||||
|
|
||||||
|
const dc = DockerController.getInstance(1);
|
||||||
|
const result = await dc.estimateManagedReclaim('images', ['any-stack']);
|
||||||
|
|
||||||
|
expect(result.reclaimableBytes).toBe(1200);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('invariant: pruneManagedOnly reports >= estimateManagedReclaim on the same inputs', async () => {
|
||||||
|
// The estimate is a conservative lower bound; the destructive path
|
||||||
|
// reports the truth (df-delta). On any input, destructive >= estimate.
|
||||||
|
// This locks the contract so future changes to either formula cannot
|
||||||
|
// accidentally flip the direction.
|
||||||
|
const dfBefore = {
|
||||||
|
LayersSize: 5000,
|
||||||
|
Images: [
|
||||||
|
{ Id: 'img-a', SharedSize: 400 },
|
||||||
|
{ Id: 'img-b', SharedSize: 400 },
|
||||||
|
],
|
||||||
|
};
|
||||||
|
const dfAfter = { LayersSize: 3400, Images: [] };
|
||||||
|
const listImages = [
|
||||||
|
{ Id: 'img-a', Containers: 0, Size: 1000 },
|
||||||
|
{ Id: 'img-b', Containers: 0, Size: 1000 },
|
||||||
|
];
|
||||||
|
|
||||||
|
// Estimate path: df called once.
|
||||||
|
mockDocker.df.mockResolvedValueOnce(dfBefore);
|
||||||
|
mockDocker.listImages.mockResolvedValue(listImages);
|
||||||
|
mockDocker.listContainers.mockResolvedValue([]);
|
||||||
|
const dc = DockerController.getInstance(1);
|
||||||
|
const estimate = await dc.estimateManagedReclaim('images', ['any-stack']);
|
||||||
|
|
||||||
|
// Reset the df mock for the destructive path: before then after.
|
||||||
|
mockDocker.df.mockReset();
|
||||||
|
mockDocker.df.mockResolvedValueOnce(dfBefore).mockResolvedValueOnce(dfAfter);
|
||||||
|
mockDocker.getImage.mockReturnValue({ remove: vi.fn().mockResolvedValue(undefined) });
|
||||||
|
const prune = await dc.pruneManagedOnly('images', ['any-stack']);
|
||||||
|
|
||||||
|
expect(prune.reclaimedBytes).toBeGreaterThanOrEqual(estimate.reclaimableBytes);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('treats images missing from the df map as having no shared layers', async () => {
|
||||||
|
// df reports only img-a; img-b is omitted (stale / race / older daemon).
|
||||||
|
// The accounting should still process img-b, defaulting its SharedSize to 0.
|
||||||
|
mockDocker.df.mockResolvedValue({ Images: [{ Id: 'img-a', SharedSize: 400 }] });
|
||||||
|
mockDocker.listImages.mockResolvedValue([
|
||||||
|
{ Id: 'img-a', Containers: 0, Size: 1000 },
|
||||||
|
{ Id: 'img-b', Containers: 0, Size: 1000 },
|
||||||
|
]);
|
||||||
|
mockDocker.listContainers.mockResolvedValue([]);
|
||||||
|
|
||||||
|
const dc = DockerController.getInstance(1);
|
||||||
|
const result = await dc.estimateManagedReclaim('images', ['any-stack']);
|
||||||
|
|
||||||
|
// img-a: 1000-400=600; img-b: 1000-0=1000; total 1600
|
||||||
|
expect(result.reclaimableBytes).toBe(1600);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('degrades to no-sharing assumption when df fails', async () => {
|
||||||
|
// If df throws, the helper returns an empty map and the accounting falls
|
||||||
|
// back to full per-image Size. This preserves prior behavior (over-report)
|
||||||
|
// rather than failing the prune.
|
||||||
|
mockDocker.df.mockRejectedValue(new Error('daemon df failed'));
|
||||||
|
mockDocker.listImages.mockResolvedValue([
|
||||||
|
{ Id: 'img-a', Containers: 0, Size: 1000 },
|
||||||
|
{ Id: 'img-b', Containers: 0, Size: 1000 },
|
||||||
|
]);
|
||||||
|
mockDocker.listContainers.mockResolvedValue([]);
|
||||||
|
|
||||||
|
const dc = DockerController.getInstance(1);
|
||||||
|
const result = await dc.estimateManagedReclaim('images', ['any-stack']);
|
||||||
|
|
||||||
|
expect(result.reclaimableBytes).toBe(2000);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
// ── getOrphanContainers ────────────────────────────────────────────────
|
// ── getOrphanContainers ────────────────────────────────────────────────
|
||||||
|
|
||||||
describe('DockerController - getOrphanContainers', () => {
|
describe('DockerController - getOrphanContainers', () => {
|
||||||
|
|||||||
@@ -363,6 +363,42 @@ class DockerController {
|
|||||||
return { images, volumes, networks };
|
return { images, volumes, networks };
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Returns the `docker df` snapshot, or null if the call fails. The
|
||||||
|
* destructive prune path uses this for a `LayersSize` before/after delta;
|
||||||
|
* the estimate path uses it for a SharedSize lookup. Null on failure lets
|
||||||
|
* each caller decide how to degrade rather than throwing mid-prune.
|
||||||
|
*/
|
||||||
|
private async safeDfSnapshot(): Promise<{
|
||||||
|
LayersSize?: number;
|
||||||
|
Images?: Array<{ Id?: string; SharedSize?: number }>;
|
||||||
|
} | null> {
|
||||||
|
try {
|
||||||
|
return await this.docker.df();
|
||||||
|
} catch {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Extracts `Id -> SharedSize` from a df snapshot. Treats missing or
|
||||||
|
* negative (Docker's "unknown" sentinel) SharedSize as 0 so the caller
|
||||||
|
* counts the image's full Size as unique, an under-report rather than an
|
||||||
|
* over-report.
|
||||||
|
*/
|
||||||
|
private static mapSharedSizesFromDf(
|
||||||
|
df: { Images?: Array<{ Id?: string; SharedSize?: number }> } | null,
|
||||||
|
): Map<string, number> {
|
||||||
|
const m = new Map<string, number>();
|
||||||
|
if (!df?.Images) return m;
|
||||||
|
for (const img of df.Images) {
|
||||||
|
if (!img?.Id) continue;
|
||||||
|
const s = typeof img.SharedSize === 'number' && img.SharedSize >= 0 ? img.SharedSize : 0;
|
||||||
|
m.set(img.Id, s);
|
||||||
|
}
|
||||||
|
return m;
|
||||||
|
}
|
||||||
|
|
||||||
public async pruneManagedOnly(
|
public async pruneManagedOnly(
|
||||||
target: 'images' | 'volumes' | 'networks',
|
target: 'images' | 'volumes' | 'networks',
|
||||||
knownStackNames: string[]
|
knownStackNames: string[]
|
||||||
@@ -422,14 +458,37 @@ class DockerController {
|
|||||||
&& !unmanagedImageIds.has(img.Id)
|
&& !unmanagedImageIds.has(img.Id)
|
||||||
&& !selfIdentity.isOwnImage(img.Id)
|
&& !selfIdentity.isOwnImage(img.Id)
|
||||||
);
|
);
|
||||||
|
// df-before / df-after delta is the only honest measurement of bytes
|
||||||
|
// actually freed. Per-image (Size - SharedSize) undercounts layers
|
||||||
|
// shared exclusively between prunable images (Docker frees the layer
|
||||||
|
// once, but the per-image formula subtracts it from every referrer).
|
||||||
|
const beforeDf = await this.safeDfSnapshot();
|
||||||
await Promise.all(prunable.map(async (img) => {
|
await Promise.all(prunable.map(async (img) => {
|
||||||
try {
|
try {
|
||||||
await this.docker.getImage(img.Id).remove({ force: true });
|
await this.docker.getImage(img.Id).remove({ force: true });
|
||||||
reclaimedBytes += img.Size ?? 0;
|
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
console.error(`[pruneManagedOnly] Failed to remove image ${img.Id}:`, e);
|
console.error(`[pruneManagedOnly] Failed to remove image ${img.Id}:`, e);
|
||||||
}
|
}
|
||||||
}));
|
}));
|
||||||
|
const afterDf = await this.safeDfSnapshot();
|
||||||
|
if (typeof beforeDf?.LayersSize === 'number' && typeof afterDf?.LayersSize === 'number') {
|
||||||
|
// Clamp to 0: a concurrent pull during the prune can grow on-disk
|
||||||
|
// bytes, and attributing that growth to "reclaimed" would mislead.
|
||||||
|
reclaimedBytes = Math.max(0, beforeDf.LayersSize - afterDf.LayersSize);
|
||||||
|
} else if (beforeDf) {
|
||||||
|
// After-snapshot failed but we have the before-snapshot, so fall
|
||||||
|
// back to a per-image lower bound. (We deliberately do not fall back
|
||||||
|
// on after-only: after the prune those image IDs are gone from the
|
||||||
|
// daemon's view, so a SharedSize map built from afterDf would treat
|
||||||
|
// every pruned image as having no sharing and over-report.)
|
||||||
|
console.warn('[pruneManagedOnly] docker df after-snapshot unavailable; reporting per-image lower bound');
|
||||||
|
const shared = DockerController.mapSharedSizesFromDf(beforeDf);
|
||||||
|
for (const img of prunable) {
|
||||||
|
reclaimedBytes += Math.max(0, (img.Size ?? 0) - (shared.get(img.Id) ?? 0));
|
||||||
|
}
|
||||||
|
} else {
|
||||||
|
console.warn('[pruneManagedOnly] docker df unavailable on both ends; reporting 0 reclaimed');
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
return { success: true, reclaimedBytes };
|
return { success: true, reclaimedBytes };
|
||||||
@@ -440,6 +499,13 @@ class DockerController {
|
|||||||
* the `/api/system/prune/estimate` route. Walks the same filter rules but
|
* the `/api/system/prune/estimate` route. Walks the same filter rules but
|
||||||
* does not call `.remove()`. Kept structurally parallel to the destructive
|
* does not call `.remove()`. Kept structurally parallel to the destructive
|
||||||
* method so the two stay in lockstep when the enumeration logic changes.
|
* method so the two stay in lockstep when the enumeration logic changes.
|
||||||
|
*
|
||||||
|
* For images, the returned figure is a **conservative lower bound** on
|
||||||
|
* bytes that will actually be freed. The formula subtracts each prunable
|
||||||
|
* image's `SharedSize`, which double-counts layers shared between two
|
||||||
|
* prunable images (the layer would only be freed once on prune). The
|
||||||
|
* exact freed bytes can only be measured by the df-delta that
|
||||||
|
* `pruneManagedOnly` reports after the destructive action.
|
||||||
*/
|
*/
|
||||||
public async estimateManagedReclaim(
|
public async estimateManagedReclaim(
|
||||||
target: 'images' | 'volumes' | 'networks',
|
target: 'images' | 'volumes' | 'networks',
|
||||||
@@ -479,7 +545,10 @@ class DockerController {
|
|||||||
&& !unmanagedImageIds.has(img.Id)
|
&& !unmanagedImageIds.has(img.Id)
|
||||||
&& !selfIdentity.isOwnImage(img.Id),
|
&& !selfIdentity.isOwnImage(img.Id),
|
||||||
);
|
);
|
||||||
for (const img of prunable) reclaimableBytes += img.Size ?? 0;
|
const sharedSizes = DockerController.mapSharedSizesFromDf(await this.safeDfSnapshot());
|
||||||
|
for (const img of prunable) {
|
||||||
|
reclaimableBytes += Math.max(0, (img.Size ?? 0) - (sharedSizes.get(img.Id) ?? 0));
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
return { reclaimableBytes };
|
return { reclaimableBytes };
|
||||||
|
|||||||
Reference in New Issue
Block a user