Infer legacy Volumes that do not have the supervised label

Change-type: patch
Closes: #1604
Signed-off-by: Miguel Casqueira <miguel@balena.io>

src/compose/volume-manager.ts

@@ -3,11 +3,12 @@ import * as Path from 'path';
 import { VolumeInspectInfo } from 'dockerode';
 
 import constants = require('../lib/constants');
-import { NotFoundError } from '../lib/errors';
+import { NotFoundError, InternalInconsistencyError } from '../lib/errors';
 import { safeRename } from '../lib/fs-utils';
 import { docker } from '../lib/docker-utils';
 import * as LogTypes from '../lib/log-types';
 import { defaultLegacyVolume } from '../lib/migration';
+import log from '../lib/supervisor-console';
 import * as logger from '../logger';
 import { ResourceRecreationAttemptError } from './errors';
 import Volume, { VolumeConfig } from './volume';
@@ -24,8 +25,21 @@ export async function get({ name, appId }: VolumeNameOpts): Promise<Volume> {
 }
 
 export async function getAll(): Promise<Volume[]> {
-	const volumeInspect = await listWithBothLabels();
-	return volumeInspect.map((inspect) => Volume.fromDockerVolume(inspect));
+	const volumes = await list();
+	// Normalize inspect information to Volume types and filter any that fail
+	return volumes.reduce((volumesList, volumeInfo) => {
+		try {
+			const volume = Volume.fromDockerVolume(volumeInfo);
+			volumesList.push(volume);
+		} catch (err) {
+			if (err instanceof InternalInconsistencyError) {
+				log.debug(`Cannot parse Volume: ${volumeInfo.Name}`);
+			} else {
+				throw err;
+			}
+		}
+		return volumesList;
+	}, [] as Volume[]);
 }
 
 export async function getAllByAppId(appId: number): Promise<Volume[]> {
@@ -139,17 +153,7 @@ export async function removeOrphanedVolumes(
 	await Promise.all(volumesToRemove.map((v) => docker.getVolume(v).remove()));
 }
 
-async function listWithBothLabels(): Promise<VolumeInspectInfo[]> {
-	const [legacyResponse, currentResponse] = await Promise.all([
-		docker.listVolumes({
-			filters: { label: ['io.resin.supervised'] },
-		}),
-		docker.listVolumes({
-			filters: { label: ['io.balena.supervised'] },
-		}),
-	]);
-
-	const legacyVolumes = _.get(legacyResponse, 'Volumes', []);
-	const currentVolumes = _.get(currentResponse, 'Volumes', []);
-	return _.unionBy(legacyVolumes, currentVolumes, 'Name');
+async function list(): Promise<VolumeInspectInfo[]> {
+	const allVolumes = await docker.listVolumes();
+	return _.get(allVolumes, 'Volumes', []);
 }

test/28-db-format.spec.ts

@@ -44,8 +44,8 @@ describe('DB Format', () => {
 				source: apiEndpoint,
 				releaseId: 123,
 				services: '[]',
-				networks: '{}',
-				volumes: '{}',
+				networks: '[]',
+				volumes: '[]',
 			},
 			{
 				appId: 2,
@@ -68,8 +68,8 @@ describe('DB Format', () => {
 						commit: 'abcdef2',
 					},
 				]),
-				networks: '{}',
-				volumes: '{}',
+				networks: '[]',
+				volumes: '[]',
 			},
 		]);
 	});

test/34-firewall.spec.ts

@@ -180,8 +180,8 @@ describe('Host Firewall', function () {
 									commit: 'abcdef2',
 								},
 							]),
-							networks: '{}',
-							volumes: '{}',
+							networks: '[]',
+							volumes: '[]',
 						},
 					]);
 
@@ -233,8 +233,8 @@ describe('Host Firewall', function () {
 									commit: 'abcdef2',
 								},
 							]),
-							networks: '{}',
-							volumes: '{}',
+							networks: '[]',
+							volumes: '[]',
 						},
 					]);
 

