diff --git a/src/compose/application-manager.ts b/src/compose/application-manager.ts index 051d65bb..38716693 100644 --- a/src/compose/application-manager.ts +++ b/src/compose/application-manager.ts @@ -1031,3 +1031,22 @@ export async function getState() { } return state; } + +export async function removeOrphanedVolumes( + inScope: (id: number) => boolean = () => true, +) { + const targetState = await getTargetApps(); + // Get list of referenced volumes to keep + const referencedVolumes = Object.values(targetState) + // Don't include volumes out of scope + .filter((app) => inScope(app.id)) + .flatMap((app) => { + const [release] = Object.values(app.releases); + // Return a list of the volume names + return Object.keys(release?.volumes ?? {}).map((volumeName) => + Volume.generateDockerName(app.id, volumeName), + ); + }); + + await volumeManager.removeOrphanedVolumes(referencedVolumes); +} diff --git a/src/device-api/actions.ts b/src/device-api/actions.ts index a949396c..e0d9ca06 100644 --- a/src/device-api/actions.ts +++ b/src/device-api/actions.ts @@ -2,6 +2,7 @@ import * as _ from 'lodash'; import { getGlobalApiKey, refreshKey } from '.'; import * as messages from './messages'; +import { AuthorizedRequest } from './api-keys'; import * as eventTracker from '../event-tracker'; import * as deviceState from '../device-state'; import * as logger from '../logger'; @@ -448,3 +449,18 @@ export const getDeviceTags = async () => { throw e; } }; + +/** + * Clean up orphaned volumes + * Used by: + * - GET /v2/cleanup-volumes + */ +export const cleanupVolumes = async ( + withScope: AuthorizedRequest['auth']['isScoped'] = () => true, +) => { + // It's better practice to access engine functionality through application-manager + // than through volume-manager directly, as the latter should be an internal module + await applicationManager.removeOrphanedVolumes((id) => + withScope({ apps: [id] }), + ); +}; diff --git a/src/device-api/v2.ts b/src/device-api/v2.ts index a155a73d..03e7bf5d 100644 --- a/src/device-api/v2.ts +++ b/src/device-api/v2.ts @@ -6,13 +6,11 @@ import * as deviceState from '../device-state'; import * as applicationManager from '../compose/application-manager'; import { CompositionStepAction } from '../compose/composition-steps'; import { Service } from '../compose/service'; -import Volume from '../compose/volume'; import * as commitStore from '../compose/commit'; import * as config from '../config'; import * as db from '../db'; import * as logger from '../logger'; import * as images from '../compose/images'; -import * as volumeManager from '../compose/volume-manager'; import * as serviceManager from '../compose/service-manager'; import { spawnJournalctl } from '../lib/journald'; import log from '../lib/supervisor-console'; @@ -523,23 +521,16 @@ router.get('/v2/device/vpn', async (_req, res, next) => { } }); -router.get('/v2/cleanup-volumes', async (req: AuthorizedRequest, res) => { - const targetState = await applicationManager.getTargetApps(); - const referencedVolumes = Object.values(targetState) - // if this app isn't in scope of the request, do not cleanup it's volumes - .filter((app) => req.auth.isScoped({ apps: [app.id] })) - .flatMap((app) => { - const [release] = Object.values(app.releases); - // Return a list of the volume names - return Object.keys(release?.volumes ?? {}).map((volumeName) => - Volume.generateDockerName(app.id, volumeName), - ); +// This should be a POST but we have to keep it a GET for interface consistency +router.get('/v2/cleanup-volumes', async (req: AuthorizedRequest, res, next) => { + try { + await actions.cleanupVolumes(req.auth.isScoped); + return res.json({ + status: 'success', }); - - await volumeManager.removeOrphanedVolumes(referencedVolumes); - res.json({ - status: 'success', - }); + } catch (e: unknown) { + next(e); + } }); router.post('/v2/journal-logs', (req, res) => { diff --git a/test/integration/device-api/actions.spec.ts b/test/integration/device-api/actions.spec.ts index 9a53ed05..13d09e2e 100644 --- a/test/integration/device-api/actions.spec.ts +++ b/test/integration/device-api/actions.spec.ts @@ -12,6 +12,7 @@ import * as deviceApi from '~/src/device-api'; import * as apiBinder from '~/src/api-binder'; import * as actions from '~/src/device-api/actions'; import * as TargetState from '~/src/device-state/target-state'; +import * as applicationManager from '~/src/compose/application-manager'; import { cleanupDocker } from '~/test-lib/docker-helper'; import { exec } from '~/src/lib/fs-utils'; @@ -894,3 +895,18 @@ describe('gets device tags', () => { expect(await actions.getDeviceTags()).to.deep.equal(fetchResponse); }); }); + +describe('cleans up orphaned volumes', () => { + let removeOrphanedVolumes: SinonStub; + before(() => { + removeOrphanedVolumes = stub(applicationManager, 'removeOrphanedVolumes'); + }); + after(() => { + removeOrphanedVolumes.restore(); + }); + + it('cleans up orphaned volumes through application-manager', async () => { + await actions.cleanupVolumes(); + expect(removeOrphanedVolumes).to.have.been.calledOnce; + }); +}); diff --git a/test/integration/device-api/v2.spec.ts b/test/integration/device-api/v2.spec.ts index 9610b814..bd4117fb 100644 --- a/test/integration/device-api/v2.spec.ts +++ b/test/integration/device-api/v2.spec.ts @@ -742,4 +742,31 @@ describe('device-api/v2', () => { .expect(500); }); }); + + describe('GET /v2/cleanup-volumes', () => { + // Actions are tested elsewhere so we can stub the dependency here + let cleanupVolumesStub: SinonStub; + before(() => { + cleanupVolumesStub = stub(actions, 'cleanupVolumes'); + }); + after(() => { + cleanupVolumesStub.restore(); + }); + + it('responds with 200', async () => { + cleanupVolumesStub.resolves(); + await request(api) + .get('/v2/cleanup-volumes') + .set('Authorization', `Bearer ${await deviceApi.getGlobalApiKey()}`) + .expect(200); + }); + + it('responds with 503 if an error occurred', async () => { + cleanupVolumesStub.throws(new Error()); + await request(api) + .get('/v2/cleanup-volumes') + .set('Authorization', `Bearer ${await deviceApi.getGlobalApiKey()}`) + .expect(503); + }); + }); });