Separate routes & actions, add tests for GET /v2/cleanup-volumes

Signed-off-by: Christina Ying Wang <christina@balena.io>

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);
+}

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] }),
+	);
+};

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) => {

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;
+	});
+});

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);
+		});
+	});
 });