From adaad786af161a7a3cdf61f2c63926e702fef790 Mon Sep 17 00:00:00 2001 From: Cameron Diver Date: Thu, 11 Jun 2020 11:43:03 +0100 Subject: [PATCH] Make volume-manager module a singleton Change-type: patch Signed-off-by: Cameron Diver --- src/application-manager.d.ts | 3 - src/application-manager.js | 294 ++++++++++++++++++++----------- src/compose/composition-steps.ts | 7 +- src/compose/volume-manager.ts | 258 +++++++++++++-------------- src/device-api/v2.ts | 3 +- src/lib/migration.ts | 7 +- test/lib/mocked-device-api.ts | 11 +- 7 files changed, 338 insertions(+), 245 deletions(-) diff --git a/src/application-manager.d.ts b/src/application-manager.d.ts index d0cbd70d..a5e2f19d 100644 --- a/src/application-manager.d.ts +++ b/src/application-manager.d.ts @@ -13,8 +13,6 @@ import DeviceState from './device-state'; import { APIBinder } from './api-binder'; import * as config from './config'; -import VolumeManager from './compose/volume-manager'; - import { CompositionStep, CompositionStepAction, @@ -48,7 +46,6 @@ class ApplicationManager extends EventEmitter { public apiBinder: APIBinder; public services: ServiceManager; - public volumes: VolumeManager; public proxyvisor: any; public timeSpentFetching: number; diff --git a/src/application-manager.js b/src/application-manager.js index 4b3b09d9..20d340a2 100644 --- a/src/application-manager.js +++ b/src/application-manager.js @@ -14,16 +14,18 @@ import { docker } from './lib/docker-utils'; import { LocalModeManager } from './local-mode'; import * as updateLock from './lib/update-lock'; import { checkTruthy, checkInt, checkString } from './lib/validation'; -import { ContractViolationError, InternalInconsistencyError } from './lib/errors'; +import { + ContractViolationError, + InternalInconsistencyError, +} from './lib/errors'; import * as dbFormat from './device-state/db-format'; -import { Network } from './compose/network'; import { ServiceManager } from './compose/service-manager'; import * as Images from './compose/images'; import { Network } from './compose/network'; import * as networkManager from './compose/network-manager'; -import { VolumeManager } from './compose/volume-manager'; +import * as volumeManager from './compose/volume-manager'; import * as compositionSteps from './compose/composition-steps'; import { Proxyvisor } from './proxyvisor'; @@ -35,7 +37,7 @@ import { serviceAction } from './device-api/common'; import * as db from './db'; // TODO: move this to an Image class? -const imageForService = service => ({ +const imageForService = (service) => ({ name: service.imageName, appId: service.appId, serviceId: service.serviceId, @@ -45,7 +47,7 @@ const imageForService = service => ({ dependent: 0, }); -const fetchAction = service => ({ +const fetchAction = (service) => ({ action: 'fetch', image: imageForService(service), serviceId: service.serviceId, @@ -76,7 +78,12 @@ export class ApplicationManager extends EventEmitter { this.fetchAction = fetchAction; this._strategySteps = { - 'download-then-kill'(current, target, needsDownload, dependenciesMetForKill) { + 'download-then-kill'( + current, + target, + needsDownload, + dependenciesMetForKill, + ) { if (needsDownload) { return fetchAction(target); } else if (dependenciesMetForKill()) { @@ -128,8 +135,12 @@ export class ApplicationManager extends EventEmitter { this._nextStepsForNetwork = this._nextStepsForNetwork.bind(this); this._nextStepForService = this._nextStepForService.bind(this); this._nextStepsForAppUpdate = this._nextStepsForAppUpdate.bind(this); - this.setTargetVolatileForService = this.setTargetVolatileForService.bind(this); - this.clearTargetVolatileForServices = this.clearTargetVolatileForServices.bind(this); + this.setTargetVolatileForService = this.setTargetVolatileForService.bind( + this, + ); + this.clearTargetVolatileForServices = this.clearTargetVolatileForServices.bind( + this, + ); this.getTargetApps = this.getTargetApps.bind(this); this.getDependentTargets = this.getDependentTargets.bind(this); this._compareImages = this._compareImages.bind(this); @@ -137,7 +148,9 @@ export class ApplicationManager extends EventEmitter { this.stopAll = this.stopAll.bind(this); this._lockingIfNecessary = this._lockingIfNecessary.bind(this); this.executeStepAction = this.executeStepAction.bind(this); - this.getExtraStateForComparison = this.getExtraStateForComparison.bind(this); + this.getExtraStateForComparison = this.getExtraStateForComparison.bind( + this, + ); this.getRequiredSteps = this.getRequiredSteps.bind(this); this.serviceNameFromId = this.serviceNameFromId.bind(this); this.removeAllVolumesForApp = this.removeAllVolumesForApp.bind(this); @@ -147,7 +160,6 @@ export class ApplicationManager extends EventEmitter { this.apiBinder = apiBinder; this.services = new ServiceManager(); - this.volumes = new VolumeManager(); this.proxyvisor = new Proxyvisor({ applications: this, }); @@ -160,13 +172,12 @@ export class ApplicationManager extends EventEmitter { this.actionExecutors = compositionSteps.getExecutors({ lockFn: this._lockingIfNecessary, services: this.services, - volumes: this.volumes, applications: this, callbacks: { - containerStarted: id => { + containerStarted: (id) => { this._containerStarted[id] = true; }, - containerKilled: id => { + containerKilled: (id) => { delete this._containerStarted[id]; }, fetchStart: () => { @@ -175,14 +186,16 @@ export class ApplicationManager extends EventEmitter { fetchEnd: () => { this.fetchesInProgress -= 1; }, - fetchTime: time => { + fetchTime: (time) => { this.timeSpentFetching += time; }, - stateReport: state => this.reportCurrentState(state), + stateReport: (state) => this.reportCurrentState(state), bestDeltaSource: this.bestDeltaSource, }, }); - this.validActions = _.keys(this.actionExecutors).concat(this.proxyvisor.validActions); + this.validActions = _.keys(this.actionExecutors).concat( + this.proxyvisor.validActions, + ); this.router = createApplicationManagerRouter(this); Images.on('change', this.reportCurrentState); this.services.on('change', this.reportCurrentState); @@ -196,7 +209,7 @@ export class ApplicationManager extends EventEmitter { await Images.initialized; await Images.cleanupDatabase(); const cleanup = () => { - return docker.listContainers({ all: true }).then(containers => { + return docker.listContainers({ all: true }).then((containers) => { return logger.clearOutOfDateDBLogs(_.map(containers, 'Id')); }); }; @@ -248,7 +261,10 @@ export class ApplicationManager extends EventEmitter { ); } if (apps[appId].services[imageId] == null) { - apps[appId].services[imageId] = _.pick(service, ['status', 'releaseId']); + apps[appId].services[imageId] = _.pick(service, [ + 'status', + 'releaseId', + ]); creationTimesAndReleases[appId][imageId] = _.pick(service, [ 'createdAt', 'releaseId', @@ -337,7 +353,7 @@ export class ApplicationManager extends EventEmitter { // multi-app warning! // This is just wrong on every level - _.each(apps, app => { + _.each(apps, (app) => { app.commit = currentCommit; }); @@ -348,7 +364,7 @@ export class ApplicationManager extends EventEmitter { return Promise.join( this.services.getAll(), networkManager.getAll(), - this.volumes.getAll(), + volumeManager.getAll(), config.get('currentCommit'), this._buildApps, ); @@ -358,7 +374,7 @@ export class ApplicationManager extends EventEmitter { return Promise.join( this.services.getAllByAppId(appId), networkManager.getAllByAppId(appId), - this.volumes.getAllByAppId(appId), + volumeManager.getAllByAppId(appId), config.get('currentCommit'), this._buildApps, ).get(appId); @@ -402,13 +418,19 @@ export class ApplicationManager extends EventEmitter { } } - const toBeMaybeUpdated = _.intersection(targetServiceIds, currentServiceIds); + const toBeMaybeUpdated = _.intersection( + targetServiceIds, + currentServiceIds, + ); const currentServicesPerId = {}; const targetServicesPerId = _.keyBy(targetServices, 'serviceId'); for (const serviceId of toBeMaybeUpdated) { const currentServiceContainers = _.filter(currentServices, { serviceId }); if (currentServiceContainers.length > 1) { - currentServicesPerId[serviceId] = _.maxBy(currentServiceContainers, 'createdAt'); + currentServicesPerId[serviceId] = _.maxBy( + currentServiceContainers, + 'createdAt', + ); // All but the latest container for this service are spurious and should be removed for (const service of _.without( @@ -428,7 +450,7 @@ export class ApplicationManager extends EventEmitter { // Returns true if a service matches its target except it should be running and it is not, but we've // already started it before. In this case it means it just exited so we don't want to start it again. - const alreadyStarted = serviceId => { + const alreadyStarted = (serviceId) => { return ( currentServicesPerId[serviceId].isEqualExceptForRunningState( targetServicesPerId[serviceId], @@ -441,7 +463,7 @@ export class ApplicationManager extends EventEmitter { const needUpdate = _.filter( toBeMaybeUpdated, - serviceId => + (serviceId) => !currentServicesPerId[serviceId].isEqual( targetServicesPerId[serviceId], containerIds, @@ -476,7 +498,7 @@ export class ApplicationManager extends EventEmitter { const toBeUpdated = _.filter( _.intersection(targetNames, currentNames), - name => !current[name].isEqualConfig(target[name]), + (name) => !current[name].isEqualConfig(target[name]), ); for (const name of toBeUpdated) { outputPairs.push({ @@ -496,7 +518,7 @@ export class ApplicationManager extends EventEmitter { } compareVolumesForUpdate({ current, target }) { - return this._compareNetworksOrVolumesForUpdate(this.volumes, { + return this._compareNetworksOrVolumesForUpdate(volumeManager, { current, target, }); @@ -509,7 +531,8 @@ export class ApplicationManager extends EventEmitter { } const hasNetwork = _.some( networkPairs, - pair => `${service.appId}_${pair.current?.name}` === service.networkMode, + (pair) => + `${service.appId}_${pair.current?.name}` === service.networkMode, ); if (hasNetwork) { return true; @@ -518,7 +541,7 @@ export class ApplicationManager extends EventEmitter { const name = _.split(volume, ':')[0]; return _.some( volumePairs, - pair => `${service.appId}_${pair.current?.name}` === name, + (pair) => `${service.appId}_${pair.current?.name}` === name, ); }); return hasVolume; @@ -526,10 +549,15 @@ export class ApplicationManager extends EventEmitter { // TODO: account for volumes-from, networks-from, links, etc // TODO: support networks instead of only networkMode - _dependenciesMetForServiceStart(target, networkPairs, volumePairs, pendingPairs) { + _dependenciesMetForServiceStart( + target, + networkPairs, + volumePairs, + pendingPairs, + ) { // for dependsOn, check no install or update pairs have that service - const dependencyUnmet = _.some(target.dependsOn, dependency => - _.some(pendingPairs, pair => pair.target?.serviceName === dependency), + const dependencyUnmet = _.some(target.dependsOn, (dependency) => + _.some(pendingPairs, (pair) => pair.target?.serviceName === dependency), ); if (dependencyUnmet) { return false; @@ -538,7 +566,7 @@ export class ApplicationManager extends EventEmitter { if ( _.some( networkPairs, - pair => `${target.appId}_${pair.target?.name}` === target.networkMode, + (pair) => `${target.appId}_${pair.target?.name}` === target.networkMode, ) ) { return false; @@ -551,7 +579,7 @@ export class ApplicationManager extends EventEmitter { } return _.some( volumePairs, - pair => `${target.appId}_${pair.target?.name}` === sourceName, + (pair) => `${target.appId}_${pair.target?.name}` === sourceName, ); }); return !volumeUnmet; @@ -560,7 +588,12 @@ export class ApplicationManager extends EventEmitter { // Unless the update strategy requires an early kill (i.e. kill-then-download, delete-then-download), we only want // to kill a service once the images for the services it depends on have been downloaded, so as to minimize // downtime (but not block the killing too much, potentially causing a deadlock) - _dependenciesMetForServiceKill(target, targetApp, availableImages, localMode) { + _dependenciesMetForServiceKill( + target, + targetApp, + availableImages, + localMode, + ) { // Because we only check for an image being available, in local mode this will always // be the case, so return true regardless. If this function ever checks for anything else, // we'll need to change the logic here @@ -575,7 +608,7 @@ export class ApplicationManager extends EventEmitter { if ( !_.some( availableImages, - image => + (image) => image.dockerImageId === dependencyService.image || Images.isSameImage(image, { name: dependencyService.imageName }), ) @@ -596,7 +629,7 @@ export class ApplicationManager extends EventEmitter { ) { // Check none of the currentApp.services use this network or volume if (current != null) { - const dependencies = _.filter(currentApp.services, service => + const dependencies = _.filter(currentApp.services, (service) => dependencyComparisonFn(service, current), ); if (_.isEmpty(dependencies)) { @@ -642,7 +675,9 @@ export class ApplicationManager extends EventEmitter { const dependencyComparisonFn = (service, curr) => _.some(service.config.volumes, function (volumeDefinition) { const [sourceName, destName] = volumeDefinition.split(':'); - return destName != null && sourceName === `${service.appId}_${curr?.name}`; + return ( + destName != null && sourceName === `${service.appId}_${curr?.name}` + ); }); return this._nextStepsForNetworkOrVolume( { current, target }, @@ -655,7 +690,10 @@ export class ApplicationManager extends EventEmitter { // Infers steps that do not require creating a new container _updateContainerStep(current, target) { - if (current.releaseId !== target.releaseId || current.imageId !== target.imageId) { + if ( + current.releaseId !== target.releaseId || + current.imageId !== target.imageId + ) { return serviceAction('updateMetadata', target.serviceId, current, target); } else if (target.config.running) { return serviceAction('start', target.serviceId, current, target); @@ -674,7 +712,12 @@ export class ApplicationManager extends EventEmitter { } } - _nextStepForService({ current, target }, updateContext, localMode, containerIds) { + _nextStepForService( + { current, target }, + updateContext, + localMode, + containerIds, + ) { const { targetApp, networkPairs, @@ -699,7 +742,7 @@ export class ApplicationManager extends EventEmitter { if (!localMode) { needsDownload = !_.some( availableImages, - image => + (image) => image.dockerImageId === target?.config.image || Images.isSameImage(image, { name: target.imageName }), ); @@ -721,7 +764,12 @@ export class ApplicationManager extends EventEmitter { const dependenciesMetForKill = () => { return ( !needsDownload && - this._dependenciesMetForServiceKill(target, targetApp, availableImages, localMode) + this._dependenciesMetForServiceKill( + target, + targetApp, + availableImages, + localMode, + ) ); }; @@ -745,7 +793,9 @@ export class ApplicationManager extends EventEmitter { dependenciesMetForStart, ); } else { - let strategy = checkString(target.config.labels['io.balena.update.strategy']); + let strategy = checkString( + target.config.labels['io.balena.update.strategy'], + ); const validStrategies = [ 'download-then-kill', 'kill-then-download', @@ -755,7 +805,9 @@ export class ApplicationManager extends EventEmitter { if (!_.includes(validStrategies, strategy)) { strategy = 'download-then-kill'; } - const timeout = checkInt(target.config.labels['io.balena.update.handover-timeout']); + const timeout = checkInt( + target.config.labels['io.balena.update.handover-timeout'], + ); return this._strategySteps[strategy]( current, target, @@ -801,8 +853,11 @@ export class ApplicationManager extends EventEmitter { if ( currentApp.services?.length === 1 && targetApp.services?.length === 1 && - targetApp.services[0].serviceName === currentApp.services[0].serviceName && - checkTruthy(currentApp.services[0].config.labels['io.balena.legacy-container']) + targetApp.services[0].serviceName === + currentApp.services[0].serviceName && + checkTruthy( + currentApp.services[0].config.labels['io.balena.legacy-container'], + ) ) { // This is a legacy preloaded app or container, so we didn't have things like serviceId. // We hack a few things to avoid an unnecessary restart of the preloaded app @@ -822,7 +877,11 @@ export class ApplicationManager extends EventEmitter { current: currentApp.volumes, target: targetApp.volumes, }); - const { removePairs, installPairs, updatePairs } = this.compareServicesForUpdate( + const { + removePairs, + installPairs, + updatePairs, + } = this.compareServicesForUpdate( currentApp.services, targetApp.services, containerIds, @@ -889,7 +948,7 @@ export class ApplicationManager extends EventEmitter { } const appId = targetApp.appId ?? currentApp.appId; - return _.map(steps, step => _.assign({}, step, { appId })); + return _.map(steps, (step) => _.assign({}, step, { appId })); } async setTarget(apps, dependent, source, maybeTrx) { @@ -927,7 +986,10 @@ export class ApplicationManager extends EventEmitter { const filteredApps = _.cloneDeep(apps); _.each( fulfilledContracts, - ({ valid, unmetServices, fulfilledServices, unmetAndOptional }, appId) => { + ( + { valid, unmetServices, fulfilledServices, unmetAndOptional }, + appId, + ) => { if (!valid) { contractViolators[apps[appId].name] = unmetServices; return delete filteredApps[appId]; @@ -969,15 +1031,17 @@ export class ApplicationManager extends EventEmitter { } clearTargetVolatileForServices(imageIds) { - return imageIds.map(imageId => (this._targetVolatilePerImageId[imageId] = {})); + return imageIds.map( + (imageId) => (this._targetVolatilePerImageId[imageId] = {}), + ); } async getTargetApps() { const apps = await dbFormat.getApps(); - _.each(apps, app => { + _.each(apps, (app) => { if (!_.isEmpty(app.services)) { - app.services = _.mapValues(app.services, svc => { + app.services = _.mapValues(app.services, (svc) => { if (this._targetVolatilePerImageId[svc.imageId] != null) { return { ...svc, @@ -1024,8 +1088,8 @@ export class ApplicationManager extends EventEmitter { // - are locally available (i.e. an image with the same digest exists) // - are not saved to the DB with all their metadata (serviceId, serviceName, etc) _compareImages(current, target, available, localMode) { - const allImagesForTargetApp = app => _.map(app.services, imageForService); - const allImagesForCurrentApp = app => + const allImagesForTargetApp = (app) => _.map(app.services, imageForService); + const allImagesForCurrentApp = (app) => _.map(app.services, function (service) { const img = _.find(available, { @@ -1034,13 +1098,13 @@ export class ApplicationManager extends EventEmitter { }) ?? _.find(available, { dockerImageId: service.config.image }); return _.omit(img, ['dockerImageId', 'id']); }); - const allImageDockerIdsForTargetApp = app => + const allImageDockerIdsForTargetApp = (app) => _(app.services) - .map(svc => [svc.imageName, svc.config.image]) - .filter(img => img[1] != null) + .map((svc) => [svc.imageName, svc.config.image]) + .filter((img) => img[1] != null) .value(); - const availableWithoutIds = _.map(available, image => + const availableWithoutIds = _.map(available, (image) => _.omit(image, ['dockerImageId', 'id']), ); const currentImages = _.flatMap(current.local.apps, allImagesForCurrentApp); @@ -1051,16 +1115,16 @@ export class ApplicationManager extends EventEmitter { const availableAndUnused = _.filter( availableWithoutIds, - image => - !_.some(currentImages.concat(targetImages), imageInUse => + (image) => + !_.some(currentImages.concat(targetImages), (imageInUse) => _.isEqual(image, imageInUse), ), ); const imagesToDownload = _.filter( targetImages, - targetImage => - !_.some(available, availableImage => + (targetImage) => + !_.some(available, (availableImage) => Images.isSameImage(availableImage, targetImage), ), ); @@ -1068,40 +1132,47 @@ export class ApplicationManager extends EventEmitter { let imagesToSave = []; if (!localMode) { imagesToSave = _.filter(targetImages, function (targetImage) { - const isActuallyAvailable = _.some(available, function (availableImage) { + const isActuallyAvailable = _.some(available, function ( + availableImage, + ) { if (Images.isSameImage(availableImage, targetImage)) { return true; } - if (availableImage.dockerImageId === targetImageDockerIds[targetImage.name]) { + if ( + availableImage.dockerImageId === + targetImageDockerIds[targetImage.name] + ) { return true; } return false; }); - const isNotSaved = !_.some(availableWithoutIds, img => + const isNotSaved = !_.some(availableWithoutIds, (img) => _.isEqual(img, targetImage), ); return isActuallyAvailable && isNotSaved; }); } - const deltaSources = _.map(imagesToDownload, image => { + const deltaSources = _.map(imagesToDownload, (image) => { return this.bestDeltaSource(image, available); }); const proxyvisorImages = this.proxyvisor.imagesInUse(current, target); const potentialDeleteThenDownload = _.filter( current.local.apps.services, - svc => - svc.config.labels['io.balena.update.strategy'] === 'delete-then-download' && - svc.status === 'Stopped', + (svc) => + svc.config.labels['io.balena.update.strategy'] === + 'delete-then-download' && svc.status === 'Stopped', ); const imagesToRemove = _.filter( availableAndUnused.concat(potentialDeleteThenDownload), function (image) { const notUsedForDelta = !_.includes(deltaSources, image.name); - const notUsedByProxyvisor = !_.some(proxyvisorImages, proxyvisorImage => - Images.isSameImage(image, { name: proxyvisorImage }), + const notUsedByProxyvisor = !_.some( + proxyvisorImages, + (proxyvisorImage) => + Images.isSameImage(image, { name: proxyvisorImage }), ); return notUsedForDelta && notUsedByProxyvisor; }, @@ -1143,8 +1214,10 @@ export class ApplicationManager extends EventEmitter { // multi-app warning: this will break let appsForVolumeRemoval; if (!localMode) { - const currentAppIds = _.keys(current.local.apps).map(n => checkInt(n)); - const targetAppIds = _.keys(target.local.apps).map(n => checkInt(n)); + const currentAppIds = _.keys(current.local.apps).map((n) => + checkInt(n), + ); + const targetAppIds = _.keys(target.local.apps).map((n) => checkInt(n)); appsForVolumeRemoval = _.difference(currentAppIds, targetAppIds); } @@ -1157,11 +1230,15 @@ export class ApplicationManager extends EventEmitter { const { services } = currentByAppId[appId]; for (const n in services) { if ( - checkTruthy(services[n].config.labels['io.balena.features.supervisor-api']) + checkTruthy( + services[n].config.labels['io.balena.features.supervisor-api'], + ) ) { containersUsingSupervisorNetwork = true; if (services[n].status !== 'Stopping') { - nextSteps.push(serviceAction('kill', services[n].serviceId, services[n])); + nextSteps.push( + serviceAction('kill', services[n].serviceId, services[n]), + ); } else { nextSteps.push({ action: 'noop' }); } @@ -1193,7 +1270,10 @@ export class ApplicationManager extends EventEmitter { } // If we have to remove any images, we do that before anything else if (_.isEmpty(nextSteps)) { - const allAppIds = _.union(_.keys(currentByAppId), _.keys(targetByAppId)); + const allAppIds = _.union( + _.keys(currentByAppId), + _.keys(targetByAppId), + ); for (const appId of allAppIds) { nextSteps = nextSteps.concat( this._nextStepsForAppUpdate( @@ -1210,13 +1290,15 @@ export class ApplicationManager extends EventEmitter { // the old app to be removed. If it has, we then // remove all of the volumes if (_.every(nextSteps, { action: 'noop' })) { - volumePromises.push(this.removeAllVolumesForApp(checkInt(appId))); + volumePromises.push( + this.removeAllVolumesForApp(checkInt(appId)), + ); } } } } } - const newDownloads = nextSteps.filter(s => s.action === 'fetch').length; + const newDownloads = nextSteps.filter((s) => s.action === 'fetch').length; if (!ignoreImages && delta && newDownloads > 0) { // Check that this is not the first pull for an @@ -1248,7 +1330,7 @@ export class ApplicationManager extends EventEmitter { nextSteps.push({ action: 'noop' }); } return _.uniqWith(nextSteps, _.isEqual); - }).then(nextSteps => + }).then((nextSteps) => Promise.all(volumePromises).then(function (volSteps) { nextSteps = nextSteps.concat(_.flatten(volSteps)); return nextSteps; @@ -1258,14 +1340,18 @@ export class ApplicationManager extends EventEmitter { stopAll({ force = false, skipLock = false } = {}) { return Promise.resolve(this.services.getAll()) - .map(service => { - return this._lockingIfNecessary(service.appId, { force, skipLock }, () => { - return this.services - .kill(service, { removeContainer: false, wait: true }) - .then(() => { - delete this._containerStarted[service.containerId]; - }); - }); + .map((service) => { + return this._lockingIfNecessary( + service.appId, + { force, skipLock }, + () => { + return this.services + .kill(service, { removeContainer: false, wait: true }) + .then(() => { + delete this._containerStarted[service.containerId]; + }); + }, + ); }) .return(); } @@ -1276,8 +1362,10 @@ export class ApplicationManager extends EventEmitter { } return config .get('lockOverride') - .then(lockOverride => lockOverride || force) - .then(lockOverridden => updateLock.lock(appId, { force: lockOverridden }, fn)); + .then((lockOverride) => lockOverride || force) + .then((lockOverridden) => + updateLock.lock(appId, { force: lockOverridden }, fn), + ); } executeStepAction(step, { force = false, skipLock = false } = {}) { @@ -1287,7 +1375,9 @@ export class ApplicationManager extends EventEmitter { if (!_.includes(this.validActions, step.action)) { return Promise.reject(new Error(`Invalid action ${step.action}`)); } - return this.actionExecutors[step.action](_.merge({}, step, { force, skipLock })); + return this.actionExecutors[step.action]( + _.merge({}, step, { force, skipLock }), + ); } getExtraStateForComparison(currentState, targetState) { @@ -1296,7 +1386,7 @@ export class ApplicationManager extends EventEmitter { .keys() .concat(_.keys(targetState.local.apps)) .uniq() - .each(id => { + .each((id) => { const intId = checkInt(id); if (intId == null) { throw new Error(`Invalid id: ${id}`); @@ -1304,7 +1394,7 @@ export class ApplicationManager extends EventEmitter { containerIdsByAppId[intId] = this.services.getContainerIdMap(intId); }); - return config.get('localMode').then(localMode => { + return config.get('localMode').then((localMode) => { return Promise.props({ cleanupNeeded: Images.isCleanupNeeded(), availableImages: Images.getAvailable(), @@ -1345,7 +1435,7 @@ export class ApplicationManager extends EventEmitter { ignoreImages, conf, containerIds, - ).then(nextSteps => { + ).then((nextSteps) => { if (ignoreImages && _.some(nextSteps, { action: 'fetch' })) { throw new Error('Cannot fetch images while executing an API action'); } @@ -1357,7 +1447,7 @@ export class ApplicationManager extends EventEmitter { targetState, nextSteps, ) - .then(proxyvisorSteps => nextSteps.concat(proxyvisorSteps)); + .then((proxyvisorSteps) => nextSteps.concat(proxyvisorSteps)); }); } @@ -1368,7 +1458,10 @@ export class ApplicationManager extends EventEmitter { // application for (const appId of Object.keys(apps)) { const app = apps[appId]; - const service = _.find(app.services, svc => svc.serviceId === serviceId); + const service = _.find( + app.services, + (svc) => svc.serviceId === serviceId, + ); if (service?.serviceName == null) { throw new InternalInconsistencyError( `Could not find service name for id: ${serviceId}`, @@ -1383,8 +1476,8 @@ export class ApplicationManager extends EventEmitter { } removeAllVolumesForApp(appId) { - return this.volumes.getAllByAppId(appId).then(volumes => - volumes.map(v => ({ + return volumeManager.getAllByAppId(appId).then((volumes) => + volumes.map((v) => ({ action: 'removeVolume', current: v, })), @@ -1403,6 +1496,11 @@ export class ApplicationManager extends EventEmitter { '. ', )}`; log.info(message); - return logger.logSystemMessage(message, {}, 'optionalContainerViolation', true); + return logger.logSystemMessage( + message, + {}, + 'optionalContainerViolation', + true, + ); } } diff --git a/src/compose/composition-steps.ts b/src/compose/composition-steps.ts index 1e9ffb89..de517a96 100644 --- a/src/compose/composition-steps.ts +++ b/src/compose/composition-steps.ts @@ -12,7 +12,7 @@ import Volume from './volume'; import { checkTruthy } from '../lib/validation'; import * as networkManager from './network-manager'; -import VolumeManager from './volume-manager'; +import * as volumeManager from './volume-manager'; interface BaseCompositionStepArgs { force?: boolean; @@ -137,7 +137,6 @@ interface CompositionCallbacks { export function getExecutors(app: { lockFn: LockingFn; services: ServiceManager; - volumes: VolumeManager; applications: ApplicationManager; callbacks: CompositionCallbacks; }) { @@ -283,13 +282,13 @@ export function getExecutors(app: { await networkManager.create(step.target); }, createVolume: async (step) => { - await app.volumes.create(step.target); + await volumeManager.create(step.target); }, removeNetwork: async (step) => { await networkManager.remove(step.current); }, removeVolume: async (step) => { - await app.volumes.remove(step.current); + await volumeManager.remove(step.current); }, ensureSupervisorNetwork: async () => { networkManager.ensureSupervisorNetwork(); diff --git a/src/compose/volume-manager.ts b/src/compose/volume-manager.ts index 8a3d88c4..a75ed59b 100644 --- a/src/compose/volume-manager.ts +++ b/src/compose/volume-manager.ts @@ -17,143 +17,139 @@ export interface VolumeNameOpts { appId: number; } -export class VolumeManager { - public async get({ name, appId }: VolumeNameOpts): Promise { - return Volume.fromDockerVolume( - await docker.getVolume(Volume.generateDockerName(appId, name)).inspect(), - ); - } +export async function get({ name, appId }: VolumeNameOpts): Promise { + return Volume.fromDockerVolume( + await docker.getVolume(Volume.generateDockerName(appId, name)).inspect(), + ); +} - public async getAll(): Promise { - const volumeInspect = await this.listWithBothLabels(); - return volumeInspect.map((inspect) => Volume.fromDockerVolume(inspect)); - } +export async function getAll(): Promise { + const volumeInspect = await listWithBothLabels(); + return volumeInspect.map((inspect) => Volume.fromDockerVolume(inspect)); +} - public async getAllByAppId(appId: number): Promise { - const all = await this.getAll(); - return _.filter(all, { appId }); - } +export async function getAllByAppId(appId: number): Promise { + const all = await getAll(); + return _.filter(all, { appId }); +} - public async create(volume: Volume): Promise { - // First we check that we're not trying to recreate a - // volume - try { - const existing = await this.get({ - name: volume.name, - appId: volume.appId, +export async function create(volume: Volume): Promise { + // First we check that we're not trying to recreate a + // volume + try { + const existing = await get({ + name: volume.name, + appId: volume.appId, + }); + + if (!volume.isEqualConfig(existing)) { + throw new ResourceRecreationAttemptError('volume', volume.name); + } + } catch (e) { + if (!NotFoundError(e)) { + logger.logSystemEvent(LogTypes.createVolumeError, { + volume: { name: volume.name }, + error: e, }); - - if (!volume.isEqualConfig(existing)) { - throw new ResourceRecreationAttemptError('volume', volume.name); - } - } catch (e) { - if (!NotFoundError(e)) { - logger.logSystemEvent(LogTypes.createVolumeError, { - volume: { name: volume.name }, - error: e, - }); - throw e; - } - - await volume.create(); + throw e; } - } - // We simply forward this to the volume object, but we - // add this method to provide a consistent interface - public async remove(volume: Volume) { - await volume.remove(); - } - - public async createFromLegacy(appId: number): Promise { - const name = defaultLegacyVolume(); - const legacyPath = Path.join( - constants.rootMountPoint, - 'mnt/data/resin-data', - appId.toString(), - ); - - try { - return await this.createFromPath({ name, appId }, {}, legacyPath); - } catch (e) { - logger.logSystemMessage( - `Warning: could not migrate legacy /data volume: ${e.message}`, - { error: e }, - 'Volume migration error', - ); - } - } - - public async createFromPath( - { name, appId }: VolumeNameOpts, - config: Partial, - oldPath: string, - ): Promise { - const volume = Volume.fromComposeObject(name, appId, config); - - await this.create(volume); - const inspect = await docker - .getVolume(Volume.generateDockerName(volume.appId, volume.name)) - .inspect(); - - const volumePath = Path.join( - constants.rootMountPoint, - 'mnt/data', - ...inspect.Mountpoint.split(Path.sep).slice(3), - ); - - await safeRename(oldPath, volumePath); - return volume; - } - - public async removeOrphanedVolumes( - referencedVolumes: string[], - ): Promise { - // Iterate through every container, and track the - // references to a volume - // Note that we're not just interested in containers - // which are part of the private state, and instead - // *all* containers. This means we don't remove - // something that's part of a sideloaded container - const [dockerContainers, dockerVolumes] = await Promise.all([ - docker.listContainers(), - docker.listVolumes(), - ]); - - const containerVolumes = _(dockerContainers) - .flatMap((c) => c.Mounts) - .filter((m) => m.Type === 'volume') - // We know that the name must be set, if the mount is - // a volume - .map((m) => m.Name as string) - .uniq() - .value(); - const volumeNames = _.map(dockerVolumes.Volumes, 'Name'); - - const volumesToRemove = _.difference( - volumeNames, - containerVolumes, - // Don't remove any volume which is still referenced - // in the target state - referencedVolumes, - ); - await Promise.all(volumesToRemove.map((v) => docker.getVolume(v).remove())); - } - - private async listWithBothLabels(): Promise { - const [legacyResponse, currentResponse] = await Promise.all([ - docker.listVolumes({ - filters: { label: ['io.resin.supervised'] }, - }), - docker.listVolumes({ - filters: { label: ['io.balena.supervised'] }, - }), - ]); - - const legacyVolumes = _.get(legacyResponse, 'Volumes', []); - const currentVolumes = _.get(currentResponse, 'Volumes', []); - return _.unionBy(legacyVolumes, currentVolumes, 'Name'); + await volume.create(); } } -export default VolumeManager; +// We simply forward this to the volume object, but we +// add this method to provide a consistent interface +export async function remove(volume: Volume) { + await volume.remove(); +} + +export async function createFromLegacy(appId: number): Promise { + const name = defaultLegacyVolume(); + const legacyPath = Path.join( + constants.rootMountPoint, + 'mnt/data/resin-data', + appId.toString(), + ); + + try { + return await createFromPath({ name, appId }, {}, legacyPath); + } catch (e) { + logger.logSystemMessage( + `Warning: could not migrate legacy /data volume: ${e.message}`, + { error: e }, + 'Volume migration error', + ); + } +} + +export async function createFromPath( + { name, appId }: VolumeNameOpts, + config: Partial, + oldPath: string, +): Promise { + const volume = Volume.fromComposeObject(name, appId, config); + + await create(volume); + const inspect = await docker + .getVolume(Volume.generateDockerName(volume.appId, volume.name)) + .inspect(); + + const volumePath = Path.join( + constants.rootMountPoint, + 'mnt/data', + ...inspect.Mountpoint.split(Path.sep).slice(3), + ); + + await safeRename(oldPath, volumePath); + return volume; +} + +export async function removeOrphanedVolumes( + referencedVolumes: string[], +): Promise { + // Iterate through every container, and track the + // references to a volume + // Note that we're not just interested in containers + // which are part of the private state, and instead + // *all* containers. This means we don't remove + // something that's part of a sideloaded container + const [dockerContainers, dockerVolumes] = await Promise.all([ + docker.listContainers(), + docker.listVolumes(), + ]); + + const containerVolumes = _(dockerContainers) + .flatMap((c) => c.Mounts) + .filter((m) => m.Type === 'volume') + // We know that the name must be set, if the mount is + // a volume + .map((m) => m.Name as string) + .uniq() + .value(); + const volumeNames = _.map(dockerVolumes.Volumes, 'Name'); + + const volumesToRemove = _.difference( + volumeNames, + containerVolumes, + // Don't remove any volume which is still referenced + // in the target state + referencedVolumes, + ); + await Promise.all(volumesToRemove.map((v) => docker.getVolume(v).remove())); +} + +async function listWithBothLabels(): Promise { + const [legacyResponse, currentResponse] = await Promise.all([ + docker.listVolumes({ + filters: { label: ['io.resin.supervised'] }, + }), + docker.listVolumes({ + filters: { label: ['io.balena.supervised'] }, + }), + ]); + + const legacyVolumes = _.get(legacyResponse, 'Volumes', []); + const currentVolumes = _.get(currentResponse, 'Volumes', []); + return _.unionBy(legacyVolumes, currentVolumes, 'Name'); +} diff --git a/src/device-api/v2.ts b/src/device-api/v2.ts index eaef1615..5b5d33d5 100644 --- a/src/device-api/v2.ts +++ b/src/device-api/v2.ts @@ -9,6 +9,7 @@ import * as config from '../config'; import * as db from '../db'; import * as logger from '../logger'; import * as images from '../compose/images'; +import * as volumeManager from '../compose/volume-manager'; import { spawnJournalctl } from '../lib/journald'; import { appNotFoundMessage, @@ -484,7 +485,7 @@ export function createV2Api(router: Router, applications: ApplicationManager) { referencedVolumes.push(Volume.generateDockerName(vol.appId, vol.name)); }); }); - await applications.volumes.removeOrphanedVolumes(referencedVolumes); + await volumeManager.removeOrphanedVolumes(referencedVolumes); res.json({ status: 'success', }); diff --git a/src/lib/migration.ts b/src/lib/migration.ts index 87759eb9..9dfc3cdf 100644 --- a/src/lib/migration.ts +++ b/src/lib/migration.ts @@ -12,6 +12,7 @@ const rimrafAsync = Bluebird.promisify(rimraf); import { ApplicationManager } from '../application-manager'; import * as config from '../config'; import * as db from '../db'; +import * as volumeManager from '../compose/volume-manager'; import DeviceState from '../device-state'; import * as constants from '../lib/constants'; import { BackupError, DatabaseParseError, NotFoundError } from '../lib/errors'; @@ -249,7 +250,7 @@ export async function normaliseLegacyDatabase( const targetApps = await application.getTargetApps(); for (const appId of _.keys(targetApps)) { - await application.volumes.createFromLegacy(parseInt(appId, 10)); + await volumeManager.createFromLegacy(parseInt(appId, 10)); } await config.set({ @@ -302,7 +303,7 @@ export async function loadBackupFromMigration( if (volumes[volumeName] != null) { log.debug(`Creating volume ${volumeName} from backup`); // If the volume exists (from a previous incomplete run of this restoreBackup), we delete it first - await deviceState.applications.volumes + await volumeManager .get({ appId, name: volumeName }) .then((volume) => { return volume.remove(); @@ -314,7 +315,7 @@ export async function loadBackupFromMigration( throw error; }); - await deviceState.applications.volumes.createFromPath( + await volumeManager.createFromPath( { appId, name: volumeName }, volumes[volumeName], path.join(backupPath, volumeName), diff --git a/test/lib/mocked-device-api.ts b/test/lib/mocked-device-api.ts index 82ea0908..2353c8c0 100644 --- a/test/lib/mocked-device-api.ts +++ b/test/lib/mocked-device-api.ts @@ -5,7 +5,7 @@ import { stub } from 'sinon'; import { ApplicationManager } from '../../src/application-manager'; import * as networkManager from '../../src/compose/network-manager'; import { ServiceManager } from '../../src/compose/service-manager'; -import { VolumeManager } from '../../src/compose/volume-manager'; +import * as volumeManager from '../../src/compose/volume-manager'; import * as config from '../../src/config'; import * as db from '../../src/db'; import { createV1Api } from '../../src/device-api/v1'; @@ -134,22 +134,23 @@ function buildRoutes(appManager: ApplicationManager): Router { } const originalNetGetAll = networkManager.getAllByAppId; +const originalVolGetAll = volumeManager.getAllByAppId; function setupStubs() { stub(ServiceManager.prototype, 'getStatus').resolves(STUBBED_VALUES.services); - stub(VolumeManager.prototype, 'getAllByAppId').resolves( - STUBBED_VALUES.volumes, - ); // @ts-expect-error Assigning to a RO property networkManager.getAllByAppId = () => Promise.resolve(STUBBED_VALUES.networks); + // @ts-expect-error Assigning to a RO property + volumeManager.getAllByAppId = () => Promise.resolve(STUBBED_VALUES.volumes); } function restoreStubs() { (ServiceManager.prototype as any).getStatus.restore(); - (VolumeManager.prototype as any).getAllByAppId.restore(); // @ts-expect-error Assigning to a RO property networkManager.getAllByAppId = originalNetGetAll; + // @ts-expect-error Assigning to a RO property + volumeManager.getAllByAppId = originalVolGetAll; } interface SupervisorAPIOpts {