Merge pull request #1370 from balena-io/singleton-images

Make images module a singleton

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
-			.get('appUpdatePollInterval')
-			.then((interval) => {
-				this.images.appUpdatePollInterval = interval;
-				return this.images.cleanupDatabase();
-			})
+		return Images.initialized
+			.then(() => 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(),
-				availableImages: this.images.getAvailable(),
-				downloading: this.images.getDownloadingImageIds(),
+				cleanupNeeded: Images.isCleanupNeeded(),
+				availableImages: Images.getAvailable(),
+				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/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) {
-			const name = await deviceState.applications.images.normalise(image.name);
+		for (const image of imgs) {
+			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(() => {
-			const err: any = new Error();
+		// @ts-expect-error Assigning to a RO property
+		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
-				.images as any).normalise(service.image);
+			const imageName = await images.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();
 }