From 19cd310da367c66cbffdea03245bd987223abc37 Mon Sep 17 00:00:00 2001 From: Cameron Diver Date: Fri, 28 Sep 2018 14:32:38 +0100 Subject: [PATCH] Support setting target state in local mode from supervisor API Change-type: minor Closes: #689 Signed-off-by: Cameron Diver --- package.json | 1 - src/api-binder.coffee | 3 +- src/application-manager.coffee | 89 ++++++++++++++++++++++------------ src/application-manager.d.ts | 2 + src/compose/images.coffee | 16 ++++-- src/compose/service.ts | 12 +++-- src/device-api/index.ts | 0 src/device-api/v2.ts | 41 +++++++--------- src/device-state.coffee | 16 ++++-- src/lib/iptables.ts | 16 +++++- src/supervisor-api.coffee | 25 +++++++++- 11 files changed, 149 insertions(+), 72 deletions(-) create mode 100644 src/device-api/index.ts diff --git a/package.json b/package.json index 8dfdde73..1acb84be 100644 --- a/package.json +++ b/package.json @@ -37,7 +37,6 @@ "@types/node": "^10.3.1", "@types/rwlock": "^5.0.2", "@types/shell-quote": "^1.6.0", - "JSONStream": "^1.1.2", "blinking": "~0.0.2", "bluebird": "^3.5.0", "body-parser": "^1.12.0", diff --git a/src/api-binder.coffee b/src/api-binder.coffee index 6e96417c..542f4af8 100644 --- a/src/api-binder.coffee +++ b/src/api-binder.coffee @@ -420,8 +420,9 @@ module.exports = class APIBinder @cachedResinApi._request(requestParams) _report: => - @config.getMany([ 'deviceId', 'apiTimeout', 'apiEndpoint', 'uuid' ]) + @config.getMany([ 'deviceId', 'apiTimeout', 'apiEndpoint', 'uuid', 'localMode' ]) .then (conf) => + return if checkTruthy(conf.localMode) stateDiff = @_getStateDiff() if _.size(stateDiff) is 0 return diff --git a/src/application-manager.coffee b/src/application-manager.coffee index 1bd4d245..c1bcdf51 100644 --- a/src/application-manager.coffee +++ b/src/application-manager.coffee @@ -141,7 +141,9 @@ module.exports = class ApplicationManager extends EventEmitter saveImage: (step) => @images.save(step.image) cleanup: (step) => - @images.cleanup() + @config.get('localMode').then (localMode) -> + if !checkTruthy(localMode) + @images.cleanup() createNetworkOrVolume: (step) => if step.model is 'network' # TODO: These step targets should be the actual compose objects, @@ -186,9 +188,13 @@ module.exports = class ApplicationManager extends EventEmitter # Returns the status of applications and their services getStatus: => + @config.get('localMode').then (localMode) => + @_getStatus(localMode) + + _getStatus: (localMode) => Promise.join( @services.getStatus() - @images.getStatus() + @images.getStatus(localMode) @config.get('currentCommit') @db.models('app').select([ 'appId', 'releaseId', 'commit' ]) (services, images, currentCommit, targetApps) -> @@ -545,7 +551,7 @@ module.exports = class ApplicationManager extends EventEmitter return null } - _nextStepForService: ({ current, target }, updateContext) => + _nextStepForService: ({ current, target }, updateContext, localMode) => { targetApp, networkPairs, volumePairs, installPairs, updatePairs, availableImages, downloading } = updateContext if current?.status == 'Stopping' # There is already a kill step in progress for this service, so we wait @@ -555,8 +561,11 @@ module.exports = class ApplicationManager extends EventEmitter # Dead containers have to be removed return serviceAction('remove', current.serviceId, current) - needsDownload = !_.some availableImages, (image) => - image.dockerImageId == target?.config.image or @images.isSameImage(image, { name: target.imageName }) + needsDownload = false + # Don't attempt to fetch any images in local mode, they should already be there + if !localMode + needsDownload = !_.some availableImages, (image) => + image.dockerImageId == target?.config.image or @images.isSameImage(image, { name: target.imageName }) # This service needs an image download but it's currently downloading, so we wait if needsDownload and target?.imageId in downloading @@ -585,7 +594,7 @@ module.exports = class ApplicationManager extends EventEmitter timeout = checkInt(target.config.labels['io.resin.update.handover-timeout']) return @_strategySteps[strategy](current, target, needsDownload, dependenciesMetForStart, dependenciesMetForKill, needsSpecialKill, timeout) - _nextStepsForAppUpdate: (currentApp, targetApp, availableImages = [], downloading = []) => + _nextStepsForAppUpdate: (currentApp, targetApp, localMode, availableImages = [], downloading = []) => emptyApp = { services: [], volumes: {}, networks: {} } if !targetApp? targetApp = emptyApp @@ -618,7 +627,7 @@ module.exports = class ApplicationManager extends EventEmitter # next step for install pairs in download - start order, but start requires dependencies, networks and volumes met # next step for update pairs in order by update strategy. start requires dependencies, networks and volumes met. for pair in installPairs.concat(updatePairs) - step = @_nextStepForService(pair, { targetApp, networkPairs, volumePairs, installPairs, updatePairs, availableImages, downloading }) + step = @_nextStepForService(pair, { targetApp, networkPairs, volumePairs, installPairs, updatePairs, availableImages, downloading }, localMode) if step? steps.push(step) # next step for network pairs - remove requires services killed, create kill if no pairs or steps affect that service @@ -730,8 +739,6 @@ module.exports = class ApplicationManager extends EventEmitter .tap (appsForDB) => Promise.map appsForDB, (app) => @db.upsertModel('app', app, { appId: app.appId }, trx) - .then (appsForDB) -> - trx('app').whereNotIn('appId', _.map(appsForDB, 'appId')).del() .then => @proxyvisor.setTargetInTransaction(dependent, trx) @@ -752,7 +759,10 @@ module.exports = class ApplicationManager extends EventEmitter @_targetVolatilePerImageId[imageId] = {} getTargetApps: => - @config.get('apiEndpoint'). then (source = '') => + @config.getMany(['apiEndpoint', 'localMode']). then ({ apiEndpoint, localMode }) => + source = apiEndpoint + if checkTruthy(localMode) + source = 'local' Promise.map(@db.models('app').where({ source }), @normaliseAndExtendAppFromDB) .map (app) => if !_.isEmpty(app.services) @@ -788,8 +798,7 @@ module.exports = class ApplicationManager extends EventEmitter # imagesToSave: images that # - 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) => - + _compareImages: (current, target, available, localMode) => allImagesForTargetApp = (app) -> _.map(app.services, imageForService) allImagesForCurrentApp = (app) -> _.map app.services, (service) -> @@ -807,9 +816,11 @@ module.exports = class ApplicationManager extends EventEmitter !_.some available, (availableImage) => @images.isSameImage(availableImage, targetImage) # Images that are available but we don't have them in the DB with the exact metadata: - imagesToSave = _.filter targetImages, (targetImage) => - _.some(available, (availableImage) => @images.isSameImage(availableImage, targetImage)) and - !_.some(availableWithoutIds, (img) -> _.isEqual(img, targetImage)) + imagesToSave = [] + if !localMode + imagesToSave = _.filter targetImages, (targetImage) => + _.some(available, (availableImage) => @images.isSameImage(availableImage, targetImage)) and + !_.some availableWithoutIds, (img) -> _.isEqual(img, targetImage) deltaSources = _.map imagesToDownload, (image) => return @bestDeltaSource(image, available) @@ -836,7 +847,7 @@ module.exports = class ApplicationManager extends EventEmitter if !ignoreImages and _.isEmpty(downloading) if cleanupNeeded nextSteps.push({ action: 'cleanup' }) - { imagesToRemove, imagesToSave } = @_compareImages(current, target, availableImages) + { imagesToRemove, imagesToSave } = @_compareImages(current, target, availableImages, localMode) for image in imagesToSave nextSteps.push({ action: 'saveImage', image }) if _.isEmpty(imagesToSave) @@ -846,7 +857,7 @@ module.exports = class ApplicationManager extends EventEmitter if _.isEmpty(nextSteps) allAppIds = _.union(_.keys(currentByAppId), _.keys(targetByAppId)) for appId in allAppIds - nextSteps = nextSteps.concat(@_nextStepsForAppUpdate(currentByAppId[appId], targetByAppId[appId], availableImages, downloading)) + nextSteps = nextSteps.concat(@_nextStepsForAppUpdate(currentByAppId[appId], targetByAppId[appId], localMode, availableImages, downloading)) newDownloads = _.filter(nextSteps, (s) -> s.action == 'fetch').length if !ignoreImages and delta and newDownloads > 0 downloadsToBlock = downloading.length + newDownloads - constants.maxDeltaDownloads @@ -882,18 +893,32 @@ module.exports = class ApplicationManager extends EventEmitter @actionExecutors[step.action](step, { force, skipLock }) getRequiredSteps: (currentState, targetState, ignoreImages = false) => - Promise.join( - @images.isCleanupNeeded() - @images.getAvailable() - @images.getDownloadingImageIds() - @networks.supervisorNetworkReady() - @config.getMany([ 'localMode', 'delta' ]) - (cleanupNeeded, availableImages, downloading, supervisorNetworkReady, conf) => - @_inferNextSteps(cleanupNeeded, availableImages, downloading, supervisorNetworkReady, currentState, targetState, ignoreImages, conf) - .then (nextSteps) => - if ignoreImages and _.some(nextSteps, action: 'fetch') - throw new Error('Cannot fetch images while executing an API action') - @proxyvisor.getRequiredSteps(availableImages, downloading, currentState, targetState, nextSteps) - .then (proxyvisorSteps) -> - return nextSteps.concat(proxyvisorSteps) - ) + @config.get('localMode').then (localMode) => + Promise.join( + @images.isCleanupNeeded() + @images.getAvailable(localMode) + @images.getDownloadingImageIds() + @networks.supervisorNetworkReady() + @config.get('delta') + (cleanupNeeded, availableImages, downloading, supervisorNetworkReady, delta) => + conf = { delta, localMode } + if localMode + cleanupNeeded = false + @_inferNextSteps(cleanupNeeded, availableImages, downloading, supervisorNetworkReady, currentState, targetState, ignoreImages, conf) + .then (nextSteps) => + if ignoreImages and _.some(nextSteps, action: 'fetch') + throw new Error('Cannot fetch images while executing an API action') + @proxyvisor.getRequiredSteps(availableImages, downloading, currentState, targetState, nextSteps) + .then (proxyvisorSteps) -> + return nextSteps.concat(proxyvisorSteps) + ) + + serviceNameFromId: (serviceId) => + @getTargetApps().then (apps) -> + # Multi-app warning! + # We assume here that there will only be a single + # application + for appId, app of apps + return _.find app.services, (svc) -> + svc.serviceId == serviceId + .get('serviceName') diff --git a/src/application-manager.d.ts b/src/application-manager.d.ts index 98ba959d..87720e5b 100644 --- a/src/application-manager.d.ts +++ b/src/application-manager.d.ts @@ -50,6 +50,8 @@ export class ApplicationManager extends EventEmitter { public getStatus(): Promise; + public serviceNameFromId(serviceId: number): Promise; + } export default ApplicationManager; diff --git a/src/compose/images.coffee b/src/compose/images.coffee index bf1a4d0b..9faecfcf 100644 --- a/src/compose/images.coffee +++ b/src/compose/images.coffee @@ -193,9 +193,16 @@ module.exports = class Images extends EventEmitter @_matchesTagOrDigest(image, dockerImage) or image.dockerImageId == dockerImage.Id # Gets all images that are supervised, in an object containing name, appId, serviceId, serviceName, imageId, dependent. - getAvailable: => + getAvailable: (localMode) => @_withImagesFromDockerAndDB (dockerImages, supervisedImages) => _.filter(supervisedImages, (image) => @_isAvailableInDocker(image, dockerImages)) + .then (images) => + if localMode + # Get all images present on the local daemon which are tagged as local images + return @_getLocalModeImages().then (localImages) -> + images.concat(localImages) + return images + getDownloadingImageIds: => Promise.try => @@ -218,8 +225,8 @@ module.exports = class Images extends EventEmitter ids = _.map(imagesToRemove, 'id') @db.models('image').del().whereIn('id', ids) - getStatus: => - @getAvailable() + getStatus: (localMode) => + @getAvailable(localMode) .map (image) -> image.status = 'Downloaded' image.downloadProgress = null @@ -304,3 +311,6 @@ module.exports = class Images extends EventEmitter return image1.name == image2.name or Images.hasSameDigest(image1.name, image2.name) isSameImage: @isSameImage + + _getLocalModeImages: => + @docker.listImages(filters: label: [ 'io.resin.local.image=1' ]) diff --git a/src/compose/service.ts b/src/compose/service.ts index fedd3c1b..096f403c 100644 --- a/src/compose/service.ts +++ b/src/compose/service.ts @@ -77,17 +77,21 @@ export class Service { appConfig = ComposeUtils.camelCaseConfig(appConfig); + const intOrNull = (val: string | number | null | undefined): number | null => { + return checkInt(val) || null; + }; + // Seperate the application information from the docker // container configuration - service.imageId = appConfig.imageId; + service.imageId = intOrNull(appConfig.imageId); delete appConfig.imageId; service.serviceName = appConfig.serviceName; delete appConfig.serviceName; - service.appId = appConfig.appId; + service.appId = intOrNull(appConfig.appId); delete appConfig.appId; - service.releaseId = appConfig.releaseId; + service.releaseId = intOrNull(appConfig.releaseId); delete appConfig.releaseId; - service.serviceId = appConfig.serviceId; + service.serviceId = intOrNull(appConfig.serviceId); delete appConfig.serviceId; service.imageName = appConfig.imageName; delete appConfig.imageName; diff --git a/src/device-api/index.ts b/src/device-api/index.ts new file mode 100644 index 00000000..e69de29b diff --git a/src/device-api/v2.ts b/src/device-api/v2.ts index 60a85770..0e160576 100644 --- a/src/device-api/v2.ts +++ b/src/device-api/v2.ts @@ -6,11 +6,24 @@ import { fs } from 'mz'; import { ApplicationManager } from '../application-manager'; import { Service } from '../compose/service'; import { appNotFoundMessage, serviceNotFoundMessage } from '../lib/messages'; +import { checkTruthy } from '../lib/validation'; import { doPurge, doRestart, serviceAction } from './common'; export function createV2Api(router: Router, applications: ApplicationManager) { - const { _lockingIfNecessary } = applications; + const { _lockingIfNecessary, deviceState } = applications; + + const messageFromError = (err?: Error | string | null): string => { + let message = 'Unknown error'; + if (err != null) { + if (_.isError(err) && err.message != null) { + message = err.message; + } else { + message = err as string; + } + } + return message; + }; const handleServiceAction = ( req: Request, @@ -51,17 +64,7 @@ export function createV2Api(router: Router, applications: ApplicationManager) { }); }) .catch((err) => { - let message; - if (err != null) { - if (err.message != null) { - message = err.message; - } else { - message = err; - } - } else { - message = 'Unknown error'; - } - res.status(503).send(message); + res.status(503).send(messageFromError(err)); }); }); }; @@ -109,16 +112,7 @@ export function createV2Api(router: Router, applications: ApplicationManager) { res.status(200).send('OK'); }) .catch((err) => { - let message; - if (err != null) { - message = err.message; - if (message == null) { - message = err; - } - } else { - message = 'Unknown error'; - } - res.status(503).send(message); + res.status(503).send(messageFromError(err)); }); }); @@ -241,8 +235,7 @@ export function createV2Api(router: Router, applications: ApplicationManager) { const force = req.body.force; const targetState = req.body; try { - await deviceState.setTarget(targetState); - await deviceState.config.set({ localModeTargetSet: true }); + await deviceState.setTarget(targetState, true); await deviceState.triggerApplyTarget({ force }); res.status(200).json({ status: 'success', diff --git a/src/device-state.coffee b/src/device-state.coffee index f3a49106..c58bffe6 100644 --- a/src/device-state.coffee +++ b/src/device-state.coffee @@ -40,6 +40,8 @@ validateState = Promise.method (state) -> if state.dependent? validateDependentState(state.dependent) +# TODO (refactor): This shouldn't be here, and instead should be part of the other +# device api stuff in ./device-api createDeviceStateRouter = (deviceState) -> router = express.Router() router.use(bodyParser.urlencoded(extended: true)) @@ -238,11 +240,14 @@ module.exports = class DeviceState extends EventEmitter usingInferStepsLock: (fn) => Promise.using @_inferStepsLock, -> fn() - setTarget: (target) -> + setTarget: (target, localSource = false) -> Promise.join( - @config.get('apiEndpoint'), + @config.getMany(['apiEndpoint', 'localMode']), validateState(target), - (source) => + ({ apiEndpoint, localMode }) => + source = apiEndpoint + if (validation.checkTruthy(localMode)) + source = 'local' @usingWriteLockTarget => # Apps, deviceConfig, dependent @db.transaction (trx) => @@ -251,7 +256,10 @@ module.exports = class DeviceState extends EventEmitter .then => @deviceConfig.setTarget(target.local.config, trx) .then => - @applications.setTarget(target.local.apps, target.dependent, source, trx) + if localSource + @applications.setTarget(target.local.apps, target.dependent, source, trx) + else + @applications.setTarget(target.local.apps, target.dependent, apiEndpoint, trx) ) getTarget: ({ initial = false, intermediate = false } = {}) => diff --git a/src/lib/iptables.ts b/src/lib/iptables.ts index 4162a920..227787f8 100644 --- a/src/lib/iptables.ts +++ b/src/lib/iptables.ts @@ -4,15 +4,19 @@ import * as childProcess from 'child_process'; // The following is exported so that we stub it in the tests export const execAsync = Promise.promisify(childProcess.exec); +function clearIptablesRule(rule: string): Promise { + return execAsync(`iptables -D ${rule}`).return(); +} + function clearAndAppendIptablesRule(rule: string): Promise { - return execAsync(`iptables -D ${rule}`) + return clearIptablesRule(rule) .catchReturn(null) .then(() => execAsync(`iptables -A ${rule}`)) .return(); } function clearAndInsertIptablesRule(rule: string): Promise { - return execAsync(`iptables -D ${rule}`) + return clearIptablesRule(rule) .catchReturn(null) .then(() => execAsync(`iptables -I ${rule}`)) .return(); @@ -30,3 +34,11 @@ export function rejectOnAllInterfacesExcept( .catch(() => clearAndAppendIptablesRule(`INPUT -p tcp --dport ${port} -j DROP`)) .return(); } + +export function removeRejections(port: number): Promise { + return clearIptablesRule(`INPUT -p tcp --dport ${port} -j REJECT`) + .catchReturn(null) + .then(() => clearIptablesRule(`INPUT -p tcp --dport ${port} -j DROP`)) + .catchReturn(null) + .return(); +} diff --git a/src/supervisor-api.coffee b/src/supervisor-api.coffee index b5425190..fc698b68 100644 --- a/src/supervisor-api.coffee +++ b/src/supervisor-api.coffee @@ -68,10 +68,33 @@ module.exports = class SupervisorAPI @_api.use(router) listen: (allowedInterfaces, port, apiTimeout) => - iptables.rejectOnAllInterfacesExcept(allowedInterfaces, port) + @config.get('localMode').then (localMode) => + @applyListeningRules(checkTruthy(localMode), port, allowedInterfaces) + .then => + # Monitor the switching of local mode, and change which interfaces will + # be listented to based on that + @config.on 'change', (changedConfig) => + if changedConfig.localMode? + @applyListeningRules(changedConfig.localMode, port, allowedInterfaces) .then => @server = @_api.listen(port) @server.timeout = apiTimeout + applyListeningRules: (allInterfaces, port, allowedInterfaces) => + Promise.try -> + if checkTruthy(allInterfaces) + iptables.removeRejections(port).then -> + console.log('Supervisor API listening on all interfaces') + else + iptables.rejectOnAllInterfacesExcept(allowedInterfaces, port).then -> + console.log('Supervisor API listening on allowed interfaces only') + .catch (e) => + # If there's an error, stop the supervisor api from answering any endpoints, + # and this will eventually be restarted by the healthcheck + console.log('Error on switching supervisor API listening rules - stopping API.') + console.log(' ', e) + if @server? + @stop() + stop: -> @server.close()