test/41-device-api-v1.spec.ts

@@ -65,8 +65,8 @@ describe('SupervisorAPI [V1 Endpoints]', () => {
 			source: 'https://api.balena-cloud.com',
 			releaseId: 1232,
 			services: JSON.stringify(services),
-			networks: '{}',
-			volumes: '{}',
+			networks: '[]',
+			volumes: '[]',
 		});
 	});
 
@@ -326,8 +326,8 @@ describe('SupervisorAPI [V1 Endpoints]', () => {
 				source: 'https://api.balena-cloud.com',
 				releaseId: 1232,
 				services: JSON.stringify([service]),
-				volumes: '{}',
-				networks: '{}',
+				volumes: '[]',
+				networks: '[]',
 			});
 
 			// Perform the test with our mocked release

test/42-device-api-v2.spec.ts

@@ -321,8 +321,8 @@ describe('SupervisorAPI [V2 Endpoints]', () => {
 				source: 'https://api.balena-cloud.com',
 				releaseId: 1232,
 				services: JSON.stringify([service]),
-				networks: '{}',
-				volumes: '{}',
+				networks: '[]',
+				volumes: '[]',
 			});
 
 			lockMock.reset();
@@ -426,8 +426,8 @@ describe('SupervisorAPI [V2 Endpoints]', () => {
 				source: 'https://api.balena-cloud.com',
 				releaseId: 1232,
 				services: JSON.stringify(mockContainers),
-				networks: '{}',
-				volumes: '{}',
+				networks: '[]',
+				volumes: '[]',
 			});
 
 			lockMock.reset();

test/44-volume-manager.spec.ts

