From a66c200ffa4e2a9f4e96c354c31b432af24bde44 Mon Sep 17 00:00:00 2001 From: Michael Kriese Date: Mon, 26 Oct 2026 13:18:09 +0100 Subject: [PATCH] feat(cli): add `cleanup` command Co-Authored-By: Claude Opus 5.5 --- docs/tools.md | 34 ++++++++++++ src/cli/cleanup/index.spec.ts | 94 ++++++++++++++++++++++++++++++++- src/cli/cleanup/index.ts | 72 ++++++++++++++++++++++--- src/cli/command/cleanup.spec.ts | 86 ++++++++++++++++++++++++++++++ src/cli/command/cleanup.ts | 51 ++++++++++++++++++ src/cli/command/index.ts | 1 + test/node/Dockerfile | 33 ++++++++++++ 7 files changed, 362 insertions(+), 9 deletions(-) create mode 100644 src/cli/command/cleanup.spec.ts create mode 100644 src/cli/command/cleanup.ts diff --git a/docs/tools.md b/docs/tools.md index efa7a0fb7c..1a3ddf3089 100644 --- a/docs/tools.md +++ b/docs/tools.md @@ -17,6 +17,7 @@ - [List](#list) - [list-tools](#list-tools-command) - [Cleanup](#cleanup) + - [cleanup](#cleanup-command) - [cleanup-cache](#cleanup-cache-command) - [cleanup-downloads](#cleanup-downloads-command) - [cleanup-tool](#cleanup-tool-command) @@ -527,6 +528,39 @@ When writing to a file, JSON output is minified. ## Cleanup +### `containerbase-cli cleanup` + +#### Description + +Runs all cleanups in this order: + +1. [`cleanup tool`](#cleanup-tool-command) removes the unlinked tools +1. [`cleanup cache`](#cleanup-cache-command) removes the known tool caches +1. [`cleanup downloads`](#cleanup-downloads-command) evicts the oldest downloads + +`CONTAINERBASE_MAX_ALLOCATED_DISK` defaults to `100`, so downloads are only evicted on a full disk, set a lower value, eg. `80`, to evict earlier, see [download cache](../README.md#download-cache). + +When a step fails, it's logged and the next step still runs. +The command fails at the end when any step failed. +It fails right away, without running any step, when the tools can't be loaded, eg. because the containerbase folders aren't writable. + +#### Usage + +`$ containerbase-cli cleanup [-d,--dry-run]` + +#### Examples + +- Runs all cleanups + ```bash + containerbase-cli cleanup + ``` +- Logs what all cleanups would remove + ```bash + containerbase-cli cleanup --dry-run + ``` + +
+ ### `containerbase-cli cleanup cache` #### Description diff --git a/src/cli/cleanup/index.spec.ts b/src/cli/cleanup/index.spec.ts index 533406998c..ff41111a3f 100644 --- a/src/cli/cleanup/index.spec.ts +++ b/src/cli/cleanup/index.spec.ts @@ -8,8 +8,16 @@ import { VersionService, createContainer, } from '../services/index.ts'; +import { NotSupported } from '../utils/codes.ts'; import { logger, pathExists } from '../utils/index.ts'; -import { cleanupCache, cleanupDownloads, cleanupTools } from './index.ts'; +import { CleanupCacheService } from './cleanup-cache.service.ts'; +import { CleanupToolService } from './cleanup-tool.service.ts'; +import { + cleanupAll, + cleanupCache, + cleanupDownloads, + cleanupTools, +} from './index.ts'; import { ensurePaths, rootPath } from '~test/path.ts'; vi.mock('del'); @@ -137,4 +145,88 @@ describe('cli/cleanup/index', () => { expect(logger.info).toHaveBeenCalledExactlyOnceWith('Nothing to evict'); }); }); + + describe('cleanupAll', () => { + test('runs all cleanups in order', async () => { + const tools = vi + .spyOn(CleanupToolService.prototype, 'cleanupTools') + .mockResolvedValue(); + const cache = vi + .spyOn(CleanupCacheService.prototype, 'cleanupCache') + .mockResolvedValue([]); + const downloads = vi + .spyOn(CacheService.prototype, 'evict') + .mockResolvedValue([]); + // called once per install container + const containers = vi.spyOn(PathService.prototype, 'findLegacyTools'); + + expect(await cleanupAll(true)).toBe(0); + + expect(containers).toHaveBeenCalledOnce(); + expect(tools).toHaveBeenCalledExactlyOnceWith(true); + expect(cache).toHaveBeenCalledExactlyOnceWith(true); + expect(downloads).toHaveBeenCalledExactlyOnceWith(true); + // the per step summaries are logged once + expect(vi.mocked(logger.info).mock.calls).toEqual([ + ['Cleanup tool ...'], + ['Cleanup cache ...'], + ['Dry run: would remove 0 cache paths'], + ['Cleanup downloads ...'], + ['Nothing to evict'], + ]); + }); + + test('continues after a failed step', async () => { + vi.spyOn(CleanupToolService.prototype, 'cleanupTools').mockResolvedValue( + NotSupported, + ); + const err = new Error('cache failed'); + vi.spyOn(CleanupCacheService.prototype, 'cleanupCache').mockRejectedValue( + err, + ); + const downloads = vi + .spyOn(CacheService.prototype, 'evict') + .mockResolvedValue([]); + + expect(await cleanupAll(true)).toBe(NotSupported); + + expect(downloads).toHaveBeenCalledOnce(); + expect(logger.warn).toHaveBeenCalledWith( + { code: NotSupported }, + 'Cleanup tool failed', + ); + expect(logger.warn).toHaveBeenCalledWith({ err }, 'Cleanup cache failed'); + }); + + test('returns 1 when a step threw', async () => { + vi.spyOn( + CleanupToolService.prototype, + 'cleanupTools', + ).mockResolvedValue(); + vi.spyOn(CleanupCacheService.prototype, 'cleanupCache').mockResolvedValue( + [], + ); + vi.spyOn(CacheService.prototype, 'evict').mockRejectedValue( + new Error('evict failed'), + ); + + expect(await cleanupAll(true)).toBe(1); + }); + + test('fails without running a step when the container cannot be created', async () => { + vi.spyOn(VersionService.prototype, 'getTypes').mockRejectedValueOnce( + new Error('db failed'), + ); + const tools = vi.spyOn(CleanupToolService.prototype, 'cleanupTools'); + const cache = vi.spyOn(CleanupCacheService.prototype, 'cleanupCache'); + const downloads = vi.spyOn(CacheService.prototype, 'evict'); + + await expect(cleanupAll(true)).rejects.toThrow('db failed'); + + expect(tools).not.toHaveBeenCalled(); + expect(cache).not.toHaveBeenCalled(); + expect(downloads).not.toHaveBeenCalled(); + expect(logger.info).not.toHaveBeenCalled(); + }); + }); }); diff --git a/src/cli/cleanup/index.ts b/src/cli/cleanup/index.ts index 684b38786b..d4ac8e2289 100644 --- a/src/cli/cleanup/index.ts +++ b/src/cli/cleanup/index.ts @@ -1,3 +1,4 @@ +import type { Container } from 'inversify'; import { prepareInstallContainer, prepareUninstallContainer, @@ -7,18 +8,30 @@ import { logger } from '../utils/index.ts'; import { CleanupCacheService } from './cleanup-cache.service.ts'; import { CleanupToolService } from './cleanup-tool.service.ts'; +/** Binds the cleanup services to an install container. */ +function bindCleanupServices(container: Container): Container { + container.bind(CleanupCacheService).toSelf(); + container.bind(CleanupToolService).toSelf(); + return container; +} + /** * Uninstalls all unlinked tool versions, see * `CleanupToolService.cleanupTools`. Generic install services are registered * for every installed `gem`, `npm` or `pip` package. * + * @param container a container from `prepareUninstallContainer` with the + * cleanup services to reuse, else a new one is created * @throws when the containerbase folders are not writable, except for a dry * run */ -export async function cleanupTools(dryRun = false): Promise { - const container = await prepareUninstallContainer(dryRun); - container.bind(CleanupToolService).toSelf(); - const svc = await container.getAsync(CleanupToolService); +export async function cleanupTools( + dryRun = false, + container?: Container, +): Promise { + const svc = await ( + container ?? bindCleanupServices(await prepareUninstallContainer(dryRun)) + ).getAsync(CleanupToolService); return svc.cleanupTools(dryRun); } @@ -26,12 +39,17 @@ export async function cleanupTools(dryRun = false): Promise { * Removes the known tool caches, see `CleanupCacheService.cleanupCache`, and * logs how many were removed. * + * @param container an install container with the cleanup services to reuse, + * else a new one is created * @returns the removed paths, with `dryRun` the paths which would be removed */ -export async function cleanupCache(dryRun = false): Promise { - const container = await prepareInstallContainer(); - container.bind(CleanupCacheService).toSelf(); - const svc = await container.getAsync(CleanupCacheService); +export async function cleanupCache( + dryRun = false, + container?: Container, +): Promise { + const svc = await ( + container ?? bindCleanupServices(await prepareInstallContainer()) + ).getAsync(CleanupCacheService); const removed = await svc.cleanupCache(dryRun); logger.info( dryRun @@ -59,3 +77,41 @@ export async function cleanupDownloads(dryRun = false): Promise { } return removed; } + +/** + * Runs all cleanups in order: the unlinked tools, the tool caches and the + * download cache, see `cleanupTools`, `cleanupCache` and `cleanupDownloads`. + * A failing step is logged and the next one still runs. The tool and cache + * cleanups share one install container, created before any step runs. + * + * @returns the exit code of the first failed step, `1` when it threw, else `0` + * @throws when the install container can't be created, eg. because the + * containerbase folders are not writable, then no step runs + */ +export async function cleanupAll(dryRun = false): Promise { + const container = bindCleanupServices( + await prepareUninstallContainer(dryRun), + ); + const steps: [string, () => Promise][] = [ + ['tool', () => cleanupTools(dryRun, container)], + ['cache', () => cleanupCache(dryRun, container)], + ['downloads', () => cleanupDownloads(dryRun)], + ]; + let code = 0; + for (const [name, step] of steps) { + logger.info(`Cleanup ${name} ...`); + let res: unknown; + try { + res = await step(); + } catch (err) { + logger.warn({ err }, `Cleanup ${name} failed`); + code ||= 1; + continue; + } + if (typeof res === 'number' && res) { + logger.warn({ code: res }, `Cleanup ${name} failed`); + code ||= res; + } + } + return code; +} diff --git a/src/cli/command/cleanup.spec.ts b/src/cli/command/cleanup.spec.ts new file mode 100644 index 0000000000..123462d160 --- /dev/null +++ b/src/cli/command/cleanup.spec.ts @@ -0,0 +1,86 @@ +import { describe, expect, test, vi } from 'vitest'; +import { logger } from '../utils/index.ts'; +import { testCli } from '~test/di.ts'; + +const mocks = vi.hoisted(() => ({ + cleanupAll: vi.fn(), + cleanupCache: vi.fn(), + cleanupDownloads: vi.fn(), + cleanupTools: vi.fn(), +})); + +const del = vi.hoisted(() => ({ deleteAsync: vi.fn() })); + +vi.mock('../cleanup/index.ts', () => mocks); +vi.mock('del', () => del); + +describe('cli/command/cleanup', () => { + test('works', async () => { + const cli = testCli('containerbase-cli'); + mocks.cleanupAll.mockResolvedValueOnce(0); + + expect(await cli.run(['cleanup'])).toBe(0); + + expect(mocks.cleanupAll).toHaveBeenCalledExactlyOnceWith(false); + expect(logger.info).toHaveBeenCalledWith( + expect.stringContaining('Cleanup succeeded'), + ); + }); + + test('dry run', async () => { + const cli = testCli('containerbase-cli'); + mocks.cleanupAll.mockResolvedValueOnce(0); + + expect(await cli.run(['cleanup', '--dry-run'])).toBe(0); + + expect(mocks.cleanupAll).toHaveBeenCalledExactlyOnceWith(true); + }); + + test('fails', async () => { + const cli = testCli('containerbase-cli'); + mocks.cleanupAll.mockResolvedValueOnce(2); + + expect(await cli.run(['cleanup'])).toBe(2); + + expect(logger.fatal).toHaveBeenCalledWith( + expect.stringContaining('Cleanup failed'), + ); + }); + + test('fails when the cleanups cannot start', async () => { + const cli = testCli('containerbase-cli'); + mocks.cleanupAll.mockRejectedValueOnce(new Error('not writable')); + + expect(await cli.run(['cleanup'])).toBe(1); + + expect(logger.error).toHaveBeenCalledWith('not writable'); + expect(logger.fatal).toHaveBeenCalledWith( + expect.stringContaining('Cleanup failed'), + ); + + // a rejection which is not an `Error` has no message to report + mocks.cleanupAll.mockRejectedValueOnce('boom'); + expect(await cli.run(['cleanup'])).toBe(1); + expect(logger.debug).toHaveBeenCalledWith('boom'); + expect(logger.error).not.toHaveBeenCalledWith('boom'); + }); + + test('keeps the sub commands', async () => { + const cli = testCli('containerbase-cli'); + mocks.cleanupCache.mockResolvedValue([]); + mocks.cleanupDownloads.mockResolvedValue([]); + + expect(await cli.run(['cleanup', 'cache'])).toBe(0); + expect(await cli.run(['cleanup', 'downloads'])).toBe(0); + expect(await cli.run(['cleanup', 'tool'])).toBe(0); + expect(await cli.run(['cleanup', 'path', '/tmp/a'])).toBe(0); + + expect(mocks.cleanupCache).toHaveBeenCalledExactlyOnceWith(false); + expect(mocks.cleanupDownloads).toHaveBeenCalledExactlyOnceWith(false); + expect(mocks.cleanupTools).toHaveBeenCalledExactlyOnceWith(false); + expect(del.deleteAsync).toHaveBeenCalledExactlyOnceWith(['/tmp/a'], { + dot: true, + }); + expect(mocks.cleanupAll).not.toHaveBeenCalled(); + }); +}); diff --git a/src/cli/command/cleanup.ts b/src/cli/command/cleanup.ts new file mode 100644 index 0000000000..7e983396dc --- /dev/null +++ b/src/cli/command/cleanup.ts @@ -0,0 +1,51 @@ +import { Command, Option } from 'clipanion'; +import prettyMilliseconds from 'pretty-ms'; +import { cleanupAll } from '../cleanup/index.ts'; +import { logger } from '../utils/index.ts'; +import { command } from './utils.ts'; + +@command('containerbase-cli') +export class CleanupCommand extends Command { + static override paths = [['cleanup']]; + + static override usage = Command.Usage({ + description: + 'Removes the unlinked tools and the tool caches, and evicts the oldest downloads.', + examples: [ + ['Runs all cleanups', '$0 cleanup'], + ['Logs what all cleanups would remove', '$0 cleanup --dry-run'], + ], + }); + + dryRun = Option.Boolean('-d,--dry-run', false); + + /** + * Runs the tool, cache and downloads cleanups, returning a non-zero exit + * code when any of them failed or they could not start. + */ + async execute(): Promise { + const start = Date.now(); + let res = 1; + try { + res = await cleanupAll(this.dryRun); + return res; + } catch (err) { + logger.debug(err); + if (err instanceof Error) { + logger.error(err.message); + } + return res; + /* v8 ignore next -- coverage bug */ + } finally { + if (res) { + logger.fatal( + `Cleanup failed in ${prettyMilliseconds(Date.now() - start)}.`, + ); + } else { + logger.info( + `Cleanup succeeded in ${prettyMilliseconds(Date.now() - start)}.`, + ); + } + } + } +} diff --git a/src/cli/command/index.ts b/src/cli/command/index.ts index b33463ed4b..0dc0bc1f9c 100644 --- a/src/cli/command/index.ts +++ b/src/cli/command/index.ts @@ -2,6 +2,7 @@ import './cleanup-cache.ts'; import './cleanup-downloads.ts'; import './cleanup-path.ts'; import './cleanup-tool.ts'; +import './cleanup.ts'; import './file-download.ts'; import './file-exists.ts'; import './init-tool.ts'; diff --git a/test/node/Dockerfile b/test/node/Dockerfile index f682eb368d..6066901dcc 100644 --- a/test/node/Dockerfile +++ b/test/node/Dockerfile @@ -819,6 +819,38 @@ RUN set -ex; \ [[ "${out}" == *"tmp=/tmp/containerbase"* ]]; \ [[ -d /tmp/containerbase && ! -L /tmp/containerbase ]] +#-------------------------------------- +# test: cleanup +#-------------------------------------- +FROM build AS test-cleanup + +# don't update!! the older version is left unlinked +RUN install-tool pnpm 10.34.7 + +# renovate: datasource=npm depName=pnpm +ARG PNPM_VERSION=12.15.0 +RUN install-tool pnpm "${PNPM_VERSION}" + +RUN set -ex; \ + mkdir -p "${USER_HOME}/.npm/_cacache" "${USER_HOME}/.local/share/pnpm/store/v10"; \ + touch "${USER_HOME}/.npm/_cacache/marker" "${USER_HOME}/.local/share/pnpm/store/v10/marker" + +RUN set -ex; \ + containerbase-cli cleanup --dry-run; \ + [[ -d /opt/containerbase/tools/pnpm/10.34.7 ]]; \ + [[ -f "${USER_HOME}/.npm/_cacache/marker" ]]; \ + [[ -f "${USER_HOME}/.local/share/pnpm/store/v10/marker" ]] + +RUN set -ex; \ + containerbase-cli cleanup; \ + [[ ! -e /opt/containerbase/tools/pnpm/10.34.7 ]]; \ + [[ ! -e "${USER_HOME}/.npm/_cacache" ]]; \ + [[ -L "${USER_HOME}/.npm" ]]; \ + [[ ! -e "${USER_HOME}/.local/share/pnpm/store" ]]; \ + [[ -d "${USER_HOME}/.local/share/pnpm" ]]; \ + node --version; \ + pnpm --version | grep "^${PNPM_VERSION}$" + #-------------------------------------- # test: vp #-------------------------------------- @@ -862,6 +894,7 @@ COPY --from=testo /.dummy /.dummy COPY --from=testp /.dummy /.dummy COPY --from=testq /.dummy /.dummy COPY --from=test-tmp-dir /.dummy /.dummy +COPY --from=test-cleanup /.dummy /.dummy COPY --from=test-vp /.dummy /.dummy COPY --from=test-v20 /.dummy /.dummy