Make images module a singleton

Change-type: patch
Signed-off-by: Cameron Diver <cameron@balena.io>

src/application-manager.d.ts

@@ -6,7 +6,7 @@ import Knex = require('knex');
 import { ServiceAction } from './device-api/common';
 import { DeviceStatus, InstancedAppState } from './types/state';
 
-import ImageManager, { Image } from './compose/images';
+import type { Image } from './compose/images';
 import ServiceManager from './compose/service-manager';
 import DeviceState from './device-state';
 
@@ -51,7 +51,6 @@ class ApplicationManager extends EventEmitter {
 	public services: ServiceManager;
 	public volumes: VolumeManager;
 	public networks: NetworkManager;
-	public images: ImageManager;
 
 	public proxyvisor: any;
 	public timeSpentFetching: number;

src/application-manager.js

@@ -28,7 +28,7 @@ import { TargetStateAccessor } from './device-state/target-state-cache';
 
 import { ServiceManager } from './compose/service-manager';
 import { Service } from './compose/service';
-import { Images } from './compose/images';
+import * as Images from './compose/images';
 import { NetworkManager } from './compose/network-manager';
 import { Network } from './compose/network';
 import { VolumeManager } from './compose/volume-manager';
@@ -172,12 +172,11 @@ export class ApplicationManager extends EventEmitter {
 		this.reportOptionalContainers = this.reportOptionalContainers.bind(this);
 		this.deviceState = deviceState;
 		this.apiBinder = apiBinder;
-		this.images = new Images();
+
 		this.services = new ServiceManager();
 		this.networks = new NetworkManager();
 		this.volumes = new VolumeManager();
 		this.proxyvisor = new Proxyvisor({
-			images: this.images,
 			applications: this,
 		});
 		this.localModeManager = new LocalModeManager();
@@ -188,19 +187,12 @@ export class ApplicationManager extends EventEmitter {
 
 		this.targetStateWrapper = new TargetStateAccessor(this);
 
-		config.on('change', (changedConfig) => {
-			if (changedConfig.appUpdatePollInterval) {
-				this.images.appUpdatePollInterval = changedConfig.appUpdatePollInterval;
-			}
-		});
-
 		this.actionExecutors = compositionSteps.getExecutors({
 			lockFn: this._lockingIfNecessary,
 			services: this.services,
 			networks: this.networks,
 			volumes: this.volumes,
 			applications: this,
-			images: this.images,
 			callbacks: {
 				containerStarted: (id) => {
 					this._containerStarted[id] = true;
@@ -225,7 +217,7 @@ export class ApplicationManager extends EventEmitter {
 			this.proxyvisor.validActions,
 		);
 		this.router = createApplicationManagerRouter(this);
-		this.images.on('change', this.reportCurrentState);
+		Images.on('change', this.reportCurrentState);
 		this.services.on('change', this.reportCurrentState);
 	}
 
@@ -234,12 +226,8 @@ export class ApplicationManager extends EventEmitter {
 	}
 
 	init() {
-		return config
+		return Images.initialized
-			.get('appUpdatePollInterval')
+			.then(() => Images.cleanupDatabase())
-			.then((interval) => {
-				this.images.appUpdatePollInterval = interval;
-				return this.images.cleanupDatabase();
-			})
 			.then(() => {
 				const cleanup = () => {
 					return docker.listContainers({ all: true }).then((containers) => {
@@ -271,7 +259,7 @@ export class ApplicationManager extends EventEmitter {
 	getStatus() {
 		return Promise.join(
 			this.services.getStatus(),
-			this.images.getStatus(),
+			Images.getStatus(),
 			config.get('currentCommit'),
 			function (services, images, currentCommit) {
 				const apps = {};
@@ -1006,7 +994,7 @@ export class ApplicationManager extends EventEmitter {
 			return service;
 		});
 		return Promise.map(services, (service) => {
-			service.image = this.images.normalise(service.image);
+			service.image = Images.normalise(service.image);
 			return Promise.props(service);
 		}).then(function ($services) {
 			const dbApp = {
@@ -1026,7 +1014,7 @@ export class ApplicationManager extends EventEmitter {
 	createTargetService(service, opts) {
 		// The image class now returns a native promise, so wrap
 		// this in a bluebird promise until we convert this to typescript
-		return Promise.resolve(this.images.inspectByName(service.image))
+		return Promise.resolve(Images.inspectByName(service.image))
 			.catchReturn(NotFoundError, undefined)
 			.then(function (imageInfo) {
 				const serviceOpts = {
@@ -1589,9 +1577,9 @@ export class ApplicationManager extends EventEmitter {
 
 		return config.get('localMode').then((localMode) => {
 			return Promise.props({
-				cleanupNeeded: this.images.isCleanupNeeded(),
+				cleanupNeeded: Images.isCleanupNeeded(),
-				availableImages: this.images.getAvailable(),
+				availableImages: Images.getAvailable(),
-				downloading: this.images.getDownloadingImageIds(),
+				downloading: Images.getDownloadingImageIds(),
 				supervisorNetworkReady: this.networks.supervisorNetworkReady(),
 				delta: config.get('delta'),
 				containerIds: Promise.props(containerIdsByAppId),

src/compose/composition-steps.ts

@@ -3,7 +3,8 @@ import * as _ from 'lodash';
 import * as config from '../config';
 
 import { ApplicationManager } from '../application-manager';
-import Images, { Image } from './images';
+import type { Image } from './images';
+import * as images from './images';
 import Network from './network';
 import Service from './service';
 import ServiceManager from './service-manager';
@@ -139,7 +140,6 @@ export function getExecutors(app: {
 	networks: NetworkManager;
 	volumes: VolumeManager;
 	applications: ApplicationManager;
-	images: Images;
 	callbacks: CompositionCallbacks;
 }) {
 	const executors: Executors<CompositionStepAction> = {
@@ -171,7 +171,7 @@ export function getExecutors(app: {
 					await app.services.kill(step.current);
 					app.callbacks.containerKilled(step.current.containerId);
 					if (_.get(step, ['options', 'removeImage'])) {
-						await app.images.removeByDockerId(step.current.config.image);
+						await images.removeByDockerId(step.current.config.image);
 					}
 				},
 			);
@@ -241,7 +241,7 @@ export function getExecutors(app: {
 			app.callbacks.fetchStart();
 			const [fetchOpts, availableImages] = await Promise.all([
 				config.get('fetchOptions'),
-				app.images.getAvailable(),
+				images.getAvailable(),
 			]);
 
 			const opts = {
@@ -249,7 +249,7 @@ export function getExecutors(app: {
 				...fetchOpts,
 			};
 
-			await app.images.triggerFetch(
+			await images.triggerFetch(
 				step.image,
 				opts,
 				async (success) => {
@@ -269,15 +269,15 @@ export function getExecutors(app: {
 			);
 		},
 		removeImage: async (step) => {
-			await app.images.remove(step.image);
+			await images.remove(step.image);
 		},
 		saveImage: async (step) => {
-			await app.images.save(step.image);
+			await images.save(step.image);
 		},
 		cleanup: async () => {
 			const localMode = await config.get('localMode');
 			if (!localMode) {
-				await app.images.cleanup();
+				await images.cleanup();
 			}
 		},
 		createNetwork: async (step) => {

src/compose/images.ts

@@ -4,6 +4,7 @@ import { EventEmitter } from 'events';
 import * as _ from 'lodash';
 import StrictEventEmitter from 'strict-event-emitter-types';
 
+import * as config from '../config';
 import * as db from '../db';
 import * as constants from '../lib/constants';
 import {
@@ -21,12 +22,6 @@ import { ImageDownloadBackoffError } from './errors';
 
 import log from '../lib/supervisor-console';
 
-interface ImageEvents {
-	change: void;
-}
-
-type ImageEventEmitter = StrictEventEmitter<EventEmitter, ImageEvents>;
-
 interface FetchProgressEvent {
 	percentage: number;
 }
@@ -52,37 +47,61 @@ type NormalisedDockerImage = Docker.ImageInfo & {
 	NormalisedRepoTags: string[];
 };
 
-export class Images extends (EventEmitter as new () => ImageEventEmitter) {
+// Setup an event emitter
-	public appUpdatePollInterval: number;
+interface ImageEvents {
+	change: void;
+}
+class ImageEventEmitter extends (EventEmitter as new () => StrictEventEmitter<
+	EventEmitter,
+	ImageEvents
+>) {}
+const events = new ImageEventEmitter();
 
-	private imageFetchFailures: Dictionary<number> = {};
+export const on: typeof events['on'] = events.on.bind(events);
-	private imageFetchLastFailureTime: Dictionary<
+export const once: typeof events['once'] = events.once.bind(events);
-		ReturnType<typeof process.hrtime>
+export const removeListener: typeof events['removeListener'] = events.removeListener.bind(
-	> = {};
+	events,
-	private imageCleanupFailures: Dictionary<number> = {};
+);
-	// A store of volatile state for images (e.g. download progress), indexed by imageId
+export const removeAllListeners: typeof events['removeAllListeners'] = events.removeAllListeners.bind(
-	private volatileState: { [imageId: number]: Image } = {};
+	events,
+);
 
-	public constructor() {
+const imageFetchFailures: Dictionary<number> = {};
-		super();
+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;
+
+export const initialized = (async () => {
+	await config.initialized;
+	appUpdatePollInterval = await config.get('appUpdatePollInterval');
+	config.on('change', (vals) => {
+		if (vals.appUpdatePollInterval != null) {
+			appUpdatePollInterval = vals.appUpdatePollInterval;
 		}
+	});
+})();
 
-	public async triggerFetch(
+export async function triggerFetch(
 	image: Image,
 	opts: FetchOptions,
 	onFinish = _.noop,
 	serviceName: string,
-	): Promise<null> {
+): Promise<void> {
-		if (this.imageFetchFailures[image.name] != null) {
+	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
 		// engine, and ensure that we wait a bit lnger
 		const minDelay = Math.min(
-				2 ** this.imageFetchFailures[image.name] * constants.backoffIncrement,
+			2 ** imageFetchFailures[image.name] * constants.backoffIncrement,
-				this.appUpdatePollInterval,
+			appUpdatePollInterval,
 		);
 		const timeSinceLastError = process.hrtime(
-				this.imageFetchLastFailureTime[image.name],
+			imageFetchLastFailureTime[image.name],
 		);
 		const timeSinceLastErrorMs =
 			timeSinceLastError[0] * 1000 + timeSinceLastError[1] / 1e6;
@@ -93,8 +112,8 @@ export class Images extends (EventEmitter as new () => ImageEventEmitter) {
 
 	const onProgress = (progress: FetchProgressEvent) => {
 		// Only report the percentage if we haven't finished fetching
-			if (this.volatileState[image.imageId] != null) {
+		if (volatileState[image.imageId] != null) {
-				this.reportChange(image.imageId, {
+			reportChange(image.imageId, {
 				downloadProgress: progress.percentage,
 			});
 		}
@@ -102,25 +121,25 @@ export class Images extends (EventEmitter as new () => ImageEventEmitter) {
 
 	let success: boolean;
 	try {
-			const imageName = await this.normalise(image.name);
+		const imageName = await normalise(image.name);
 		image = _.clone(image);
 		image.name = imageName;
 
-			await this.markAsSupervised(image);
+		await markAsSupervised(image);
 
-			const img = await this.inspectByName(image.name);
+		const img = await inspectByName(image.name);
 		await db.models('image').update({ dockerImageId: img.Id }).where(image);
 
 		onFinish(true);
-			return null;
+		return;
 	} catch (e) {
 		if (!NotFoundError(e)) {
 			if (!(e instanceof ImageDownloadBackoffError)) {
-					this.addImageFailure(image.name);
+				addImageFailure(image.name);
 			}
 			throw e;
 		}
-			this.reportChange(
+		reportChange(
 			image.imageId,
 			_.merge(_.clone(image), { status: 'Downloading', downloadProgress: 0 }),
 		);
@@ -128,17 +147,17 @@ export class Images extends (EventEmitter as new () => ImageEventEmitter) {
 		try {
 			let id;
 			if (opts.delta && (opts as DeltaFetchOptions).deltaSource != null) {
-					id = await this.fetchDelta(image, opts, onProgress, serviceName);
+				id = await fetchDelta(image, opts, onProgress, serviceName);
 			} else {
-					id = await this.fetchImage(image, opts, onProgress);
+				id = await fetchImage(image, opts, onProgress);
 			}
 
 			await db.models('image').update({ dockerImageId: id }).where(image);
 
 			logger.logSystemEvent(LogTypes.downloadImageSuccess, { image });
 			success = true;
-				delete this.imageFetchFailures[image.name];
+			delete imageFetchFailures[image.name];
-				delete this.imageFetchLastFailureTime[image.name];
+			delete imageFetchLastFailureTime[image.name];
 		} catch (err) {
 			if (err instanceof DeltaStillProcessingError) {
 				// If this is a delta image pull, and the delta still hasn't finished generating,
@@ -146,7 +165,7 @@ export class Images extends (EventEmitter as new () => ImageEventEmitter) {
 				// processing
 				logger.logSystemEvent(LogTypes.deltaStillProcessingError, {});
 			} else {
-					this.addImageFailure(image.name);
+				addImageFailure(image.name);
 				logger.logSystemEvent(LogTypes.downloadImageError, {
 					image,
 					error: err,
@@ -156,14 +175,13 @@ export class Images extends (EventEmitter as new () => ImageEventEmitter) {
 		}
 	}
 
-		this.reportChange(image.imageId);
+	reportChange(image.imageId);
 	onFinish(success);
-		return null;
+}
-	}
 
-	public async remove(image: Image): Promise<void> {
+export async function remove(image: Image): Promise<void> {
 	try {
-			await this.removeImageIfNotNeeded(image);
+		await removeImageIfNotNeeded(image);
 	} catch (e) {
 		logger.logSystemEvent(LogTypes.deleteImageError, {
 			image,
@@ -171,94 +189,93 @@ export class Images extends (EventEmitter as new () => ImageEventEmitter) {
 		});
 		throw e;
 	}
-	}
+}
 
-	public async getByDockerId(id: string): Promise<Image> {
+export function getByDockerId(id: string): Promise<Image> {
-		return await db.models('image').where({ dockerImageId: id }).first();
+	return db.models('image').where({ dockerImageId: id }).first();
-	}
+}
 
-	public async removeByDockerId(id: string): Promise<void> {
+export async function removeByDockerId(id: string): Promise<void> {
-		const image = await this.getByDockerId(id);
+	const image = await getByDockerId(id);
-		await this.remove(image);
+	await remove(image);
-	}
+}
 
-	private async getNormalisedTags(image: Docker.ImageInfo): Promise<string[]> {
+export async function getNormalisedTags(
+	image: Docker.ImageInfo,
+): Promise<string[]> {
 	return await Bluebird.map(
 		image.RepoTags != null ? image.RepoTags : [],
-			this.normalise.bind(this),
+		normalise,
 	);
-	}
+}
 
-	private async withImagesFromDockerAndDB<T>(
+async function withImagesFromDockerAndDB<T>(
 	cb: (dockerImages: NormalisedDockerImage[], composeImages: Image[]) => T,
-	) {
+) {
 	const [normalisedImages, dbImages] = await Promise.all([
 		Bluebird.map(docker.listImages({ digests: true }), async (image) => {
 			const newImage = _.clone(image) as NormalisedDockerImage;
-				newImage.NormalisedRepoTags = await this.getNormalisedTags(image);
+			newImage.NormalisedRepoTags = await getNormalisedTags(image);
 			return newImage;
 		}),
 		db.models('image').select(),
 	]);
 	return cb(normalisedImages, dbImages);
-	}
+}
 
-	private addImageFailure(imageName: string, time = process.hrtime()) {
+function addImageFailure(imageName: string, time = process.hrtime()) {
-		this.imageFetchLastFailureTime[imageName] = time;
+	imageFetchLastFailureTime[imageName] = time;
-		this.imageFetchFailures[imageName] =
+	imageFetchFailures[imageName] =
-			this.imageFetchFailures[imageName] != null
+		imageFetchFailures[imageName] != null
-				? this.imageFetchFailures[imageName] + 1
+			? imageFetchFailures[imageName] + 1
 			: 1;
-	}
+}
 
-	private matchesTagOrDigest(
+function matchesTagOrDigest(
 	image: Image,
 	dockerImage: NormalisedDockerImage,
-	): boolean {
+): boolean {
 	return (
 		_.includes(dockerImage.NormalisedRepoTags, image.name) ||
 		_.some(dockerImage.RepoDigests, (digest) =>
-				Images.hasSameDigest(image.name, digest),
+			hasSameDigest(image.name, digest),
 		)
 	);
-	}
+}
 
-	private isAvailableInDocker(
+function isAvailableInDocker(
 	image: Image,
 	dockerImages: NormalisedDockerImage[],
-	): boolean {
+): boolean {
 	return _.some(
 		dockerImages,
 		(dockerImage) =>
-				this.matchesTagOrDigest(image, dockerImage) ||
+			matchesTagOrDigest(image, dockerImage) ||
 			image.dockerImageId === dockerImage.Id,
 	);
-	}
+}
 
-	public async getAvailable(): Promise<Image[]> {
+export async function getAvailable(): Promise<Image[]> {
-		const images = await this.withImagesFromDockerAndDB(
+	return withImagesFromDockerAndDB((dockerImages, supervisedImages) =>
-			(dockerImages, supervisedImages) =>
 		_.filter(supervisedImages, (image) =>
-					this.isAvailableInDocker(image, dockerImages),
+			isAvailableInDocker(image, dockerImages),
 		),
 	);
+}
 
-		return images;
+// TODO: Why does this need a Bluebird.try?
-	}
+export function getDownloadingImageIds() {
-
-	// TODO: Why does this need a Bluebird.try?
-	public getDownloadingImageIds() {
 	return Bluebird.try(() =>
-			_(this.volatileState)
+		_(volatileState)
 			.pickBy({ status: 'Downloading' })
 			.keys()
 			.map(validation.checkInt)
 			.value(),
 	);
-	}
+}
 
-	public async cleanupDatabase(): Promise<void> {
+export async function cleanupDatabase(): Promise<void> {
-		const imagesToRemove = await this.withImagesFromDockerAndDB(
+	const imagesToRemove = await withImagesFromDockerAndDB(
 		async (dockerImages, supervisedImages) => {
 			for (const supervisedImage of supervisedImages) {
 				// If the supervisor was interrupted between fetching an image and storing its id,
@@ -266,7 +283,7 @@ export class Images extends (EventEmitter as new () => ImageEventEmitter) {
 				if (supervisedImage.dockerImageId == null) {
 					const id = _.get(
 						_.find(dockerImages, (dockerImage) =>
-								this.matchesTagOrDigest(supervisedImage, dockerImage),
+							matchesTagOrDigest(supervisedImage, dockerImage),
 						),
 						'Id',
 					);
@@ -281,46 +298,46 @@ export class Images extends (EventEmitter as new () => ImageEventEmitter) {
 				}
 			}
 			return _.reject(supervisedImages, (image) =>
-					this.isAvailableInDocker(image, dockerImages),
+				isAvailableInDocker(image, dockerImages),
 			);
 		},
 	);
 
 	const ids = _(imagesToRemove).map('id').compact().value();
 	await db.models('image').del().whereIn('id', ids);
-	}
+}
 
-	public async getStatus() {
+export const getStatus = async () => {
-		const images = await this.getAvailable();
+	const images = await getAvailable();
 	for (const image of images) {
 		image.status = 'Downloaded';
 		image.downloadProgress = null;
 	}
-		const status = _.clone(this.volatileState);
+	const status = _.clone(volatileState);
 	for (const image of images) {
 		if (status[image.imageId] == null) {
 			status[image.imageId] = image;
 		}
 	}
 	return _.values(status);
-	}
+};
 
-	public async update(image: Image): Promise<void> {
+export async function update(image: Image): Promise<void> {
-		const formattedImage = this.format(image);
+	const formattedImage = format(image);
 	await db
 		.models('image')
 		.update(formattedImage)
 		.where({ name: formattedImage.name });
-	}
+}
 
-	public async save(image: Image): Promise<void> {
+export const save = async (image: Image): Promise<void> => {
-		const img = await this.inspectByName(image.name);
+	const img = await inspectByName(image.name);
 	image = _.clone(image);
 	image.dockerImageId = img.Id;
-		await this.markAsSupervised(image);
+	await markAsSupervised(image);
-	}
+};
 
-	private async getImagesForCleanup(): Promise<string[]> {
+async function getImagesForCleanup(): Promise<string[]> {
 	const images: string[] = [];
 
 	const [
@@ -360,12 +377,9 @@ export class Images extends (EventEmitter as new () => ImageEventEmitter) {
 	const dockerImages = await docker.listImages({ digests: true });
 	for (const image of dockerImages) {
 		// Cleanup should remove truly dangling images (i.e dangling and with no digests)
-			if (Images.isDangling(image) && !_.includes(usedImageIds, image.Id)) {
+		if (isDangling(image) && !_.includes(usedImageIds, image.Id)) {
 			images.push(image.Id);
-			} else if (
+		} else if (!_.isEmpty(image.RepoTags) && image.Id !== supervisorImage.Id) {
-				!_.isEmpty(image.RepoTags) &&
-				image.Id !== supervisorImage.Id
-			) {
 			// We also remove images from the supervisor repository with a different tag
 			for (const tag of image.RepoTags) {
 				const imageNameComponents = await dockerToolbelt.getRegistryAndName(
@@ -382,16 +396,16 @@ export class Images extends (EventEmitter as new () => ImageEventEmitter) {
 		.uniq()
 		.filter(
 			(image) =>
-					this.imageCleanupFailures[image] == null ||
+				imageCleanupFailures[image] == null ||
-					Date.now() - this.imageCleanupFailures[image] >
+				Date.now() - imageCleanupFailures[image] >
 					constants.imageCleanupErrorIgnoreTimeout,
 		)
 		.value();
-	}
+}
 
-	public async inspectByName(
+export async function inspectByName(
 	imageName: string,
-	): Promise<Docker.ImageInspectInfo> {
+): Promise<Docker.ImageInspectInfo> {
 	try {
 		return await docker.getImage(imageName).inspect();
 	} catch (e) {
@@ -417,63 +431,60 @@ export class Images extends (EventEmitter as new () => ImageEventEmitter) {
 		}
 		throw e;
 	}
-	}
+}
 
-	public async isCleanupNeeded() {
+export async function isCleanupNeeded() {
-		return !_.isEmpty(await this.getImagesForCleanup());
+	return !_.isEmpty(await getImagesForCleanup());
-	}
+}
 
-	public async cleanup() {
+export async function cleanup() {
-		const images = await this.getImagesForCleanup();
+	const images = await getImagesForCleanup();
 	for (const image of images) {
 		log.debug(`Cleaning up ${image}`);
 		try {
 			await docker.getImage(image).remove({ force: true });
-				delete this.imageCleanupFailures[image];
+			delete imageCleanupFailures[image];
 		} catch (e) {
 			logger.logSystemMessage(
 				`Error cleaning up ${image}: ${e.message} - will ignore for 1 hour`,
 				{ error: e },
 				'Image cleanup error',
 			);
-				this.imageCleanupFailures[image] = Date.now();
+			imageCleanupFailures[image] = Date.now();
-			}
 		}
 	}
+}
 
-	public static isSameImage(
+export function isSameImage(
 	image1: Pick<Image, 'name'>,
 	image2: Pick<Image, 'name'>,
-	): boolean {
+): boolean {
-		return (
+	return image1.name === image2.name || hasSameDigest(image1.name, image2.name);
-			image1.name === image2.name ||
+}
-			Images.hasSameDigest(image1.name, image2.name)
-		);
-	}
 
-	public normalise(imageName: string): Bluebird<string> {
+export function normalise(imageName: string): Bluebird<string> {
 	return dockerToolbelt.normaliseImageName(imageName);
-	}
+}
 
-	private static isDangling(image: Docker.ImageInfo): boolean {
+function isDangling(image: Docker.ImageInfo): boolean {
 	return (
 		(_.isEmpty(image.RepoTags) ||
 			_.isEqual(image.RepoTags, ['<none>:<none>'])) &&
 		(_.isEmpty(image.RepoDigests) ||
 			_.isEqual(image.RepoDigests, ['<none>@<none>']))
 	);
-	}
+}
 
-	private static hasSameDigest(
+function hasSameDigest(
 	name1: Nullable<string>,
 	name2: Nullable<string>,
-	): boolean {
+): boolean {
 	const hash1 = name1 != null ? name1.split('@')[1] : null;
 	const hash2 = name2 != null ? name2.split('@')[1] : null;
 	return hash1 != null && hash1 === hash2;
-	}
+}
 
-	private async removeImageIfNotNeeded(image: Image): Promise<void> {
+async function removeImageIfNotNeeded(image: Image): Promise<void> {
 	let removed: boolean;
 
 	// We first fetch the image from the DB to ensure it exists,
@@ -498,20 +509,18 @@ export class Images extends (EventEmitter as new () => ImageEventEmitter) {
 				.select();
 			if (
 				imagesFromDb.length === 1 &&
-					_.isEqual(this.format(imagesFromDb[0]), this.format(img))
+				_.isEqual(format(imagesFromDb[0]), format(img))
 			) {
-					this.reportChange(
+				reportChange(
 					image.imageId,
 					_.merge(_.clone(image), { status: 'Deleting' }),
 				);
 				logger.logSystemEvent(LogTypes.deleteImage, { image });
 				docker.getImage(img.dockerImageId).remove({ force: true });
 				removed = true;
-				} else if (!Images.hasDigest(img.name)) {
+			} else if (!hasDigest(img.name)) {
 				// Image has a regular tag, so we might have to remove unnecessary tags
-					const dockerImage = await docker
+				const dockerImage = await docker.getImage(img.dockerImageId).inspect();
-						.getImage(img.dockerImageId)
-						.inspect();
 				const differentTags = _.reject(imagesFromDb, { name: img.name });
 
 				if (
@@ -535,7 +544,7 @@ export class Images extends (EventEmitter as new () => ImageEventEmitter) {
 			throw e;
 		}
 	} finally {
-			this.reportChange(image.imageId);
+		reportChange(image.imageId);
 	}
 
 	await db.models('image').del().where({ id: img.id });
@@ -543,10 +552,10 @@ export class Images extends (EventEmitter as new () => ImageEventEmitter) {
 	if (removed) {
 		logger.logSystemEvent(LogTypes.deleteImageSuccess, { image });
 	}
-	}
+}
 
-	private async markAsSupervised(image: Image): Promise<void> {
+async function markAsSupervised(image: Image): Promise<void> {
-		const formattedImage = this.format(image);
+	const formattedImage = format(image);
 	await db.upsertModel(
 		'image',
 		formattedImage,
@@ -554,9 +563,9 @@ export class Images extends (EventEmitter as new () => ImageEventEmitter) {
 		// and currently acts like an "insert if not exists"
 		formattedImage,
 	);
-	}
+}
 
-	private format(image: Image): Omit<Image, 'id'> {
+function format(image: Image): Omit<Image, 'id'> {
 	return _(image)
 		.defaults({
 			serviceId: null,
@@ -568,18 +577,18 @@ export class Images extends (EventEmitter as new () => ImageEventEmitter) {
 		})
 		.omit('id')
 		.value();
-	}
+}
 
-	private async fetchDelta(
+async function fetchDelta(
 	image: Image,
 	opts: FetchOptions,
 	onProgress: (evt: FetchProgressEvent) => void,
 	serviceName: string,
-	): Promise<string> {
+): Promise<string> {
 	logger.logSystemEvent(LogTypes.downloadImageDelta, { image });
 
 	const deltaOpts = (opts as unknown) as DeltaFetchOptions;
-		const srcImage = await this.inspectByName(deltaOpts.deltaSource);
+	const srcImage = await inspectByName(deltaOpts.deltaSource);
 
 	deltaOpts.deltaSourceId = srcImage.Id;
 	const id = await dockerUtils.fetchDeltaWithProgress(
@@ -589,47 +598,44 @@ export class Images extends (EventEmitter as new () => ImageEventEmitter) {
 		serviceName,
 	);
 
-		if (!Images.hasDigest(image.name)) {
+	if (!hasDigest(image.name)) {
 		const { repo, tag } = await dockerUtils.getRepoAndTag(image.name);
 		await docker.getImage(id).tag({ repo, tag });
 	}
 
 	return id;
-	}
+}
 
-	private fetchImage(
+function fetchImage(
 	image: Image,
 	opts: FetchOptions,
 	onProgress: (evt: FetchProgressEvent) => void,
-	): Promise<string> {
+): Promise<string> {
 	logger.logSystemEvent(LogTypes.downloadImage, { image });
 	return dockerUtils.fetchImageWithProgress(image.name, opts, onProgress);
-	}
+}
 
-	// TODO: find out if imageId can actually be null
+// TODO: find out if imageId can actually be null
-	private reportChange(imageId: Nullable<number>, status?: Partial<Image>) {
+function reportChange(imageId: Nullable<number>, status?: Partial<Image>) {
 	if (imageId == null) {
 		return;
 	}
 	if (status != null) {
-			if (this.volatileState[imageId] == null) {
+		if (volatileState[imageId] == null) {
-				this.volatileState[imageId] = { imageId } as Image;
+			volatileState[imageId] = { imageId } as Image;
-			}
-			_.merge(this.volatileState[imageId], status);
-			return this.emit('change');
-		} else if (this.volatileState[imageId] != null) {
-			delete this.volatileState[imageId];
-			return this.emit('change');
 		}
+		_.merge(volatileState[imageId], status);
+		return events.emit('change');
+	} else if (volatileState[imageId] != null) {
+		delete volatileState[imageId];
+		return events.emit('change');
 	}
+}
 
-	private static hasDigest(name: Nullable<string>): boolean {
+function hasDigest(name: Nullable<string>): boolean {
 	if (name == null) {
 		return false;
 	}
 	const parts = name.split('@');
 	return parts[1] != null;
-	}
 }
-
-export default Images;

src/device-api/v2.ts

@@ -8,6 +8,7 @@ import Volume from '../compose/volume';
 import * as config from '../config';
 import * as db from '../db';
 import * as logger from '../logger';
+import * as images from '../compose/images';
 import { spawnJournalctl } from '../lib/journald';
 import {
 	appNotFoundMessage,
@@ -152,11 +153,11 @@ export function createV2Api(router: Router, applications: ApplicationManager) {
 			// maybe refactor this code
 			Bluebird.join(
 				applications.services.getStatus(),
-				applications.images.getStatus(),
+				images.getStatus(),
 				db.models('app').select(['appId', 'commit', 'name']),
 				(
 					services,
-					images,
+					imgs,
 					apps: Array<{ appId: string; commit: string; name: string }>,
 				) => {
 					// Create an object which is keyed my application name
@@ -187,7 +188,7 @@ export function createV2Api(router: Router, applications: ApplicationManager) {
 						appNameById[appId] = app.name;
 					});
 
-					images.forEach((img) => {
+					imgs.forEach((img) => {
 						const appName = appNameById[img.appId];
 						if (appName == null) {
 							log.warn(
@@ -406,7 +407,7 @@ export function createV2Api(router: Router, applications: ApplicationManager) {
 
 		let downloadProgressTotal = 0;
 		let downloads = 0;
-		const imagesStates = (await applications.images.getStatus()).map((img) => {
+		const imagesStates = (await images.getStatus()).map((img) => {
 			if (img.downloadProgress != null) {
 				downloadProgressTotal += img.downloadProgress;
 				downloads += 1;

src/device-state/preload.ts

@@ -5,6 +5,7 @@ import { Image } from '../compose/images';
 import DeviceState from '../device-state';
 import * as config from '../config';
 import * as eventTracker from '../event-tracker';
+import * as images from '../compose/images';
 
 import constants = require('../lib/constants');
 import { AppsJsonParseError, EISDIR, ENOENT } from '../lib/errors';
@@ -47,7 +48,7 @@ export async function loadTargetFromFile(
 			return;
 		}
 
-		const images: Image[] = [];
+		const imgs: Image[] = [];
 		const appIds = _.keys(preloadState.apps);
 		for (const appId of appIds) {
 			const app = preloadState.apps[appId];
@@ -67,14 +68,14 @@ export async function loadTargetFromFile(
 					releaseId: app.releaseId,
 					appId,
 				};
-				images.push(deviceState.applications.imageForService(svc));
+				imgs.push(deviceState.applications.imageForService(svc));
 			}
 		}
 
-		for (const image of images) {
+		for (const image of imgs) {
-			const name = await deviceState.applications.images.normalise(image.name);
+			const name = await images.normalise(image.name);
 			image.name = name;
-			await deviceState.applications.images.save(image);
+			await images.save(image);
 		}
 
 		const deviceConf = await deviceState.deviceConfig.getCurrent();

src/proxyvisor.js

@@ -14,6 +14,7 @@ import * as mkdirp from 'mkdirp';
 import * as bodyParser from 'body-parser';
 import * as url from 'url';
 
+import { normalise } from './compose/images';
 import { log } from './lib/supervisor-console';
 import * as db from './db';
 import * as config from './config';
@@ -346,7 +347,7 @@ const createProxyvisorRouter = function (proxyvisor) {
 };
 
 export class Proxyvisor {
-	constructor({ images, applications }) {
+	constructor({ applications }) {
 		this.bindToAPI = this.bindToAPI.bind(this);
 		this.executeStepAction = this.executeStepAction.bind(this);
 		this.getCurrentStates = this.getCurrentStates.bind(this);
@@ -362,7 +363,6 @@ export class Proxyvisor {
 		this.sendUpdate = this.sendUpdate.bind(this);
 		this.sendDeleteHook = this.sendDeleteHook.bind(this);
 		this.sendUpdates = this.sendUpdates.bind(this);
-		this.images = images;
 		this.applications = applications;
 		this.acknowledgedState = {};
 		this.lastRequestForDevice = {};
@@ -536,7 +536,7 @@ export class Proxyvisor {
 	normaliseDependentAppForDB(app) {
 		let image;
 		if (app.image != null) {
-			image = this.images.normalise(app.image);
+			image = normalise(app.image);
 		} else {
 			image = null;
 		}

test/05-device-state.spec.ts

@@ -4,10 +4,12 @@ import * as _ from 'lodash';
 import { SinonSpy, SinonStub, spy, stub } from 'sinon';
 
 import chai = require('./lib/chai-config');
+import { StatusCodeError } from '../src/lib/errors';
 import prepare = require('./lib/prepare');
 import Log from '../src/lib/supervisor-console';
 import * as dockerUtils from '../src/lib/docker-utils';
 import * as config from '../src/config';
+import * as images from '../src/compose/images';
 import { RPiConfigBackend } from '../src/config/backend';
 import DeviceState from '../src/device-state';
 import { loadTargetFromFile } from '../src/device-state/preload';
@@ -209,6 +211,8 @@ const testTargetInvalid = {
 
 describe('deviceState', () => {
 	let deviceState: DeviceState;
+	const originalImagesSave = images.save;
+	const originalImagesInspect = images.inspectByName;
 	before(async () => {
 		await prepare();
 
@@ -230,11 +234,15 @@ describe('deviceState', () => {
 			Promise.resolve('172.17.0.1'),
 		);
 
-		stub(deviceState.applications.images, 'inspectByName').callsFake(() => {
+		// @ts-expect-error Assigning to a RO property
-			const err: any = new Error();
+		images.save = () => Promise.resolve();
+
+		// @ts-expect-error Assigning to a RO property
+		images.inspectByName = () => {
+			const err: StatusCodeError = new Error();
 			err.statusCode = 404;
 			return Promise.reject(err);
-		});
+		};
 
 		(deviceState as any).deviceConfig.configBackend = new RPiConfigBackend();
 	});
@@ -242,12 +250,14 @@ describe('deviceState', () => {
 	after(() => {
 		(Service as any).extendEnvVars.restore();
 		(dockerUtils.getNetworkGateway as sinon.SinonStub).restore();
-		(deviceState.applications.images
+
-			.inspectByName as sinon.SinonStub).restore();
+		// @ts-expect-error Assigning to a RO property
+		images.save = originalImagesSave;
+		// @ts-expect-error Assigning to a RO property
+		images.inspectByName = originalImagesInspect;
 	});
 
 	it('loads a target state from an apps.json file and saves it as target state, then returns it', async () => {
-		stub(deviceState.applications.images, 'save').returns(Promise.resolve());
 		stub(deviceState.deviceConfig, 'getCurrent').returns(
 			Promise.resolve(mockedInitialConfig),
 		);
@@ -272,13 +282,11 @@ describe('deviceState', () => {
 				JSON.parse(JSON.stringify(testTarget)),
 			);
 		} finally {
-			(deviceState.applications.images.save as sinon.SinonStub).restore();
 			(deviceState.deviceConfig.getCurrent as sinon.SinonStub).restore();
 		}
 	});
 
 	it('stores info for pinning a device after loading an apps.json with a pinDevice field', async () => {
-		stub(deviceState.applications.images, 'save').returns(Promise.resolve());
 		stub(deviceState.deviceConfig, 'getCurrent').returns(
 			Promise.resolve(mockedInitialConfig),
 		);
@@ -286,7 +294,6 @@ describe('deviceState', () => {
 			process.env.ROOT_MOUNTPOINT + '/apps-pin.json',
 			deviceState,
 		);
-		(deviceState as any).applications.images.save.restore();
 		(deviceState as any).deviceConfig.getCurrent.restore();
 
 		const pinned = await config.get('pinDevice');
@@ -306,8 +313,7 @@ describe('deviceState', () => {
 
 		const services: Service[] = [];
 		for (const service of testTarget.local.apps['1234'].services) {
-			const imageName = await (deviceState.applications
+			const imageName = await images.normalise(service.image);
-				.images as any).normalise(service.image);
 			service.image = imageName;
 			(service as any).imageName = imageName;
 			services.push(

test/14-application-manager.spec.ts

@@ -8,6 +8,7 @@ import Service from '../src/compose/service';
 import Volume from '../src/compose/volume';
 import DeviceState from '../src/device-state';
 import * as dockerUtils from '../src/lib/docker-utils';
+import * as images from '../src/compose/images';
 
 import chai = require('./lib/chai-config');
 import prepare = require('./lib/prepare');
@@ -123,14 +124,17 @@ const dependentDBFormat = {
 };
 
 describe('ApplicationManager', function () {
+	const originalInspectByName = images.inspectByName;
 	before(async function () {
 		await prepare();
 		this.deviceState = new DeviceState({
 			apiBinder: null as any,
 		});
 		this.applications = this.deviceState.applications;
-		stub(this.applications.images, 'inspectByName').callsFake((_imageName) =>
+
-			Bluebird.Promise.resolve({
+		// @ts-expect-error assigning to a RO property
+		images.inspectByName = () =>
+			Promise.resolve({
 				Config: {
 					Cmd: ['someCommand'],
 					Entrypoint: ['theEntrypoint'],
@@ -138,8 +142,8 @@ describe('ApplicationManager', function () {
 					Labels: {},
 					Volumes: [],
 				},
-			}),
+			});
-		);
+
 		stub(dockerUtils, 'getNetworkGateway').returns(
 			Bluebird.Promise.resolve('172.17.0.1'),
 		);
@@ -223,7 +227,8 @@ describe('ApplicationManager', function () {
 	);
 
 	after(function () {
-		this.applications.images.inspectByName.restore();
+		// @ts-expect-error Assigning to a RO property
+		images.inspectByName = originalInspectByName;
 		// @ts-expect-error restore on non-stubbed type
 		dockerUtils.getNetworkGateway.restore();
 		// @ts-expect-error restore on non-stubbed type

test/21-supervisor-api.spec.ts

@@ -5,6 +5,7 @@ import * as supertest from 'supertest';
 import APIBinder from '../src/api-binder';
 import DeviceState from '../src/device-state';
 import Log from '../src/lib/supervisor-console';
+import * as images from '../src/compose/images';
 import SupervisorAPI from '../src/supervisor-api';
 import sampleResponses = require('./data/device-api-responses.json');
 import mockedAPI = require('./lib/mocked-device-api');
@@ -21,6 +22,7 @@ describe('SupervisorAPI', () => {
 	let api: SupervisorAPI;
 	let healthCheckStubs: SinonStub[];
 	const request = supertest(`http://127.0.0.1:${mockedOptions.listenPort}`);
+	const originalGetStatus = images.getStatus;
 
 	before(async () => {
 		// Stub health checks so we can modify them whenever needed
@@ -31,6 +33,10 @@ describe('SupervisorAPI', () => {
 		// The mockedAPI contains stubs that might create unexpected results
 		// See the module to know what has been stubbed
 		api = await mockedAPI.create();
+
+		// @ts-expect-error assigning to a RO property
+		images.getStatus = () => Promise.resolve([]);
+
 		// Start test API
 		return api.listen(
 			ALLOWED_INTERFACES,
@@ -51,6 +57,9 @@ describe('SupervisorAPI', () => {
 		healthCheckStubs.forEach((hc) => hc.restore);
 		// Remove any test data generated
 		await mockedAPI.cleanUp();
+
+		// @ts-expect-error assigning to a RO property
+		images.getStatus = originalGetStatus;
 	});
 
 	describe('/ping', () => {

test/lib/mocked-device-api.ts

@@ -3,7 +3,6 @@ import { fs } from 'mz';
 import { stub } from 'sinon';
 
 import { ApplicationManager } from '../../src/application-manager';
-import { Images } from '../../src/compose/images';
 import { NetworkManager } from '../../src/compose/network-manager';
 import { ServiceManager } from '../../src/compose/service-manager';
 import { VolumeManager } from '../../src/compose/volume-manager';
@@ -136,7 +135,6 @@ function buildRoutes(appManager: ApplicationManager): Router {
 
 function setupStubs() {
 	stub(ServiceManager.prototype, 'getStatus').resolves(STUBBED_VALUES.services);
-	stub(Images.prototype, 'getStatus').resolves(STUBBED_VALUES.images);
 	stub(NetworkManager.prototype, 'getAllByAppId').resolves(
 		STUBBED_VALUES.networks,
 	);
@@ -147,7 +145,6 @@ function setupStubs() {
 
 function restoreStubs() {
 	(ServiceManager.prototype as any).getStatus.restore();
-	(Images.prototype as any).getStatus.restore();
 	(NetworkManager.prototype as any).getAllByAppId.restore();
 	(VolumeManager.prototype as any).getAllByAppId.restore();
 }