Remove releaseId and imageId from container names

src/compose/app.ts

@@ -658,7 +658,10 @@ export class App {
 
 	private generateContainerStep(current: Service, target: Service) {
 		// if the services release doesn't match, then rename the container...
-		if (current.commit !== target.commit) {
+		if (
+			current.commit !== target.commit ||
+			current.containerName !== target.containerName
+		) {
 			return generateStep('updateMetadata', { current, target });
 		} else if (target.config.running !== current.config.running) {
 			if (target.config.running) {

src/compose/application-manager.ts

@@ -350,6 +350,8 @@ export async function getCurrentApps(): Promise<InstancedAppState> {
 				);
 
 				s.imageName = imageForService?.name ?? s.imageName;
+				s.releaseId = imageForService?.releaseId ?? s.releaseId;
+				s.imageId = imageForService?.imageId ?? s.imageId;
 				return s;
 			});
 
@@ -695,7 +697,10 @@ function saveAndRemoveImages(
 		availableWithoutIds,
 		(image) =>
 			!_.some(currentImages.concat(targetImages), (imageInUse) => {
-				return _.isEqual(image, _.omit(imageInUse, ['dockerImageId', 'id']));
+				return _.isEqual(
+					_.omit(image, ['releaseId', 'imageId']),
+					_.omit(imageInUse, ['dockerImageId', 'id', 'releaseId', 'imageId']),
+				);
 			}),
 	);
 
@@ -829,8 +834,16 @@ export async function getLegacyState() {
 	// We iterate over the current running services and add them to the current state
 	// of the app they belong to.
 	for (const service of services) {
-		const { appId, imageId } = service;
-		if (!appId) {
+		const { appId } = service;
+
+		// We get the image for the service so we can get service metadata not
+		// in the containers
+		const imageForService = images.find(
+			(img) =>
+				img.serviceName === service.serviceName &&
+				img.commit === service.commit,
+		);
+		if (!appId || !imageForService) {
 			continue;
 		}
 		if (apps[appId] == null) {
@@ -840,17 +853,19 @@ export async function getLegacyState() {
 		if (apps[appId].services == null) {
 			apps[appId].services = {};
 		}
+
+		const { releaseId: rId, imageId } = imageForService;
+
+		// Replace the service releaseId with the one from the image table above
+		service.releaseId = rId;
+
 		// We only send commit if all services have the same release, and it matches the target release
 		if (releaseId == null) {
-			({ releaseId } = service);
-		} else if (releaseId !== service.releaseId) {
+			releaseId = service.releaseId;
+		} else if (service.releaseId != null && releaseId !== service.releaseId) {
 			releaseId = false;
 		}
-		if (imageId == null) {
-			throw new InternalInconsistencyError(
-				`imageId not defined in ApplicationManager.getLegacyApplicationsState: ${service}`,
-			);
-		}
+
 		if (apps[appId].services[imageId] == null) {
 			apps[appId].services[imageId] = _.pick(service, ['status', 'releaseId']);
 			creationTimesAndReleases[appId][imageId] = _.pick(service, [
@@ -869,20 +884,16 @@ export async function getLegacyState() {
 	}
 
 	for (const image of images) {
-		const { appId } = image;
+		const { appId, imageId } = image;
 		if (apps[appId] == null) {
 			apps[appId] = {};
 		}
 		if (apps[appId].services == null) {
 			apps[appId].services = {};
 		}
-		if (apps[appId].services[image.imageId] == null) {
-			apps[appId].services[image.imageId] = _.pick(image, [
-				'status',
-				'releaseId',
-			]);
-			apps[appId].services[image.imageId].download_progress =
-				image.downloadProgress;
+		if (apps[appId].services[imageId] == null) {
+			apps[appId].services[imageId] = _.pick(image, ['status', 'releaseId']);
+			apps[appId].services[imageId].download_progress = image.downloadProgress;
 		}
 	}
 

src/compose/images.ts

@@ -179,7 +179,11 @@ export function imageFromService(service: ServiceInfo): Image {
 		appUuid: service.appUuid!,
 		serviceId: service.serviceId!,
 		serviceName: service.serviceName!,
+		// Image id is optional in service, but it is always available
+		// on the target state
 		imageId: service.imageId!,
+		// Release id is optional in service, but it is always available
+		// on the target state
 		releaseId: service.releaseId!,
 		commit: service.commit!,
 	};

src/compose/service-manager.ts

@@ -123,9 +123,7 @@ export async function getState() {
 			status[service.containerId] = _.pick(service, [
 				'appId',
 				'appUuid',
-				'imageId',
 				'status',
-				'releaseId',
 				'commit',
 				'createdAt',
 				'serviceName',
@@ -159,7 +157,7 @@ export async function updateMetadata(service: Service, target: Service) {
 
 	try {
 		await docker.getContainer(svc.containerId).rename({
-			name: `${service.serviceName}_${target.imageId}_${target.releaseId}_${target.commit}`,
+			name: target.containerName,
 		});
 	} catch (e) {
 		if (isNotFoundError(e)) {
@@ -352,14 +350,13 @@ export async function start(service: Service) {
 		}
 
 		const serviceId = service.serviceId;
-		const imageId = service.imageId;
-		if (serviceId == null || imageId == null) {
+		if (serviceId == null) {
 			throw new InternalInconsistencyError(
-				`serviceId and imageId not defined for service: ${service.serviceName} in ServiceManager.start`,
+				`serviceId not defined for service: ${service.serviceName} in ServiceManager.start`,
 			);
 		}
 
-		logger.attach(container.id, { serviceId, imageId });
+		logger.attach(container.id, { serviceId });
 
 		if (!alreadyStarted) {
 			logger.logSystemEvent(LogTypes.startServiceSuccess, { service });
@@ -415,15 +412,13 @@ export function listenToEvents() {
 								});
 
 								const serviceId = service.serviceId;
-								const imageId = service.imageId;
-								if (serviceId == null || imageId == null) {
+								if (serviceId == null) {
 									throw new InternalInconsistencyError(
-										`serviceId and imageId not defined for service: ${service.serviceName} in ServiceManager.listenToEvents`,
+										`serviceId not defined for service: ${service.serviceName} in ServiceManager.listenToEvents`,
 									);
 								}
 								logger.attach(data.id, {
 									serviceId,
-									imageId,
 								});
 							} else if (status === 'destroy') {
 								await logMonitor.detach(data.id);
@@ -467,10 +462,9 @@ export async function attachToRunning() {
 	for (const service of services) {
 		if (service.status === 'Running') {
 			const serviceId = service.serviceId;
-			const imageId = service.imageId;
-			if (serviceId == null || imageId == null) {
+			if (serviceId == null) {
 				throw new InternalInconsistencyError(
-					`serviceId and imageId not defined for service: ${service.serviceName} in ServiceManager.start`,
+					`serviceId not defined for service: ${service.serviceName} in ServiceManager.start`,
 				);
 			}
 
@@ -481,7 +475,6 @@ export async function attachToRunning() {
 			}
 			logger.attach(service.containerId, {
 				serviceId,
-				imageId,
 			});
 		}
 	}
@@ -522,15 +515,7 @@ function reportNewStatus(
 		containerId,
 		_.merge(
 			{ status },
-			_.pick(service, [
-				'imageId',
-				'appId',
-				'appUuid',
-				'serviceName',
-				'releaseId',
-				'createdAt',
-				'commit',
-			]),
+			_.pick(service, ['appUuid', 'serviceName', 'createdAt', 'commit']),
 		),
 	);
 }
@@ -546,9 +531,7 @@ async function killContainer(
 	// TODO: Remove the need for the wait flag
 
 	logger.logSystemEvent(LogTypes.stopService, { service });
-	if (service.imageId != null) {
-		reportNewStatus(containerId, service, 'Stopping');
-	}
+	reportNewStatus(containerId, service, 'Stopping');
 
 	const containerObj = docker.getContainer(containerId);
 	const killPromise = containerObj
@@ -594,9 +577,7 @@ async function killContainer(
 			});
 		})
 		.finally(() => {
-			if (service.imageId != null) {
-				reportChange(containerId);
-			}
+			reportChange(containerId);
 		});
 
 	if (wait) {
@@ -633,7 +614,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.releaseId}_${service.commit}`,
+		name: `old_${service.serviceName}_${service.commit}`,
 	});
 }
 

src/compose/service.ts

@@ -52,14 +52,25 @@ export type ServiceStatus =
 export class Service {
 	public appId: number;
 	public appUuid?: string;
-	public imageId: number;
+	/**
+	 * We make this available as it still needed by application manager to
+	 * compare images, but it is only available in the target state.
+	 * @deprecated
+	 */
+	public imageId?: number;
 	public config: ServiceConfig;
 	public serviceName: string;
 	public commit: string;
-	public releaseId: number;
+	/**
+	 * We make this available as it still needed by application manager
+	 * but it is only available on the target state
+	 * @deprecated
+	 */
+	public releaseId?: number;
 	public serviceId: number;
 	public imageName: string | null;
 	public containerId: string | null;
+	public containerName: string;
 	public exitErrorMessage: string | null;
 
 	public dependsOn: string[] | null;
@@ -143,6 +154,9 @@ export class Service {
 		service.commit = appConfig.commit;
 		service.appUuid = appConfig.appUuid;
 
+		// The target container name
+		service.containerName = `${service.serviceName}_${service.commit}`;
+
 		// dependsOn is used by other parts of the step
 		// calculation so we delete it from the composition
 		service.dependsOn = appConfig.composition?.dependsOn || null;
@@ -617,13 +631,19 @@ 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+))?(?:_([a-f0-9]*))?$/,
+		);
 		if (nameMatch == null) {
 			throw new InternalInconsistencyError(
-				`Expected supervised container to have name '<serviceName>_<imageId>_<releaseId>_<commit>', got: ${container.Name}`,
+				`Expected supervised container to have name '<serviceName>_<commit>', got: ${container.Name}`,
 			);
 		}
 
+		// The current container name, minus the initial '/'
+		svc.containerName = container.Name.slice(1);
+
+		// If we have not renamed the service yet we can still use the image id
 		svc.imageId = parseInt(nameMatch[1], 10);
 		svc.releaseId = parseInt(nameMatch[2], 10);
 		svc.commit = nameMatch[3];
@@ -674,7 +694,7 @@ export class Service {
 			this.config.networkMode = `container:${containerId}`;
 		}
 		return {
-			name: `${this.serviceName}_${this.imageId}_${this.releaseId}_${this.commit}`,
+			name: `${this.serviceName}_${this.commit}`,
 			Tty: this.config.tty,
 			Cmd: this.config.command,
 			Volumes: volumes,
@@ -880,7 +900,8 @@ export class Service {
 	): boolean {
 		return (
 			this.isEqualConfig(service, currentContainerIds) &&
-			this.commit === service.commit
+			this.commit === service.commit &&
+			this.containerName === service.containerName
 		);
 	}
 
@@ -1134,7 +1155,7 @@ export class Service {
 				log.warn(
 					`Ignoring invalid compose volume definition ${
 						isString ? volume : JSON.stringify(volume)
-					}`,
+					} `,
 				);
 			}
 		}

src/device-api/actions.ts

@@ -8,6 +8,7 @@ import * as logger from '../logger';
 import * as config from '../config';
 import * as hostConfig from '../host-config';
 import * as applicationManager from '../compose/application-manager';
+import * as imageManager from '../compose/images';
 import {
 	CompositionStepAction,
 	generateStep,
@@ -273,9 +274,7 @@ export const executeServiceAction = async ({
 		);
 	}
 	const targetService = targetApp.services.find(
-		(s) =>
-			s.imageId === currentService.imageId ||
-			s.serviceName === currentService.serviceName,
+		(s) => s.serviceName === currentService.serviceName,
 	);
 	if (targetService == null) {
 		throw new NotFoundError(messages.targetServiceNotFound);
@@ -331,6 +330,15 @@ export const getSingleContainerApp = async (appId: number) => {
 		);
 	}
 
+	// releaseId is not part of the documented outputs of this endpoint
+	// but it has been there for a while, we query the value from
+	// the image list if available. But we won't error if not found
+	const images = await imageManager.getState();
+	service.releaseId = images.find(
+		(img) =>
+			img.serviceName === service.serviceName && img.commit === service.commit,
+	)?.releaseId;
+
 	// Because we only have a single app, we can fetch the commit for that
 	// app, and maintain backwards compatability
 	const commit = await commitStore.getCommitForApp(appId);

src/device-api/v2.ts

@@ -424,25 +424,32 @@ router.get('/v2/containerId', async (req: AuthorizedRequest, res) => {
 router.get('/v2/state/status', async (req: AuthorizedRequest, res) => {
 	const appIds: number[] = [];
 	const pending = deviceState.isApplyInProgress();
+	const allImages = await images.getState();
 	const containerStates = (await serviceManager.getAll())
 		.filter((service) => req.auth.isScoped({ apps: [service.appId] }))
 		.map((svc) => {
 			appIds.push(svc.appId);
-			return _.pick(
+			const s = _.pick(
 				svc,
 				'status',
 				'serviceName',
 				'appId',
-				'imageId',
 				'serviceId',
+				'imageId',
 				'containerId',
 				'createdAt',
 			);
+
+			s.imageId = allImages.find(
+				(img) => img.commit === svc.commit && img.serviceName === s.serviceName,
+			)?.imageId;
+
+			return s;
 		});
 
 	let downloadProgressTotal = 0;
 	let downloads = 0;
-	const imagesStates = (await images.getState())
+	const imagesStates = allImages
 		.filter((img) => req.auth.isScoped({ apps: [img.appId] }))
 		.map((img) => {
 			appIds.push(img.appId);