Merge pull request #1753 from balena-os/no-db-ids

Remove comparisons based on image, release, and service ids

src/compose/app.ts

@@ -31,7 +31,6 @@ export interface AppConstructOpts {
 	appId: number;
 	appName?: string;
 	commit?: string;
-	releaseId?: number;
 	source?: string;
 
 	services: Service[];
@@ -43,7 +42,7 @@ export interface UpdateState {
 	localMode: boolean;
 	availableImages: Image[];
 	containerIds: Dictionary<string>;
-	downloading: number[];
+	downloading: string[];
 }
 
 interface ChangingPair<T> {
@@ -56,7 +55,6 @@ export class App {
 	// When setting up an application from current state, these values are not available
 	public appName?: string;
 	public commit?: string;
-	public releaseId?: number;
 	public source?: string;
 
 	// Services are stored as an array, as at any one time we could have more than one
@@ -69,7 +67,6 @@ export class App {
 		this.appId = opts.appId;
 		this.appName = opts.appName;
 		this.commit = opts.commit;
-		this.releaseId = opts.releaseId;
 		this.source = opts.source;
 		this.services = opts.services;
 		this.volumes = opts.volumes;
@@ -266,34 +263,30 @@ export class App {
 		removePairs: Array<ChangingPair<Service>>;
 		updatePairs: Array<ChangingPair<Service>>;
 	} {
-		const currentByServiceId = _.keyBy(current, 'serviceId');
+		const currentByServiceName = _.keyBy(current, 'serviceName');
-		const targetByServiceId = _.keyBy(target, 'serviceId');
+		const targetByServiceName = _.keyBy(target, 'serviceName');
 
-		const currentServiceIds = Object.keys(currentByServiceId).map((i) =>
+		const currentServiceNames = Object.keys(currentByServiceName);
-			parseInt(i, 10),
+		const targetServiceNames = Object.keys(targetByServiceName);
-		);
-		const targetServiceIds = Object.keys(targetByServiceId).map((i) =>
-			parseInt(i, 10),
-		);
 
-		const toBeRemoved = _(currentServiceIds)
+		const toBeRemoved = _(currentServiceNames)
-			.difference(targetServiceIds)
+			.difference(targetServiceNames)
-			.map((id) => ({ current: currentByServiceId[id] }))
+			.map((id) => ({ current: currentByServiceName[id] }))
 			.value();
 
-		const toBeInstalled = _(targetServiceIds)
+		const toBeInstalled = _(targetServiceNames)
-			.difference(currentServiceIds)
+			.difference(currentServiceNames)
-			.map((id) => ({ target: targetByServiceId[id] }))
+			.map((id) => ({ target: targetByServiceName[id] }))
 			.value();
 
-		const maybeUpdate = _.intersection(targetServiceIds, currentServiceIds);
+		const maybeUpdate = _.intersection(targetServiceNames, currentServiceNames);
 
-		// Build up a list of services for a given service ID, always using the latest created
+		// Build up a list of services for a given service name, always using the latest created
 		// service. Any older services will have kill steps emitted
-		for (const serviceId of maybeUpdate) {
+		for (const serviceName of maybeUpdate) {
-			const currentServiceContainers = _.filter(current, { serviceId });
+			const currentServiceContainers = _.filter(current, { serviceName });
 			if (currentServiceContainers.length > 1) {
-				currentByServiceId[serviceId] = _.maxBy(
+				currentByServiceName[serviceName] = _.maxBy(
 					currentServiceContainers,
 					'createdAt',
 				)!;
@@ -302,13 +295,13 @@ export class App {
 				// be removed
 				const otherContainers = _.without(
 					currentServiceContainers,
-					currentByServiceId[serviceId],
+					currentByServiceName[serviceName],
 				);
 				for (const service of otherContainers) {
 					toBeRemoved.push({ current: service });
 				}
 			} else {
-				currentByServiceId[serviceId] = currentServiceContainers[0];
+				currentByServiceName[serviceName] = currentServiceContainers[0];
 			}
 		}
 
@@ -374,9 +367,9 @@ export class App {
 		 * Filter all the services which should be updated due to run state change, or config mismatch.
 		 */
 		const toBeUpdated = maybeUpdate
-			.map((serviceId) => ({
+			.map((serviceName) => ({
-				current: currentByServiceId[serviceId],
+				current: currentByServiceName[serviceName],
-				target: targetByServiceId[serviceId],
+				target: targetByServiceName[serviceName],
 			}))
 			.filter(
 				({ current: c, target: t }) =>
@@ -456,7 +449,7 @@ export class App {
 		context: {
 			localMode: boolean;
 			availableImages: Image[];
-			downloading: number[];
+			downloading: string[];
 			targetApp: App;
 			containerIds: Dictionary<string>;
 			networkPairs: Array<ChangingPair<Network>>;
@@ -486,7 +479,7 @@ export class App {
 			);
 		}
 
-		if (needsDownload && context.downloading.includes(target?.imageId!)) {
+		if (needsDownload && context.downloading.includes(target?.imageName!)) {
 			// The image needs to be downloaded, and it's currently downloading. We simply keep
 			// the application loop alive
 			return generateStep('noop', {});
@@ -563,7 +556,7 @@ export class App {
 			service.status !== 'Stopping' &&
 			!_.some(
 				changingServices,
-				({ current }) => current?.serviceId !== service.serviceId,
+				({ current }) => current?.serviceName !== service.serviceName,
 			)
 		) {
 			return [generateStep('kill', { current: service })];
@@ -595,11 +588,8 @@ export class App {
 	}
 
 	private generateContainerStep(current: Service, target: Service) {
-		// if the services release/image don't match, then rename the container...
+		// if the services release doesn't match, then rename the container...
-		if (
+		if (current.commit !== target.commit) {
-			current.releaseId !== target.releaseId ||
-			current.imageId !== target.imageId
-		) {
 			return generateStep('updateMetadata', { current, target });
 		} else if (target.config.running !== current.config.running) {
 			if (target.config.running) {
@@ -728,7 +718,7 @@ export class App {
 					!_.some(
 						availableImages,
 						(image) =>
-							image.dockerImageId === dependencyService?.imageId ||
+							image.dockerImageId === dependencyService?.config.image ||
 							imageManager.isSameImage(image, {
 								name: dependencyService?.imageName!,
 							}),
@@ -825,7 +815,6 @@ export class App {
 			{
 				appId: app.appId,
 				commit: app.commit,
-				releaseId: app.releaseId,
 				appName: app.name,
 				source: app.source,
 				services,

src/compose/application-manager.ts

@@ -125,7 +125,6 @@ let targetVolatilePerImageId: {
 export const initialized = (async () => {
 	await config.initialized;
 
-	await imageManager.initialized;
 	await imageManager.cleanImageData();
 	const cleanup = async () => {
 		const containers = await docker.listContainers({ all: true });
@@ -180,7 +179,7 @@ export async function getRequiredSteps(
 ): Promise<CompositionStep[]> {
 	// get some required data
 	const [downloading, availableImages, currentApps] = await Promise.all([
-		imageManager.getDownloadingImageIds(),
+		imageManager.getDownloadingImageNames(),
 		imageManager.getAvailable(),
 		getCurrentApps(),
 	]);
@@ -200,7 +199,7 @@ export async function inferNextSteps(
 	targetApps: InstancedAppState,
 	{
 		ignoreImages = false,
-		downloading = [] as number[],
+		downloading = [] as string[],
 		availableImages = [] as Image[],
 		containerIdsByAppId = {} as { [appId: number]: Dictionary<string> },
 	} = {},
@@ -675,7 +674,7 @@ function saveAndRemoveImages(
 			(svc) =>
 				_.find(availableImages, {
 					dockerImageId: svc.config.image,
-					imageId: svc.imageId,
+					name: svc.imageName,
 				}) ?? _.find(availableImages, { dockerImageId: svc.config.image }),
 		),
 	) as imageManager.Image[];

src/compose/images.ts

@@ -15,7 +15,6 @@ import {
 	StatusError,
 } from '../lib/errors';
 import * as LogTypes from '../lib/log-types';
-import * as validation from '../lib/validation';
 import * as logger from '../logger';
 import { ImageDownloadBackoffError } from './errors';
 
@@ -68,20 +67,90 @@ const imageFetchLastFailureTime: Dictionary<ReturnType<
 	typeof process.hrtime
 >> = {};
 const imageCleanupFailures: Dictionary<number> = {};
-// A store of volatile state for images (e.g. download progress), indexed by imageId
-const volatileState: { [imageId: number]: Image } = {};
 
-let appUpdatePollInterval: number;
+type ImageState = Pick<Image, 'status' | 'downloadProgress'>;
+type ImageTask = {
+	// Indicates whether the task has been finished
+	done?: boolean;
 
-export const initialized = (async () => {
+	// Current image state of the task
-	await config.initialized;
+	context: Image;
-	appUpdatePollInterval = await config.get('appUpdatePollInterval');
+
-	config.on('change', (vals) => {
+	// Update the task with new context. This is a pure function
-		if (vals.appUpdatePollInterval != null) {
+	// meaning it doesn't modify the original task
-			appUpdatePollInterval = vals.appUpdatePollInterval;
+	update: (change?: ImageState) => ImageTaskUpdate;
+
+	// Finish the task. This is a pure function
+	// meaning it doesn't modify the original task
+	finish: () => ImageTaskUpdate;
+};
+
+type ImageTaskUpdate = [ImageTask, boolean];
+
+// Create new running task with the given initial context
+function createTask(initialContext: Image) {
+	// Task has only two state, is either running or finished
+	const running = (context: Image): ImageTask => {
+		return {
+			context,
+			update: ({ status, downloadProgress }: ImageState) =>
+				// Keep current state
+				[
+					running({
+						...context,
+						...(status && { status }),
+						...(downloadProgress && { downloadProgress }),
+					}),
+					// Only mark the task as changed if there is new data
+					[status, downloadProgress].some((v) => !!v),
+				],
+			finish: () => [finished(context), true],
+		};
+	};
+
+	// Once the task is finished, it cannot go back to a running state
+	const finished = (context: Image): ImageTask => {
+		return {
+			done: true,
+			context,
+			update: () => [finished(context), false],
+			finish: () => [finished(context), false],
+		};
+	};
+
+	return running(initialContext);
+}
+
+const runningTasks: { [imageName: string]: ImageTask } = {};
+function reportEvent(event: 'start' | 'update' | 'finish', state: Image) {
+	const { name: imageName } = state;
+
+	// Emit by default if a start event is reported
+	let emitChange = event === 'start';
+
+	// Get the current task and update it in memory
+	const currentTask =
+		event === 'start' ? createTask(state) : runningTasks[imageName];
+	runningTasks[imageName] = currentTask;
+
+	// TODO: should we assert that the current task exists at this point?
+	// On update, update the corresponding task with the new state if it exists
+	if (event === 'update' && currentTask) {
+		const [updatedTask, changed] = currentTask.update(state);
+		runningTasks[imageName] = updatedTask;
+		emitChange = changed;
 	}
-	});
+
-})();
+	// On update, update the corresponding task with the new state if it exists
+	if (event === 'finish' && currentTask) {
+		[, emitChange] = currentTask.finish();
+		delete runningTasks[imageName];
+	}
+
+	if (emitChange) {
+		events.emit('change');
+	}
+}
 
 type ServiceInfo = Pick<
 	Service,
@@ -106,6 +175,8 @@ export async function triggerFetch(
 	onFinish = _.noop,
 	serviceName: string,
 ): Promise<void> {
+	const appUpdatePollInterval = await config.get('appUpdatePollInterval');
+
 	if (imageFetchFailures[image.name] != null) {
 		// If we are retrying a pull within the backoff time of the last failure,
 		// we need to throw an error, which will be caught in the device-state
@@ -125,12 +196,7 @@ export async function triggerFetch(
 	}
 
 	const onProgress = (progress: FetchProgressEvent) => {
-		// Only report the percentage if we haven't finished fetching
+		reportEvent('update', { ...image, downloadProgress: progress.percentage });
-		if (volatileState[image.imageId] != null) {
-			reportChange(image.imageId, {
-				downloadProgress: progress.percentage,
-			});
-		}
 	};
 
 	let success: boolean;
@@ -157,10 +223,13 @@ export async function triggerFetch(
 			}
 			throw e;
 		}
-		reportChange(
+
-			image.imageId,
+		// Report a fetch start
-			_.merge(_.clone(image), { status: 'Downloading', downloadProgress: 0 }),
+		reportEvent('start', {
-		);
+			...image,
+			status: 'Downloading',
+			downloadProgress: 0,
+		});
 
 		try {
 			let id;
@@ -197,7 +266,7 @@ export async function triggerFetch(
 		}
 	}
 
-	reportChange(image.imageId);
+	reportEvent('finish', image);
 	onFinish(success);
 }
 
@@ -280,9 +349,15 @@ export async function getAvailable(): Promise<Image[]> {
 }
 
 export function getDownloadingImageIds(): number[] {
-	return _.keys(_.pickBy(volatileState, { status: 'Downloading' })).map((i) =>
+	return Object.values(runningTasks)
-		validation.checkInt(i),
+		.filter((t) => t.context.status === 'Downloading')
-	) as number[];
+		.map((t) => t.context.imageId);
+}
+
+export function getDownloadingImageNames(): string[] {
+	return Object.values(runningTasks)
+		.filter((t) => t.context.status === 'Downloading')
+		.map((t) => t.context.name);
 }
 
 export async function cleanImageData(): Promise<void> {
@@ -331,18 +406,22 @@ export async function cleanImageData(): Promise<void> {
 }
 
 export const getStatus = async () => {
-	const images = await getAvailable();
+	const images = (await getAvailable()).map((img) => ({
-	for (const image of images) {
+		...img,
-		image.status = 'Downloaded';
+		status: 'Downloaded' as Image['status'],
-		image.downloadProgress = null;
+		downloadImageSuccess: null,
-	}
+	}));
-	const status = _.clone(volatileState);
+
-	for (const image of images) {
+	const imagesFromRunningTasks = Object.values(runningTasks).map(
-		if (status[image.imageId] == null) {
+		(task) => task.context,
-			status[image.imageId] = image;
+	);
-		}
+	const runningImageIds = imagesFromRunningTasks.map((img) => img.imageId);
-	}
+
-	return _.values(status);
+	// TODO: this is possibly wrong, the value from getAvailable should be more reliable
+	// than the value from running tasks
+	return imagesFromRunningTasks.concat(
+		images.filter((img) => !runningImageIds.includes(img.imageId)),
+	);
 };
 
 export async function update(image: Image): Promise<void> {
@@ -593,10 +672,7 @@ async function removeImageIfNotNeeded(image: Image): Promise<void> {
 			[] as string[],
 		);
 
-		reportChange(
+		reportEvent('start', { ...image, status: 'Deleting' });
-			image.imageId,
-			_.merge(_.clone(image), { status: 'Deleting' }),
-		);
 		logger.logSystemEvent(LogTypes.deleteImage, { image });
 
 		// The engine doesn't handle concurrency too well. If two requests to
@@ -641,7 +717,7 @@ async function removeImageIfNotNeeded(image: Image): Promise<void> {
 			throw e;
 		}
 	} finally {
-		reportChange(image.imageId);
+		reportEvent('finish', image);
 	}
 
 	await db.models('image').del().where({ id: img.id });
@@ -706,20 +782,3 @@ function fetchImage(
 	logger.logSystemEvent(LogTypes.downloadImage, { image });
 	return dockerUtils.fetchImageWithProgress(image.name, opts, onProgress);
 }
-
-// TODO: find out if imageId can actually be null
-function reportChange(imageId: Nullable<number>, status?: Partial<Image>) {
-	if (imageId == null) {
-		return;
-	}
-	if (status != null) {
-		if (volatileState[imageId] == null) {
-			volatileState[imageId] = { imageId } as Image;
-		}
-		_.merge(volatileState[imageId], status);
-		return events.emit('change');
-	} else if (volatileState[imageId] != null) {
-		delete volatileState[imageId];
-		return events.emit('change');
-	}
-}

src/compose/service-manager.ts

@@ -20,7 +20,7 @@ import {
 } from '../lib/errors';
 import * as LogTypes from '../lib/log-types';
 import { checkInt, isValidDeviceName } from '../lib/validation';
-import { Service } from './service';
+import { Service, ServiceStatus } from './service';
 import { serviceNetworksToDockerNetworks } from './utils';
 
 import log from '../lib/supervisor-console';
@@ -88,7 +88,7 @@ export async function get(service: Service) {
 	// Get the container ids for special network handling
 	const containerIds = await getContainerIdMap(service.appId!);
 	const services = (
-		await getAll(`service-id=${service.serviceId}`)
+		await getAll(`service-name=${service.serviceName}`)
 	).filter((currentService) =>
 		currentService.isEqualConfig(service, containerIds),
 	);
@@ -151,7 +151,7 @@ export async function updateMetadata(service: Service, target: Service) {
 	}
 
 	await docker.getContainer(svc.containerId).rename({
-		name: `${service.serviceName}_${target.imageId}_${target.releaseId}`,
+		name: `${service.serviceName}_${target.imageId}_${target.releaseId}_${target.commit}`,
 	});
 }
 
@@ -294,7 +294,7 @@ export async function start(service: Service) {
 		containerId = container.id;
 		logger.logSystemEvent(LogTypes.startService, { service });
 
-		reportNewStatus(containerId, service, 'Starting');
+		reportNewStatus(containerId, service, 'Starting' as ServiceStatus);
 
 		let shouldRemove = false;
 		let err: Error | undefined;
@@ -498,7 +498,7 @@ function reportChange(containerId?: string, status?: Partial<Service>) {
 function reportNewStatus(
 	containerId: string,
 	service: Partial<Service>,
-	status: string,
+	status: ServiceStatus,
 ) {
 	reportChange(
 		containerId,
@@ -611,7 +611,7 @@ async function prepareForHandover(service: Service) {
 	const container = docker.getContainer(svc.containerId);
 	await container.update({ RestartPolicy: {} });
 	return await container.rename({
-		name: `old_${service.serviceName}_${service.imageId}_${service.imageId}_${service.releaseId}`,
+		name: `old_${service.serviceName}_${service.imageId}_${service.releaseId}_${service.commit}`,
 	});
 }
 

src/compose/service.ts

@@ -44,7 +44,8 @@ export class Service {
 	public appId: number;
 	public imageId: number;
 	public config: ServiceConfig;
-	public serviceName: string | null;
+	public serviceName: string;
+	public commit: string;
 	public releaseId: number;
 	public serviceId: number;
 	public imageName: string | null;
@@ -135,12 +136,11 @@ export class Service {
 		delete appConfig.dependsOn;
 		service.createdAt = appConfig.createdAt;
 		delete appConfig.createdAt;
+		service.commit = appConfig.commit;
+		delete appConfig.commit;
 
 		delete appConfig.contract;
 
-		// We don't need this value
-		delete appConfig.commit;
-
 		// Get rid of any extra values and report them to the user
 		const config = sanitiseComposeConfig(appConfig);
 
@@ -600,15 +600,16 @@ export class Service {
 				'Attempt to build Service class from container with malformed labels',
 			);
 		}
-		const nameMatch = container.Name.match(/.*_(\d+)_(\d+)$/);
+		const nameMatch = container.Name.match(/.*_(\d+)_(\d+)(?:_(.*?))?$/);
 		if (nameMatch == null) {
 			throw new InternalInconsistencyError(
-				'Attempt to build Service class from container with malformed name',
+				`Expected supervised container to have name '<serviceName>_<imageId>_<releaseId>_<commit>', got: ${container.Name}`,
 			);
 		}
 
 		svc.imageId = parseInt(nameMatch[1], 10);
 		svc.releaseId = parseInt(nameMatch[2], 10);
+		svc.commit = nameMatch[3];
 		svc.containerId = container.Id;
 		svc.dockerImageId = container.Config.Image;
 
@@ -656,7 +657,7 @@ export class Service {
 			this.config.networkMode = `container:${containerId}`;
 		}
 		return {
-			name: `${this.serviceName}_${this.imageId}_${this.releaseId}`,
+			name: `${this.serviceName}_${this.imageId}_${this.releaseId}_${this.commit}`,
 			Tty: this.config.tty,
 			Cmd: this.config.command,
 			Volumes: volumes,
@@ -862,8 +863,7 @@ export class Service {
 	): boolean {
 		return (
 			this.isEqualConfig(service, currentContainerIds) &&
-			this.releaseId === service.releaseId &&
+			this.commit === service.commit
-			this.imageId === service.imageId
 		);
 	}
 

src/compose/utils.ts

@@ -373,7 +373,7 @@ export async function addFeaturesFromLabels(
 			// create a app/service specific API secret
 			const apiSecret = await apiKeys.generateScopedKey(
 				service.appId,
-				service.serviceId,
+				service.serviceName,
 			);
 
 			const host = (() => {

src/device-api/common.js

@@ -121,13 +121,6 @@ export async function doPurge(appId, force) {
 		});
 }
 
-export function serviceAction(action, serviceId, current, target, options) {
-	if (options == null) {
-		options = {};
-	}
-	return { action, serviceId, current, target, options };
-}
-
 /**
  * This doesn't truly return an InstancedDeviceState, but it's close enough to mostly work where it's used
  *

src/lib/api-keys.ts

@@ -15,7 +15,7 @@ export class KeyNotFoundError extends Error {}
 interface DbApiSecret {
 	id: number;
 	appId: number;
-	serviceId: number;
+	serviceName: string;
 	scopes: string;
 	key: string;
 }
@@ -199,17 +199,17 @@ export async function getScopesForKey(key: string): Promise<Scope[] | null> {
 
 export async function generateScopedKey(
 	appId: number,
-	serviceId: number,
+	serviceName: string,
 	options?: Partial<GenerateKeyOptions>,
 ): Promise<string> {
 	await initialized;
-	return await generateKey(appId, serviceId, options);
+	return await generateKey(appId, serviceName, options);
 }
 
 export async function generateCloudKey(
 	force: boolean = false,
 ): Promise<string> {
-	cloudApiKey = await generateKey(0, 0, {
+	cloudApiKey = await generateKey(0, null, {
 		force,
 		scopes: [{ type: 'global' }],
 	});
@@ -223,15 +223,15 @@ export async function refreshKey(key: string): Promise<string> {
 		throw new KeyNotFoundError();
 	}
 
-	const { appId, serviceId, scopes } = apiKey;
+	const { appId, serviceName, scopes } = apiKey;
 
 	// if this is a cloud key that is being refreshed
-	if (appId === 0 && serviceId === 0) {
+	if (appId === 0 && serviceName === null) {
 		return await generateCloudKey(true);
 	}
 
 	// generate a new key, expiring the old one...
-	const newKey = await generateScopedKey(appId, serviceId, {
+	const newKey = await generateScopedKey(appId, serviceName, {
 		force: true,
 		scopes: deserialiseScopes(scopes),
 	});
@@ -244,15 +244,15 @@ export async function refreshKey(key: string): Promise<string> {
  * A cached lookup of the database key
  */
 const getApiKeyForService = memoizee(
-	async (appId: number, serviceId: number): Promise<DbApiSecret[]> => {
+	async (appId: number, serviceName: string | null): Promise<DbApiSecret[]> => {
 		await db.initialized;
 
-		return await db.models('apiSecret').where({ appId, serviceId }).select();
+		return await db.models('apiSecret').where({ appId, serviceName }).select();
 	},
 	{
 		promise: true,
 		maxAge: 60000, // 1 minute
-		normalizer: ([appId, serviceId]) => `${appId}-${serviceId}`,
+		normalizer: ([appId, serviceName]) => `${appId}-${serviceName}`,
 	},
 );
 
@@ -276,12 +276,12 @@ const getApiKeyByKey = memoizee(
  * All key generate logic should come though this method. It handles cache clearing.
  *
  * @param appId
- * @param serviceId
+ * @param serviceName
  * @param options
  */
 async function generateKey(
 	appId: number,
-	serviceId: number,
+	serviceName: string | null,
 	options?: Partial<GenerateKeyOptions>,
 ): Promise<string> {
 	// set default options
@@ -292,13 +292,13 @@ async function generateKey(
 	};
 
 	// grab the existing API key info
-	const secrets = await getApiKeyForService(appId, serviceId);
+	const secrets = await getApiKeyForService(appId, serviceName);
 
 	// if we need a new key
 	if (secrets.length === 0 || force) {
 		// are forcing a new key?
 		if (force) {
-			await db.models('apiSecret').where({ appId, serviceId }).del();
+			await db.models('apiSecret').where({ appId, serviceName }).del();
 		}
 
 		// remove the cached lookup for the key
@@ -308,10 +308,10 @@ async function generateKey(
 		}
 
 		// remove the cached value for this lookup
-		getApiKeyForService.clear(appId, serviceId);
+		getApiKeyForService.clear(appId, serviceName);
 
 		// return a new API key
-		return await createNewKey(appId, serviceId, scopes);
+		return await createNewKey(appId, serviceName, scopes);
 	}
 
 	// grab the current secret and scopes
@@ -333,21 +333,25 @@ async function generateKey(
 	}
 
 	// forcibly get a new key...
-	return await generateKey(appId, serviceId, { ...options, force: true });
+	return await generateKey(appId, serviceName, { ...options, force: true });
 }
 
 /**
  * Generates a new key value and inserts it into the DB.
  *
  * @param appId
- * @param serviceId
+ * @param serviceName
  * @param scopes
  */
-async function createNewKey(appId: number, serviceId: number, scopes: Scope[]) {
+async function createNewKey(
+	appId: number,
+	serviceName: string | null,
+	scopes: Scope[],
+) {
 	const key = generateUniqueKey();
 	await db.models('apiSecret').insert({
 		appId,
-		serviceId,
+		serviceName,
 		key,
 		scopes: serialiseScopes(scopes),
 	});

src/migrations/M00007.js

@@ -0,0 +1,41 @@
+export async function up(knex) {
+	// Add serviceName to apiSecret schema
+	await knex.schema.table('apiSecret', (table) => {
+		table.string('serviceName');
+		table.unique(['appId', 'serviceName']);
+	});
+
+	const targetServices = (await knex('app').select(['appId', 'services']))
+		.map(({ appId, services }) => ({
+			appId,
+			// Extract service name and id per app
+			services: JSON.parse(services).map(({ serviceId, serviceName }) => ({
+				serviceId,
+				serviceName,
+			})),
+		}))
+		.reduce(
+			// Convert to array of {appId, serviceId, serviceName}
+			(apps, { appId, services }) =>
+				apps.concat(services.map((svc) => ({ appId, ...svc }))),
+			[],
+		);
+
+	// Update all API secret entries so services can still access the API after
+	// the change
+	await Promise.all(
+		targetServices.map(({ appId, serviceId, serviceName }) =>
+			knex('apiSecret').update({ serviceName }).where({ appId, serviceId }),
+		),
+	);
+
+	// Update the table schema deleting the serviceId column
+	await knex.schema.table('apiSecret', (table) => {
+		table.dropUnique(['appId', 'serviceId']);
+		table.dropColumn('serviceId');
+	});
+}
+
+export function down() {
+	return Promise.reject(new Error('Not Implemented'));
+}

test/21-supervisor-api.spec.ts

@@ -64,7 +64,7 @@ describe('SupervisorAPI', () => {
 	describe('API Key Scope', () => {
 		it('should generate a key which is scoped for a single application', async () => {
 			// single app scoped key...
-			const appScopedKey = await apiKeys.generateScopedKey(1, 1);
+			const appScopedKey = await apiKeys.generateScopedKey(1, 'main');
 
 			await request
 				.get('/v2/applications/1/state')
@@ -74,7 +74,7 @@ describe('SupervisorAPI', () => {
 		});
 		it('should generate a key which is scoped for multiple applications', async () => {
 			// multi-app scoped key...
-			const multiAppScopedKey = await apiKeys.generateScopedKey(1, 2, {
+			const multiAppScopedKey = await apiKeys.generateScopedKey(1, 'other', {
 				scopes: [1, 2].map((appId) => {
 					return { type: 'app', appId };
 				}),
@@ -135,7 +135,7 @@ describe('SupervisorAPI', () => {
 		});
 		it('should regenerate a key and invalidate the old one', async () => {
 			// single app scoped key...
-			const appScopedKey = await apiKeys.generateScopedKey(1, 1);
+			const appScopedKey = await apiKeys.generateScopedKey(1, 'main');
 
 			await request
 				.get('/v2/applications/1/state')

test/28-db-format.spec.ts

@@ -86,7 +86,6 @@ describe('DB Format', () => {
 		expect(app).to.be.an.instanceOf(App);
 		expect(app).to.have.property('appId').that.equals(1);
 		expect(app).to.have.property('commit').that.equals('abcdef');
-		expect(app).to.have.property('releaseId').that.equals(123);
 		expect(app).to.have.property('appName').that.equals('test-app');
 		expect(app)
 			.to.have.property('source')

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

@@ -1232,7 +1232,7 @@ describe('SupervisorAPI [V1 Endpoints]', () => {
 			// resolve to true for any isScoped check
 			const scopedKey = await apiKeys.generateScopedKey(
 				2,
-				containers[0].serviceId,
+				containers[0].serviceName,
 			);
 
 			await request

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

@@ -142,7 +142,7 @@ describe('SupervisorAPI [V2 Endpoints]', () => {
 
 		describe('Scoped API Keys', () => {
 			it('returns 409 because app is out of scope of the key', async () => {
-				const apiKey = await apiKeys.generateScopedKey(3, 1);
+				const apiKey = await apiKeys.generateScopedKey(3, 'main');
 				await request
 					.get('/v2/applications/2/state')
 					.set('Accept', 'application/json')
@@ -164,7 +164,7 @@ describe('SupervisorAPI [V2 Endpoints]', () => {
 
 		it('should return scoped application', async () => {
 			// Create scoped key for application
-			const appScopedKey = await apiKeys.generateScopedKey(1658654, 640681);
+			const appScopedKey = await apiKeys.generateScopedKey(1658654, 'main');
 			// Setup device conditions
 			serviceManagerMock.resolves([mockedAPI.mockService({ appId: 1658654 })]);
 			imagesMock.resolves([mockedAPI.mockImage({ appId: 1658654 })]);
@@ -188,7 +188,7 @@ describe('SupervisorAPI [V2 Endpoints]', () => {
 
 		it('should return no application info due to lack of scope', async () => {
 			// Create scoped key for wrong application
-			const appScopedKey = await apiKeys.generateScopedKey(1, 1);
+			const appScopedKey = await apiKeys.generateScopedKey(1, 'main');
 			// Setup device conditions
 			serviceManagerMock.resolves([mockedAPI.mockService({ appId: 1658654 })]);
 			imagesMock.resolves([mockedAPI.mockImage({ appId: 1658654 })]);
@@ -211,7 +211,7 @@ describe('SupervisorAPI [V2 Endpoints]', () => {
 
 		it('should return success when device has no applications', async () => {
 			// Create scoped key for any application
-			const appScopedKey = await apiKeys.generateScopedKey(1658654, 1658654);
+			const appScopedKey = await apiKeys.generateScopedKey(1658654, 'main');
 			// Setup device conditions
 			serviceManagerMock.resolves([]);
 			imagesMock.resolves([]);
@@ -234,7 +234,7 @@ describe('SupervisorAPI [V2 Endpoints]', () => {
 
 		it('should only return 1 application when N > 1 applications on device', async () => {
 			// Create scoped key for application
-			const appScopedKey = await apiKeys.generateScopedKey(1658654, 640681);
+			const appScopedKey = await apiKeys.generateScopedKey(1658654, 'main');
 			// Setup device conditions
 			serviceManagerMock.resolves([
 				mockedAPI.mockService({ appId: 1658654 }),
@@ -330,7 +330,7 @@ describe('SupervisorAPI [V2 Endpoints]', () => {
 
 		before(async () => {
 			// Create scoped key for application
-			appScopedKey = await apiKeys.generateScopedKey(1658654, 640681);
+			appScopedKey = await apiKeys.generateScopedKey(1658654, 'main');
 
 			// Mock target state cache
 			targetStateCacheMock = stub(targetStateCache, 'getTargetApp');
@@ -439,7 +439,7 @@ describe('SupervisorAPI [V2 Endpoints]', () => {
 
 		before(async () => {
 			// Create scoped key for application
-			appScopedKey = await apiKeys.generateScopedKey(1658654, 640681);
+			appScopedKey = await apiKeys.generateScopedKey(1658654, 'main');
 
 			// Mock target state cache
 			targetStateCacheMock = stub(targetStateCache, 'getTargetApp');

test/src/compose/app.spec.ts

@@ -15,9 +15,9 @@ import log from '../../../src/lib/supervisor-console';
 
 const defaultContext = {
 	localMode: false,
-	availableImages: [],
+	availableImages: [] as Image[],
 	containerIds: {},
-	downloading: [],
+	downloading: [] as string[],
 };
 
 function createApp({
@@ -42,26 +42,22 @@ function createApp({
 }
 
 async function createService(
-	conf = {} as Partial<ServiceComposeConfig>,
 	{
 		appId = 1,
 		serviceName = 'test',
-		releaseId = 2,
+		commit = 'test-commit',
-		serviceId = 3,
+		...conf
-		imageId = 4,
+	} = {} as Partial<ServiceComposeConfig>,
-		state = {} as Partial<Service>,
+	{ state = {} as Partial<Service>, options = {} as any } = {},
-	} = {},
 ) {
 	const svc = await Service.fromComposeObject(
 		{
 			appId,
 			serviceName,
-			releaseId,
+			commit,
-			serviceId,
-			imageId,
 			...conf,
 		},
-		{} as any,
+		options,
 	);
 
 	// Add additonal configuration
@@ -71,6 +67,18 @@ async function createService(
 	return svc;
 }
 
+function createImage(
+	{
+		appId = 1,
+		dependent = 0,
+		name = 'test-image',
+		serviceName = 'test',
+		...extra
+	} = {} as Partial<Image>,
+) {
+	return { appId, dependent, name, serviceName, ...extra } as Image;
+}
+
 const expectSteps = (
 	action: CompositionStepAction,
 	steps: CompositionStep[],
@@ -298,15 +306,10 @@ describe('compose/app', () => {
 				...defaultContext,
 				...{
 					availableImages: [
-						{
+						createImage({
 							appId: service.appId,
-							dependent: 0,
-							imageId: service.imageId,
-							releaseId: service.releaseId,
-							serviceId: service.serviceId,
 							name: 'test-image',
-							serviceName: service.serviceName,
+						}),
-						} as Image,
 					],
 				},
 			};
@@ -488,7 +491,7 @@ describe('compose/app', () => {
 				networks: [Network.fromComposeObject('test-network', 1, {})],
 			});
 			const target = createApp({
-				services: [await createService({})],
+				services: [await createService()],
 				networks: [],
 				isTarget: true,
 			});
@@ -557,24 +560,13 @@ describe('compose/app', () => {
 		it('should create a kill step for service which is no longer referenced', async () => {
 			const current = createApp({
 				services: [
-					await createService(
+					await createService({ appId: 1, serviceName: 'main' }),
-						{},
+					await createService({ appId: 1, serviceName: 'aux' }),
-						{ appId: 1, serviceName: 'main', releaseId: 1, serviceId: 1 },
-					),
-					await createService(
-						{},
-						{ appId: 1, serviceName: 'aux', releaseId: 1, serviceId: 2 },
-					),
 				],
 				networks: [Network.fromComposeObject('test-network', 1, {})],
 			});
 			const target = createApp({
-				services: [
+				services: [await createService({ appId: 1, serviceName: 'main' })],
-					await createService(
-						{},
-						{ appId: 1, serviceName: 'main', releaseId: 1, serviceId: 1 },
-					),
-				],
 				networks: [Network.fromComposeObject('test-network', 1, {})],
 				isTarget: true,
 			});
@@ -590,8 +582,8 @@ describe('compose/app', () => {
 			const current = createApp({
 				services: [
 					await createService(
-						{},
+						{ serviceName: 'main' },
-						{ serviceName: 'main', state: { status: 'Stopping' } },
+						{ state: { status: 'Stopping' } },
 					),
 				],
 			});
@@ -608,13 +600,13 @@ describe('compose/app', () => {
 			const current = createApp({
 				services: [
 					await createService(
-						{},
+						{ serviceName: 'main' },
-						{ serviceName: 'main', state: { status: 'Dead' } },
+						{ state: { status: 'Dead' } },
 					),
 				],
 			});
 			const target = createApp({
-				services: [await createService({}, { serviceName: 'main' })],
+				services: [await createService({ serviceName: 'main' })],
 				isTarget: true,
 			});
 
@@ -630,8 +622,8 @@ describe('compose/app', () => {
 			const current = createApp({
 				services: [
 					await createService(
-						{},
+						{ serviceName: 'main' },
-						{ serviceName: 'main', state: { status: 'Dead' } },
+						{ state: { status: 'Dead' } },
 					),
 				],
 			});
@@ -649,13 +641,13 @@ describe('compose/app', () => {
 			const current = createApp({ services: [] });
 			const target = createApp({
 				services: [
-					await createService({}, { serviceName: 'main', imageId: 123 }),
+					await createService({ image: 'main-image', serviceName: 'main' }),
 				],
 				isTarget: true,
 			});
 
 			const steps = current.nextStepsForAppUpdate(
-				{ ...defaultContext, ...{ downloading: [123] } },
+				{ ...defaultContext, ...{ downloading: ['main-image'] } },
 				target,
 			);
 			expectSteps('noop', steps);
@@ -665,12 +657,12 @@ describe('compose/app', () => {
 		it('should emit an updateMetadata step when a service has not changed but the release has', async () => {
 			const current = createApp({
 				services: [
-					await createService({}, { serviceName: 'main', releaseId: 1 }),
+					await createService({ serviceName: 'main', commit: 'old-release' }),
 				],
 			});
 			const target = createApp({
 				services: [
-					await createService({}, { serviceName: 'main', releaseId: 2 }),
+					await createService({ serviceName: 'main', commit: 'new-release' }),
 				],
 				isTarget: true,
 			});
@@ -680,20 +672,20 @@ describe('compose/app', () => {
 
 			expect(updateMetadataStep)
 				.to.have.property('current')
-				.to.deep.include({ serviceName: 'main', releaseId: 1 });
+				.to.deep.include({ serviceName: 'main', commit: 'old-release' });
 
 			expect(updateMetadataStep)
 				.to.have.property('target')
-				.to.deep.include({ serviceName: 'main', releaseId: 2 });
+				.to.deep.include({ serviceName: 'main', commit: 'new-release' });
 		});
 
 		it('should stop a container which has `running: false` as its target', async () => {
 			const current = createApp({
-				services: [await createService({}, { serviceName: 'main' })],
+				services: [await createService({ serviceName: 'main' })],
 			});
 			const target = createApp({
 				services: [
-					await createService({ running: false }, { serviceName: 'main' }),
+					await createService({ running: false, serviceName: 'main' }),
 				],
 				isTarget: true,
 			});
@@ -745,30 +737,23 @@ describe('compose/app', () => {
 				...defaultContext,
 				...{
 					availableImages: [
-						{
+						createImage({ appId: 1, serviceName: 'main', name: 'main-image' }),
-							appId: 1,
-							dependent: 0,
-							imageId: 1,
-							releaseId: 1,
-							serviceId: 1,
-							name: 'main-image',
-							serviceName: 'main',
-						},
 					],
 				},
 			};
 
 			const current = createApp({
-				services: [await createService({}, { appId: 1, serviceName: 'main' })],
+				services: [await createService({ appId: 1, serviceName: 'main' })],
 				// Default network was already created
 				networks: [defaultNetwork],
 			});
 			const target = createApp({
 				services: [
-					await createService(
+					await createService({
-						{ privileged: true },
+						privileged: true,
-						{ appId: 1, serviceName: 'main' },
+						appId: 1,
-					),
+						serviceName: 'main',
+					}),
 				],
 				networks: [defaultNetwork],
 				isTarget: true,
@@ -809,36 +794,20 @@ describe('compose/app', () => {
 
 		it('should not start a container when it depends on a service which is being installed', async () => {
 			const availableImages = [
-				{
+				createImage({ appId: 1, serviceName: 'main', name: 'main-image' }),
-					appId: 1,
+				createImage({ appId: 1, serviceName: 'dep', name: 'dep-image' }),
-					dependent: 0,
-					imageId: 1,
-					releaseId: 1,
-					serviceId: 1,
-					name: 'main-image',
-					serviceName: 'main',
-				},
-				{
-					appId: 1,
-					dependent: 0,
-					imageId: 2,
-					releaseId: 1,
-					serviceId: 2,
-					name: 'dep-image',
-					serviceName: 'dep',
-				},
 			];
 			const contextWithImages = { ...defaultContext, ...{ availableImages } };
 
 			const current = createApp({
 				services: [
 					await createService(
-						{ running: false },
 						{
+							running: false,
 							appId: 1,
 							serviceName: 'dep',
-							serviceId: 2,
+						},
-							imageId: 2,
+						{
 							state: {
 								status: 'Installing',
 								containerId: 'dep-id',
@@ -850,25 +819,15 @@ describe('compose/app', () => {
 			});
 			const target = createApp({
 				services: [
-					await createService(
+					await createService({
-						{},
-						{
 						appId: 1,
 						serviceName: 'main',
-							serviceId: 1,
+						dependsOn: ['dep'],
-							imageId: 1,
+					}),
-							state: { dependsOn: ['dep'] },
+					await createService({
-						},
-					),
-					await createService(
-						{},
-						{
 						appId: 1,
 						serviceName: 'dep',
-							serviceId: 2,
+					}),
-							imageId: 2,
-						},
-					),
 				],
 				networks: [defaultNetwork],
 				isTarget: true,
@@ -889,16 +848,8 @@ describe('compose/app', () => {
 			const intermediate = createApp({
 				services: [
 					await createService(
-						{},
+						{ appId: 1, serviceName: 'dep' },
-						{
+						{ state: { containerId: 'dep-id' } },
-							appId: 1,
-							serviceName: 'dep',
-							serviceId: 2,
-							imageId: 2,
-							state: {
-								containerId: 'dep-id',
-							},
-						},
 					),
 				],
 				networks: [defaultNetwork],
@@ -931,26 +882,18 @@ describe('compose/app', () => {
 				...defaultContext,
 				...{
 					availableImages: [
-						{
+						createImage({ appId: 1, name: 'main-image', serviceName: 'main' }),
-							appId: 1,
-							dependent: 0,
-							imageId: 1,
-							releaseId: 1,
-							serviceId: 1,
-							name: 'main-image',
-							serviceName: 'main',
-						},
 					],
 				},
 			};
 			const current = createApp({
 				services: [
-					await createService({ running: false }, { serviceName: 'main' }),
+					await createService({ running: false, serviceName: 'main' }),
 				],
 				networks: [defaultNetwork],
 			});
 			const target = createApp({
-				services: [await createService({}, { serviceName: 'main' })],
+				services: [await createService({ serviceName: 'main' })],
 				networks: [defaultNetwork],
 				isTarget: true,
 			});
@@ -969,15 +912,7 @@ describe('compose/app', () => {
 				...defaultContext,
 				...{
 					availableImages: [
-						{
+						createImage({ appId: 1, name: 'main-image', serviceName: 'main' }),
-							appId: 1,
-							dependent: 0,
-							imageId: 1,
-							releaseId: 1,
-							serviceId: 1,
-							name: 'main-image',
-							serviceName: 'main',
-						},
 					],
 				},
 			};
@@ -988,31 +923,23 @@ describe('compose/app', () => {
 
 			const current = createApp({
 				services: [
-					await createService(
+					await createService({
-						{ labels, image: 'main-image' },
+						labels,
-						{
+						image: 'main-image',
-							appId: 1,
 						serviceName: 'main',
-							releaseId: 1,
+						commit: 'old-release',
-							serviceId: 1,
+					}),
-							imageId: 1,
-						},
-					),
 				],
 				networks: [defaultNetwork],
 			});
 			const target = createApp({
 				services: [
-					await createService(
+					await createService({
-						{ labels, image: 'main-image-2' },
+						labels,
-						{
+						image: 'main-image-2',
-							appId: 1,
 						serviceName: 'main',
-							releaseId: 2, // new release
+						commit: 'new-release',
-							serviceId: 1,
+					}),
-							imageId: 2, // new image id
-						},
-					),
 				],
 				networks: [defaultNetwork],
 				isTarget: true,
@@ -1049,86 +976,52 @@ describe('compose/app', () => {
 			const contextWithImages = {
 				...defaultContext,
 				...{
-					downloading: [4], // The depended service image is being downloaded
+					downloading: ['dep-image-2'], // The depended service image is being downloaded
 					availableImages: [
-						{
+						createImage({ appId: 1, name: 'main-image', serviceName: 'main' }),
+						createImage({ appId: 1, name: 'dep-image', serviceName: 'dep' }),
+						createImage({
 							appId: 1,
-							releaseId: 1,
-							dependent: 0,
-							name: 'main-image',
-							imageId: 1,
-							serviceName: 'main',
-							serviceId: 1,
-						},
-						{
-							appId: 1,
-							releaseId: 1,
-							dependent: 0,
-							name: 'dep-image',
-							imageId: 2,
-							serviceName: 'dep',
-							serviceId: 2,
-						},
-						{
-							appId: 1,
-							releaseId: 2,
-							dependent: 0,
 							name: 'main-image-2',
-							imageId: 3,
 							serviceName: 'main',
-							serviceId: 1,
+						}),
-						},
 					],
 				},
 			};
 
 			const current = createApp({
 				services: [
-					await createService(
+					await createService({
-						{ image: 'main-image', dependsOn: ['dep'] },
+						image: 'main-image',
-						{
+						dependsOn: ['dep'],
 						appId: 1,
 						serviceName: 'main',
-							releaseId: 1,
+						commit: 'old-release',
-							serviceId: 1,
+					}),
-							imageId: 1,
+					await createService({
-						},
+						image: 'dep-image',
-					),
-					await createService(
-						{ image: 'dep-image' },
-						{
 						appId: 1,
 						serviceName: 'dep',
-							releaseId: 1,
+						commit: 'old-release',
-							serviceId: 2,
+					}),
-							imageId: 2,
-						},
-					),
 				],
 				networks: [defaultNetwork],
 			});
 			const target = createApp({
 				services: [
-					await createService(
+					await createService({
-						{ image: 'main-image-2', dependsOn: ['dep'] },
+						image: 'main-image-2',
-						{
+						dependsOn: ['dep'],
 						appId: 1,
 						serviceName: 'main',
-							releaseId: 2, // new release
+						commit: 'new-release',
-							serviceId: 1,
+					}),
-							imageId: 3, // image has changed
+					await createService({
-						},
+						image: 'dep-image-2',
-					),
-					await createService(
-						{ image: 'dep-image-2' },
-						{
 						appId: 1,
 						serviceName: 'dep',
-							releaseId: 2,
+						commit: 'new-release',
-							serviceId: 2,
+					}),
-							imageId: 4,
-						},
-					),
 				],
 				networks: [defaultNetwork],
 				isTarget: true,
@@ -1144,44 +1037,34 @@ describe('compose/app', () => {
 				...defaultContext,
 				...{
 					availableImages: [
-						{
+						createImage({ appId: 1, name: 'main-image', serviceName: 'main' }),
+						createImage({
 							appId: 1,
-							releaseId: 1,
-							dependent: 0,
-							name: 'main-image',
-							imageId: 1,
-							serviceName: 'main',
-							serviceId: 1,
-						},
-						{
-							appId: 1,
-							releaseId: 2,
-							dependent: 0,
 							name: 'main-image-2',
-							imageId: 2,
 							serviceName: 'main',
-							serviceId: 1,
+						}),
-						},
 					],
 				},
 			};
 
 			const current = createApp({
 				services: [
-					await createService(
+					await createService({
-						{ image: 'main-image' },
+						image: 'main-image',
-						{ serviceName: 'main', releaseId: 1, serviceId: 1, imageId: 1 },
+						serviceName: 'main',
-					),
+						commit: 'old-release',
+					}),
 				],
 				networks: [defaultNetwork],
 			});
 			const target = createApp({
 				services: [
-					await createService(
+					await createService({
-						{ image: 'main-image-2' },
+						image: 'main-image-2',
 						// new release as target
-						{ serviceName: 'main', releaseId: 2, serviceId: 1, imageId: 2 },
+						serviceName: 'main',
-					),
+						commit: 'new-release',
+					}),
 				],
 				networks: [defaultNetwork],
 				isTarget: true,
@@ -1238,7 +1121,7 @@ describe('compose/app', () => {
 		it('should not create a service when a network it depends on is not ready', async () => {
 			const current = createApp({ networks: [defaultNetwork] });
 			const target = createApp({
-				services: [await createService({ networks: ['test'] }, { appId: 1 })],
+				services: [await createService({ networks: ['test'], appId: 1 })],
 				networks: [defaultNetwork, Network.fromComposeObject('test', 1, {})],
 				isTarget: true,
 			});
@@ -1256,50 +1139,30 @@ describe('compose/app', () => {
 		it('should create several kill steps as long as there are no unmet dependencies', async () => {
 			const current = createApp({
 				services: [
-					await createService(
+					await createService({
-						{},
-						{
 						appId: 1,
 						serviceName: 'one',
-							releaseId: 1,
+						commit: 'old-release',
-							serviceId: 1,
+					}),
-							imageId: 1,
+					await createService({
-						},
-					),
-					await createService(
-						{},
-						{
 						appId: 1,
 						serviceName: 'two',
-							releaseId: 1,
+						commit: 'old-release',
-							serviceId: 2,
+					}),
-							imageId: 2,
+					await createService({
-						},
-					),
-					await createService(
-						{},
-						{
 						appId: 1,
 						serviceName: 'three',
-							releaseId: 1,
+						commit: 'old-release',
-							serviceId: 3,
+					}),
-							imageId: 3,
-						},
-					),
 				],
 			});
 			const target = createApp({
 				services: [
-					await createService(
+					await createService({
-						{},
-						{
 						appId: 1,
 						serviceName: 'three',
-							releaseId: 1,
+						commit: 'new-release',
-							serviceId: 3,
+					}),
-							imageId: 3,
-						},
-					),
 				],
 				isTarget: true,
 			});
@@ -1313,7 +1176,7 @@ describe('compose/app', () => {
 		it('should emit a fetch step when an image has not been downloaded for a service', async () => {
 			const current = createApp({ services: [] });
 			const target = createApp({
-				services: [await createService({}, { serviceName: 'main' })],
+				services: [await createService({ serviceName: 'main' })],
 				isTarget: true,
 			});
 
@@ -1328,13 +1191,13 @@ describe('compose/app', () => {
 			const contextWithDownloading = {
 				...defaultContext,
 				...{
-					downloading: [1],
+					downloading: ['image2'],
 				},
 			};
 			const current = createApp({ services: [] });
 			const target = createApp({
 				services: [
-					await createService({}, { serviceName: 'main', imageId: 1 }),
+					await createService({ image: 'image2', serviceName: 'main' }),
 				],
 				isTarget: true,
 			});

test/src/compose/application-manager.spec.ts

@@ -18,24 +18,25 @@ import * as dbHelper from '../../lib/db-helper';
 const DEFAULT_NETWORK = Network.fromComposeObject('default', 1, {});
 
 async function createService(
-	conf = {} as Partial<ServiceComposeConfig>,
 	{
 		appId = 1,
 		serviceName = 'main',
-		releaseId = 1,
+		commit = 'main-commit',
-		serviceId = 1,
+		...conf
-		imageId = 1,
+	} = {} as Partial<ServiceComposeConfig>,
-		state = {} as Partial<Service>,
+	{ state = {} as Partial<Service>, options = {} as any } = {},
-		options = {} as any,
-	} = {},
 ) {
 	const svc = await Service.fromComposeObject(
 		{
 			appId,
 			serviceName,
-			releaseId,
+			commit,
-			serviceId,
+			// db ids should not be used for target state calculation, but images
-			imageId,
+			// are compared using _.isEqual so leaving this here to have image comparisons
+			// match
+			serviceId: 1,
+			imageId: 1,
+			releaseId: 1,
 			...conf,
 		},
 		options,
@@ -48,11 +49,28 @@ async function createService(
 	return svc;
 }
 
-function createImage(svc: Service) {
+function createImage(
+	{
+		appId = 1,
+		dependent = 0,
+		name = 'test-image',
+		serviceName = 'test',
+		...extra
+	} = {} as Partial<Image>,
+) {
 	return {
-		dockerImageId: svc.config.image,
+		appId,
-		...imageManager.imageFromService(svc),
+		dependent,
-	};
+		name,
+		serviceName,
+		// db ids should not be used for target state calculation, but images
+		// are compared using _.isEqual so leaving this here to have image comparisons
+		// match
+		imageId: 1,
+		releaseId: 1,
+		serviceId: 1,
+		...extra,
+	} as Image;
 }
 
 function createApps(
@@ -110,8 +128,12 @@ function createCurrentState({
 	services = [] as Service[],
 	networks = [] as Network[],
 	volumes = [] as Volume[],
-	images = services.map((s) => createImage(s)) as Image[],
+	images = services.map((s) => ({
-	downloading = [] as number[],
+		// Infer images from services by default
+		dockerImageId: s.config.image,
+		...imageManager.imageFromService(s),
+	})) as Image[],
+	downloading = [] as string[],
 }) {
 	const currentApps = createApps({ services, networks, volumes });
 
@@ -183,7 +205,7 @@ describe('compose/application-manager', () => {
 	it('infers a start step when all that changes is a running state', async () => {
 		const targetApps = createApps(
 			{
-				services: [await createService({ running: true }, { appId: 1 })],
+				services: [await createService({ running: true, appId: 1 })],
 				networks: [DEFAULT_NETWORK],
 			},
 			true,
@@ -194,7 +216,7 @@ describe('compose/application-manager', () => {
 			downloading,
 			containerIdsByAppId,
 		} = createCurrentState({
-			services: [await createService({ running: false }, { appId: 1 })],
+			services: [await createService({ running: false, appId: 1 })],
 			networks: [DEFAULT_NETWORK],
 		});
 
@@ -251,12 +273,7 @@ describe('compose/application-manager', () => {
 	it('infers a fetch step when a service has to be updated', async () => {
 		const targetApps = createApps(
 			{
-				services: [
+				services: [await createService({ image: 'image-new', appId: 1 })],
-					await createService(
-						{ image: 'image-new' },
-						{ appId: 1, imageId: 2, options: {} },
-					),
-				],
 				networks: [DEFAULT_NETWORK],
 			},
 			true,
@@ -267,7 +284,7 @@ describe('compose/application-manager', () => {
 			downloading,
 			containerIdsByAppId,
 		} = createCurrentState({
-			services: [await createService({}, { appId: 1, imageId: 1 })],
+			services: [await createService({ appId: 1 })],
 			networks: [DEFAULT_NETWORK],
 			images: [],
 		});
@@ -291,9 +308,7 @@ describe('compose/application-manager', () => {
 	it('does not infer a fetch step when the download is already in progress', async () => {
 		const targetApps = createApps(
 			{
-				services: [
+				services: [await createService({ image: 'image-new', appId: 1 })],
-					await createService({ image: 'image-new' }, { appId: 1, imageId: 2 }),
-				],
 				networks: [DEFAULT_NETWORK],
 			},
 			true,
@@ -304,9 +319,9 @@ describe('compose/application-manager', () => {
 			downloading,
 			containerIdsByAppId,
 		} = createCurrentState({
-			services: [await createService({}, { appId: 1, imageId: 1 })],
+			services: [await createService({ appId: 1 })],
 			networks: [DEFAULT_NETWORK],
-			downloading: [2],
+			downloading: ['image-new'],
 		});
 
 		const [noopStep, ...nextSteps] = await applicationManager.inferNextSteps(
@@ -330,10 +345,12 @@ describe('compose/application-manager', () => {
 		const targetApps = createApps(
 			{
 				services: [
-					await createService(
+					await createService({
-						{ image: 'image-new', labels },
+						image: 'image-new',
-						{ appId: 1, imageId: 2 },
+						labels,
-					),
+						appId: 1,
+						commit: 'new-release',
+					}),
 				],
 				networks: [DEFAULT_NETWORK],
 			},
@@ -346,10 +363,12 @@ describe('compose/application-manager', () => {
 			containerIdsByAppId,
 		} = createCurrentState({
 			services: [
-				await createService(
+				await createService({
-					{ image: 'image-old', labels },
+					image: 'image-old',
-					{ appId: 1, imageId: 1 },
+					labels,
-				),
+					appId: 1,
+					commit: 'old-release',
+				}),
 			],
 			networks: [DEFAULT_NETWORK],
 		});
@@ -374,26 +393,19 @@ describe('compose/application-manager', () => {
 		const targetApps = createApps(
 			{
 				services: [
-					await createService(
+					await createService({
-						{ image: 'main-image', dependsOn: ['dep'] },
+						image: 'main-image',
-						{
+						dependsOn: ['dep'],
 						appId: 1,
-							imageId: 3,
+						commit: 'new-release',
-							serviceId: 1,
 						serviceName: 'main',
-							releaseId: 2,
+					}),
-						},
+					await createService({
-					),
+						image: 'dep-image',
-					await createService(
-						{ image: 'dep-image' },
-						{
 						appId: 1,
-							imageId: 4,
+						commit: 'new-release',
-							serviceId: 2,
 						serviceName: 'dep',
-							releaseId: 2,
+					}),
-						},
-					),
 				],
 				networks: [DEFAULT_NETWORK],
 			},
@@ -406,28 +418,27 @@ describe('compose/application-manager', () => {
 			containerIdsByAppId,
 		} = createCurrentState({
 			services: [
-				await createService(
+				await createService({
-					{ dependsOn: ['dep'] },
+					dependsOn: ['dep'],
-					{ appId: 1, imageId: 1, serviceId: 1, serviceName: 'main' },
+					appId: 1,
-				),
+					commit: 'old-release',
-				await createService(
+					serviceName: 'main',
-					{},
+				}),
-					{ appId: 1, imageId: 2, serviceId: 2, serviceName: 'dep' },
+				await createService({
-				),
+					appId: 1,
+					commit: 'old-release',
+					serviceName: 'dep',
+				}),
 			],
 			networks: [DEFAULT_NETWORK],
-			downloading: [4], // dep-image is still being downloaded
+			downloading: ['dep-image'], // dep-image is still being downloaded
 			images: [
 				// main-image was already downloaded
-				{
+				createImage({
 					appId: 1,
-					releaseId: 2,
 					name: 'main-image',
-					imageId: 3,
 					serviceName: 'main',
-					serviceId: 1,
+				}),
-					dependent: 0,
-				},
 			],
 		});
 
@@ -449,26 +460,19 @@ describe('compose/application-manager', () => {
 		const targetApps = createApps(
 			{
 				services: [
-					await createService(
+					await createService({
-						{ image: 'main-image', dependsOn: ['dep'] },
+						image: 'main-image',
-						{
+						dependsOn: ['dep'],
 						appId: 1,
-							imageId: 3,
+						commit: 'new-release',
-							serviceId: 1,
 						serviceName: 'main',
-							releaseId: 2,
+					}),
-						},
+					await createService({
-					),
+						image: 'dep-image',
-					await createService(
-						{ image: 'dep-image' },
-						{
 						appId: 1,
-							imageId: 4,
+						commit: 'new-release',
-							serviceId: 2,
 						serviceName: 'dep',
-							releaseId: 2,
+					}),
-						},
-					),
 				],
 				networks: [DEFAULT_NETWORK],
 			},
@@ -482,36 +486,31 @@ describe('compose/application-manager', () => {
 			containerIdsByAppId,
 		} = createCurrentState({
 			services: [
-				await createService(
+				await createService({
-					{ dependsOn: ['dep'] },
+					dependsOn: ['dep'],
-					{ appId: 1, imageId: 1, serviceId: 1, serviceName: 'main' },
+					appId: 1,
-				),
+					commit: 'old-release',
-				await createService(
+					serviceName: 'main',
-					{},
+				}),
-					{ appId: 1, imageId: 2, serviceId: 2, serviceName: 'dep' },
+				await createService({
-				),
+					appId: 1,
+					commit: 'old-release',
+					serviceName: 'dep',
+				}),
 			],
 			networks: [DEFAULT_NETWORK],
 			images: [
 				// Both images have been downloaded
-				{
+				createImage({
 					appId: 1,
-					releaseId: 2,
 					name: 'main-image',
-					imageId: 3,
 					serviceName: 'main',
-					serviceId: 1,
+				}),
-					dependent: 0,
+				createImage({
-				},
-				{
 					appId: 1,
-					releaseId: 2,
 					name: 'dep-image',
-					imageId: 4,
 					serviceName: 'dep',
-					serviceId: 2,
+				}),
-					dependent: 0,
-				},
 			],
 		});
 
@@ -542,22 +541,17 @@ describe('compose/application-manager', () => {
 		const targetApps = createApps(
 			{
 				services: [
-					await createService(
+					await createService({
-						{ image: 'main-image', dependsOn: ['dep'] },
+						image: 'main-image',
-						{
+						dependsOn: ['dep'],
-							imageId: 1,
-							serviceId: 1,
 						serviceName: 'main',
-						},
+						commit: 'new-release',
-					),
+					}),
-					await createService(
+					await createService({
-						{ image: 'dep-image' },
+						image: 'dep-image',
-						{
-							imageId: 2,
-							serviceId: 2,
 						serviceName: 'dep',
-						},
+						commit: 'new-release',
-					),
+					}),
 				],
 				networks: [DEFAULT_NETWORK],
 			},
@@ -574,24 +568,16 @@ describe('compose/application-manager', () => {
 			networks: [DEFAULT_NETWORK],
 			images: [
 				// Both images have been downloaded
-				{
+				createImage({
 					appId: 1,
-					releaseId: 1,
 					name: 'main-image',
-					imageId: 1,
 					serviceName: 'main',
-					serviceId: 1,
+				}),
-					dependent: 0,
+				createImage({
-				},
-				{
 					appId: 1,
-					releaseId: 1,
 					name: 'dep-image',
-					imageId: 2,
 					serviceName: 'dep',
-					serviceId: 2,
+				}),
-					dependent: 0,
-				},
 			],
 		});
 
@@ -619,22 +605,17 @@ describe('compose/application-manager', () => {
 		const targetApps = createApps(
 			{
 				services: [
-					await createService(
+					await createService({
-						{ image: 'main-image', dependsOn: ['dep'] },
+						image: 'main-image',
-						{
+						dependsOn: ['dep'],
-							imageId: 1,
-							serviceId: 1,
 						serviceName: 'main',
-						},
+						commit: 'new-release',
-					),
+					}),
-					await createService(
+					await createService({
-						{ image: 'dep-image' },
+						image: 'dep-image',
-						{
-							imageId: 2,
-							serviceId: 2,
 						serviceName: 'dep',
-						},
+						commit: 'new-release',
-					),
+					}),
 				],
 				networks: [DEFAULT_NETWORK],
 			},
@@ -648,36 +629,25 @@ describe('compose/application-manager', () => {
 			containerIdsByAppId,
 		} = createCurrentState({
 			services: [
-				await createService(
+				await createService({
-					{ image: 'dep-image' },
+					image: 'dep-image',
-					{
-						imageId: 2,
-						serviceId: 2,
 					serviceName: 'dep',
-					},
+					commit: 'new-release',
-				),
+				}),
 			],
 			networks: [DEFAULT_NETWORK],
 			images: [
 				// Both images have been downloaded
-				{
+				createImage({
 					appId: 1,
-					releaseId: 1,
 					name: 'main-image',
-					imageId: 1,
 					serviceName: 'main',
-					serviceId: 1,
+				}),
-					dependent: 0,
+				createImage({
-				},
-				{
 					appId: 1,
-					releaseId: 1,
 					name: 'dep-image',
-					imageId: 2,
 					serviceName: 'dep',
-					serviceId: 2,
+				}),
-					dependent: 0,
-				},
 			],
 		});
 
@@ -714,27 +684,15 @@ describe('compose/application-manager', () => {
 			downloading,
 			containerIdsByAppId,
 		} = createCurrentState({
-			services: [
+			services: [await createService({ appId: 5, serviceName: 'old-service' })],
-				await createService(
-					{},
-					{
-						appId: 5,
-						serviceName: 'old-service',
-					},
-				),
-			],
 			networks: [DEFAULT_NETWORK],
 			images: [
-				// Both images have been downloaded
+				// Image has been downloaded
-				{
+				createImage({
 					appId: 1,
-					releaseId: 1,
 					name: 'main-image',
-					imageId: 1,
 					serviceName: 'main',
-					serviceId: 1,
+				}),
-					dependent: 0,
-				},
 			],
 		});
 
@@ -822,7 +780,7 @@ describe('compose/application-manager', () => {
 		(networkManager.supervisorNetworkReady as sinon.SinonStub).resolves(false);
 
 		const targetApps = createApps(
-			{ services: [await createService({})], networks: [DEFAULT_NETWORK] },
+			{ services: [await createService()], networks: [DEFAULT_NETWORK] },
 			true,
 		);
 		const {
@@ -955,26 +913,18 @@ describe('compose/application-manager', () => {
 			networks: [DEFAULT_NETWORK],
 			images: [
 				// An image for a service that no longer exists
-				{
+				createImage({
 					name: 'old-image',
 					appId: 5,
-					serviceId: 5,
 					serviceName: 'old-service',
-					imageId: 5,
-					dependent: 0,
-					releaseId: 5,
 					dockerImageId: 'sha256:aaaa',
-				},
+				}),
-				{
+				createImage({
 					name: 'main-image',
 					appId: 1,
-					serviceId: 1,
 					serviceName: 'main',
-					imageId: 1,
-					dependent: 0,
-					releaseId: 1,
 					dockerImageId: 'sha256:bbbb',
-				},
+				}),
 			],
 		});
 
@@ -1021,26 +971,18 @@ describe('compose/application-manager', () => {
 			networks: [DEFAULT_NETWORK],
 			images: [
 				// An image for a service that no longer exists
-				{
+				createImage({
 					name: 'old-image',
 					appId: 5,
-					serviceId: 5,
 					serviceName: 'old-service',
-					imageId: 5,
-					dependent: 0,
-					releaseId: 5,
 					dockerImageId: 'sha256:aaaa',
-				},
+				}),
-				{
+				createImage({
 					name: 'main-image',
 					appId: 1,
-					serviceId: 1,
 					serviceName: 'main',
-					imageId: 1,
-					dependent: 0,
-					releaseId: 1,
 					dockerImageId: 'sha256:bbbb',
-				},
+				}),
 			],
 		});
 
@@ -1109,14 +1051,18 @@ describe('compose/application-manager', () => {
 		const targetApps = createApps(
 			{
 				services: [
-					await createService(
+					await createService({
-						{ running: true, image: 'main-image-1' },
+						running: true,
-						{ appId: 1, serviceId: 1, imageId: 1 },
+						image: 'main-image-1',
-					),
+						appId: 1,
-					await createService(
+						commit: 'commit-for-app-1',
-						{ running: true, image: 'main-image-2' },
+					}),
-						{ appId: 2, serviceId: 2, imageId: 2 },
+					await createService({
-					),
+						running: true,
+						image: 'main-image-2',
+						appId: 2,
+						commit: 'commit-for-app-2',
+					}),
 				],
 				networks: [
 					// Default networks for two apps
@@ -1139,24 +1085,16 @@ describe('compose/application-manager', () => {
 				Network.fromComposeObject('default', 2, {}),
 			],
 			images: [
-				{
+				createImage({
 					name: 'main-image-1',
 					appId: 1,
-					serviceId: 1,
 					serviceName: 'main',
-					imageId: 1,
+				}),
-					dependent: 0,
+				createImage({
-					releaseId: 1,
-				},
-				{
 					name: 'main-image-2',
 					appId: 2,
-					serviceId: 2,
 					serviceName: 'main',
-					imageId: 2,
+				}),
-					dependent: 0,
-					releaseId: 1,
-				},
 			],
 		});