@@ -0,0 +1,171 @@
+import { expect } from 'chai';
+import { stub, SinonStub } from 'sinon';
+
+import * as mockedDockerode from './lib/mocked-dockerode';
+import * as volumeManager from '../src/compose/volume-manager';
+import log from '../src/lib/supervisor-console';
+import Volume from '../src/compose/volume';
+
+describe('Volume Manager', () => {
+	let logDebug: SinonStub;
+	before(() => {
+		logDebug = stub(log, 'debug');
+	});
+	after(() => {
+		logDebug.restore();
+	});
+
+	afterEach(() => {
+		// Clear Dockerode actions recorded for each test
+		mockedDockerode.resetHistory();
+		logDebug.reset();
+	});
+
+	it('gets all supervised Volumes', async () => {
+		// Setup volume data
+		const volumeData = [
+			createVolumeInspectInfo(Volume.generateDockerName(1, 'redis'), {
+				'io.balena.supervised': '1', // Recently created volumes contain io.balena.supervised label
+			}),
+			createVolumeInspectInfo(Volume.generateDockerName(1, 'mysql'), {
+				'io.balena.supervised': '1', // Recently created volumes contain io.balena.supervised label
+			}),
+			createVolumeInspectInfo(Volume.generateDockerName(1, 'backend')), // Old Volumes will not have labels
+			createVolumeInspectInfo('user_created_volume'), // Volume not created by the Supervisor
+			createVolumeInspectInfo('decoy', { 'io.balena.supervised': '1' }), // Added decoy to really test the inference (should not return)
+		];
+		// Perform test
+		await mockedDockerode.testWithData({ volumes: volumeData }, async () => {
+			await expect(volumeManager.getAll()).to.eventually.deep.equal([
+				{
+					appId: 1,
+					config: {
+						driver: 'local',
+						driverOpts: {},
+						labels: {
+							'io.balena.supervised': '1',
+						},
+					},
+					name: 'redis',
+				},
+				{
+					appId: 1,
+					config: {
+						driver: 'local',
+						driverOpts: {},
+						labels: {
+							'io.balena.supervised': '1',
+						},
+					},
+					name: 'mysql',
+				},
+				{
+					appId: 1,
+					config: {
+						driver: 'local',
+						driverOpts: {},
+						labels: {},
+					},
+					name: 'backend',
+				},
+			]);
+			// Check that debug message was logged saying we found a Volume not created by us
+			expect(logDebug.lastCall.lastArg).to.equal('Cannot parse Volume: decoy');
+		});
+	});
+
+	it('gets a Volume for an application', async () => {
+		// Setup volume data
+		const volumeData = [
+			createVolumeInspectInfo(Volume.generateDockerName(111, 'app'), {
+				'io.balena.supervised': '1',
+			}),
+			createVolumeInspectInfo(Volume.generateDockerName(222, 'otherApp'), {
+				'io.balena.supervised': '1',
+			}),
+		];
+		// Perform test
+		await mockedDockerode.testWithData({ volumes: volumeData }, async () => {
+			await expect(volumeManager.getAllByAppId(111)).to.eventually.deep.equal([
+				{
+					appId: 111,
+					config: {
+						driver: 'local',
+						driverOpts: {},
+						labels: {
+							'io.balena.supervised': '1',
+						},
+					},
+					name: 'app',
+				},
+			]);
+		});
+	});
+
+	it('creates a Volume', async () => {
+		// Setup volume data
+		const volumeData: Dictionary<any> = [];
+		// Perform test
+		await mockedDockerode.testWithData({ volumes: volumeData }, async () => {
+			// Volume to create
+			const volume = Volume.fromComposeObject('main', 111, {});
+			stub(volume, 'create');
+			// Create volume
+			await volumeManager.create(volume);
+			// Check volume was created
+			expect(volume.create as SinonStub).to.be.calledOnce;
+		});
+	});
+
+	it('does not try to create a volume that already exists', async () => {
+		// Setup volume data
+		const volumeData = [
+			createVolumeInspectInfo(Volume.generateDockerName(111, 'main'), {
+				'io.balena.supervised': '1',
+			}),
+		];
+		// Perform test
+		await mockedDockerode.testWithData({ volumes: volumeData }, async () => {
+			// Volume to try again create
+			const volume = Volume.fromComposeObject('main', 111, {});
+			stub(volume, 'create');
+			// Create volume
+			await volumeManager.create(volume);
+			// Check volume was not created
+			expect(volume.create as SinonStub).to.not.be.called;
+		});
+	});
+
+	it('removes a Volume', async () => {
+		// Setup volume data
+		const volumeData = [
+			createVolumeInspectInfo(Volume.generateDockerName(111, 'main'), {
+				'io.balena.supervised': '1',
+			}),
+		];
+		// Perform test
+		await mockedDockerode.testWithData({ volumes: volumeData }, async () => {
+			// Volume to remove
+			const volume = Volume.fromComposeObject('main', 111, {});
+			stub(volume, 'remove');
+			// Remove volume
+			await volumeManager.remove(volume);
+			// Check volume was removed
+			expect(volume.remove as SinonStub).to.be.calledOnce;
+		});
+	});
+});
+
+function createVolumeInspectInfo(
+	name: string,
+	labels: { [key: string]: string } = {},
+	driver: string = 'local',
+	options: { [key: string]: string } | null = null,
+) {
+	return {
+		Name: name,
+		Driver: driver,
+		Labels: labels,
+		Options: options,
+	};
+}

test/lib/mocked-dockerode.ts

@@ -26,7 +26,12 @@ export function resetHistory() {
 	actions = [];
 }
 
