From 8fc97b9de89392bb9e6fd562e12d261ced3990cb Mon Sep 17 00:00:00 2001 From: Cameron Diver Date: Thu, 11 Jun 2020 11:05:43 +0100 Subject: [PATCH 1/3] Make network-manager module a singleton Change-type: patch Signed-off-by: Cameron Diver --- .gitignore | 1 + src/application-manager.d.ts | 2 - src/application-manager.js | 296 ++++++++++--------------------- src/compose/composition-steps.ts | 9 +- src/compose/network-manager.ts | 281 +++++++++++++++-------------- test/lib/mocked-device-api.ts | 13 +- 6 files changed, 249 insertions(+), 353 deletions(-) diff --git a/.gitignore b/.gitignore index eb7c4a22..60274643 100644 --- a/.gitignore +++ b/.gitignore @@ -17,3 +17,4 @@ test/data/led_file report.xml .DS_Store .tsbuildinfo +.prettierrc diff --git a/src/application-manager.d.ts b/src/application-manager.d.ts index fd96afee..d0cbd70d 100644 --- a/src/application-manager.d.ts +++ b/src/application-manager.d.ts @@ -13,7 +13,6 @@ import DeviceState from './device-state'; import { APIBinder } from './api-binder'; import * as config from './config'; -import NetworkManager from './compose/network-manager'; import VolumeManager from './compose/volume-manager'; import { @@ -50,7 +49,6 @@ class ApplicationManager extends EventEmitter { public services: ServiceManager; public volumes: VolumeManager; - public networks: NetworkManager; public proxyvisor: any; public timeSpentFetching: number; diff --git a/src/application-manager.js b/src/application-manager.js index 9ddd8321..4b3b09d9 100644 --- a/src/application-manager.js +++ b/src/application-manager.js @@ -14,17 +14,15 @@ 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 { NetworkManager } from './compose/network-manager'; +import { Network } from './compose/network'; +import * as networkManager from './compose/network-manager'; import { VolumeManager } from './compose/volume-manager'; import * as compositionSteps from './compose/composition-steps'; @@ -37,7 +35,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, @@ -47,7 +45,7 @@ const imageForService = (service) => ({ dependent: 0, }); -const fetchAction = (service) => ({ +const fetchAction = service => ({ action: 'fetch', image: imageForService(service), serviceId: service.serviceId, @@ -78,12 +76,7 @@ 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()) { @@ -135,12 +128,8 @@ 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); @@ -148,9 +137,7 @@ 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); @@ -160,7 +147,6 @@ export class ApplicationManager extends EventEmitter { this.apiBinder = apiBinder; this.services = new ServiceManager(); - this.networks = new NetworkManager(); this.volumes = new VolumeManager(); this.proxyvisor = new Proxyvisor({ applications: this, @@ -174,14 +160,13 @@ export class ApplicationManager extends EventEmitter { this.actionExecutors = compositionSteps.getExecutors({ lockFn: this._lockingIfNecessary, services: this.services, - networks: this.networks, volumes: this.volumes, applications: this, callbacks: { - containerStarted: (id) => { + containerStarted: id => { this._containerStarted[id] = true; }, - containerKilled: (id) => { + containerKilled: id => { delete this._containerStarted[id]; }, fetchStart: () => { @@ -190,16 +175,14 @@ 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); @@ -213,7 +196,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')); }); }; @@ -265,10 +248,7 @@ 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', @@ -357,7 +337,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; }); @@ -367,7 +347,7 @@ export class ApplicationManager extends EventEmitter { getCurrentForComparison() { return Promise.join( this.services.getAll(), - this.networks.getAll(), + networkManager.getAll(), this.volumes.getAll(), config.get('currentCommit'), this._buildApps, @@ -377,7 +357,7 @@ export class ApplicationManager extends EventEmitter { getCurrentApp(appId) { return Promise.join( this.services.getAllByAppId(appId), - this.networks.getAllByAppId(appId), + networkManager.getAllByAppId(appId), this.volumes.getAllByAppId(appId), config.get('currentCommit'), this._buildApps, @@ -422,19 +402,13 @@ 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( @@ -454,7 +428,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], @@ -467,7 +441,7 @@ export class ApplicationManager extends EventEmitter { const needUpdate = _.filter( toBeMaybeUpdated, - (serviceId) => + serviceId => !currentServicesPerId[serviceId].isEqual( targetServicesPerId[serviceId], containerIds, @@ -502,7 +476,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({ @@ -515,7 +489,7 @@ export class ApplicationManager extends EventEmitter { } compareNetworksForUpdate({ current, target }) { - return this._compareNetworksOrVolumesForUpdate(this.networks, { + return this._compareNetworksOrVolumesForUpdate(networkManager, { current, target, }); @@ -535,8 +509,7 @@ 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; @@ -545,7 +518,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; @@ -553,15 +526,10 @@ 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; @@ -570,7 +538,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; @@ -583,7 +551,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; @@ -592,12 +560,7 @@ 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 @@ -612,7 +575,7 @@ export class ApplicationManager extends EventEmitter { if ( !_.some( availableImages, - (image) => + image => image.dockerImageId === dependencyService.image || Images.isSameImage(image, { name: dependencyService.imageName }), ) @@ -633,7 +596,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)) { @@ -679,9 +642,7 @@ 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 }, @@ -694,10 +655,7 @@ 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); @@ -716,12 +674,7 @@ export class ApplicationManager extends EventEmitter { } } - _nextStepForService( - { current, target }, - updateContext, - localMode, - containerIds, - ) { + _nextStepForService({ current, target }, updateContext, localMode, containerIds) { const { targetApp, networkPairs, @@ -746,7 +699,7 @@ export class ApplicationManager extends EventEmitter { if (!localMode) { needsDownload = !_.some( availableImages, - (image) => + image => image.dockerImageId === target?.config.image || Images.isSameImage(image, { name: target.imageName }), ); @@ -768,12 +721,7 @@ export class ApplicationManager extends EventEmitter { const dependenciesMetForKill = () => { return ( !needsDownload && - this._dependenciesMetForServiceKill( - target, - targetApp, - availableImages, - localMode, - ) + this._dependenciesMetForServiceKill(target, targetApp, availableImages, localMode) ); }; @@ -797,9 +745,7 @@ 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', @@ -809,9 +755,7 @@ 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, @@ -857,11 +801,8 @@ 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 @@ -881,11 +822,7 @@ 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, @@ -952,7 +889,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) { @@ -990,10 +927,7 @@ 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]; @@ -1035,17 +969,15 @@ 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, @@ -1092,8 +1024,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, { @@ -1102,13 +1034,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); @@ -1119,16 +1051,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), ), ); @@ -1136,47 +1068,40 @@ 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; }, @@ -1218,10 +1143,8 @@ 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); } @@ -1234,15 +1157,11 @@ 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' }); } @@ -1274,10 +1193,7 @@ 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( @@ -1294,15 +1210,13 @@ 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 @@ -1334,7 +1248,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; @@ -1344,18 +1258,14 @@ 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(); } @@ -1366,10 +1276,8 @@ 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 } = {}) { @@ -1379,9 +1287,7 @@ 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) { @@ -1390,7 +1296,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}`); @@ -1398,12 +1304,12 @@ 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(), downloading: Images.getDownloadingImageIds(), - supervisorNetworkReady: this.networks.supervisorNetworkReady(), + supervisorNetworkReady: networkManager.supervisorNetworkReady(), delta: config.get('delta'), containerIds: Promise.props(containerIdsByAppId), localMode, @@ -1439,7 +1345,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'); } @@ -1451,7 +1357,7 @@ export class ApplicationManager extends EventEmitter { targetState, nextSteps, ) - .then((proxyvisorSteps) => nextSteps.concat(proxyvisorSteps)); + .then(proxyvisorSteps => nextSteps.concat(proxyvisorSteps)); }); } @@ -1462,10 +1368,7 @@ 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}`, @@ -1480,8 +1383,8 @@ export class ApplicationManager extends EventEmitter { } removeAllVolumesForApp(appId) { - return this.volumes.getAllByAppId(appId).then((volumes) => - volumes.map((v) => ({ + return this.volumes.getAllByAppId(appId).then(volumes => + volumes.map(v => ({ action: 'removeVolume', current: v, })), @@ -1500,11 +1403,6 @@ 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 4514e57f..1e9ffb89 100644 --- a/src/compose/composition-steps.ts +++ b/src/compose/composition-steps.ts @@ -11,7 +11,7 @@ import ServiceManager from './service-manager'; import Volume from './volume'; import { checkTruthy } from '../lib/validation'; -import { NetworkManager } from './network-manager'; +import * as networkManager from './network-manager'; import VolumeManager from './volume-manager'; interface BaseCompositionStepArgs { @@ -137,7 +137,6 @@ interface CompositionCallbacks { export function getExecutors(app: { lockFn: LockingFn; services: ServiceManager; - networks: NetworkManager; volumes: VolumeManager; applications: ApplicationManager; callbacks: CompositionCallbacks; @@ -281,19 +280,19 @@ export function getExecutors(app: { } }, createNetwork: async (step) => { - await app.networks.create(step.target); + await networkManager.create(step.target); }, createVolume: async (step) => { await app.volumes.create(step.target); }, removeNetwork: async (step) => { - await app.networks.remove(step.current); + await networkManager.remove(step.current); }, removeVolume: async (step) => { await app.volumes.remove(step.current); }, ensureSupervisorNetwork: async () => { - app.networks.ensureSupervisorNetwork(); + networkManager.ensureSupervisorNetwork(); }, }; diff --git a/src/compose/network-manager.ts b/src/compose/network-manager.ts index b5fe3c45..3fa52988 100644 --- a/src/compose/network-manager.ts +++ b/src/compose/network-manager.ts @@ -12,150 +12,147 @@ import { Network } from './network'; import log from '../lib/supervisor-console'; import { ResourceRecreationAttemptError } from './errors'; -export class NetworkManager { - public getAll(): Bluebird { - return this.getWithBothLabels().map((network: { Name: string }) => { - return docker - .getNetwork(network.Name) - .inspect() - .then((net) => { - return Network.fromDockerNetwork(net); - }); - }); - } - - public getAllByAppId(appId: number): Bluebird { - return this.getAll().filter((network: Network) => network.appId === appId); - } - - public async get(network: { name: string; appId: number }): Promise { - const dockerNet = await docker - .getNetwork(Network.generateDockerName(network.appId, network.name)) - .inspect(); - return Network.fromDockerNetwork(dockerNet); - } - - public async create(network: Network) { - try { - const existing = await this.get({ - name: network.name, - appId: network.appId, - }); - if (!network.isEqualConfig(existing)) { - throw new ResourceRecreationAttemptError('network', network.name); - } - - // We have a network with the same config and name - // already created, we can skip this - } catch (e) { - if (!NotFoundError(e)) { - logger.logSystemEvent(logTypes.createNetworkError, { - network: { name: network.name, appId: network.appId }, - error: e, - }); - throw e; - } - - // If we got a not found error, create the network - await network.create(); - } - } - - public async remove(network: Network) { - // We simply forward this to the network object, but we - // add this method to provide a consistent interface - await network.remove(); - } - - public supervisorNetworkReady(): Bluebird { - return Bluebird.resolve( - fs.stat(`/sys/class/net/${constants.supervisorNetworkInterface}`), - ) - .then(() => { - return docker - .getNetwork(constants.supervisorNetworkInterface) - .inspect(); - }) - .then((network) => { - return ( - network.Options['com.docker.network.bridge.name'] === - constants.supervisorNetworkInterface && - network.IPAM.Config[0].Subnet === constants.supervisorNetworkSubnet && - network.IPAM.Config[0].Gateway === constants.supervisorNetworkGateway - ); - }) - .catchReturn(NotFoundError, false) - .catchReturn(ENOENT, false); - } - - public ensureSupervisorNetwork(): Bluebird { - const removeIt = () => { - return Bluebird.resolve( - docker.getNetwork(constants.supervisorNetworkInterface).remove(), - ).then(() => { - return docker - .getNetwork(constants.supervisorNetworkInterface) - .inspect(); - }); - }; - - return Bluebird.resolve( - docker.getNetwork(constants.supervisorNetworkInterface).inspect(), - ) +export function getAll(): Bluebird { + return getWithBothLabels().map((network: { Name: string }) => { + return docker + .getNetwork(network.Name) + .inspect() .then((net) => { - if ( - net.Options['com.docker.network.bridge.name'] !== - constants.supervisorNetworkInterface || - net.IPAM.Config[0].Subnet !== constants.supervisorNetworkSubnet || - net.IPAM.Config[0].Gateway !== constants.supervisorNetworkGateway - ) { - return removeIt(); - } else { - return Bluebird.resolve( - fs.stat(`/sys/class/net/${constants.supervisorNetworkInterface}`), - ) - .catch(ENOENT, removeIt) - .return(); - } - }) - .catch(NotFoundError, () => { - log.debug(`Creating ${constants.supervisorNetworkInterface} network`); - return Bluebird.resolve( - docker.createNetwork({ - Name: constants.supervisorNetworkInterface, - Options: { - 'com.docker.network.bridge.name': - constants.supervisorNetworkInterface, - }, - IPAM: { - Driver: 'default', - Config: [ - { - Subnet: constants.supervisorNetworkSubnet, - Gateway: constants.supervisorNetworkGateway, - }, - ], - }, - }), - ); + return Network.fromDockerNetwork(net); }); - } + }); +} - private getWithBothLabels() { - return Bluebird.join( - docker.listNetworks({ - filters: { - label: ['io.resin.supervised'], - }, - }), - docker.listNetworks({ - filters: { - label: ['io.balena.supervised'], - }, - }), - (legacyNetworks, currentNetworks) => { - return _.unionBy(currentNetworks, legacyNetworks, 'Id'); - }, - ); +export function getAllByAppId(appId: number): Bluebird { + return getAll().filter((network: Network) => network.appId === appId); +} + +export async function get(network: { + name: string; + appId: number; +}): Promise { + const dockerNet = await docker + .getNetwork(Network.generateDockerName(network.appId, network.name)) + .inspect(); + return Network.fromDockerNetwork(dockerNet); +} + +export async function create(network: Network) { + try { + const existing = await get({ + name: network.name, + appId: network.appId, + }); + if (!network.isEqualConfig(existing)) { + throw new ResourceRecreationAttemptError('network', network.name); + } + + // We have a network with the same config and name + // already created, we can skip this + } catch (e) { + if (!NotFoundError(e)) { + logger.logSystemEvent(logTypes.createNetworkError, { + network: { name: network.name, appId: network.appId }, + error: e, + }); + throw e; + } + + // If we got a not found error, create the network + await network.create(); } } + +export async function remove(network: Network) { + // We simply forward this to the network object, but we + // add this method to provide a consistent interface + await network.remove(); +} + +export function supervisorNetworkReady(): Bluebird { + return Bluebird.resolve( + fs.stat(`/sys/class/net/${constants.supervisorNetworkInterface}`), + ) + .then(() => { + return docker.getNetwork(constants.supervisorNetworkInterface).inspect(); + }) + .then((network) => { + return ( + network.Options['com.docker.network.bridge.name'] === + constants.supervisorNetworkInterface && + network.IPAM.Config[0].Subnet === constants.supervisorNetworkSubnet && + network.IPAM.Config[0].Gateway === constants.supervisorNetworkGateway + ); + }) + .catchReturn(NotFoundError, false) + .catchReturn(ENOENT, false); +} + +export function ensureSupervisorNetwork(): Bluebird { + const removeIt = () => { + return Bluebird.resolve( + docker.getNetwork(constants.supervisorNetworkInterface).remove(), + ).then(() => { + return docker.getNetwork(constants.supervisorNetworkInterface).inspect(); + }); + }; + + return Bluebird.resolve( + docker.getNetwork(constants.supervisorNetworkInterface).inspect(), + ) + .then((net) => { + if ( + net.Options['com.docker.network.bridge.name'] !== + constants.supervisorNetworkInterface || + net.IPAM.Config[0].Subnet !== constants.supervisorNetworkSubnet || + net.IPAM.Config[0].Gateway !== constants.supervisorNetworkGateway + ) { + return removeIt(); + } else { + return Bluebird.resolve( + fs.stat(`/sys/class/net/${constants.supervisorNetworkInterface}`), + ) + .catch(ENOENT, removeIt) + .return(); + } + }) + .catch(NotFoundError, () => { + log.debug(`Creating ${constants.supervisorNetworkInterface} network`); + return Bluebird.resolve( + docker.createNetwork({ + Name: constants.supervisorNetworkInterface, + Options: { + 'com.docker.network.bridge.name': + constants.supervisorNetworkInterface, + }, + IPAM: { + Driver: 'default', + Config: [ + { + Subnet: constants.supervisorNetworkSubnet, + Gateway: constants.supervisorNetworkGateway, + }, + ], + }, + }), + ); + }); +} + +function getWithBothLabels() { + return Bluebird.join( + docker.listNetworks({ + filters: { + label: ['io.resin.supervised'], + }, + }), + docker.listNetworks({ + filters: { + label: ['io.balena.supervised'], + }, + }), + (legacyNetworks, currentNetworks) => { + return _.unionBy(currentNetworks, legacyNetworks, 'Id'); + }, + ); +} diff --git a/test/lib/mocked-device-api.ts b/test/lib/mocked-device-api.ts index f139bcfa..82ea0908 100644 --- a/test/lib/mocked-device-api.ts +++ b/test/lib/mocked-device-api.ts @@ -3,7 +3,7 @@ import { fs } from 'mz'; import { stub } from 'sinon'; import { ApplicationManager } from '../../src/application-manager'; -import { NetworkManager } from '../../src/compose/network-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 config from '../../src/config'; @@ -133,20 +133,23 @@ function buildRoutes(appManager: ApplicationManager): Router { return router; } +const originalNetGetAll = networkManager.getAllByAppId; function setupStubs() { stub(ServiceManager.prototype, 'getStatus').resolves(STUBBED_VALUES.services); - stub(NetworkManager.prototype, 'getAllByAppId').resolves( - STUBBED_VALUES.networks, - ); stub(VolumeManager.prototype, 'getAllByAppId').resolves( STUBBED_VALUES.volumes, ); + + // @ts-expect-error Assigning to a RO property + networkManager.getAllByAppId = () => Promise.resolve(STUBBED_VALUES.networks); } function restoreStubs() { (ServiceManager.prototype as any).getStatus.restore(); - (NetworkManager.prototype as any).getAllByAppId.restore(); (VolumeManager.prototype as any).getAllByAppId.restore(); + + // @ts-expect-error Assigning to a RO property + networkManager.getAllByAppId = originalNetGetAll; } interface SupervisorAPIOpts { From adaad786af161a7a3cdf61f2c63926e702fef790 Mon Sep 17 00:00:00 2001 From: Cameron Diver Date: Thu, 11 Jun 2020 11:43:03 +0100 Subject: [PATCH 2/3] 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 { From 0e8d92e08af5f18fce9aa9e3e1e7057c98dee1c3 Mon Sep 17 00:00:00 2001 From: Cameron Diver Date: Mon, 15 Jun 2020 11:31:26 +0100 Subject: [PATCH 3/3] Make service-manager module a singleton Change-type: patch Signed-off-by: Cameron Diver --- src/application-manager.d.ts | 3 - src/application-manager.js | 22 +- src/compose/composition-steps.ts | 19 +- src/compose/service-manager.ts | 1119 +++++++++++++++--------------- src/device-api/v2.ts | 7 +- src/lib/migration.ts | 3 +- test/lib/mocked-device-api.ts | 16 +- 7 files changed, 588 insertions(+), 601 deletions(-) diff --git a/src/application-manager.d.ts b/src/application-manager.d.ts index a5e2f19d..ee77c42d 100644 --- a/src/application-manager.d.ts +++ b/src/application-manager.d.ts @@ -7,7 +7,6 @@ import { ServiceAction } from './device-api/common'; import { DeviceStatus, InstancedAppState } from './types/state'; import type { Image } from './compose/images'; -import ServiceManager from './compose/service-manager'; import DeviceState from './device-state'; import { APIBinder } from './api-binder'; @@ -45,8 +44,6 @@ class ApplicationManager extends EventEmitter { public deviceState: DeviceState; public apiBinder: APIBinder; - public services: ServiceManager; - public proxyvisor: any; public timeSpentFetching: number; public fetchesInProgress: number; diff --git a/src/application-manager.js b/src/application-manager.js index 20d340a2..25a97ec2 100644 --- a/src/application-manager.js +++ b/src/application-manager.js @@ -21,11 +21,11 @@ import { import * as dbFormat from './device-state/db-format'; -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 * as volumeManager from './compose/volume-manager'; +import * as serviceManager from './compose/service-manager'; import * as compositionSteps from './compose/composition-steps'; import { Proxyvisor } from './proxyvisor'; @@ -159,7 +159,6 @@ export class ApplicationManager extends EventEmitter { this.deviceState = deviceState; this.apiBinder = apiBinder; - this.services = new ServiceManager(); this.proxyvisor = new Proxyvisor({ applications: this, }); @@ -171,7 +170,6 @@ export class ApplicationManager extends EventEmitter { this.actionExecutors = compositionSteps.getExecutors({ lockFn: this._lockingIfNecessary, - services: this.services, applications: this, callbacks: { containerStarted: (id) => { @@ -198,7 +196,7 @@ export class ApplicationManager extends EventEmitter { ); this.router = createApplicationManagerRouter(this); Images.on('change', this.reportCurrentState); - this.services.on('change', this.reportCurrentState); + serviceManager.on('change', this.reportCurrentState); } reportCurrentState(data) { @@ -223,14 +221,14 @@ export class ApplicationManager extends EventEmitter { // But also run it in on startup await cleanup(); await this.localModeManager.init(); - await this.services.attachToRunning(); - await this.services.listenToEvents(); + await serviceManager.attachToRunning(); + await serviceManager.listenToEvents(); } // Returns the status of applications and their services getStatus() { return Promise.join( - this.services.getStatus(), + serviceManager.getStatus(), Images.getStatus(), config.get('currentCommit'), function (services, images, currentCommit) { @@ -362,7 +360,7 @@ export class ApplicationManager extends EventEmitter { getCurrentForComparison() { return Promise.join( - this.services.getAll(), + serviceManager.getAll(), networkManager.getAll(), volumeManager.getAll(), config.get('currentCommit'), @@ -372,7 +370,7 @@ export class ApplicationManager extends EventEmitter { getCurrentApp(appId) { return Promise.join( - this.services.getAllByAppId(appId), + serviceManager.getAllByAppId(appId), networkManager.getAllByAppId(appId), volumeManager.getAllByAppId(appId), config.get('currentCommit'), @@ -1339,13 +1337,13 @@ export class ApplicationManager extends EventEmitter { } stopAll({ force = false, skipLock = false } = {}) { - return Promise.resolve(this.services.getAll()) + return Promise.resolve(serviceManager.getAll()) .map((service) => { return this._lockingIfNecessary( service.appId, { force, skipLock }, () => { - return this.services + return serviceManager .kill(service, { removeContainer: false, wait: true }) .then(() => { delete this._containerStarted[service.containerId]; @@ -1391,7 +1389,7 @@ export class ApplicationManager extends EventEmitter { if (intId == null) { throw new Error(`Invalid id: ${id}`); } - containerIdsByAppId[intId] = this.services.getContainerIdMap(intId); + containerIdsByAppId[intId] = serviceManager.getContainerIdMap(intId); }); return config.get('localMode').then((localMode) => { diff --git a/src/compose/composition-steps.ts b/src/compose/composition-steps.ts index de517a96..d20d86f8 100644 --- a/src/compose/composition-steps.ts +++ b/src/compose/composition-steps.ts @@ -7,7 +7,7 @@ import type { Image } from './images'; import * as images from './images'; import Network from './network'; import Service from './service'; -import ServiceManager from './service-manager'; +import * as serviceManager from './service-manager'; import Volume from './volume'; import { checkTruthy } from '../lib/validation'; @@ -136,7 +136,6 @@ interface CompositionCallbacks { export function getExecutors(app: { lockFn: LockingFn; - services: ServiceManager; applications: ApplicationManager; callbacks: CompositionCallbacks; }) { @@ -150,7 +149,7 @@ export function getExecutors(app: { }, async () => { const wait = _.get(step, ['options', 'wait'], false); - await app.services.kill(step.current, { + await serviceManager.kill(step.current, { removeContainer: false, wait, }); @@ -166,7 +165,7 @@ export function getExecutors(app: { skipLock: step.skipLock || _.get(step, ['options', 'skipLock']), }, async () => { - await app.services.kill(step.current); + await serviceManager.kill(step.current); app.callbacks.containerKilled(step.current.containerId); if (_.get(step, ['options', 'removeImage'])) { await images.removeByDockerId(step.current.config.image); @@ -177,7 +176,7 @@ export function getExecutors(app: { remove: async (step) => { // Only called for dead containers, so no need to // take locks - await app.services.remove(step.current); + await serviceManager.remove(step.current); }, updateMetadata: (step) => { const skipLock = @@ -190,7 +189,7 @@ export function getExecutors(app: { skipLock: skipLock || _.get(step, ['options', 'skipLock']), }, async () => { - await app.services.updateMetadata(step.current, step.target); + await serviceManager.updateMetadata(step.current, step.target); }, ); }, @@ -202,9 +201,9 @@ export function getExecutors(app: { skipLock: step.skipLock || _.get(step, ['options', 'skipLock']), }, async () => { - await app.services.kill(step.current, { wait: true }); + await serviceManager.kill(step.current, { wait: true }); app.callbacks.containerKilled(step.current.containerId); - const container = await app.services.start(step.target); + const container = await serviceManager.start(step.target); app.callbacks.containerStarted(container.id); }, ); @@ -216,7 +215,7 @@ export function getExecutors(app: { }); }, start: async (step) => { - const container = await app.services.start(step.target); + const container = await serviceManager.start(step.target); app.callbacks.containerStarted(container.id); }, updateCommit: async (step) => { @@ -230,7 +229,7 @@ export function getExecutors(app: { skipLock: step.skipLock || _.get(step, ['options', 'skipLock']), }, async () => { - await app.services.handover(step.current, step.target); + await serviceManager.handover(step.current, step.target); }, ); }, diff --git a/src/compose/service-manager.ts b/src/compose/service-manager.ts index 814fd452..4784cce1 100644 --- a/src/compose/service-manager.ts +++ b/src/compose/service-manager.ts @@ -32,322 +32,428 @@ type ServiceManagerEventEmitter = StrictEventEmitter< EventEmitter, ServiceManagerEvents >; +const events: ServiceManagerEventEmitter = new EventEmitter(); interface KillOpts { removeContainer?: boolean; wait?: boolean; } -export class ServiceManager extends (EventEmitter as new () => ServiceManagerEventEmitter) { - // Whether a container has died, indexed by ID - private containerHasDied: Dictionary = {}; - private listening = false; - // Volatile state of containers, indexed by containerId (or random strings if - // we don't yet have an id) - private volatileState: Dictionary> = {}; +export const on: typeof events['on'] = events.on.bind(events); +export const once: typeof events['once'] = events.once.bind(events); +export const removeListener: typeof events['removeListener'] = events.removeListener.bind( + events, +); +export const removeAllListeners: typeof events['removeAllListeners'] = events.removeAllListeners.bind( + events, +); - public constructor() { - super(); - } +// Whether a container has died, indexed by ID +const containerHasDied: Dictionary = {}; +let listening = false; +// Volatile state of containers, indexed by containerId (or random strings if +// we don't yet have an id) +const volatileState: Dictionary> = {}; - public async getAll( - extraLabelFilters: string | string[] = [], - ): Promise { - const filterLabels = ['supervised'].concat(extraLabelFilters); - const containers = await this.listWithBothLabels(filterLabels); +export async function getAll( + extraLabelFilters: string | string[] = [], +): Promise { + const filterLabels = ['supervised'].concat(extraLabelFilters); + const containers = await listWithBothLabels(filterLabels); - const services = await Bluebird.map(containers, async (container) => { - try { - const serviceInspect = await docker - .getContainer(container.Id) - .inspect(); - const service = Service.fromDockerContainer(serviceInspect); - // We know that the containerId is set below, because `fromDockerContainer` - // always sets it - const vState = this.volatileState[service.containerId!]; - if (vState != null && vState.status != null) { - service.status = vState.status; - } - return service; - } catch (e) { - if (NotFoundError(e)) { - return null; - } - throw e; + const services = await Bluebird.map(containers, async (container) => { + try { + const serviceInspect = await docker.getContainer(container.Id).inspect(); + const service = Service.fromDockerContainer(serviceInspect); + // We know that the containerId is set below, because `fromDockerContainer` + // always sets it + const vState = volatileState[service.containerId!]; + if (vState != null && vState.status != null) { + service.status = vState.status; + } + return service; + } catch (e) { + if (NotFoundError(e)) { + return null; } - }); - - return services.filter((s) => s != null) as Service[]; - } - - public async get(service: Service) { - // Get the container ids for special network handling - const containerIds = await this.getContainerIdMap(service.appId!); - const services = ( - await this.getAll(`service-id=${service.serviceId}`) - ).filter((currentService) => - currentService.isEqualConfig(service, containerIds), - ); - - if (services.length === 0) { - const e: StatusCodeError = new Error( - 'Could not find a container matching this service definition', - ); - e.statusCode = 404; throw e; } - return services[0]; - } + }); - public async getStatus() { - const services = await this.getAll(); - const status = _.clone(this.volatileState); + return services.filter((s) => s != null) as Service[]; +} - for (const service of services) { - if (service.containerId == null) { - throw new InternalInconsistencyError( - `containerId not defined in ServiceManager.getStatus: ${service}`, - ); - } - if (status[service.containerId] == null) { - status[service.containerId] = _.pick(service, [ - 'appId', - 'imageId', - 'status', - 'releaseId', - 'commit', - 'createdAt', - 'serviceName', - ]) as Partial; - } - } +export async function get(service: Service) { + // Get the container ids for special network handling + const containerIds = await getContainerIdMap(service.appId!); + const services = ( + await getAll(`service-id=${service.serviceId}`) + ).filter((currentService) => + currentService.isEqualConfig(service, containerIds), + ); - return _.values(status); - } - - public async getByDockerContainerId( - containerId: string, - ): Promise { - const container = await docker.getContainer(containerId).inspect(); - if ( - container.Config.Labels['io.balena.supervised'] == null && - container.Config.Labels['io.resin.supervised'] == null - ) { - return null; - } - return Service.fromDockerContainer(container); - } - - public async updateMetadata( - service: Service, - metadata: { imageId: number; releaseId: number }, - ) { - const svc = await this.get(service); - if (svc.containerId == null) { - throw new InternalInconsistencyError( - `No containerId provided for service ${service.serviceName} in ServiceManager.updateMetadata. Service: ${service}`, - ); - } - - await docker.getContainer(svc.containerId).rename({ - name: `${service.serviceName}_${metadata.imageId}_${metadata.releaseId}`, - }); - } - - public async handover(current: Service, target: Service) { - // We set the running container to not restart so that in case of a poweroff - // it doesn't come back after boot. - await this.prepareForHandover(current); - await this.start(target); - await this.waitToKill( - current, - target.config.labels['io.balena.update.handover-timeout'], + if (services.length === 0) { + const e: StatusCodeError = new Error( + 'Could not find a container matching this service definition', ); - await this.kill(current); + e.statusCode = 404; + throw e; } + return services[0]; +} - public async killAllLegacy(): Promise { - // Containers haven't been normalized (this is an updated supervisor) - // so we need to stop and remove them - const supervisorImageId = ( - await docker.getImage(constants.supervisorImage).inspect() - ).Id; +export async function getStatus() { + const services = await getAll(); + const status = _.clone(volatileState); - for (const container of await docker.listContainers({ all: true })) { - if (container.ImageID !== supervisorImageId) { - await this.killContainer(container.Id, { - serviceName: 'legacy', - }); - } - } - } - - public kill(service: Service, opts: KillOpts = {}) { + for (const service of services) { if (service.containerId == null) { throw new InternalInconsistencyError( - `Attempt to kill container without containerId! Service :${service}`, + `containerId not defined in ServiceManager.getStatus: ${service}`, ); } - return this.killContainer(service.containerId, service, opts); + if (status[service.containerId] == null) { + status[service.containerId] = _.pick(service, [ + 'appId', + 'imageId', + 'status', + 'releaseId', + 'commit', + 'createdAt', + 'serviceName', + ]) as Partial; + } } - public async remove(service: Service) { - logger.logSystemEvent(LogTypes.removeDeadService, { service }); - const existingService = await this.get(service); + return _.values(status); +} - if (existingService.containerId == null) { +export async function getByDockerContainerId( + containerId: string, +): Promise { + const container = await docker.getContainer(containerId).inspect(); + if ( + container.Config.Labels['io.balena.supervised'] == null && + container.Config.Labels['io.resin.supervised'] == null + ) { + return null; + } + return Service.fromDockerContainer(container); +} + +export async function updateMetadata( + service: Service, + metadata: { imageId: number; releaseId: number }, +) { + const svc = await get(service); + if (svc.containerId == null) { + throw new InternalInconsistencyError( + `No containerId provided for service ${service.serviceName} in ServiceManager.updateMetadata. Service: ${service}`, + ); + } + + await docker.getContainer(svc.containerId).rename({ + name: `${service.serviceName}_${metadata.imageId}_${metadata.releaseId}`, + }); +} + +export async function handover(current: Service, target: Service) { + // We set the running container to not restart so that in case of a poweroff + // it doesn't come back after boot. + await prepareForHandover(current); + await start(target); + await waitToKill( + current, + target.config.labels['io.balena.update.handover-timeout'], + ); + await kill(current); +} + +export async function killAllLegacy(): Promise { + // Containers haven't been normalized (this is an updated supervisor) + const supervisorImageId = ( + await docker.getImage(constants.supervisorImage).inspect() + ).Id; + + for (const container of await docker.listContainers({ all: true })) { + if (container.ImageID !== supervisorImageId) { + await killContainer(container.Id, { + serviceName: 'legacy', + }); + } + } +} + +export function kill(service: Service, opts: KillOpts = {}) { + if (service.containerId == null) { + throw new InternalInconsistencyError( + `Attempt to kill container without containerId! Service :${service}`, + ); + } + return killContainer(service.containerId, service, opts); +} + +export async function remove(service: Service) { + logger.logSystemEvent(LogTypes.removeDeadService, { service }); + const existingService = await get(service); + + if (existingService.containerId == null) { + throw new InternalInconsistencyError( + `No containerId provided for service ${service.serviceName} in ServiceManager.updateMetadata. Service: ${service}`, + ); + } + + try { + await docker.getContainer(existingService.containerId).remove({ v: true }); + } catch (e) { + if (!NotFoundError(e)) { + logger.logSystemEvent(LogTypes.removeDeadServiceError, { + service, + error: e, + }); + throw e; + } + } +} +export function getAllByAppId(appId: number) { + return getAll(`app-id=${appId}`); +} + +export async function stopAllByAppId(appId: number) { + for (const app of await getAllByAppId(appId)) { + await kill(app, { removeContainer: false }); + } +} + +export async function create(service: Service) { + const mockContainerId = config.newUniqueKey(); + try { + const existing = await get(service); + if (existing.containerId == null) { throw new InternalInconsistencyError( `No containerId provided for service ${service.serviceName} in ServiceManager.updateMetadata. Service: ${service}`, ); } - - try { - await docker - .getContainer(existingService.containerId) - .remove({ v: true }); - } catch (e) { - if (!NotFoundError(e)) { - logger.logSystemEvent(LogTypes.removeDeadServiceError, { - service, - error: e, - }); - throw e; - } - } - } - public getAllByAppId(appId: number) { - return this.getAll(`app-id=${appId}`); - } - - public async stopAllByAppId(appId: number) { - for (const app of await this.getAllByAppId(appId)) { - await this.kill(app, { removeContainer: false }); - } - } - - public async create(service: Service) { - const mockContainerId = config.newUniqueKey(); - try { - const existing = await this.get(service); - if (existing.containerId == null) { - throw new InternalInconsistencyError( - `No containerId provided for service ${service.serviceName} in ServiceManager.updateMetadata. Service: ${service}`, - ); - } - return docker.getContainer(existing.containerId); - } catch (e) { - if (!NotFoundError(e)) { - logger.logSystemEvent(LogTypes.installServiceError, { - service, - error: e, - }); - throw e; - } - - const deviceName = await config.get('name'); - if (!isValidDeviceName(deviceName)) { - throw new Error( - 'The device name contains a newline, which is unsupported by balena. ' + - 'Please fix the device name', - ); - } - - // Get all created services so far - if (service.appId == null) { - throw new InternalInconsistencyError( - 'Attempt to start a service without an existing application ID', - ); - } - const serviceContainerIds = await this.getContainerIdMap(service.appId); - const conf = service.toDockerContainer({ - deviceName, - containerIds: serviceContainerIds, + return docker.getContainer(existing.containerId); + } catch (e) { + if (!NotFoundError(e)) { + logger.logSystemEvent(LogTypes.installServiceError, { + service, + error: e, }); - const nets = serviceNetworksToDockerNetworks( - service.extraNetworksToJoin(), + throw e; + } + + const deviceName = await config.get('name'); + if (!isValidDeviceName(deviceName)) { + throw new Error( + 'The device name contains a newline, which is unsupported by balena. ' + + 'Please fix the device name', ); + } - logger.logSystemEvent(LogTypes.installService, { service }); - this.reportNewStatus(mockContainerId, service, 'Installing'); - - const container = await docker.createContainer(conf); - service.containerId = container.id; - - await Promise.all( - _.map((nets || {}).EndpointsConfig, (endpointConfig, name) => - docker.getNetwork(name).connect({ - Container: container.id, - EndpointConfig: endpointConfig, - }), - ), + // Get all created services so far + if (service.appId == null) { + throw new InternalInconsistencyError( + 'Attempt to start a service without an existing application ID', ); + } + const serviceContainerIds = await getContainerIdMap(service.appId); + const conf = service.toDockerContainer({ + deviceName, + containerIds: serviceContainerIds, + }); + const nets = serviceNetworksToDockerNetworks(service.extraNetworksToJoin()); - logger.logSystemEvent(LogTypes.installServiceSuccess, { service }); - return container; + logger.logSystemEvent(LogTypes.installService, { service }); + reportNewStatus(mockContainerId, service, 'Installing'); + + const container = await docker.createContainer(conf); + service.containerId = container.id; + + await Promise.all( + _.map((nets || {}).EndpointsConfig, (endpointConfig, name) => + docker.getNetwork(name).connect({ + Container: container.id, + EndpointConfig: endpointConfig, + }), + ), + ); + + logger.logSystemEvent(LogTypes.installServiceSuccess, { service }); + return container; + } finally { + reportChange(mockContainerId); + } +} + +export async function start(service: Service) { + let alreadyStarted = false; + let containerId: string | null = null; + + try { + const container = await create(service); + containerId = container.id; + logger.logSystemEvent(LogTypes.startService, { service }); + + reportNewStatus(containerId, service, 'Starting'); + + let shouldRemove = false; + let err: Error | undefined; + try { + await container.start(); + } catch (e) { + // Get the statusCode from the original cause and make sure it's + // definitely an int for comparison reasons + const maybeStatusCode = PermissiveNumber.decode(e.statusCode); + if (isLeft(maybeStatusCode)) { + shouldRemove = true; + err = new Error(`Could not parse status code from docker error: ${e}`); + throw err; + } + const statusCode = maybeStatusCode.right; + const message = e.message; + + // 304 means the container was already started, precisely what we want + if (statusCode === 304) { + alreadyStarted = true; + } else if ( + statusCode === 500 && + _.isString(message) && + message.trim().match(/exec format error$/) + ) { + // Provide a friendlier error message for "exec format error" + const deviceType = await config.get('deviceType'); + err = new Error( + `Application architecture incompatible with ${deviceType}: exec format error`, + ); + throw err; + } else { + // rethrow the same error + err = e; + throw e; + } } finally { - this.reportChange(mockContainerId); + if (shouldRemove) { + // If starting the container fialed, we remove it so that it doesn't litter + await container.remove({ v: true }).catch(_.noop); + logger.logSystemEvent(LogTypes.startServiceError, { + service, + error: err, + }); + } + } + + const serviceId = service.serviceId; + const imageId = service.imageId; + if (serviceId == null || imageId == null) { + throw new InternalInconsistencyError( + `serviceId and imageId not defined for service: ${service.serviceName} in ServiceManager.start`, + ); + } + + logger.attach(container.id, { serviceId, imageId }); + + if (!alreadyStarted) { + logger.logSystemEvent(LogTypes.startServiceSuccess, { service }); + } + + service.config.running = true; + return container; + } finally { + if (containerId != null) { + reportChange(containerId); } } +} - public async start(service: Service) { - let alreadyStarted = false; - let containerId: string | null = null; +export function listenToEvents() { + if (listening) { + return; + } - try { - const container = await this.create(service); - containerId = container.id; - logger.logSystemEvent(LogTypes.startService, { service }); + listening = true; - this.reportNewStatus(containerId, service, 'Starting'); + const listen = async () => { + const stream = await docker.getEvents({ + filters: { type: ['container'] } as any, + }); - let remove = false; - let err: Error | undefined; - try { - await container.start(); - } catch (e) { - // Get the statusCode from the original cause and make sure it's - // definitely an int for comparison reasons - const maybeStatusCode = PermissiveNumber.decode(e.statusCode); - if (isLeft(maybeStatusCode)) { - remove = true; - err = new Error( - `Could not parse status code from docker error: ${e}`, - ); - throw err; - } - const statusCode = maybeStatusCode.right; - const message = e.message; + stream.on('error', (e) => { + log.error(`Error on docker events stream:`, e); + }); + const parser = JSONStream.parse(); + parser.on('data', async (data: { status: string; id: string }) => { + if (data != null) { + const status = data.status; + if (status === 'die' || status === 'start') { + try { + let service: Service | null = null; + try { + service = await getByDockerContainerId(data.id); + } catch (e) { + if (!NotFoundError(e)) { + throw e; + } + } + if (service != null) { + events.emit('change'); + if (status === 'die') { + logger.logSystemEvent(LogTypes.serviceExit, { service }); + containerHasDied[data.id] = true; + } else if (status === 'start' && containerHasDied[data.id]) { + delete containerHasDied[data.id]; + logger.logSystemEvent(LogTypes.serviceRestart, { + service, + }); - // 304 means the container was already started, precisely what we want - if (statusCode === 304) { - alreadyStarted = true; - } else if ( - statusCode === 500 && - _.isString(message) && - message.trim().match(/exec format error$/) - ) { - // Provide a friendlier error message for "exec format error" - const deviceType = await config.get('deviceType'); - err = new Error( - `Application architecture incompatible with ${deviceType}: exec format error`, - ); - throw err; - } else { - // rethrow the same error - err = e; - throw e; - } - } finally { - if (remove) { - // If starting the container fialed, we remove it so that it doesn't litter - await container.remove({ v: true }).catch(_.noop); - logger.logSystemEvent(LogTypes.startServiceError, { - service, - error: err, - }); + const serviceId = service.serviceId; + const imageId = service.imageId; + if (serviceId == null || imageId == null) { + throw new InternalInconsistencyError( + `serviceId and imageId not defined for service: ${service.serviceName} in ServiceManager.listenToEvents`, + ); + } + logger.attach(data.id, { + serviceId, + imageId, + }); + } + } + } catch (e) { + log.error('Error on docker event:', e, e.stack); + } } } + }); + return new Promise((resolve, reject) => { + parser + .on('error', (e: Error) => { + log.error('Error on docker events stream:', e); + reject(e); + }) + .on('end', resolve); + stream.pipe(parser); + }); + }; + + Bluebird.resolve(listen()) + .catch((e) => { + log.error('Error listening to events:', e, e.stack); + }) + .finally(() => { + listening = false; + setTimeout(listenToEvents, 1000); + }); + + return; +} + +export async function attachToRunning() { + const services = await getAll(); + for (const service of services) { + if (service.status === 'Running') { const serviceId = service.serviceId; const imageId = service.imageId; if (serviceId == null || imageId == null) { @@ -356,302 +462,187 @@ export class ServiceManager extends (EventEmitter as new () => ServiceManagerEve ); } - logger.attach(container.id, { serviceId, imageId }); - - if (!alreadyStarted) { - logger.logSystemEvent(LogTypes.startServiceSuccess, { service }); - } - - service.config.running = true; - return container; - } finally { - if (containerId != null) { - this.reportChange(containerId); + if (service.containerId == null) { + throw new InternalInconsistencyError( + `containerId not defined for service: ${service.serviceName} in ServiceManager.attachToRunning`, + ); } + logger.attach(service.containerId, { + serviceId, + imageId, + }); } } - - public listenToEvents() { - if (this.listening) { - return; - } - - this.listening = true; - - const listen = async () => { - const stream = await docker.getEvents({ - filters: { type: ['container'] } as any, - }); - - stream.on('error', (e) => { - log.error(`Error on docker events stream:`, e); - }); - const parser = JSONStream.parse(); - parser.on('data', async (data: { status: string; id: string }) => { - if (data != null) { - const status = data.status; - if (status === 'die' || status === 'start') { - try { - let service: Service | null = null; - try { - service = await this.getByDockerContainerId(data.id); - } catch (e) { - if (!NotFoundError(e)) { - throw e; - } - } - if (service != null) { - this.emit('change'); - if (status === 'die') { - logger.logSystemEvent(LogTypes.serviceExit, { service }); - this.containerHasDied[data.id] = true; - } else if ( - status === 'start' && - this.containerHasDied[data.id] - ) { - delete this.containerHasDied[data.id]; - logger.logSystemEvent(LogTypes.serviceRestart, { - service, - }); - - const serviceId = service.serviceId; - const imageId = service.imageId; - if (serviceId == null || imageId == null) { - throw new InternalInconsistencyError( - `serviceId and imageId not defined for service: ${service.serviceName} in ServiceManager.listenToEvents`, - ); - } - logger.attach(data.id, { - serviceId, - imageId, - }); - } - } - } catch (e) { - log.error('Error on docker event:', e, e.stack); - } - } - } - }); - - return new Promise((resolve, reject) => { - parser - .on('error', (e: Error) => { - log.error('Error on docker events stream:', e); - reject(e); - }) - .on('end', resolve); - stream.pipe(parser); - }); - }; - - Bluebird.resolve(listen()) - .catch((e) => { - log.error('Error listening to events:', e, e.stack); - }) - .finally(() => { - this.listening = false; - setTimeout(() => this.listenToEvents(), 1000); - }); - - return; - } - - public async attachToRunning() { - const services = await this.getAll(); - for (const service of services) { - if (service.status === 'Running') { - const serviceId = service.serviceId; - const imageId = service.imageId; - if (serviceId == null || imageId == null) { - throw new InternalInconsistencyError( - `serviceId and imageId not defined for service: ${service.serviceName} in ServiceManager.start`, - ); - } - - if (service.containerId == null) { - throw new InternalInconsistencyError( - `containerId not defined for service: ${service.serviceName} in ServiceManager.attachToRunning`, - ); - } - logger.attach(service.containerId, { - serviceId, - imageId, - }); - } - } - } - - public async getContainerIdMap(appId: number): Promise> { - return _(await this.getAllByAppId(appId)) - .keyBy('serviceName') - .mapValues('containerId') - .value() as Dictionary; - } - - private reportChange(containerId?: string, status?: Partial) { - if (containerId != null) { - if (status != null) { - this.volatileState[containerId] = {}; - _.merge(this.volatileState[containerId], status); - } else if (this.volatileState[containerId] != null) { - delete this.volatileState[containerId]; - } - } - this.emit('change'); - } - - private reportNewStatus( - containerId: string, - service: Partial, - status: string, - ) { - this.reportChange( - containerId, - _.merge( - { status }, - _.pick(service, ['imageId', 'appId', 'releaseId', 'commit']), - ), - ); - } - - private killContainer( - containerId: string, - service: Partial = {}, - { removeContainer = true, wait = false }: KillOpts = {}, - ): Bluebird { - // To maintain compatibility of the `wait` flag, this function is not - // async, but it feels like whether or not the promise should be waited on - // should performed by the caller - // TODO: Remove the need for the wait flag - - return Bluebird.try(() => { - logger.logSystemEvent(LogTypes.stopService, { service }); - if (service.imageId != null) { - this.reportNewStatus(containerId, service, 'Stopping'); - } - - const containerObj = docker.getContainer(containerId); - const killPromise = Bluebird.resolve(containerObj.stop()) - .then(() => { - if (removeContainer) { - return containerObj.remove({ v: true }); - } - }) - .catch((e) => { - // Get the statusCode from the original cause and make sure it's - // definitely an int for comparison reasons - const maybeStatusCode = PermissiveNumber.decode(e.statusCode); - if (isLeft(maybeStatusCode)) { - throw new Error( - `Could not parse status code from docker error: ${e}`, - ); - } - const statusCode = maybeStatusCode.right; - - // 304 means the container was already stopped, so we can just remove it - if (statusCode === 304) { - logger.logSystemEvent(LogTypes.stopServiceNoop, { service }); - // Why do we attempt to remove the container again? - if (removeContainer) { - return containerObj.remove({ v: true }); - } - } else if (statusCode === 404) { - // 404 means the container doesn't exist, precisely what we want! - logger.logSystemEvent(LogTypes.stopRemoveServiceNoop, { - service, - }); - } else { - throw e; - } - }) - .tap(() => { - delete this.containerHasDied[containerId]; - logger.logSystemEvent(LogTypes.stopServiceSuccess, { service }); - }) - .catch((e) => { - logger.logSystemEvent(LogTypes.stopServiceError, { - service, - error: e, - }); - }) - .finally(() => { - if (service.imageId != null) { - this.reportChange(containerId); - } - }); - - if (wait) { - return killPromise; - } - return; - }); - } - - private async listWithBothLabels( - labelList: string[], - ): Promise { - const listWithPrefix = (prefix: string) => - docker.listContainers({ - all: true, - filters: { - label: _.map(labelList, (v) => `${prefix}${v}`), - }, - }); - - const [legacy, current] = await Promise.all([ - listWithPrefix('io.resin.'), - listWithPrefix('io.balena.'), - ]); - - return _.unionBy(legacy, current, 'Id'); - } - - private async prepareForHandover(service: Service) { - const svc = await this.get(service); - if (svc.containerId == null) { - throw new InternalInconsistencyError( - `No containerId provided for service ${service.serviceName} in ServiceManager.prepareForHandover. Service: ${service}`, - ); - } - const container = docker.getContainer(svc.containerId); - await container.update({ RestartPolicy: {} }); - return await container.rename({ - name: `old_${service.serviceName}_${service.imageId}_${service.imageId}_${service.releaseId}`, - }); - } - - private waitToKill(service: Service, timeout: number | string) { - const pollInterval = 100; - timeout = checkInt(timeout, { positive: true }) || 60000; - const deadline = Date.now() + timeout; - - const handoverCompletePaths = service.handoverCompleteFullPathsOnHost(); - - const wait = (): Bluebird => - Bluebird.any( - handoverCompletePaths.map((file) => - fs.stat(file).then(() => fs.unlink(file).catch(_.noop)), - ), - ).catch(async () => { - if (Date.now() < deadline) { - await Bluebird.delay(pollInterval); - return wait(); - } else { - log.info( - `Handover timeout has passed, assuming handover was completed for service ${service.serviceName}`, - ); - } - }); - - log.info( - `Waiting for handover to be completed for service: ${service.serviceName}`, - ); - - return wait().then(() => { - log.success(`Handover complete for service ${service.serviceName}`); - }); - } } -export default ServiceManager; +export async function getContainerIdMap( + appId: number, +): Promise> { + return _(await getAllByAppId(appId)) + .keyBy('serviceName') + .mapValues('containerId') + .value() as Dictionary; +} + +function reportChange(containerId?: string, status?: Partial) { + if (containerId != null) { + if (status != null) { + volatileState[containerId] = { ...status }; + } else if (volatileState[containerId] != null) { + delete volatileState[containerId]; + } + } + events.emit('change'); +} + +function reportNewStatus( + containerId: string, + service: Partial, + status: string, +) { + reportChange( + containerId, + _.merge( + { status }, + _.pick(service, ['imageId', 'appId', 'releaseId', 'commit']), + ), + ); +} + +function killContainer( + containerId: string, + service: Partial = {}, + { removeContainer = true, wait = false }: KillOpts = {}, +): Bluebird { + // To maintain compatibility of the `wait` flag, this function is not + // async, but it feels like whether or not the promise should be waited on + // should performed by the caller + // TODO: Remove the need for the wait flag + + return Bluebird.try(() => { + logger.logSystemEvent(LogTypes.stopService, { service }); + if (service.imageId != null) { + reportNewStatus(containerId, service, 'Stopping'); + } + + const containerObj = docker.getContainer(containerId); + const killPromise = Bluebird.resolve(containerObj.stop()) + .then(() => { + if (removeContainer) { + return containerObj.remove({ v: true }); + } + }) + .catch((e) => { + // Get the statusCode from the original cause and make sure it's + // definitely an int for comparison reasons + const maybeStatusCode = PermissiveNumber.decode(e.statusCode); + if (isLeft(maybeStatusCode)) { + throw new Error( + `Could not parse status code from docker error: ${e}`, + ); + } + const statusCode = maybeStatusCode.right; + + // 304 means the container was already stopped, so we can just remove it + if (statusCode === 304) { + logger.logSystemEvent(LogTypes.stopServiceNoop, { service }); + // Why do we attempt to remove the container again? + if (removeContainer) { + return containerObj.remove({ v: true }); + } + } else if (statusCode === 404) { + // 404 means the container doesn't exist, precisely what we want! + logger.logSystemEvent(LogTypes.stopRemoveServiceNoop, { + service, + }); + } else { + throw e; + } + }) + .tap(() => { + delete containerHasDied[containerId]; + logger.logSystemEvent(LogTypes.stopServiceSuccess, { service }); + }) + .catch((e) => { + logger.logSystemEvent(LogTypes.stopServiceError, { + service, + error: e, + }); + }) + .finally(() => { + if (service.imageId != null) { + reportChange(containerId); + } + }); + + if (wait) { + return killPromise; + } + return; + }); +} + +async function listWithBothLabels( + labelList: string[], +): Promise { + const listWithPrefix = (prefix: string) => + docker.listContainers({ + all: true, + filters: { + label: _.map(labelList, (v) => `${prefix}${v}`), + }, + }); + + const [legacy, current] = await Promise.all([ + listWithPrefix('io.resin.'), + listWithPrefix('io.balena.'), + ]); + + return _.unionBy(legacy, current, 'Id'); +} + +async function prepareForHandover(service: Service) { + const svc = await get(service); + if (svc.containerId == null) { + throw new InternalInconsistencyError( + `No containerId provided for service ${service.serviceName} in ServiceManager.prepareForHandover. Service: ${service}`, + ); + } + const container = docker.getContainer(svc.containerId); + await container.update({ RestartPolicy: {} }); + return await container.rename({ + name: `old_${service.serviceName}_${service.imageId}_${service.imageId}_${service.releaseId}`, + }); +} + +function waitToKill(service: Service, timeout: number | string) { + const pollInterval = 100; + timeout = checkInt(timeout, { positive: true }) || 60000; + const deadline = Date.now() + timeout; + + const handoverCompletePaths = service.handoverCompleteFullPathsOnHost(); + + const wait = (): Bluebird => + Bluebird.any( + handoverCompletePaths.map((file) => + fs.stat(file).then(() => fs.unlink(file).catch(_.noop)), + ), + ).catch(async () => { + if (Date.now() < deadline) { + await Bluebird.delay(pollInterval); + return wait(); + } else { + log.info( + `Handover timeout has passed, assuming handover was completed for service ${service.serviceName}`, + ); + } + }); + + log.info( + `Waiting for handover to be completed for service: ${service.serviceName}`, + ); + + return wait().then(() => { + log.success(`Handover complete for service ${service.serviceName}`); + }); +} diff --git a/src/device-api/v2.ts b/src/device-api/v2.ts index 5b5d33d5..d07ba145 100644 --- a/src/device-api/v2.ts +++ b/src/device-api/v2.ts @@ -10,6 +10,7 @@ import * as db from '../db'; import * as logger from '../logger'; import * as images from '../compose/images'; import * as volumeManager from '../compose/volume-manager'; +import * as serviceManager from '../compose/service-manager'; import { spawnJournalctl } from '../lib/journald'; import { appNotFoundMessage, @@ -153,7 +154,7 @@ export function createV2Api(router: Router, applications: ApplicationManager) { // It's kinda hacky to access the services and db via the application manager // maybe refactor this code Bluebird.join( - applications.services.getStatus(), + serviceManager.getStatus(), images.getStatus(), db.models('app').select(['appId', 'commit', 'name']), ( @@ -359,7 +360,7 @@ export function createV2Api(router: Router, applications: ApplicationManager) { }); router.get('/v2/containerId', async (req, res) => { - const services = await applications.services.getAll(); + const services = await serviceManager.getAll(); if (req.query.serviceName != null || req.query.service != null) { const serviceName = req.query.serviceName || req.query.service; @@ -393,7 +394,7 @@ export function createV2Api(router: Router, applications: ApplicationManager) { const currentRelease = await config.get('currentCommit'); const pending = applications.deviceState.applyInProgress; - const containerStates = (await applications.services.getAll()).map((svc) => + const containerStates = (await serviceManager.getAll()).map((svc) => _.pick( svc, 'status', diff --git a/src/lib/migration.ts b/src/lib/migration.ts index 9dfc3cdf..b4b7f968 100644 --- a/src/lib/migration.ts +++ b/src/lib/migration.ts @@ -13,6 +13,7 @@ import { ApplicationManager } from '../application-manager'; import * as config from '../config'; import * as db from '../db'; import * as volumeManager from '../compose/volume-manager'; +import * as serviceManager from '../compose/service-manager'; import DeviceState from '../device-state'; import * as constants from '../lib/constants'; import { BackupError, DatabaseParseError, NotFoundError } from '../lib/errors'; @@ -244,7 +245,7 @@ export async function normaliseLegacyDatabase( } log.debug('Killing legacy containers'); - await application.services.killAllLegacy(); + await serviceManager.killAllLegacy(); log.debug('Migrating legacy app volumes'); const targetApps = await application.getTargetApps(); diff --git a/test/lib/mocked-device-api.ts b/test/lib/mocked-device-api.ts index 2353c8c0..8380e082 100644 --- a/test/lib/mocked-device-api.ts +++ b/test/lib/mocked-device-api.ts @@ -1,10 +1,9 @@ import { Router } from 'express'; import { fs } from 'mz'; -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 * as serviceManager from '../../src/compose/service-manager'; import * as volumeManager from '../../src/compose/volume-manager'; import * as config from '../../src/config'; import * as db from '../../src/db'; @@ -135,22 +134,23 @@ function buildRoutes(appManager: ApplicationManager): Router { const originalNetGetAll = networkManager.getAllByAppId; const originalVolGetAll = volumeManager.getAllByAppId; +const originalSvcGetStatus = serviceManager.getStatus; function setupStubs() { - stub(ServiceManager.prototype, 'getStatus').resolves(STUBBED_VALUES.services); - // @ts-expect-error Assigning to a RO property - networkManager.getAllByAppId = () => Promise.resolve(STUBBED_VALUES.networks); + networkManager.getAllByAppId = async () => STUBBED_VALUES.networks; // @ts-expect-error Assigning to a RO property - volumeManager.getAllByAppId = () => Promise.resolve(STUBBED_VALUES.volumes); + volumeManager.getAllByAppId = async () => STUBBED_VALUES.volumes; + // @ts-expect-error Assigning to a RO property + serviceManager.getStatus = async () => STUBBED_VALUES.services; } function restoreStubs() { - (ServiceManager.prototype as any).getStatus.restore(); - // @ts-expect-error Assigning to a RO property networkManager.getAllByAppId = originalNetGetAll; // @ts-expect-error Assigning to a RO property volumeManager.getAllByAppId = originalVolGetAll; + // @ts-expect-error Assigning to a RO property + serviceManager.getStatus = originalSvcGetStatus; } interface SupervisorAPIOpts {