diff --git a/backend/src/__tests__/fleet-action-card-endpoints.test.ts b/backend/src/__tests__/fleet-action-card-endpoints.test.ts index 0f7ec1a0..26d2a016 100644 --- a/backend/src/__tests__/fleet-action-card-endpoints.test.ts +++ b/backend/src/__tests__/fleet-action-card-endpoints.test.ts @@ -1301,6 +1301,7 @@ describe('POST /api/fleet/labels/fleet-prune with dryRun: true', () => { executePrunePlan.mockResolvedValue({ success: true, reclaimedBytes: 0, + mutated: true, outcomes: [{ target: 'images', id: 'image', status: 'removed' }], }); const res = await request(app) @@ -1337,7 +1338,12 @@ describe('POST /api/fleet/labels/fleet-prune with dryRun: true', () => { nodeId: local.id, scope: 'managed', targets: ['images'], items: [...testCase.items], reclaimableBytes: 0, fingerprint, createdAt: 1, }); - executePrunePlan.mockResolvedValue({ success: true, reclaimedBytes: 0, outcomes: [...testCase.outcomes] }); + executePrunePlan.mockResolvedValue({ + success: true, + reclaimedBytes: 0, + mutated: false, + outcomes: [...testCase.outcomes], + }); invalidateNodeCaches.mockClear(); const res = await request(app) .post('/api/fleet/labels/fleet-prune') diff --git a/backend/src/__tests__/fleet-prune.test.ts b/backend/src/__tests__/fleet-prune.test.ts index 13a83f48..e1db5aed 100644 --- a/backend/src/__tests__/fleet-prune.test.ts +++ b/backend/src/__tests__/fleet-prune.test.ts @@ -3,6 +3,7 @@ import request from 'supertest'; import jwt from 'jsonwebtoken'; import { cleanupTestDb, setupTestDb, TEST_JWT_SECRET, TEST_USERNAME } from './helpers/setupTestDb'; import type { PruneItemOutcome, PrunePlan, PrunePlanItem } from '../services/prunePlan'; +import { CacheService } from '../services/CacheService'; let tmpDir: string; let app: import('express').Express; @@ -62,9 +63,11 @@ function mockLocal(planFactory: (nodeId: number) => PrunePlan = (nodeId) => plan success: boolean; reclaimedBytes: number; outcomes: PruneItemOutcome[]; + mutated: boolean; }> => ({ success: true, reclaimedBytes: reviewedPlan.reclaimableBytes, + mutated: reviewedPlan.items.length > 0, outcomes: reviewedPlan.items.map((entry) => ({ id: entry.id, target: entry.target, @@ -212,6 +215,30 @@ describe('POST /api/fleet/labels/fleet-prune', () => { expect(activeBulkActions.size).toBe(0); }); + it('invalidates local node caches when a failed image outcome is mutated', async () => { + const fake = mockLocal(); + fake.executePrunePlan.mockResolvedValue({ + success: false, + reclaimedBytes: 0, + mutated: true, + outcomes: [{ target: 'images', id: 'sha256:image', status: 'failed', error: 'tags remain' }], + }); + const invalidate = vi.spyOn(CacheService.getInstance(), 'invalidate'); + const local = DatabaseService.getInstance().getNodes().find((node) => node.type === 'local')!; + const response = await request(app) + .post('/api/fleet/labels/fleet-prune') + .set('Authorization', authHeader) + .send({ + targets: ['images'], scope: 'managed', dryRun: false, + reviewedNodes: [{ nodeId: local.id, reachable: true }], + plans: [{ nodeId: local.id, fingerprint: `fingerprint-${local.id}` }], + }); + expect(response.status).toBe(200); + expect(response.body.results[0].outcomes[0].status).toBe('failed'); + expect(invalidate).toHaveBeenCalledWith(`stats:${local.id}`); + expect(invalidate).toHaveBeenCalledWith(`stack-statuses:${local.id}`); + }); + it('fails closed when a local prune lock is active', async () => { const fake = mockLocal(); const { local, body } = localReview(`fingerprint-${DatabaseService.getInstance().getNodes()[0].id}`); @@ -440,6 +467,7 @@ describe('POST /api/fleet/labels/fleet-prune', () => { fake.executePrunePlan.mockResolvedValue({ success: false, reclaimedBytes: 100, + mutated: true, outcomes: [ { target: 'images', id: 'removed', status: 'removed', sizeBytes: 100 }, { target: 'images', id: 'skipped', status: 'skipped', reason: 'became active' }, diff --git a/backend/src/__tests__/prune-plan.test.ts b/backend/src/__tests__/prune-plan.test.ts index 54168fd3..517f766c 100644 --- a/backend/src/__tests__/prune-plan.test.ts +++ b/backend/src/__tests__/prune-plan.test.ts @@ -659,9 +659,77 @@ describe('DockerController.buildPrunePlan', () => { expect(plan.items.map((i) => i.id)).toEqual(['img-free']); }); + + it('excludes a fully synthetic sencho-rb hold image from the plan even without a DB hold', async () => { + mockDocker.listImages.mockResolvedValue([ + { + Id: 'img-orphan-hold', + RepoTags: ['sencho-rb/abc123456789/web:hold', 'sencho-rb/abc123456789/api:hold'], + Size: 50, + Containers: 0, + }, + { Id: 'img-free', RepoTags: ['app:2'], Size: 100, Containers: 0 }, + ]); + + const dc = DockerController.getInstance(1); + const plan = await dc.buildPrunePlan(['images'], 'all', [], 1, () => false); + + expect(plan.items.map((i) => i.id)).toEqual(['img-free']); + }); + + it('still plans a dual-tagged image that carries a registry tag and a sencho-rb hold tag', async () => { + mockDocker.listImages.mockResolvedValue([ + { + Id: 'img-dual', + RepoTags: ['myregistry/app:1.4', 'sencho-rb/abc123456789/app:hold'], + Size: 100, + Containers: 0, + }, + ]); + + const dc = DockerController.getInstance(1); + const plan = await dc.buildPrunePlan(['images'], 'all', [], 1); + + expect(plan.items.map((i) => i.id)).toEqual(['img-dual']); + expect(plan.items[0]?.image?.references).toEqual([ + 'myregistry/app:1.4', + 'sencho-rb/abc123456789/app:hold', + ]); + }); }); describe('DockerController.executePrunePlan', () => { + const multiTagImage = { + Id: 'img-multi', + RepoTags: ['qa1769/app:v0', 'qa1769-external/keep:v0'], + Size: 50, + Containers: 0, + }; + + function mockImageRemove( + onRemove?: (name: string, opts?: { force?: boolean }) => void | Promise, + ): Array<{ name: string; force?: boolean }> { + const removes: Array<{ name: string; force?: boolean }> = []; + mockDocker.getImage.mockImplementation((name: string) => ({ + remove: vi.fn().mockImplementation(async (opts?: { force?: boolean }) => { + removes.push({ name, force: opts?.force }); + await onRemove?.(name, opts); + }), + })); + return removes; + } + + async function executeForcedFreshImagePlan(listings: Array) { + for (const listing of listings) { + if (listing instanceof Error) mockDocker.listImages.mockRejectedValueOnce(listing); + else mockDocker.listImages.mockResolvedValueOnce(listing); + } + const dc = DockerController.getInstance(1); + const plan = await dc.buildPrunePlan(['images'], 'all', [], 1); + vi.spyOn(dc, 'assertPlanFresh').mockResolvedValue(plan); + return dc.executePrunePlan(plan, []); + } + it('throws PrunePlanStaleError when the fingerprint no longer matches', async () => { mockDocker.listContainers.mockResolvedValue([ { @@ -852,6 +920,49 @@ describe('DockerController.executePrunePlan', () => { ]); }); + it('skips a fully synthetic hold image even under a forced-fresh plan', async () => { + const holdImg = { + Id: 'img-orphan-hold', + RepoTags: ['sencho-rb/abc123456789/web:hold', 'sencho-rb/abc123456789/api:hold'], + Size: 50, + Containers: 0, + }; + mockDocker.listImages.mockResolvedValue([holdImg]); + const removes = mockImageRemove(); + + const dc = DockerController.getInstance(1); + const plan = { + scope: 'all' as const, + targets: ['images' as const], + items: [{ + target: 'images' as const, + id: holdImg.Id, + name: holdImg.RepoTags[0], + sizeBytes: 50, + managed: false, + reason: 'Image is not used by any container', + image: { references: holdImg.RepoTags }, + }], + reclaimableBytes: 50, + fingerprint: 'forced-hold', + createdAt: Date.now(), + nodeId: 1, + }; + vi.spyOn(dc, 'assertPlanFresh').mockResolvedValue(plan); + const result = await dc.executePrunePlan(plan, []); + + expect(removes).toEqual([]); + expect(result.mutated).toBe(false); + expect(result.outcomes).toEqual([ + expect.objectContaining({ + id: holdImg.Id, + target: 'images', + status: 'skipped', + reason: 'Sencho rollback-hold image', + }), + ]); + }); + it('marks the plan stale when a free image gains a new RepoTag between plan and rebuild', async () => { const img = { Id: 'img-retag', @@ -896,29 +1007,177 @@ describe('DockerController.executePrunePlan', () => { })); }); - it('surfaces multi-repository refuse without a silent partial untag report', async () => { - mockDocker.listImages.mockResolvedValue([ - { - Id: 'img-multi', - RepoTags: ['qa1769/app:v0', 'qa1769-external/keep:v0'], - Size: 50, - Containers: 0, - }, - ]); - const imageRemove = vi.fn().mockRejectedValue( - Object.assign(new Error('conflict: unable to delete (must be forced) - image is referenced in multiple repositories'), { statusCode: 409 }), - ); - mockDocker.getImage.mockReturnValue({ remove: imageRemove }); + it('untags each reviewed name and reports removed once the image is gone', async () => { + const removes = mockImageRemove(); + const result = await executeForcedFreshImagePlan([[multiTagImage], [multiTagImage], []]); + + expect(removes.map((entry) => entry.name)).toEqual(['qa1769/app:v0', 'qa1769-external/keep:v0']); + expect(result.outcomes[0]).toEqual(expect.objectContaining({ + id: 'img-multi', + status: 'removed', + })); + expect(result.mutated).toBe(true); + expect(result.success).toBe(true); + }); + + it('treats a first-ref 404 as absent and still removes when a later reviewed tag succeeds', async () => { + const removes = mockImageRemove(async (name) => { + if (name === 'qa1769/app:v0') { + throw Object.assign(new Error('No such image: qa1769/app:v0'), { statusCode: 404 }); + } + }); + const result = await executeForcedFreshImagePlan([[multiTagImage], [multiTagImage], []]); + + expect(removes).toEqual([ + { name: 'qa1769/app:v0', force: false }, + { name: 'qa1769-external/keep:v0', force: false }, + ]); + expect(result.outcomes[0]).toEqual(expect.objectContaining({ status: 'removed' })); + expect(result.mutated).toBe(true); + expect(result.success).toBe(true); + }); + + it('does not report removed when a later 404 leaves another reviewed tag present', async () => { + mockImageRemove(async (name) => { + if (name === 'qa1769-external/keep:v0') { + throw Object.assign(new Error('No such image'), { statusCode: 404 }); + } + }); + const result = await executeForcedFreshImagePlan([ + [multiTagImage], + [multiTagImage], + [{ ...multiTagImage, RepoTags: ['qa1769/app:v0'] }], + ]); - const dc = DockerController.getInstance(1); - const plan = await dc.buildPrunePlan(['images'], 'all', [], 1); - vi.spyOn(dc, 'assertPlanFresh').mockResolvedValue(plan); - const result = await dc.executePrunePlan(plan, []); - expect(imageRemove).toHaveBeenCalled(); expect(result.outcomes[0]).toEqual(expect.objectContaining({ status: 'failed', - error: expect.stringMatching(/multiple repositories/i), + error: expect.stringMatching(/qa1769\/app:v0/), })); - expect(String(result.outcomes[0] && 'error' in result.outcomes[0] ? result.outcomes[0].error : '')).toMatch(/qa1769\/app:v0/); + expect(result.outcomes[0]).not.toEqual(expect.objectContaining({ status: 'removed' })); + expect(result.mutated).toBe(true); + expect(result.success).toBe(false); + }); + + it('stops on a hard untag error, lists remaining tags, and marks mutated', async () => { + const removes = mockImageRemove(async (name) => { + if (name === 'qa1769-external/keep:v0') { + throw Object.assign(new Error('conflict: unable to delete (must be forced)'), { statusCode: 409 }); + } + }); + const result = await executeForcedFreshImagePlan([ + [multiTagImage], + [multiTagImage], + [{ ...multiTagImage, RepoTags: ['qa1769-external/keep:v0'] }], + ]); + + expect(removes.map((entry) => entry.name)).toEqual(['qa1769/app:v0', 'qa1769-external/keep:v0']); + expect(result.outcomes[0]).toEqual(expect.objectContaining({ + status: 'failed', + error: expect.stringMatching(/qa1769-external\/keep:v0/), + })); + expect(result.mutated).toBe(true); + expect(result.success).toBe(false); + }); + + it('never removes an unexpected tag that appears during execution', async () => { + const img = { Id: 'img-multi', RepoTags: ['qa1769/app:v0'], Size: 50, Containers: 0 }; + const removes = mockImageRemove(); + const result = await executeForcedFreshImagePlan([ + [img], + [img], + [{ ...img, RepoTags: ['qa1769-external/keep:v0'] }], + ]); + + expect(removes.map((entry) => entry.name)).toEqual(['qa1769/app:v0']); + expect(removes.map((entry) => entry.name)).not.toContain('qa1769-external/keep:v0'); + expect(result.outcomes[0]).toEqual(expect.objectContaining({ + status: 'failed', + error: expect.stringMatching(/not in the reviewed set/i), + })); + expect(result.mutated).toBe(true); + }); + + it('removes a dangling planned image by id with force false', async () => { + const img = { Id: 'img-dang', RepoTags: [':'], Size: 20, Containers: 0 }; + const removes = mockImageRemove(); + const result = await executeForcedFreshImagePlan([[img], [img], [img]]); + + expect(removes).toEqual([{ name: 'img-dang', force: false }]); + expect(result.outcomes[0]).toEqual(expect.objectContaining({ status: 'removed' })); + expect(result.mutated).toBe(true); + expect(result.success).toBe(true); + }); + + it('id-removes leftover none tags after reviewed names are gone', async () => { + const img = { Id: 'img-multi', RepoTags: ['qa1769/app:v0'], Size: 50, Containers: 0 }; + const removes = mockImageRemove(); + const result = await executeForcedFreshImagePlan([ + [img], + [img], + [{ ...img, RepoTags: [':'] }], + ]); + + expect(removes).toEqual([ + { name: 'qa1769/app:v0', force: false }, + { name: 'img-multi', force: false }, + ]); + expect(result.outcomes[0]).toEqual(expect.objectContaining({ status: 'removed' })); + expect(result.mutated).toBe(true); + expect(result.success).toBe(true); + }); + + it('does not treat a 409 as missing when the conflict text contains 404', async () => { + const img = { + Id: 'sha256:abc404def', + RepoTags: ['qa1769/app:v0', 'qa1769/app:latest'], + Size: 50, + Containers: 0, + }; + const removes = mockImageRemove(async () => { + throw Object.assign( + new Error('(HTTP code 409) conflict - image sha256:abc404def is being used by running container'), + { statusCode: 409 }, + ); + }); + const result = await executeForcedFreshImagePlan([[img], [img], [img]]); + + expect(removes).toEqual([{ name: 'qa1769/app:v0', force: false }]); + expect(result.outcomes[0]).toEqual(expect.objectContaining({ + status: 'failed', + error: expect.stringMatching(/qa1769\/app:v0/), + })); + expect(result.mutated).toBe(false); + expect(result.success).toBe(false); + }); + + it('does not report dangling id-remove 409 as removed when the message contains 404', async () => { + const img = { Id: 'sha256:abc404def', RepoTags: [':'], Size: 20, Containers: 0 }; + mockImageRemove(async () => { + throw Object.assign( + new Error('(HTTP code 409) conflict - unable to delete sha256:abc404def (must be forced)'), + { statusCode: 409 }, + ); + }); + const result = await executeForcedFreshImagePlan([[img], [img], [img]]); + + expect(result.outcomes[0]).toEqual(expect.objectContaining({ + status: 'failed', + error: expect.stringMatching(/dangling image could not be removed/i), + })); + expect(result.mutated).toBe(false); + expect(result.success).toBe(false); + }); + + it('fails honestly when completion re-list throws after a successful untag', async () => { + const img = { Id: 'img-multi', RepoTags: ['qa1769/app:v0'], Size: 50, Containers: 0 }; + mockImageRemove(); + const result = await executeForcedFreshImagePlan([[img], [img], new Error('daemon busy')]); + + expect(result.outcomes[0]).toEqual(expect.objectContaining({ + status: 'failed', + error: expect.stringMatching(/could not confirm remaining/i), + })); + expect(result.mutated).toBe(true); + expect(result.success).toBe(false); }); }); diff --git a/backend/src/__tests__/system-maintenance-prune.test.ts b/backend/src/__tests__/system-maintenance-prune.test.ts index cba48ddf..98c8f9c6 100644 --- a/backend/src/__tests__/system-maintenance-prune.test.ts +++ b/backend/src/__tests__/system-maintenance-prune.test.ts @@ -174,6 +174,7 @@ describe('Prune plan routes', () => { outcomes: [{ id: 'v1', target: 'volumes', status: 'removed', sizeBytes: 42 }], reclaimedBytes: 42, success: true, + mutated: true, }); vi.spyOn(DockerController, 'getInstance').mockReturnValue({ buildPrunePlan: vi.fn().mockResolvedValue(plan), @@ -190,6 +191,7 @@ describe('Prune plan routes', () => { expect(res.body.success).toBe(true); expect(res.body.reclaimedBytes).toBe(42); expect(res.body.outcomes).toHaveLength(1); + expect(res.body).not.toHaveProperty('mutated'); expect(executePrunePlan).toHaveBeenCalled(); expect(invalidate).toHaveBeenCalledWith('stats:1'); expect(invalidate).toHaveBeenCalledWith('stack-statuses:1'); @@ -268,4 +270,80 @@ describe('Prune plan routes', () => { expect(res.body.items).toHaveLength(1); expect(executePrunePlan).not.toHaveBeenCalled(); }); + + it('invalidates node caches when a failed image outcome is mutated', async () => { + stubFsStacks(); + const plan = samplePlan('fp-partial'); + vi.spyOn(DockerController, 'getInstance').mockReturnValue({ + buildPrunePlan: vi.fn().mockResolvedValue(plan), + executePrunePlan: vi.fn().mockResolvedValue({ + outcomes: [{ id: 'img-multi', target: 'images', status: 'failed', error: 'tags remain' }], + reclaimedBytes: 0, + success: false, + mutated: true, + }), + } as unknown as ReturnType); + const invalidate = vi.spyOn(CacheService.getInstance(), 'invalidate'); + + const res = await request(app) + .post('/api/system/prune/system') + .set('Authorization', authHeader) + .send({ target: 'volumes', scope: 'managed', planFingerprint: 'fp-partial' }); + + expect(res.status).toBe(200); + expect(res.body.success).toBe(false); + expect(res.body.outcomes[0].status).toBe('failed'); + expect(invalidate).toHaveBeenCalledWith('stats:1'); + expect(invalidate).toHaveBeenCalledWith('stack-statuses:1'); + }); + + it('does not invalidate node caches when a failed outcome is not mutated', async () => { + stubFsStacks(); + const plan = samplePlan('fp-nomut'); + vi.spyOn(DockerController, 'getInstance').mockReturnValue({ + buildPrunePlan: vi.fn().mockResolvedValue(plan), + executePrunePlan: vi.fn().mockResolvedValue({ + outcomes: [{ id: 'img-multi', target: 'images', status: 'failed', error: 'references changed' }], + reclaimedBytes: 0, + success: false, + mutated: false, + }), + } as unknown as ReturnType); + const invalidate = vi.spyOn(CacheService.getInstance(), 'invalidate'); + + const res = await request(app) + .post('/api/system/prune/system') + .set('Authorization', authHeader) + .send({ target: 'volumes', scope: 'managed', planFingerprint: 'fp-nomut' }); + + expect(res.status).toBe(200); + expect(invalidate).not.toHaveBeenCalled(); + }); + + it('legacy no-fingerprint containers prune omits mutated from the JSON body', async () => { + stubFsStacks(); + const executePrunePlan = vi.fn().mockResolvedValue({ + outcomes: [{ id: 'c1', target: 'containers', status: 'removed' }], + reclaimedBytes: 0, + success: true, + mutated: true, + }); + vi.spyOn(DockerController, 'getInstance').mockReturnValue({ + buildPrunePlan: vi.fn().mockResolvedValue(samplePlan('unused')), + executePrunePlan, + } as unknown as ReturnType); + + const res = await request(app) + .post('/api/system/prune/system') + .set('Authorization', authHeader) + .send({ target: 'containers', scope: 'managed' }); + + expect(res.status).toBe(200); + expect(res.body.success).toBe(true); + expect(res.body.reclaimedBytes).toBe(0); + expect(res.body.outcomes).toEqual([ + { id: 'c1', target: 'containers', status: 'removed' }, + ]); + expect(res.body).not.toHaveProperty('mutated'); + }); }); diff --git a/backend/src/helpers/fleetPrune.ts b/backend/src/helpers/fleetPrune.ts index 46bd4896..d814bbd1 100644 --- a/backend/src/helpers/fleetPrune.ts +++ b/backend/src/helpers/fleetPrune.ts @@ -427,7 +427,7 @@ async function executeLocal(entry: Preflight, targets: FleetPruneTarget[]): Prom const knownStacks = await FileSystemService.getInstance(entry.node.id).getStacks(); const isImageHeld = ServiceUpdateRecoveryService.getInstance().buildHeldImagePredicate(entry.node.id); const result = await DockerController.getInstance(entry.node.id).executePrunePlan(plan, knownStacks, isImageHeld); - if (result.outcomes.some((outcome) => outcome.status === 'removed')) { + if (result.mutated) { try { invalidateNodeCaches(entry.node.id); } catch (error) { diff --git a/backend/src/routes/systemMaintenance.ts b/backend/src/routes/systemMaintenance.ts index aafe9319..3263dd4f 100644 --- a/backend/src/routes/systemMaintenance.ts +++ b/backend/src/routes/systemMaintenance.ts @@ -6,7 +6,7 @@ import DockerController, { type PruneScope, type PruneTarget, } from '../services/DockerController'; -import { isPruneTarget } from '../services/prunePlan'; +import { isPruneTarget, type PruneItemOutcome } from '../services/prunePlan'; import { FileSystemService } from '../services/FileSystemService'; import { ServiceUpdateRecoveryService } from '../services/ServiceUpdateRecoveryService'; import { StackUpdateRecoveryService, shortGenerationId } from '../services/StackUpdateRecoveryService'; @@ -250,7 +250,7 @@ systemMaintenanceRouter.post('/prune/system', async (req: Request, res: Response success: result.success, }); } - if (result.outcomes.some((outcome) => outcome.status === 'removed')) { + if (result.mutated) { invalidateNodeCaches(req.nodeId); } res.json({ @@ -271,7 +271,7 @@ systemMaintenanceRouter.post('/prune/system', async (req: Request, res: Response const target = targets[0]; console.log(`[Resources] System prune: ${sanitizeForLog(target)} (scope: ${pruneScope})`); const pruneStartedAt = Date.now(); - let result: { success: boolean; reclaimedBytes: number }; + let result: { success: boolean; reclaimedBytes: number; outcomes?: PruneItemOutcome[] }; if (pruneScope === 'managed' && target !== 'containers') { result = await dockerController.pruneManagedOnly( target as 'images' | 'volumes' | 'networks', @@ -296,7 +296,12 @@ systemMaintenanceRouter.post('/prune/system', async (req: Request, res: Response if (target === 'containers') { invalidateNodeCaches(req.nodeId); } - res.json({ message: 'Prune completed', ...result }); + res.json({ + message: 'Prune completed', + success: result.success, + reclaimedBytes: result.reclaimedBytes, + ...(result.outcomes !== undefined ? { outcomes: result.outcomes } : {}), + }); } catch (error: unknown) { if (error instanceof TimeoutError) { console.warn('System prune: docker disk usage timed out'); diff --git a/backend/src/services/DockerController.ts b/backend/src/services/DockerController.ts index 506e861c..485d5276 100644 --- a/backend/src/services/DockerController.ts +++ b/backend/src/services/DockerController.ts @@ -12,6 +12,7 @@ import { FileSystemService } from './FileSystemService'; import SelfIdentityService from './SelfIdentityService'; import { fingerprintPrunePlan, + normalizePruneImageReferences, normalizePruneTargets, projectPruneOwnershipLabels, pruneImageReferencesEqual, @@ -1019,7 +1020,8 @@ class DockerController { /** * Build an itemized prune plan using the same eligibility predicates that - * `executePrunePlan` revalidates before each delete. Never calls remove APIs. + * `executePrunePlan` revalidates against live tags before mutation. Never + * calls remove APIs. */ public async buildPrunePlan( targets: PruneTarget[], @@ -1214,6 +1216,7 @@ class DockerController { for (const img of rawImages) { if (selfIdentity.isOwnImage(img.Id)) continue; if (isImageHeld?.(img.Id)) continue; + if (isFullySyntheticHoldImage(img.RepoTags ?? [])) continue; const refs = imageToContainerIds.get(img.Id) ?? []; const becomesFree = freeingImages && refs.length > 0 @@ -1311,14 +1314,16 @@ class DockerController { } /** - * Execute a previously previewed plan: revalidate fingerprint, then delete - * each planned item serially with force:false. Never deletes unplanned items. + * Execute a previously previewed plan: revalidate fingerprint, then apply each + * planned item serially with force:false. Images untag reviewed names, then + * re-list to decide removed, remaining-tag failure, or dangling ID-remove. + * Never mutates unplanned items. */ public async executePrunePlan( plan: PrunePlan, knownStackNames: string[], isImageHeld?: (imageId: string) => boolean, - ): Promise<{ outcomes: PruneItemOutcome[]; reclaimedBytes: number; success: boolean }> { + ): Promise<{ outcomes: PruneItemOutcome[]; reclaimedBytes: number; success: boolean; mutated: boolean }> { const fresh = await this.assertPlanFresh(plan, knownStackNames, isImageHeld); if (!fresh) throw new PrunePlanStaleError(); @@ -1330,6 +1335,7 @@ class DockerController { const outcomes: PruneItemOutcome[] = []; let reclaimedBytes = 0; + let planMutated = false; /** Image IDs whose planned container removal failed or was skipped. */ const blockedImageIds = new Set(); @@ -1415,10 +1421,11 @@ class DockerController { continue; } } - const outcome = await this.executePlannedImage( + const { outcome, mutated: imageMutated } = await this.executePlannedImage( item, selfIdentity, ); outcomes.push(outcome); + if (imageMutated) planMutated = true; if (outcome.status === 'removed') reclaimedBytes += outcome.sizeBytes ?? item.sizeBytes ?? 0; } } catch (e) { @@ -1433,7 +1440,8 @@ class DockerController { } const success = outcomes.every((o) => o.status !== 'failed'); - return { outcomes, reclaimedBytes, success }; + const mutated = planMutated || outcomes.some((outcome) => outcome.status === 'removed'); + return { outcomes, reclaimedBytes, success, mutated }; } private async lookupContainerImageId(containerId: string): Promise { @@ -1588,35 +1596,66 @@ class DockerController { return { id: item.id, target: 'networks', status: 'removed' }; } + private static dockerIsNotFound(error: unknown): boolean { + const statusCode = (error as { statusCode?: number }).statusCode; + if (statusCode === 404) return true; + if (typeof statusCode === 'number') return false; + const message = error instanceof Error ? error.message : String(error); + return /no such image/i.test(message); + } + + private static listedImageMatch(images: T[], id: string): T | undefined { + return images.find((image) => image.Id === id || image.Id.startsWith(id) || id.startsWith(image.Id)); + } + + private static plannedImageResult( + outcome: PruneItemOutcome, + mutated = false, + ): { outcome: PruneItemOutcome; mutated: boolean } { + return { outcome, mutated }; + } + + private async listedImageById(id: string): Promise<{ Id: string; RepoTags?: string[] | null } | undefined> { + const images = await this.docker.listImages({ all: false }) as Array<{ + Id: string; + RepoTags?: string[] | null; + }>; + return DockerController.listedImageMatch(images, id); + } + private async executePlannedImage( item: PrunePlanItem, selfIdentity: SelfIdentityService, - ): Promise { + ): Promise<{ outcome: PruneItemOutcome; mutated: boolean }> { if (selfIdentity.isOwnImage(item.id)) { - return { id: item.id, target: 'images', status: 'skipped', reason: 'Sencho self image' }; + return DockerController.plannedImageResult({ + id: item.id, target: 'images', status: 'skipped', reason: 'Sencho self image', + }); } if (item.target !== 'images') { - return { id: item.id, target: 'images', status: 'failed', error: 'Plan item is not an image target' }; + return DockerController.plannedImageResult({ + id: item.id, target: 'images', status: 'failed', error: 'Plan item is not an image target', + }); } const plannedRefs = item.image.references; - const rawImages = await this.docker.listImages({ all: false }) as Array<{ - Id: string; - Size?: number; - RepoTags?: string[] | null; - }>; - const img = rawImages.find((i) => i.Id === item.id || i.Id.startsWith(item.id) || item.id.startsWith(i.Id)); + const img = await this.listedImageById(item.id); if (!img) { - return { id: item.id, target: 'images', status: 'skipped', reason: 'Image no longer exists' }; + return DockerController.plannedImageResult({ + id: item.id, target: 'images', status: 'skipped', reason: 'Image no longer exists', + }); + } + if (isFullySyntheticHoldImage(img.RepoTags ?? [])) { + return DockerController.plannedImageResult({ + id: item.id, target: 'images', status: 'skipped', reason: 'Sencho rollback-hold image', + }); } - // Defense in depth when fleet preflight/assertPlanFresh already matched: - // refuse mutation if live tags drifted from the reviewed reference set. if (!pruneImageReferencesEqual(plannedRefs, img.RepoTags)) { - return { + return DockerController.plannedImageResult({ id: item.id, target: 'images', status: 'failed', error: 'Image references changed since the plan was built; refresh and confirm again', - }; + }); } const allContainers = await this.docker.listContainers({ all: true }) as Array<{ ImageID?: string; @@ -1626,51 +1665,87 @@ class DockerController { || Boolean(container.ImageID?.startsWith(img.Id)) || img.Id.startsWith(container.ImageID ?? '')); if (containerRefs.length > 0) { - return { id: item.id, target: 'images', status: 'skipped', reason: 'Image still has container references' }; + return DockerController.plannedImageResult({ + id: item.id, target: 'images', status: 'skipped', reason: 'Image still has container references', + }); + } + + let mutated = false; + for (const ref of plannedRefs) { + try { + await this.docker.getImage(ref).remove({ force: false }); + mutated = true; + } catch (error) { + if (!DockerController.dockerIsNotFound(error)) break; + } + } + return this.completePlannedImage(item, plannedRefs, mutated); + } + + private static remainingImageFailure( + item: PrunePlanItem, + plannedRefs: string[], + remaining: string[], + ): PruneItemOutcome { + const prefix = remaining.some((ref) => !plannedRefs.includes(ref)) + ? 'Image still has repository tags that were not in the reviewed set: ' + : 'Image still has repository tags after prune: '; + return { + id: item.id, + target: 'images', + status: 'failed', + error: prefix + remaining.join(', ') + + '. Refresh the plan and confirm remaining references before pruning again.', + }; + } + + private async completePlannedImage( + item: PrunePlanItem, + plannedRefs: string[], + mutated: boolean, + ): Promise<{ outcome: PruneItemOutcome; mutated: boolean }> { + const removed = DockerController.plannedImageResult( + { id: item.id, target: 'images', status: 'removed', sizeBytes: item.sizeBytes ?? 0 }, + true, + ); + let listed: { Id: string; RepoTags?: string[] | null } | undefined; + try { + listed = await this.listedImageById(item.id); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + console.error( + `[executePrunePlan] Could not confirm remaining tags for ${sanitizeForLog(item.id)}:`, + sanitizeForLog(message), + ); + return DockerController.plannedImageResult({ + id: item.id, + target: 'images', + status: 'failed', + error: 'Could not confirm remaining image tags after prune. ' + + 'Refresh the plan and confirm remaining references before pruning again.', + }, mutated); + } + if (!listed) return removed; + const remaining = normalizePruneImageReferences(listed.RepoTags); + if (remaining.length > 0) { + return DockerController.plannedImageResult( + DockerController.remainingImageFailure(item, plannedRefs, remaining), + mutated, + ); } try { await this.docker.getImage(item.id).remove({ force: false }); - } catch (e) { - const message = e instanceof Error ? e.message : String(e); - // Re-read tags so a multi-repository conflict that partially untagged is not - // reported as a no-op when live names already diverged. - let afterTags: string[] | null | undefined; - try { - const afterList = await this.docker.listImages({ all: false }) as Array<{ - Id: string; - RepoTags?: string[] | null; - }>; - afterTags = afterList.find((i) => i.Id === img.Id || i.Id.startsWith(img.Id) || img.Id.startsWith(i.Id)) - ?.RepoTags; - } catch { - afterTags = undefined; - } - if (afterTags !== undefined && !pruneImageReferencesEqual(plannedRefs, afterTags)) { - return { - id: item.id, - target: 'images', - status: 'failed', - error: 'Image delete did not fully remove the reviewed image; some repository tags changed. ' - + 'Refresh the plan and confirm remaining references before pruning again. ' - + message, - }; - } - const lower = message.toLowerCase(); - if (lower.includes('multiple repositories') || lower.includes('must be forced') || lower.includes('conflict')) { - return { - id: item.id, - target: 'images', - status: 'failed', - error: 'Docker refused to delete this image because it is tagged in multiple repositories. ' - + 'Reviewed tags: ' + (plannedRefs.join(', ') || '(none)') + '. ' - + 'Resolve tags on the host or re-scope the plan, then dry-run again. ' - + message, - }; - } - throw e; + return removed; + } catch (error) { + if (DockerController.dockerIsNotFound(error)) return removed; + const message = error instanceof Error ? error.message : String(error); + return DockerController.plannedImageResult({ + id: item.id, + target: 'images', + status: 'failed', + error: 'Dangling image could not be removed after reviewed tags were cleared. ' + message, + }, mutated); } - // Prefer plan unique-bytes; do not fall back to full Size (shared layers). - return { id: item.id, target: 'images', status: 'removed', sizeBytes: item.sizeBytes ?? 0 }; } public async getDiskUsageClassified(knownStackNames: string[]): Promise<{ diff --git a/docs/features/resources.mdx b/docs/features/resources.mdx index 153eafa6..b8831141 100644 --- a/docs/features/resources.mdx +++ b/docs/features/resources.mdx @@ -172,4 +172,11 @@ When no unmanaged containers are detected, the tab shows a success state with th The banner starts from what Docker reports as reclaimable on the node, subtracts images Sencho holds as rollback protection points, and counts only the resources a standard prune can actually remove. Held images are never part of that total and never appear in a prune plan, so the residue after a prune is normally the storage driver still reporting a few megabytes that the prune did not free. Held images are listed in the **Rollback** tab and are freed once the hold is released or superseded. To stop the banner drawing attention to a stubborn remainder, dismiss it with the **×** in its top-right corner; it stays hidden until the reclaimable total grows past that point. To turn it off for the node entirely, switch off **Show reclaimable-space banner** in **Settings → Monitoring → Docker & Storage**. + + Unused images often carry more than one name, such as a version tag and `latest`. Sencho includes every reviewed tag in the prune confirmation. Confirming removes those tags one by one, and Docker deletes the image once no names remain. + + + Prune confirmation dialog listing an unused image with both a version tag and latest in the reviewed items + + diff --git a/docs/images/resources/resources-prune-confirm.png b/docs/images/resources/resources-prune-confirm.png new file mode 100644 index 00000000..b3f51025 Binary files /dev/null and b/docs/images/resources/resources-prune-confirm.png differ diff --git a/e2e/screenshots.spec.ts b/e2e/screenshots.spec.ts index c57bcf99..0ce17bac 100644 --- a/e2e/screenshots.spec.ts +++ b/e2e/screenshots.spec.ts @@ -192,3 +192,52 @@ test.describe('classified change-plan docs screenshots', () => { }, stackName); }); }); + +test.describe('resources prune confirm docs screenshot', () => { + test.use({ viewport: { width: 1920, height: 1080 } }); + + test('prune confirm lists extra repository tags', async ({ page }) => { + await page.route('**/system/prune/plan', async (route) => { + if (route.request().method() !== 'POST') { + await route.continue(); + return; + } + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ + scope: 'managed', + targets: ['images'], + items: [{ + target: 'images', + id: 'sha256:docs-prune-multi-tag', + name: 'ghcr.io/example/app:1.5.2', + sizeBytes: 48234496, + managed: true, + reason: 'Image is not used by any container', + image: { + references: [ + 'ghcr.io/example/app:1.5.2', + 'ghcr.io/example/app:latest', + ], + }, + }], + reclaimableBytes: 48234496, + fingerprint: 'fp-docs-prune-confirm', + createdAt: 1_700_000_000_000, + nodeId: 1, + }), + }); + }); + + await loginAs(page); + await page.getByRole('button', { name: /resources/i }).click(); + await page.getByRole('button', { name: /Prune Unused Images/ }).click(); + const pruneDialog = page.getByRole('alertdialog').filter({ hasText: 'Prune Sencho-managed images' }); + await expect(pruneDialog).toBeVisible(); + await expect(pruneDialog.getByText('ghcr.io/example/app:latest')).toBeVisible(); + await pruneDialog.screenshot({ + path: path.join(DOCS_IMAGES, 'resources', 'resources-prune-confirm.png'), + }); + }); +}); diff --git a/frontend/src/components/ResourcesView.tsx b/frontend/src/components/ResourcesView.tsx index b74b47ab..8ef1e6dc 100644 --- a/frontend/src/components/ResourcesView.tsx +++ b/frontend/src/components/ResourcesView.tsx @@ -126,16 +126,25 @@ function PrunePlanPreview({ {plan.items.length} {plan.items.length === 1 ? 'item' : 'items'} {plan.reclaimableBytes > 0 ? ` · ${formatBytes(plan.reclaimableBytes)}` : ''}

-
    - {shown.map((item) => ( -
  • - {item.target} - {' · '} - {item.name} - {item.sizeBytes != null && item.sizeBytes > 0 ? ` · ${formatBytes(item.sizeBytes)}` : ''} -
    • - ))} -
+ +
    + {shown.map((item) => ( +
  • + + {item.target} + {' · '} + {item.name} + {item.sizeBytes != null && item.sizeBytes > 0 ? ` · ${formatBytes(item.sizeBytes)}` : ''} + + {item.target === 'images' && item.image.references + .filter((ref) => ref !== item.name) + .map((ref) => ( + {ref} + ))} +
    • + ))} +
+ {remaining > 0 && (

and {remaining} more

)} diff --git a/frontend/src/components/__tests__/ResourcesView.test.tsx b/frontend/src/components/__tests__/ResourcesView.test.tsx index 980fe545..fb1de730 100644 --- a/frontend/src/components/__tests__/ResourcesView.test.tsx +++ b/frontend/src/components/__tests__/ResourcesView.test.tsx @@ -241,6 +241,43 @@ describe('ResourcesView', () => { expect(body.planFingerprint).toBe('fp-test'); }); + it('discloses extra repository tags in the prune confirm list', async () => { + mockedFetch.mockImplementation((url: string, opts?: RequestInit) => { + if (url === '/system/prune/plan' && opts?.method === 'POST') { + return Promise.resolve(jsonResponse(samplePrunePlan({ + items: [{ + target: 'images', + id: 'img-multi', + name: 'ghcr.io/example/very-long-app-name:1.5.2-build.1844', + sizeBytes: 1000, + managed: false, + reason: 'Image is not used by any container', + image: { + references: [ + 'ghcr.io/example/very-long-app-name:1.5.2-build.1844', + 'ghcr.io/example/very-long-app-name:latest', + ], + }, + }], + }))); + } + if (url === '/system/resources') { + return Promise.resolve(jsonResponse({ images: [], volumes: [], networks: [] })); + } + return Promise.resolve(jsonResponse({})); + }); + + const user = userEvent.setup(); + render(); + await waitFor(() => expect(mockedFetch).toHaveBeenCalledWith('/system/resources')); + await user.click(screen.getByRole('button', { name: /Prune Unused Images/ })); + + const extraRef = await screen.findByText('ghcr.io/example/very-long-app-name:latest'); + expect(extraRef).toHaveClass('break-all'); + expect(extraRef).not.toHaveClass('truncate'); + expect(screen.getByText(/1\.5\.2-build\.1844/)).toBeInTheDocument(); + }); + it('surfaces the server error on a failed prune instead of a false success (M-2)', async () => { mockedFetch.mockImplementation((url: string, opts?: RequestInit) => { if (url === '/system/prune/plan' && opts?.method === 'POST') {