-function addAction(name: string, parameters: Dictionary<any>) {
+/**
+ * Tracks actions performed on a mocked dockerode instance
+ * @param name action called
+ * @param parameters data passed
+ */
+function addAction(name: string, parameters: Dictionary<any> = {}) {
 	actions.push({
 		name,
 		parameters,
@@ -52,7 +57,6 @@ for (const fn of Object.getOwnPropertyNames(dockerode.prototype)) {
 }
 
 // default overrides needed to startup...
-registerOverride('listImages', async () => []);
 registerOverride(
 	'getEvents',
 	async () =>
@@ -63,6 +67,11 @@ registerOverride(
 		}),
 );
 
+/**
+ * Used to add or modifying functions on the mocked dockerode
+ * @param name function name to override
+ * @param fn function to execute
+ */
 export function registerOverride<
 	T extends DockerodeFunction,
 	P extends Parameters<dockerode[T]>,
@@ -76,25 +85,65 @@ export interface TestData {
 	networks: Dictionary<any>;
 	images: Dictionary<any>;
 	containers: Dictionary<any>;
+	volumes: Dictionary<any>;
 }
 
 function createMockedDockerode(data: TestData) {
 	const mockedDockerode = dockerode.prototype;
+
+	mockedDockerode.listImages = async () => [];
+
+	mockedDockerode.listVolumes = async () => {
+		addAction('listVolumes');
+		return {
+			Volumes: data.volumes as dockerode.VolumeInspectInfo[],
+			Warnings: [],
+		};
+	};
+
+	mockedDockerode.getVolume = (name: string) => {
+		addAction('getVolume');
+		const picked = data.volumes.filter((v: Dictionary<any>) => v.Name === name);
+		if (picked.length !== 1) {
+			throw new NotFoundError();
+		}
+		const volume = picked[0];
+		return {
+			...volume,
+			inspect: async () => {
+				addAction('inspect');
+				// TODO fully implement volume inspect.
+				// This should return VolumeInspectInfo not Volume
+				return volume;
+			},
+			remove: async (options?: {}) => {
+				addAction('remove', options);
+				data.volumes = _.reject(data.volumes, { name: volume.name });
+			},
+			name: volume.name,
+			modem: {},
+		} as dockerode.Volume;
+	};
+
 	mockedDockerode.createContainer = async (
 		options: dockerode.ContainerCreateOptions,
 	) => {
 		addAction('createContainer', { options });
 		return {
 			start: async () => {
-				addAction('start', {});
+				addAction('start');
 			},
 		} as dockerode.Container;
 	};
+
 	mockedDockerode.getContainer = (id: string) => {
 		addAction('getContainer', { id });
 		return {
+			inspect: async () => {
+				return data.containers.filter((c: Dictionary<any>) => c.id === id);
+			},
 			start: async () => {
-				addAction('start', {});
+				addAction('start');
 				data.containers = data.containers.map((c: any) => {
 					if (c.containerId === id) {
 						c.status = 'Installing';
@@ -103,7 +152,7 @@ function createMockedDockerode(data: TestData) {
 				});
 			},
 			stop: async () => {
-				addAction('stop', {});
+				addAction('stop');
 				data.containers = data.containers.map((c: any) => {
 					if (c.containerId === id) {
 						c.status = 'Stopping';
@@ -112,7 +161,7 @@ function createMockedDockerode(data: TestData) {
 				});
 			},
 			remove: async () => {
-				addAction('remove', {});
+				addAction('remove');
 				data.containers = data.containers.map((c: any) => {
 					if (c.containerId === id) {
 						c.status = 'removing';
@@ -122,11 +171,12 @@ function createMockedDockerode(data: TestData) {
 			},
 		} as dockerode.Container;
 	};
+
 	mockedDockerode.getNetwork = (id: string) => {
 		addAction('getNetwork', { id });
 		return {
 			inspect: async () => {
-				addAction('inspect', {});
+				addAction('inspect');
 				return data.networks[id];
 			},
 		} as dockerode.Network;
@@ -136,11 +186,11 @@ function createMockedDockerode(data: TestData) {
 		addAction('getImage', { name });
 		return {
 			inspect: async () => {
-				addAction('inspect', {});
+				addAction('inspect');
 				return data.images[name];
 			},
 			remove: async () => {
-				addAction('remove', {});
+				addAction('remove');
 				data.images = _.reject(data.images, {
 					name,
 				});
@@ -157,9 +207,10 @@ export async function testWithData(
 ) {
 	const mockedData: TestData = {
 		...{
-			networks: {},
-			images: {},
-			containers: {},
+			networks: [],
+			images: [],
+			containers: [],
+			volumes: [],
 		},
 		...data,
 	};