From ed5ca9c4f690d07dc1ccf25c3a8557e57780410d Mon Sep 17 00:00:00 2001 From: Anso Date: Thu, 23 Jul 2026 08:25:54 -0400 Subject: [PATCH] fix(fleet): refresh prune reclaimable estimate after successful run (#1675) * fix(fleet): refresh prune reclaimable estimate after successful run The live estimate only re-ran when targets or scope changed, so the toolbar and per-node reclaimable figures stayed stale after a real prune. Invalidate via estimateEpoch when any non-dry-run target succeeds. * fix(fleet): toast partial prune success honestly A reachable node with mixed per-target outcomes made okNodes zero and showed a total-failure toast even when some targets mutated Docker. Gate the failure toast on zero successful targets anywhere. --- docs/features/fleet-actions.mdx | 2 +- .../cards/FleetPruneCard.test.tsx | 165 +++++++++++++++++- .../FleetActions/cards/FleetPruneCard.tsx | 25 ++- 3 files changed, 185 insertions(+), 7 deletions(-) diff --git a/docs/features/fleet-actions.mdx b/docs/features/fleet-actions.mdx index 4cbe6556..43b70633 100644 --- a/docs/features/fleet-actions.mdx +++ b/docs/features/fleet-actions.mdx @@ -139,7 +139,7 @@ Scope is a segmented control with two options: ### Live estimate and behaviour -- Changing a target or the scope re-triggers a debounced call to `POST /api/fleet/prune/estimate`, which walks the same Docker enumeration the destructive path uses so the estimate matches what pruning would actually reclaim. **Prune fleet** stays disabled until the estimate resolves: you cannot confirm a destructive fleet-wide prune with no context on what it will reclaim. +- Changing a target or the scope re-triggers a debounced call to `POST /api/fleet/prune/estimate`, which walks the same Docker enumeration the destructive path uses so the estimate matches what pruning would actually reclaim. A completed real prune that succeeds on at least one target re-triggers the same estimate so the toolbar total and per-node list reflect post-prune Docker state. **Prune fleet** stays disabled until the estimate resolves: you cannot confirm a destructive fleet-wide prune with no context on what it will reclaim. - Each remote node receives one `POST /api/system/prune/system` call per selected target, with a 120-second timeout. If a transport error fires for one target, the remaining targets on that node are short-circuited with the same error rather than retried, so a dead node doesn't absorb the full multi-target timeout budget. - Local nodes serialize against a per-node lock (`bulk-prune:`). A second fleet prune launched against the same local node while the first is still in flight returns *A prune is already running on this node* for each target. - Reclaimed bytes are reported by the Docker daemon and are approximate. Per-node rows in the results panel sum the per-target reclaim; the per-target children show how much each individual prune actually freed. diff --git a/frontend/src/components/fleet/FleetActions/cards/FleetPruneCard.test.tsx b/frontend/src/components/fleet/FleetActions/cards/FleetPruneCard.test.tsx index df538584..6ad8850b 100644 --- a/frontend/src/components/fleet/FleetActions/cards/FleetPruneCard.test.tsx +++ b/frontend/src/components/fleet/FleetActions/cards/FleetPruneCard.test.tsx @@ -13,12 +13,13 @@ vi.mock('@/lib/api', () => ({ apiFetch: vi.fn() })); const toastError = vi.fn(); const toastSuccess = vi.fn(); +const toastWarning = vi.fn(); vi.mock('@/components/ui/toast-store', () => ({ toast: { error: (...a: unknown[]) => toastError(...a), success: (...a: unknown[]) => toastSuccess(...a), info: vi.fn(), - warning: vi.fn(), + warning: (...a: unknown[]) => toastWarning(...a), loading: vi.fn(() => 'toast-id'), dismiss: vi.fn(), }, @@ -112,3 +113,165 @@ it('surfaces an error toast when the prune returns non-ok', async () => { await user.click(screen.getByRole('button', { name: 'Dry run' })); await waitFor(() => expect(toastError).toHaveBeenCalledWith('prune blew up')); }); + +function estimateCallCount(): number { + return mockedFetch.mock.calls.filter(c => c[0] === '/fleet/prune/estimate').length; +} + +it('re-fetches the estimate after a partial-success real prune', async () => { + const user = userEvent.setup(); + let estimatePhase: 'pre' | 'post' = 'pre'; + mockedFetch.mockImplementation((url: string) => { + if (url === '/fleet/prune/estimate') { + if (estimatePhase === 'pre') { + return Promise.resolve(jsonResponse(200, { + totalBytes: 1024, + perNode: [{ nodeId: 1, nodeName: 'central', reclaimableBytes: 1024, reachable: true }], + })); + } + return Promise.resolve(jsonResponse(200, { + totalBytes: 0, + perNode: [{ nodeId: 1, nodeName: 'central', reclaimableBytes: 0, reachable: true }], + })); + } + if (url === '/fleet/labels/fleet-prune') { + // One target succeeded before a later failure left the node unreachable. + // reachable:false must not suppress the refresh (target.success is the signal). + return Promise.resolve(jsonResponse(200, { + results: [{ + nodeId: 1, + nodeName: 'central', + reachable: false, + error: 'transport failed after images', + targets: [ + { target: 'images', success: true, reclaimedBytes: 1024 }, + { target: 'volumes', success: false, reclaimedBytes: 0, error: 'unreachable' }, + ], + }], + })); + } + return Promise.resolve(jsonResponse(404, {})); + }); + + render(); + await waitFor(() => expect(screen.getByRole('button', { name: 'Prune fleet' })).toBeEnabled()); + expect(screen.getByText('~ 1 KB reclaimable')).toBeInTheDocument(); + expect(screen.getByText('1 KB')).toBeInTheDocument(); + const callsBeforePrune = estimateCallCount(); + + await user.click(screen.getByRole('button', { name: 'Prune fleet' })); + const dialog = await screen.findByRole('alertdialog'); + estimatePhase = 'post'; + await user.click(within(dialog).getByRole('button', { name: 'Prune managed' })); + + await waitFor(() => expect(estimateCallCount()).toBe(callsBeforePrune + 1)); + const postEstimateCall = [...mockedFetch.mock.calls].reverse().find(c => c[0] === '/fleet/prune/estimate'); + expect(postEstimateCall).toBeTruthy(); + expect(JSON.parse(postEstimateCall![1].body)).toEqual({ targets: ['images'], scope: 'managed' }); + await waitFor(() => expect(screen.getByText('0 reclaimable')).toBeInTheDocument()); + await waitFor(() => expect(screen.getByText('0 Bytes')).toBeInTheDocument()); +}); + +it('does not re-fetch the estimate after a dry run', async () => { + const user = userEvent.setup(); + mockedFetch.mockImplementation((url: string) => { + if (url === '/fleet/prune/estimate') { + return Promise.resolve(jsonResponse(200, { + totalBytes: 1024, + perNode: [{ nodeId: 1, nodeName: 'central', reclaimableBytes: 1024, reachable: true }], + })); + } + if (url === '/fleet/labels/fleet-prune') { + return Promise.resolve(jsonResponse(200, { + results: [{ + nodeId: 1, + nodeName: 'central', + reachable: true, + targets: [{ target: 'images', success: true, reclaimedBytes: 4096, dryRun: true }], + }], + })); + } + return Promise.resolve(jsonResponse(404, {})); + }); + + render(); + await waitFor(() => expect(screen.getByRole('button', { name: 'Prune fleet' })).toBeEnabled()); + const callsBefore = estimateCallCount(); + await user.click(screen.getByRole('button', { name: 'Dry run' })); + await waitFor(() => expect(toastSuccess).toHaveBeenCalled()); + // Debounce window: a refresh would schedule another estimate within 350ms. + await new Promise(r => setTimeout(r, 500)); + expect(estimateCallCount()).toBe(callsBefore); +}); + +it('does not re-fetch the estimate when every target fails on a 2xx response', async () => { + const user = userEvent.setup(); + mockedFetch.mockImplementation((url: string) => { + if (url === '/fleet/prune/estimate') { + return Promise.resolve(jsonResponse(200, { + totalBytes: 1024, + perNode: [{ nodeId: 1, nodeName: 'central', reclaimableBytes: 1024, reachable: true }], + })); + } + if (url === '/fleet/labels/fleet-prune') { + return Promise.resolve(jsonResponse(200, { + results: [{ + nodeId: 1, + nodeName: 'central', + reachable: true, + targets: [{ target: 'images', success: false, reclaimedBytes: 0, error: 'A prune is already running on this node' }], + }], + })); + } + return Promise.resolve(jsonResponse(404, {})); + }); + + render(); + await waitFor(() => expect(screen.getByRole('button', { name: 'Prune fleet' })).toBeEnabled()); + const callsBefore = estimateCallCount(); + await user.click(screen.getByRole('button', { name: 'Prune fleet' })); + const dialog = await screen.findByRole('alertdialog'); + await user.click(within(dialog).getByRole('button', { name: 'Prune managed' })); + await waitFor(() => expect(toastError).toHaveBeenCalled()); + await new Promise(r => setTimeout(r, 500)); + expect(estimateCallCount()).toBe(callsBefore); +}); + +it('warns instead of claiming total failure when a reachable node has mixed per-target outcomes', async () => { + const user = userEvent.setup(); + mockedFetch.mockImplementation((url: string) => { + if (url === '/fleet/prune/estimate') { + return Promise.resolve(jsonResponse(200, { + totalBytes: 1024, + perNode: [{ nodeId: 1, nodeName: 'central', reclaimableBytes: 1024, reachable: true }], + })); + } + if (url === '/fleet/labels/fleet-prune') { + // Images succeed, volumes fail on the same reachable node. Pre-fix toast + // logic treated the node as fully failed (okNodes=0) and said every node + // failed even though Docker mutated for images. + return Promise.resolve(jsonResponse(200, { + results: [{ + nodeId: 1, + nodeName: 'central', + reachable: true, + targets: [ + { target: 'images', success: true, reclaimedBytes: 1024 }, + { target: 'volumes', success: false, reclaimedBytes: 0, error: 'volume prune failed' }, + ], + }], + })); + } + return Promise.resolve(jsonResponse(404, {})); + }); + + render(); + await waitFor(() => expect(screen.getByRole('button', { name: 'Prune fleet' })).toBeEnabled()); + await user.click(screen.getByRole('button', { name: 'Prune fleet' })); + const dialog = await screen.findByRole('alertdialog'); + await user.click(within(dialog).getByRole('button', { name: 'Prune managed' })); + + await waitFor(() => expect(toastWarning).toHaveBeenCalled()); + expect(toastWarning.mock.calls[0][0]).toMatch(/1\/1 nodes reclaimed space/); + expect(toastError).not.toHaveBeenCalledWith('Prune failed on every node. See results below.'); +}); diff --git a/frontend/src/components/fleet/FleetActions/cards/FleetPruneCard.tsx b/frontend/src/components/fleet/FleetActions/cards/FleetPruneCard.tsx index a2f8cebb..34729407 100644 --- a/frontend/src/components/fleet/FleetActions/cards/FleetPruneCard.tsx +++ b/frontend/src/components/fleet/FleetActions/cards/FleetPruneCard.tsx @@ -48,6 +48,9 @@ export function FleetPruneCard({ nodes }: Props) { const [running, setRunning] = useState(false); const [results, setResults] = useState([]); const [estimate, setEstimate] = useState({ kind: 'idle' }); + // Bumped after a real prune mutates at least one target so the estimate + // effect re-runs without changing targets/scope. + const [estimateEpoch, setEstimateEpoch] = useState(0); const toggleTarget = (target: PruneTarget) => { setTargets(prev => { @@ -92,7 +95,7 @@ export function FleetPruneCard({ nodes }: Props) { cancelled = true; if (estimateDebounceRef.current) clearTimeout(estimateDebounceRef.current); }; - }, [targets, scope]); + }, [targets, scope, estimateEpoch]); async function run(opts: { dryRun: boolean }) { if (targets.size === 0) return; @@ -113,6 +116,13 @@ export function FleetPruneCard({ nodes }: Props) { return; } const apiResults = (body.results as FleetPruneNodeResult[]) ?? []; + // HTTP 200 still arrives when every target fails; only a successful + // target means Docker state changed and the live estimate is stale. + // Scan targets independently of node.reachable (an early remote target + // can succeed before a later transport error marks the node unreachable). + if (!opts.dryRun && apiResults.some(node => node.targets.some(target => target.success))) { + setEstimateEpoch(e => e + 1); + } const rows: ResultRow[] = apiResults.map((node) => { const totalBytes = node.targets.reduce((sum, t) => sum + (t.reclaimedBytes ?? 0), 0); const allOk = node.reachable && node.targets.every(t => t.success); @@ -133,19 +143,24 @@ export function FleetPruneCard({ nodes }: Props) { }); setResults(rows); const totalNodes = apiResults.length; - const okNodes = apiResults.filter(n => n.reachable && n.targets.every(t => t.success)).length; + // Fully OK: every target on a reachable node succeeded. Partial: at + // least one target succeeded anywhere (mixed per-target on one node + // still counts). "Failed on every node" only when zero targets succeeded. + const fullyOkNodes = apiResults.filter(n => n.reachable && n.targets.every(t => t.success)).length; + const nodesWithAnySuccess = apiResults.filter(n => n.targets.some(t => t.success)).length; + const anyTargetSucceeded = nodesWithAnySuccess > 0; const totalReclaimed = apiResults.reduce( (sum, n) => sum + n.targets.reduce((s, t) => s + (t.reclaimedBytes ?? 0), 0), 0, ); if (opts.dryRun) { toast.success(`Dry run: ${formatBytes(totalReclaimed)} would be reclaimed across ${totalNodes} node${totalNodes === 1 ? '' : 's'}.`); - } else if (okNodes === totalNodes && totalNodes > 0) { + } else if (fullyOkNodes === totalNodes && totalNodes > 0) { toast.success(`Reclaimed ${formatBytes(totalReclaimed)} across ${totalNodes} node${totalNodes === 1 ? '' : 's'}.`); - } else if (okNodes === 0) { + } else if (!anyTargetSucceeded) { toast.error('Prune failed on every node. See results below.'); } else { - toast.warning(`${okNodes}/${totalNodes} nodes succeeded · ${formatBytes(totalReclaimed)} reclaimed. See results below.`); + toast.warning(`${nodesWithAnySuccess}/${totalNodes} nodes reclaimed space · ${formatBytes(totalReclaimed)} reclaimed. See results below.`); } } catch (err) { toast.dismiss(toastId);