From 0fb31014ef38f2243e86c8a49d622359f2d6bbdd Mon Sep 17 00:00:00 2001 From: damienrunnable Date: Mon, 12 Jun 2017 16:24:11 -0700 Subject: [PATCH 01/12] SAN-6078 Initial checkin --- lib/models/mongo/context-version.js | 6 ++- lib/models/mongo/schemas/context-version.js | 28 +++++++++++ lib/models/services/build-service.js | 4 +- lib/models/services/image-service.js | 54 +++++++++++++++++++++ lib/workers/build.container.died.js | 35 ++++++------- unit/models/mongo/context-version.js | 32 +++++++++++- 6 files changed, 134 insertions(+), 25 deletions(-) create mode 100644 lib/models/services/image-service.js diff --git a/lib/models/mongo/context-version.js b/lib/models/mongo/context-version.js index d3f1d1776..90a601c55 100644 --- a/lib/models/mongo/context-version.js +++ b/lib/models/mongo/context-version.js @@ -721,7 +721,7 @@ ContextVersionSchema.statics.updateAndGetFailedBuild = (buildId, errorMessage) = * @param {string} buildId - build id associated with context version * @return {Promise} */ -ContextVersionSchema.statics.updateAndGetSuccessfulBuild = (buildId) => { +ContextVersionSchema.statics.updateAndGetSuccessfulBuild = (buildId, imageData) => { var log = logger.child({ buildId: buildId, method: 'updateAndGetSuccessfulBuild' @@ -736,6 +736,10 @@ ContextVersionSchema.statics.updateAndGetSuccessfulBuild = (buildId) => { } } + if (imageData) { + update.$set.imageData = imageData + } + return ContextVersion._updateByBuildIdAndEmit(buildId, update) } diff --git a/lib/models/mongo/schemas/context-version.js b/lib/models/mongo/schemas/context-version.js index 35e17eb02..cb17ca728 100644 --- a/lib/models/mongo/schemas/context-version.js +++ b/lib/models/mongo/schemas/context-version.js @@ -210,6 +210,34 @@ var ContextVersionSchema = module.exports = new Schema({ }, log: Schema.Types.Mixed, failed: Boolean + }, + imageData: { + type: { + ports: { + type: [{ + protocol: { + type: String + }, + port: { + type: String + } + }], + 'default': [] + }, + cmd: [{ + type: String, + 'default': [] + }], + entryPoint: [{ + type: String, + 'default': [] + }] + }, + 'default': { + ports: [], + cmd: [], + entryPoint: [] + } } }) diff --git a/lib/models/services/build-service.js b/lib/models/services/build-service.js index 57826114a..56d0587f7 100644 --- a/lib/models/services/build-service.js +++ b/lib/models/services/build-service.js @@ -337,11 +337,11 @@ BuildService._buildBuild = function (build, buildInfo, sessionUser) { * @param {String} buildId * @return {Promise} */ -BuildService.updateSuccessfulBuild = (buildId) => { +BuildService.updateSuccessfulBuild = (buildId, imageData) => { var log = logger.child({ method: 'updateSuccessfulBuild' }) log.info({ buildId }, 'updateSuccessfulBuild called') - return ContextVersion.updateAndGetSuccessfulBuild(buildId) + return ContextVersion.updateAndGetSuccessfulBuild(buildId, imageData) .tap((SuccessfullContextVersions) => { var contextVersionIds = SuccessfullContextVersions.map(pluck('_id')) diff --git a/lib/models/services/image-service.js b/lib/models/services/image-service.js new file mode 100644 index 000000000..fa2e26e16 --- /dev/null +++ b/lib/models/services/image-service.js @@ -0,0 +1,54 @@ +/** + * @module lib/models/services/image-service + */ +'use strict' + +require('loadenv')('models/services/infracode-version-service') + +const keypather = require('keypather')() +const logger = require('logger') +const ImageService = module.exports = {} + +ImageService.logger = logger.child({ + module: 'ImageService' +}) + +ImageService.parsePorts = function (ports) { + if (keypather.get(ports, 'length')) { + return ports.reduce((acc, portObj) => { + let keys = Object.keys(portObj) + let splitPort = keys[0].split('/') + + if (splitPort.length === 2) { + acc.push({ + protocol: splitPort[1], + port: splitPort[0] + }) + } + + return acc + }, []) + } + + return [] +} + +ImageService.getImageDataFromJob = function (job) { + let rawImageData = keypather.get(job, 'inspectImageData.Config') + let imageData = {} + + if (!rawImageData) { + return { + ports: [], + cmd: [], + entryPoint: [] + } + } else { + imageData.port = ImageService.parsePorts( + keypather.get(rawImageData, 'ExposedPorts')) + imageData.cmd = keypather.get(rawImageData, 'Cmd') || [] + imageData.entryPoint = keypather.get(rawImageData, 'Entrypoint') || [] + + return imageData + } +} diff --git a/lib/workers/build.container.died.js b/lib/workers/build.container.died.js index 6ac4459c8..7ce31658c 100644 --- a/lib/workers/build.container.died.js +++ b/lib/workers/build.container.died.js @@ -19,34 +19,17 @@ const Promise = require('bluebird') const error = require('error') const InstanceService = require('models/services/instance-service') const BuildService = require('models/services/build-service') +const ImageService = require('models/services/image-service') + const Isolation = require('models/mongo/isolation') const logger = require('logger') const rabbitMQ = require('models/rabbitmq') -const ContainerImageBuilderDied = {} - -module.exports = ContainerImageBuilderDied - -ContainerImageBuilderDied.jobSchema = joi.object({ - from: joi.string().required(), - host: joi.string().uri({ scheme: 'http' }).required(), - id: joi.string().required(), - time: joi.number().required(), - inspectData: joi.object({ - Config: joi.object({ - Labels: joi.object({ - 'contextVersion.build._id': joi.string().required(), - 'ownerUsername': joi.string().required(), - 'sessionUserGithubId': joi.number().required() - }).unknown().required() - }).unknown().required() - }).unknown().required() -}).unknown().required() - class Worker { constructor (job) { this.host = job.host this.id = job.id + this.inspectImageData = ImageService.getImageDataFromJob(job) this.inspectData = job.inspectData this.contextVersionBuildId = job.inspectData.Config.Labels['contextVersion.build._id'] this.ownerUsername = job.inspectData.Config.Labels.ownerUsername @@ -103,7 +86,7 @@ class Worker { imageTag: this.dockerImageTag }) - return BuildService.updateSuccessfulBuild(this.contextVersionBuildId) + return BuildService.updateSuccessfulBuild(this.contextVersionBuildId, this.inspectImageData) } /** @@ -323,6 +306,16 @@ module.exports = { from: joi.string().required(), host: joi.string().uri({ scheme: 'http' }).required(), id: joi.string().required(), + time: joi.number().required(), + inspectImageData: joi.object({ + exposedPorts: joi.array(joi.object({}).unknown()).default([]), + cmd: joi.array(joi.string()).default([]), + entrypoint: joi.array(joi.string()).default([]) + }).default({ + exposedPorts: [], + cmd: [], + entrypoint: [] + }).unknown().required(), inspectData: joi.object({ Config: joi.object({ Labels: joi.object({ diff --git a/unit/models/mongo/context-version.js b/unit/models/mongo/context-version.js index 9999798a6..9b66e3950 100644 --- a/unit/models/mongo/context-version.js +++ b/unit/models/mongo/context-version.js @@ -309,7 +309,7 @@ describe('Context Version Unit Test', function () { done() }) - it('should save a successful build', (done) => { + it('should save a successful build with no image data', (done) => { const buildId = 12341 ContextVersion.updateAndGetSuccessfulBuild(buildId).asCallback(() => { @@ -329,6 +329,36 @@ describe('Context Version Unit Test', function () { done() }) }) + + it('should save a successful build with image data', (done) => { + const buildId = 12341 + const imageData = { + ports: [{ + protocol: 'Omega', + port: '13' + }], + cmd: ['cmd 1'], + entryPoint: ['entry 1'] + } + + ContextVersion.updateAndGetSuccessfulBuild(buildId, imageData).asCallback(() => { + sinon.assert.calledOnce(ContextVersion.updateByAsync) + sinon.assert.calledWith(ContextVersion.updateByAsync, + 'build._id', + buildId, { + $set: { + 'build.failed': false, + 'build.completed': sinon.match.number, + 'state': ContextVersion.states.buildSucceeded, + 'imageData': imageData + } + }) + + sinon.assert.calledOnce(ContextVersion.findByAsync) + sinon.assert.calledWith(ContextVersion.findByAsync, 'build._id', buildId) + done() + }) + }) }) // end updateAndGetSuccessfulBuild describe('findOneCreating', function () { From 708116211fbe5305040580df59c6a329b1d4dd2c Mon Sep 17 00:00:00 2001 From: damienrunnable Date: Fri, 23 Jun 2017 16:28:46 -0700 Subject: [PATCH 02/12] SAN-6078 Adding logging --- lib/models/services/image-service.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/lib/models/services/image-service.js b/lib/models/services/image-service.js index fa2e26e16..e868f7a9b 100644 --- a/lib/models/services/image-service.js +++ b/lib/models/services/image-service.js @@ -34,10 +34,17 @@ ImageService.parsePorts = function (ports) { } ImageService.getImageDataFromJob = function (job) { + const log = ImageService.logger.child({ + method: 'getImageDataFromJob' + }) + let rawImageData = keypather.get(job, 'inspectImageData.Config') let imageData = {} if (!rawImageData) { + log.info('no raw data, returning empty object') + log.info(job) + return { ports: [], cmd: [], @@ -49,6 +56,8 @@ ImageService.getImageDataFromJob = function (job) { imageData.cmd = keypather.get(rawImageData, 'Cmd') || [] imageData.entryPoint = keypather.get(rawImageData, 'Entrypoint') || [] + log.info(imageData) + return imageData } } From 9004c98e36f4cdafc3e4954adf908070ff84137a Mon Sep 17 00:00:00 2001 From: damienrunnable Date: Fri, 23 Jun 2017 16:56:18 -0700 Subject: [PATCH 03/12] SAN-6078 Adding logging --- lib/models/services/image-service.js | 2 ++ lib/workers/build.container.died.js | 3 +++ 2 files changed, 5 insertions(+) diff --git a/lib/models/services/image-service.js b/lib/models/services/image-service.js index e868f7a9b..1d9501efb 100644 --- a/lib/models/services/image-service.js +++ b/lib/models/services/image-service.js @@ -38,6 +38,8 @@ ImageService.getImageDataFromJob = function (job) { method: 'getImageDataFromJob' }) + log.info('called') + let rawImageData = keypather.get(job, 'inspectImageData.Config') let imageData = {} diff --git a/lib/workers/build.container.died.js b/lib/workers/build.container.died.js index 7ce31658c..63b61e612 100644 --- a/lib/workers/build.container.died.js +++ b/lib/workers/build.container.died.js @@ -27,6 +27,9 @@ const rabbitMQ = require('models/rabbitmq') class Worker { constructor (job) { + const log = this.log.child({ method: 'constructor' }) + log.info('called') + this.host = job.host this.id = job.id this.inspectImageData = ImageService.getImageDataFromJob(job) From 5374e39d40d16057028ecea16821a6d1f6c54a6e Mon Sep 17 00:00:00 2001 From: damienrunnable Date: Fri, 23 Jun 2017 17:21:13 -0700 Subject: [PATCH 04/12] SAN-6078 Adding logging --- lib/workers/build.container.died.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/workers/build.container.died.js b/lib/workers/build.container.died.js index 63b61e612..61d623246 100644 --- a/lib/workers/build.container.died.js +++ b/lib/workers/build.container.died.js @@ -27,8 +27,13 @@ const rabbitMQ = require('models/rabbitmq') class Worker { constructor (job) { + this.log = logger.child({ + job: job, + module: 'ContainerImageBuilderDied' + }) const log = this.log.child({ method: 'constructor' }) log.info('called') + log.info('my magic string to find this invocation') this.host = job.host this.id = job.id @@ -39,10 +44,7 @@ class Worker { this.sessionUserGithubId = job.inspectData.Config.Labels.sessionUserGithubId this.dockerImageTag = job.inspectData.Config.Labels.dockerTag - this.log = logger.child({ - job: job, - module: 'ContainerImageBuilderDied' - }) + log.info('my magic string to end this invocation') } /** From 048f0779576b1d99fd0ee8207c7be3d92a1de043 Mon Sep 17 00:00:00 2001 From: damienrunnable Date: Mon, 26 Jun 2017 13:41:40 -0700 Subject: [PATCH 05/12] SAN-6078 Adding logging --- lib/workers/build.container.died.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/workers/build.container.died.js b/lib/workers/build.container.died.js index 61d623246..4b663cddf 100644 --- a/lib/workers/build.container.died.js +++ b/lib/workers/build.container.died.js @@ -52,6 +52,11 @@ class Worker { * @returns {Promise} */ task () { + const log = this.log.child({ method: 'taskConstructor' }) + log.info('called') + log.info('magic') + log.info(this) + return Promise.try(this._clearBuildResources.bind(this)) .bind(this) .then(this._updateModelsAndGetUpdatedContextVersions) From b937e8940f4fc1c0e212f19b4142e3b805c22172 Mon Sep 17 00:00:00 2001 From: damienrunnable Date: Mon, 26 Jun 2017 14:20:05 -0700 Subject: [PATCH 06/12] SAN-6078 Adding logging --- lib/workers/build.container.died.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/workers/build.container.died.js b/lib/workers/build.container.died.js index 4b663cddf..7c8b3e127 100644 --- a/lib/workers/build.container.died.js +++ b/lib/workers/build.container.died.js @@ -53,12 +53,14 @@ class Worker { */ task () { const log = this.log.child({ method: 'taskConstructor' }) - log.info('called') - log.info('magic') - log.info(this) return Promise.try(this._clearBuildResources.bind(this)) .bind(this) + .tap(() => { + log.info('called') + log.info('magic') + log.info(this) + }) .then(this._updateModelsAndGetUpdatedContextVersions) .tap(rabbitMQ.publishContainerLogsStore.bind(rabbitMQ, {containerId: this.id})) .then(this._emitUpdateAndGetInstances) From 2208562552f609fe790ef74e42978b992febd7a3 Mon Sep 17 00:00:00 2001 From: damienrunnable Date: Mon, 26 Jun 2017 15:03:08 -0700 Subject: [PATCH 07/12] SAN-6078 Adding logging --- lib/models/services/image-service.js | 6 +++--- lib/workers/build.container.died.js | 12 ------------ 2 files changed, 3 insertions(+), 15 deletions(-) diff --git a/lib/models/services/image-service.js b/lib/models/services/image-service.js index 1d9501efb..404fef102 100644 --- a/lib/models/services/image-service.js +++ b/lib/models/services/image-service.js @@ -44,9 +44,6 @@ ImageService.getImageDataFromJob = function (job) { let imageData = {} if (!rawImageData) { - log.info('no raw data, returning empty object') - log.info(job) - return { ports: [], cmd: [], @@ -58,6 +55,9 @@ ImageService.getImageDataFromJob = function (job) { imageData.cmd = keypather.get(rawImageData, 'Cmd') || [] imageData.entryPoint = keypather.get(rawImageData, 'Entrypoint') || [] + log.info('rawImageData') + log.info(rawImageData) + log.info('the returned image data') log.info(imageData) return imageData diff --git a/lib/workers/build.container.died.js b/lib/workers/build.container.died.js index 7c8b3e127..f053e13b8 100644 --- a/lib/workers/build.container.died.js +++ b/lib/workers/build.container.died.js @@ -31,9 +31,6 @@ class Worker { job: job, module: 'ContainerImageBuilderDied' }) - const log = this.log.child({ method: 'constructor' }) - log.info('called') - log.info('my magic string to find this invocation') this.host = job.host this.id = job.id @@ -43,8 +40,6 @@ class Worker { this.ownerUsername = job.inspectData.Config.Labels.ownerUsername this.sessionUserGithubId = job.inspectData.Config.Labels.sessionUserGithubId this.dockerImageTag = job.inspectData.Config.Labels.dockerTag - - log.info('my magic string to end this invocation') } /** @@ -52,15 +47,8 @@ class Worker { * @returns {Promise} */ task () { - const log = this.log.child({ method: 'taskConstructor' }) - return Promise.try(this._clearBuildResources.bind(this)) .bind(this) - .tap(() => { - log.info('called') - log.info('magic') - log.info(this) - }) .then(this._updateModelsAndGetUpdatedContextVersions) .tap(rabbitMQ.publishContainerLogsStore.bind(rabbitMQ, {containerId: this.id})) .then(this._emitUpdateAndGetInstances) From 85796d4a2118d9bc40452febce3963221ce2c54a Mon Sep 17 00:00:00 2001 From: damienrunnable Date: Mon, 26 Jun 2017 15:47:41 -0700 Subject: [PATCH 08/12] SAN-6078 Fix the port parse --- lib/models/services/image-service.js | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/models/services/image-service.js b/lib/models/services/image-service.js index 404fef102..55481f258 100644 --- a/lib/models/services/image-service.js +++ b/lib/models/services/image-service.js @@ -14,23 +14,23 @@ ImageService.logger = logger.child({ }) ImageService.parsePorts = function (ports) { - if (keypather.get(ports, 'length')) { - return ports.reduce((acc, portObj) => { - let keys = Object.keys(portObj) - let splitPort = keys[0].split('/') + let portList = [] + let splitPort + + if (ports) { + for (let key of Object.keys(ports)) { + splitPort = key.split('/') if (splitPort.length === 2) { - acc.push({ + portList.push({ protocol: splitPort[1], port: splitPort[0] }) } - - return acc - }, []) + } } - return [] + return portList } ImageService.getImageDataFromJob = function (job) { From e519fb7621a5f149dedcdd7fa3dd17bcd4f0aea0 Mon Sep 17 00:00:00 2001 From: damienrunnable Date: Mon, 26 Jun 2017 16:31:05 -0700 Subject: [PATCH 09/12] SAN-6078 cleanup --- lib/models/services/image-service.js | 27 +++++++++------------------ 1 file changed, 9 insertions(+), 18 deletions(-) diff --git a/lib/models/services/image-service.js b/lib/models/services/image-service.js index 55481f258..cc5150877 100644 --- a/lib/models/services/image-service.js +++ b/lib/models/services/image-service.js @@ -37,29 +37,20 @@ ImageService.getImageDataFromJob = function (job) { const log = ImageService.logger.child({ method: 'getImageDataFromJob' }) - log.info('called') let rawImageData = keypather.get(job, 'inspectImageData.Config') - let imageData = {} - - if (!rawImageData) { + if (rawImageData) { return { - ports: [], - cmd: [], - entryPoint: [] + port: ImageService.parsePorts(keypather.get(rawImageData, 'ExposedPorts')), + cmd: keypather.get(rawImageData, 'Cmd') || [], + entryPoint: keypather.get(rawImageData, 'Entrypoint') || [] } - } else { - imageData.port = ImageService.parsePorts( - keypather.get(rawImageData, 'ExposedPorts')) - imageData.cmd = keypather.get(rawImageData, 'Cmd') || [] - imageData.entryPoint = keypather.get(rawImageData, 'Entrypoint') || [] - - log.info('rawImageData') - log.info(rawImageData) - log.info('the returned image data') - log.info(imageData) + } - return imageData + return { + ports: [], + cmd: [], + entryPoint: [] } } From 933f31544b854734991d12b0f88ed28567675bac Mon Sep 17 00:00:00 2001 From: damienrunnable Date: Tue, 27 Jun 2017 18:00:17 -0700 Subject: [PATCH 10/12] SAN-6078 Code review changes --- lib/models/mongo/context-version.js | 7 +--- lib/models/services/image-service.js | 56 ---------------------------- lib/workers/build.container.died.js | 39 ++++++++++++++++--- unit/models/mongo/context-version.js | 30 ++++++++++++++- 4 files changed, 64 insertions(+), 68 deletions(-) delete mode 100644 lib/models/services/image-service.js diff --git a/lib/models/mongo/context-version.js b/lib/models/mongo/context-version.js index 90a601c55..e42eef35b 100644 --- a/lib/models/mongo/context-version.js +++ b/lib/models/mongo/context-version.js @@ -732,14 +732,11 @@ ContextVersionSchema.statics.updateAndGetSuccessfulBuild = (buildId, imageData) $set: { 'build.completed': Date.now(), 'build.failed': false, - 'state': ContextVersion.states.buildSucceeded + 'state': ContextVersion.states.buildSucceeded, + 'imageData': imageData } } - if (imageData) { - update.$set.imageData = imageData - } - return ContextVersion._updateByBuildIdAndEmit(buildId, update) } diff --git a/lib/models/services/image-service.js b/lib/models/services/image-service.js deleted file mode 100644 index cc5150877..000000000 --- a/lib/models/services/image-service.js +++ /dev/null @@ -1,56 +0,0 @@ -/** - * @module lib/models/services/image-service - */ -'use strict' - -require('loadenv')('models/services/infracode-version-service') - -const keypather = require('keypather')() -const logger = require('logger') -const ImageService = module.exports = {} - -ImageService.logger = logger.child({ - module: 'ImageService' -}) - -ImageService.parsePorts = function (ports) { - let portList = [] - let splitPort - - if (ports) { - for (let key of Object.keys(ports)) { - splitPort = key.split('/') - - if (splitPort.length === 2) { - portList.push({ - protocol: splitPort[1], - port: splitPort[0] - }) - } - } - } - - return portList -} - -ImageService.getImageDataFromJob = function (job) { - const log = ImageService.logger.child({ - method: 'getImageDataFromJob' - }) - log.info('called') - - let rawImageData = keypather.get(job, 'inspectImageData.Config') - if (rawImageData) { - return { - port: ImageService.parsePorts(keypather.get(rawImageData, 'ExposedPorts')), - cmd: keypather.get(rawImageData, 'Cmd') || [], - entryPoint: keypather.get(rawImageData, 'Entrypoint') || [] - } - } - - return { - ports: [], - cmd: [], - entryPoint: [] - } -} diff --git a/lib/workers/build.container.died.js b/lib/workers/build.container.died.js index f053e13b8..85cbd0489 100644 --- a/lib/workers/build.container.died.js +++ b/lib/workers/build.container.died.js @@ -19,7 +19,6 @@ const Promise = require('bluebird') const error = require('error') const InstanceService = require('models/services/instance-service') const BuildService = require('models/services/build-service') -const ImageService = require('models/services/image-service') const Isolation = require('models/mongo/isolation') const logger = require('logger') @@ -34,7 +33,7 @@ class Worker { this.host = job.host this.id = job.id - this.inspectImageData = ImageService.getImageDataFromJob(job) + this.inspectImageData = this._getImageDataFromJob(job) this.inspectData = job.inspectData this.contextVersionBuildId = job.inspectData.Config.Labels['contextVersion.build._id'] this.ownerUsername = job.inspectData.Config.Labels.ownerUsername @@ -292,6 +291,38 @@ class Worker { return false } + + _parsePorts (ports) { + let portList = [] + let splitPort + + if (ports) { + for (let key of Object.keys(ports)) { + splitPort = key.split('/') + + if (splitPort.length === 2) { + portList.push({ + protocol: splitPort[1], + port: splitPort[0] + }) + } + } + } + + return portList + } + + _getImageDataFromJob (job) { + const log = this.log.child({ job, method: '_getImageDataFromJob' }) + log.info('called') + + let rawImageData = keypather.get(job, 'inspectImageData.Config') + return { + port: this._parsePorts(keypather.get(rawImageData, 'ExposedPorts')), + cmd: keypather.get(rawImageData, 'Cmd') || [], + entryPoint: keypather.get(rawImageData, 'Entrypoint') || [] + } + } } module.exports = { @@ -311,10 +342,6 @@ module.exports = { exposedPorts: joi.array(joi.object({}).unknown()).default([]), cmd: joi.array(joi.string()).default([]), entrypoint: joi.array(joi.string()).default([]) - }).default({ - exposedPorts: [], - cmd: [], - entrypoint: [] }).unknown().required(), inspectData: joi.object({ Config: joi.object({ diff --git a/unit/models/mongo/context-version.js b/unit/models/mongo/context-version.js index 9b66e3950..e558d7aa3 100644 --- a/unit/models/mongo/context-version.js +++ b/unit/models/mongo/context-version.js @@ -320,7 +320,8 @@ describe('Context Version Unit Test', function () { $set: { 'build.failed': false, 'build.completed': sinon.match.number, - 'state': ContextVersion.states.buildSucceeded + 'state': ContextVersion.states.buildSucceeded, + imageData: undefined } }) @@ -359,6 +360,33 @@ describe('Context Version Unit Test', function () { done() }) }) + + it('should save a successful build with empty image data', (done) => { + const buildId = 12341 + const imageData = { + ports: [], + cmd: [], + entryPoint: [] + } + + ContextVersion.updateAndGetSuccessfulBuild(buildId, imageData).asCallback(() => { + sinon.assert.calledOnce(ContextVersion.updateByAsync) + sinon.assert.calledWith(ContextVersion.updateByAsync, + 'build._id', + buildId, { + $set: { + 'build.failed': false, + 'build.completed': sinon.match.number, + 'state': ContextVersion.states.buildSucceeded, + 'imageData': imageData + } + }) + + sinon.assert.calledOnce(ContextVersion.findByAsync) + sinon.assert.calledWith(ContextVersion.findByAsync, 'build._id', buildId) + done() + }) + }) }) // end updateAndGetSuccessfulBuild describe('findOneCreating', function () { From 8ef03036a3e19e5ea9583f319943125fa3aa9a89 Mon Sep 17 00:00:00 2001 From: damienrunnable Date: Thu, 29 Jun 2017 11:26:52 -0700 Subject: [PATCH 11/12] SAN-6078 Code review changes --- lib/models/mongo/context-version.js | 1 + lib/models/services/build-service.js | 1 + lib/workers/build.container.died.js | 26 ++++++++++++++++++-------- 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/lib/models/mongo/context-version.js b/lib/models/mongo/context-version.js index e42eef35b..fc1daed8a 100644 --- a/lib/models/mongo/context-version.js +++ b/lib/models/mongo/context-version.js @@ -719,6 +719,7 @@ ContextVersionSchema.statics.updateAndGetFailedBuild = (buildId, errorMessage) = /** * @param {string} buildId - build id associated with context version + * @param {object} imageData - the image data to attach to the context version, * @return {Promise} */ ContextVersionSchema.statics.updateAndGetSuccessfulBuild = (buildId, imageData) => { diff --git a/lib/models/services/build-service.js b/lib/models/services/build-service.js index 56d0587f7..d6fc17f6c 100644 --- a/lib/models/services/build-service.js +++ b/lib/models/services/build-service.js @@ -335,6 +335,7 @@ BuildService._buildBuild = function (build, buildInfo, sessionUser) { /** * @param {String} buildId + * @param {object} imageData - the image data to attach to the context version, * @return {Promise} */ BuildService.updateSuccessfulBuild = (buildId, imageData) => { diff --git a/lib/workers/build.container.died.js b/lib/workers/build.container.died.js index 85cbd0489..34463cf8c 100644 --- a/lib/workers/build.container.died.js +++ b/lib/workers/build.container.died.js @@ -292,6 +292,11 @@ class Worker { return false } + /** + * @param ports {Object} Object who's properties are in the format {/:} + * @returns {Array} Array of ports parsed into {protocol: , port: + * @private + */ _parsePorts (ports) { let portList = [] let splitPort @@ -300,18 +305,23 @@ class Worker { for (let key of Object.keys(ports)) { splitPort = key.split('/') - if (splitPort.length === 2) { - portList.push({ - protocol: splitPort[1], - port: splitPort[0] - }) - } + portList.push({ + protocol: splitPort[1], + port: splitPort[0] + }) } } return portList } + /** + * Returns the image data formatted for the context version model. + * + * @param job + * @returns {{port: Array, cmd: Array, entryPoint: Array}} + * @private + */ _getImageDataFromJob (job) { const log = this.log.child({ job, method: '_getImageDataFromJob' }) log.info('called') @@ -319,8 +329,8 @@ class Worker { let rawImageData = keypather.get(job, 'inspectImageData.Config') return { port: this._parsePorts(keypather.get(rawImageData, 'ExposedPorts')), - cmd: keypather.get(rawImageData, 'Cmd') || [], - entryPoint: keypather.get(rawImageData, 'Entrypoint') || [] + cmd: keypather.get(rawImageData, 'Cmd'), + entryPoint: keypather.get(rawImageData, 'Entrypoint') } } } From 11a3f7523a99e3b3c4dc835069397188d888f876 Mon Sep 17 00:00:00 2001 From: damienrunnable Date: Thu, 29 Jun 2017 13:12:01 -0700 Subject: [PATCH 12/12] SAN-6078 Code review changes --- unit/models/mongo/context-version.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/unit/models/mongo/context-version.js b/unit/models/mongo/context-version.js index e558d7aa3..2195e5bde 100644 --- a/unit/models/mongo/context-version.js +++ b/unit/models/mongo/context-version.js @@ -326,7 +326,7 @@ describe('Context Version Unit Test', function () { }) sinon.assert.calledOnce(ContextVersion.findByAsync) - sinon.assert.calledWith(ContextVersion.findByAsync, 'build._id', buildId) + sinon.assert.calledWithExactly(ContextVersion.findByAsync, 'build._id', buildId) done() }) }) @@ -356,7 +356,7 @@ describe('Context Version Unit Test', function () { }) sinon.assert.calledOnce(ContextVersion.findByAsync) - sinon.assert.calledWith(ContextVersion.findByAsync, 'build._id', buildId) + sinon.assert.calledWithExactly(ContextVersion.findByAsync, 'build._id', buildId) done() }) }) @@ -383,7 +383,7 @@ describe('Context Version Unit Test', function () { }) sinon.assert.calledOnce(ContextVersion.findByAsync) - sinon.assert.calledWith(ContextVersion.findByAsync, 'build._id', buildId) + sinon.assert.calledWithExactly(ContextVersion.findByAsync, 'build._id', buildId) done() }) })