diff --git a/backend/src/__tests__/docker-controller.test.ts b/backend/src/__tests__/docker-controller.test.ts index b01eff85..2cbe5830 100644 --- a/backend/src/__tests__/docker-controller.test.ts +++ b/backend/src/__tests__/docker-controller.test.ts @@ -382,6 +382,48 @@ describe('DockerController - getDiskUsage', () => { expect(usage.reclaimableContainers).toBe(1_500); expect(usage.reclaimableContainerCount).toBe(2); }); + + it('estimateSystemReclaim returns same bytes as getDiskUsage for every target type', async () => { + mockDocker.df.mockResolvedValue({ + LayersSize: 1_000_000_000, + Images: [ + { Id: 'a', Containers: 0, Size: 500_000_000, SharedSize: 0 }, + { Id: 'b', Containers: 1, Size: 300_000_000, SharedSize: 0 }, + ], + Containers: [ + { State: 'exited', SizeRw: 200 }, + ], + Volumes: [ + { UsageData: { RefCount: 0, Size: 400 } }, + ], + }); + + const dc = DockerController.getInstance(1); + const usage = await dc.getDiskUsage(); + + // Each estimateSystemReclaim(target) must equal the corresponding + // getDiskUsage() field. knownStackNames is unused in the fast path + // but retained for call-site symmetry. + const expectations: Array<[target: 'images' | 'containers' | 'volumes' | 'networks', expected: number]> = [ + ['images', usage.reclaimableImages], + ['containers', usage.reclaimableContainers], + ['volumes', usage.reclaimableVolumes], + ['networks', 0], + ]; + for (const [target, expected] of expectations) { + const estimate = await dc.estimateSystemReclaim(target, []); + expect(estimate.reclaimableBytes).toBe(expected); + } + + // The fast path uses only docker.df(), never the classified-resources + // API calls that estimateSystemReclaim previously incurred via + // getDiskUsageClassified (listImages, listVolumes, etc.). + expect(mockDocker.df).toHaveBeenCalledTimes(5); // 1 getDiskUsage + 4 estimateSystemReclaim + expect(mockDocker.listImages).not.toHaveBeenCalled(); + expect(mockDocker.listVolumes).not.toHaveBeenCalled(); + expect(mockDocker.listNetworks).not.toHaveBeenCalled(); + expect(mockDocker.listContainers).not.toHaveBeenCalled(); + }); }); // ── pruneSystem ──────────────────────────────────────────────────────── diff --git a/backend/src/__tests__/fleet-prune-df-timeout.test.ts b/backend/src/__tests__/fleet-prune-df-timeout.test.ts index 76504dc5..7bd40a90 100644 --- a/backend/src/__tests__/fleet-prune-df-timeout.test.ts +++ b/backend/src/__tests__/fleet-prune-df-timeout.test.ts @@ -1,6 +1,6 @@ /** * F-6 regression: Fleet itemized plan enumeration and byte estimation both - * bound the slow `docker system df` call (8s) and surface a recognizable + * bound the slow `docker system df` call (12s) and surface a recognizable * timeout message to the operator. * * Covers: @@ -9,7 +9,7 @@ * * Uses real timers because supertest dispatches lazily and the in-route * `withTimeout` setTimeout cannot be advanced via vi.useFakeTimers from - * outside the request lifecycle. + * outside the request lifecycle. Three timeout tests add ~25s to the file. */ import { describe, it, expect, beforeAll, afterAll, afterEach, vi } from 'vitest'; import request from 'supertest'; @@ -29,7 +29,7 @@ beforeAll(async () => { ({ default: DockerController } = await import('../services/DockerController')); ({ FileSystemService } = await import('../services/FileSystemService')); ({ activeBulkActions } = await import('../routes/labels')); - // 10-minute expiry survives the file even with two ~8s timeout tests. + // 10-minute expiry survives the file even with three ~12s timeout tests. const token = jwt.sign({ username: TEST_USERNAME }, TEST_JWT_SECRET, { expiresIn: '10m' }); authHeader = `Bearer ${token}`; }); @@ -56,7 +56,7 @@ function stubLocalEstimate( vi.spyOn(FileSystemService.prototype, 'getStacks').mockResolvedValue([]); } -describe('Fleet prune routes bound docker df at 8s on local nodes (F-6)', () => { +describe('Fleet prune routes bound docker df at 12s on local nodes (F-6)', () => { it('POST /api/fleet/labels/fleet-prune dry-run surfaces a busy-daemon error on local timeout', async () => { stubLocalEstimate( () => Promise.resolve({ reclaimableBytes: 0 }), @@ -94,6 +94,27 @@ describe('Fleet prune routes bound docker df at 8s on local nodes (F-6)', () => expect(local.error).toMatch(/Docker daemon is busy/); }, 20_000); + it('POST /api/fleet/prune/estimate marks the local node unreachable on managed timeout', async () => { + vi.spyOn(FileSystemService.prototype, 'getStacks').mockResolvedValue([]); + // estimateManagedReclaim never settles so the managed path hits + // FLEET_DF_TIMEOUT_MS and surfaces the busy-daemon error. + vi.spyOn(DockerController, 'getInstance').mockReturnValue({ + estimateManagedReclaim: vi.fn().mockImplementation(() => new Promise(() => { /* never resolves */ })), + estimateSystemReclaim: vi.fn().mockResolvedValue({ reclaimableBytes: 0 }), + } as unknown as ReturnType); + + const res = await request(app) + .post('/api/fleet/prune/estimate') + .set('Authorization', authHeader) + .send({ targets: ['images'], scope: 'managed' }); + + expect(res.status).toBe(200); + expect(Array.isArray(res.body.perNode)).toBe(true); + const local = res.body.perNode[0]; + expect(local.reachable).toBe(false); + expect(local.error).toMatch(/Docker daemon is busy/); + }, 20_000); + it('fleet-prune dry-run succeeds normally when estimateSystemReclaim resolves quickly', async () => { stubLocalEstimate( () => Promise.resolve({ reclaimableBytes: 256 }), diff --git a/backend/src/__tests__/system-maintenance-prune.test.ts b/backend/src/__tests__/system-maintenance-prune.test.ts index 7ca13686..cba48ddf 100644 --- a/backend/src/__tests__/system-maintenance-prune.test.ts +++ b/backend/src/__tests__/system-maintenance-prune.test.ts @@ -4,7 +4,7 @@ * * Uses real timers because supertest dispatches lazily and vi.useFakeTimers * does not compose cleanly with that pattern. Each timeout test waits the - * full 8s withTimeout budget, so two such tests add ~17s to the file. + * full 12s withTimeout budget, so three such tests add ~36s to the file. */ import { describe, it, expect, beforeAll, afterAll, afterEach, vi } from 'vitest'; import request from 'supertest'; @@ -45,9 +45,12 @@ function stubFsStacks() { } as unknown as ReturnType); } -function stubEstimate(impl: () => Promise<{ reclaimableBytes: number }>) { +function stubEstimate( + impl: () => Promise<{ reclaimableBytes: number }>, + method: 'estimateSystemReclaim' | 'estimateManagedReclaim' = 'estimateSystemReclaim', +) { vi.spyOn(DockerController, 'getInstance').mockReturnValue({ - estimateSystemReclaim: vi.fn().mockImplementation(impl), + [method]: vi.fn().mockImplementation(impl), } as unknown as ReturnType); } @@ -122,6 +125,27 @@ describe('Prune estimate endpoints return 503 on slow docker df (F-6)', () => { expect(res.status).toBe(500); expect(res.body.code).not.toBe('docker_df_slow'); }); + + it('managed images estimate returns 503 docker_df_slow when estimateManagedReclaim never settles', async () => { + stubFsStacks(); + stubEstimate( + () => new Promise(() => { /* never resolves */ }), + 'estimateManagedReclaim', + ); + + const t0 = Date.now(); + const res = await request(app) + .post('/api/system/prune/estimate') + .set('Authorization', authHeader) + .send({ target: 'images', scope: 'managed' }); + const elapsed = Date.now() - t0; + + expect(res.status).toBe(503); + expect(res.body.code).toBe('docker_df_slow'); + expect(res.body.error).toMatch(/Docker daemon is busy/); + expect(elapsed).toBeGreaterThanOrEqual(7_500); + expect(elapsed).toBeLessThan(15_000); + }, 20_000); }); describe('Prune plan routes', () => { diff --git a/backend/src/routes/fleet.ts b/backend/src/routes/fleet.ts index 2947df29..6ebd77a3 100644 --- a/backend/src/routes/fleet.ts +++ b/backend/src/routes/fleet.ts @@ -38,8 +38,8 @@ import { buildTargetImageRef, isRepinBlocked, type ImagePinKind } from '../helpe import { withTimeout, TimeoutError } from '../utils/withTimeout'; // Mirror the system-maintenance route timeout so fleet's local-node prune -// paths cap the slow `docker system df` call at the same 8s budget (F-6). -const FLEET_DF_TIMEOUT_MS = 8_000; +// paths cap the slow `docker system df` call at the same 12 s budget (F-6). +const FLEET_DF_TIMEOUT_MS = 12_000; import { POLICY_SEVERITIES } from '../utils/severity'; import { isNoOpBlockingPolicy } from '../utils/policy-risk'; import { sanitizeForLog, redactSensitiveText } from '../utils/safeLog'; @@ -2370,15 +2370,13 @@ fleetRouter.post('/prune/estimate', authMiddleware, async (req: Request, res: Re const dockerController = DockerController.getInstance(node.id); let nodeBytes = 0; for (const target of targets) { - // estimateSystemReclaim hits `docker system df`; bound it so a - // slow local daemon doesn't hang the fleet estimate (F-6). - const result = scope === 'managed' - ? await dockerController.estimateManagedReclaim(target, knownStacks) - : await withTimeout( - dockerController.estimateSystemReclaim(target, knownStacks), - FLEET_DF_TIMEOUT_MS, - 'docker disk usage', - ); + // Bound so a slow local daemon does not hang the fleet + // estimate (F-6). Both managed and all scopes bound each + // per-target call at the same 12 s. + const estimate = scope === 'managed' + ? dockerController.estimateManagedReclaim(target, knownStacks) + : dockerController.estimateSystemReclaim(target, knownStacks); + const result = await withTimeout(estimate, FLEET_DF_TIMEOUT_MS, 'docker disk usage'); nodeBytes += result.reclaimableBytes; } return { nodeId: node.id, nodeName: node.name, reclaimableBytes: nodeBytes, reachable: true }; diff --git a/backend/src/routes/systemMaintenance.ts b/backend/src/routes/systemMaintenance.ts index 5e1d248d..80db7f84 100644 --- a/backend/src/routes/systemMaintenance.ts +++ b/backend/src/routes/systemMaintenance.ts @@ -28,11 +28,15 @@ import { buildStackNetworkFacts } from '../services/network/composeNetworkInspec import { evaluateNetworkDeleteGuard } from '../services/network/networkDeleteGuards'; import { loadNetworkingSnapshot } from '../services/network/networkingAggregate'; -// `docker system df` (the call backing estimateSystemReclaim) can take 30+ -// seconds on Docker Desktop with many volumes; 8s matches the MonitorService -// janitor timeout so the daemon never has more than ~16s of concurrent -// pressure from Sencho's own paths even when prune and janitor collide. -const PRUNE_ESTIMATE_TIMEOUT_MS = 8_000; +// The prune estimate and plan paths are bounded at 12 s. `docker system df` +// cost scales with image-store size (measured ~7.4 s on a 34 GB store), so +// the estimate budget must leave headroom above a single df call. 12 s sits +// strictly below the hub's 15 s AbortSignal.timeout on the fleet estimate +// fetch, keeping the remote 503 the actionable failure instead of a hub-side +// abort. The MonitorService janitor keeps its own 8 s budget +// (JANITOR_TIMEOUT_MS) so Sencho's destructive paths never stack more than +// ~16 s of concurrent daemon pressure with the janitor. +const PRUNE_ESTIMATE_TIMEOUT_MS = 12_000; function respondDfSlow(res: Response): Response { return res.status(503).json({ @@ -322,26 +326,26 @@ systemMaintenanceRouter.post('/prune/estimate', async (req: Request, res: Respon } const pruneScope = scope === 'managed' ? 'managed' : 'all'; const dockerController = DockerController.getInstance(req.nodeId); - const knownStacks = await FileSystemService.getInstance(req.nodeId).getStacks(); + // Skip the filesystem walk for all scope: estimateSystemReclaim uses + // only docker system df and ignores knownStackNames. Mirroring the + // fleet route's conditional at fleet.ts:2367. + const knownStacks = pruneScope === 'managed' + ? await FileSystemService.getInstance(req.nodeId).getStacks() + : []; - let result: { reclaimableBytes: number }; - if (pruneScope === 'managed' && target !== 'containers') { - result = await dockerController.estimateManagedReclaim( - target as 'images' | 'volumes' | 'networks', - knownStacks, - ); - } else { - // estimateSystemReclaim calls `docker system df`; bound it so a slow - // daemon doesn't hang the admin's tab (F-6). - result = await withTimeout( - dockerController.estimateSystemReclaim( + // Both estimate paths run under the same 12 s budget (F-6). The all-scope + // fast path uses only `docker system df` via getDiskUsage(); the managed + // path enumerates stacks but stays within the same bound. + const estimate = pruneScope === 'managed' && target !== 'containers' + ? dockerController.estimateManagedReclaim( + target as 'images' | 'volumes' | 'networks', + knownStacks, + ) + : dockerController.estimateSystemReclaim( target as 'containers' | 'images' | 'networks' | 'volumes', knownStacks, - ), - PRUNE_ESTIMATE_TIMEOUT_MS, - 'docker disk usage', - ); - } + ); + const result = await withTimeout(estimate, PRUNE_ESTIMATE_TIMEOUT_MS, 'docker disk usage'); res.json({ reclaimableBytes: result.reclaimableBytes }); } catch (error: unknown) { if (error instanceof TimeoutError) { diff --git a/backend/src/services/DockerController.ts b/backend/src/services/DockerController.ts index 0776bcce..6b466148 100644 --- a/backend/src/services/DockerController.ts +++ b/backend/src/services/DockerController.ts @@ -964,15 +964,17 @@ class DockerController { } /** - * Non-destructive estimate for the `all` (system) prune scope. Reuses the - * same disk-usage source as the resources page so the readout matches what - * the operator already sees there. + * Non-destructive estimate for the `all` (system) prune scope. Uses only + * `docker system df` so the readout matches the daemon's own reclaimable + * totals for each resource type. `knownStackNames` is retained for + * call-site symmetry with {@link estimateManagedReclaim} and is unused + * by this method. */ public async estimateSystemReclaim( target: 'containers' | 'images' | 'networks' | 'volumes', - knownStackNames: string[], + _knownStackNames: string[], ): Promise<{ reclaimableBytes: number }> { - const df = await this.getDiskUsageClassified(knownStackNames); + const df = await this.getDiskUsage(); if (target === 'images') return { reclaimableBytes: df.reclaimableImages }; if (target === 'containers') return { reclaimableBytes: df.reclaimableContainers }; if (target === 'volumes') return { reclaimableBytes: df.reclaimableVolumes };