From bd6999eb1e6df68e6c7c8da5337e7de8c5defdf0 Mon Sep 17 00:00:00 2001 From: Henry Mollman Date: Mon, 12 Jun 2017 18:21:17 -0700 Subject: [PATCH 01/18] Add do not autofork option --- lib/models/services/cluster-config-service.js | 7 +++++-- lib/routes/docker-compose-cluster.js | 10 +++++++--- lib/workers/cluster.create.js | 5 +++-- 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/lib/models/services/cluster-config-service.js b/lib/models/services/cluster-config-service.js index 9060f6965..21eed43ee 100644 --- a/lib/models/services/cluster-config-service.js +++ b/lib/models/services/cluster-config-service.js @@ -142,7 +142,8 @@ module.exports = class ClusterConfigService { } const buildOpts = { repoFullName, - triggeredAction: data.triggeredAction + triggeredAction: data.triggeredAction, + shouldNotAutoFork: data.shouldNotAutoFork } return ClusterConfigService.createFromRunnableConfig( sessionUser, @@ -674,6 +675,8 @@ module.exports = class ClusterConfigService { */ static _createInstance (sessionUser, parsedInstanceData, parentBuildId, testingOpts, buildOpts) { const serviceName = keypather.get(parsedInstanceData, 'metadata.name') + const isMain = keypather.get(parsedInstanceData, 'metadata.isMain') + const shouldNotAutoFork = isMain ? buildOpts.shouldNotAutoFork : true const configData = parsedInstanceData.instance const inputInstanceOpts = pick(configData, ['aliases', 'env', 'containerStartCommand', 'name', 'ports']) if (buildOpts.isolated) { @@ -690,7 +693,7 @@ module.exports = class ClusterConfigService { ipWhitelist: { enabled: false }, - shouldNotAutofork: !keypather.get(parsedInstanceData, 'metadata.isMain'), + shouldNotAutofork: shouldNotAutoFork, isolated: buildOpts.isolated, isTesting: testingOpts.isTesting, isTestReporter: testingOpts.isTestReporter diff --git a/lib/routes/docker-compose-cluster.js b/lib/routes/docker-compose-cluster.js index fe82dc6e2..21cda2c25 100644 --- a/lib/routes/docker-compose-cluster.js +++ b/lib/routes/docker-compose-cluster.js @@ -19,7 +19,8 @@ const postSchema = joi.object({ isTesting: joi.boolean().optional(), testReporters: joi.array().optional(), githubId: joi.number().optional(), - parentInputClusterConfigId: joi.string().allow('').optional() + parentInputClusterConfigId: joi.string().allow('').optional(), + shouldNotAutoFork: joi.boolean().optional() }).unknown().required() const deleteSchema = joi.object({ @@ -50,6 +51,7 @@ const postRoute = function (req, res, next) { const parentInputClusterConfigId = keypather.get(req, 'body.parentInputClusterConfigId') const isTesting = keypather.get(req, 'body.isTesting') const testReporters = keypather.get(req, 'body.testReporters') + const shouldNotAutoFork = keypather.get(req, 'body.shouldNotAutoFork') const log = logger.child({ method: 'post', repoFullName, @@ -58,7 +60,8 @@ const postRoute = function (req, res, next) { githubId, isTesting, clusterName, - parentInputClusterConfigId + parentInputClusterConfigId, + shouldNotAutoFork }) log.info('called') return joi.validateOrBoomAsync(req.body, postSchema) @@ -74,7 +77,8 @@ const postRoute = function (req, res, next) { isTesting: isTesting || false, testReporters: testReporters || [], clusterName, - parentInputClusterConfigId: parentInputClusterConfigId || '' + parentInputClusterConfigId: parentInputClusterConfigId || '', + shouldNotAutoFork }) const message = 'cluster.create job enqueued' return { json: { message }, status: 202 } diff --git a/lib/workers/cluster.create.js b/lib/workers/cluster.create.js index 24ed8d59f..14c4e1bf5 100644 --- a/lib/workers/cluster.create.js +++ b/lib/workers/cluster.create.js @@ -22,7 +22,8 @@ module.exports.jobSchema = joi.object({ isTesting: joi.boolean().required(), clusterName: joi.string().required(), parentInputClusterConfigId: joi.string().allow(''), - testReporters: joi.array() + testReporters: joi.array(), + shouldNotAutoFork: joi.boolean().required() }).unknown().required() /** @@ -37,7 +38,7 @@ module.exports.task = (job) => { }) return UserService.getCompleteUserByBigPoppaId(job.sessionUserBigPoppaId) .then(sessionUser => { - const props = [ 'triggeredAction', 'repoFullName', 'branchName', 'filePath', 'isTesting', 'clusterName', 'testReporters', 'parentInputClusterConfigId' ] + const props = [ 'triggeredAction', 'repoFullName', 'branchName', 'filePath', 'isTesting', 'clusterName', 'testReporters', 'parentInputClusterConfigId', 'shouldNotAutoFork' ] const opts = pick(job, props) if (opts.parentInputClusterConfigId === '') { delete opts.parentInputClusterConfigId From 381eba8873418e24856dab4931b92313c90defa9 Mon Sep 17 00:00:00 2001 From: Henry Mollman Date: Tue, 13 Jun 2017 10:36:58 -0700 Subject: [PATCH 02/18] Fix tests --- lib/routes/docker-compose-cluster.js | 2 +- unit/models/services/cluster-config-service.js | 10 ++++++---- unit/routes/docker-compose-cluster.js | 6 ++++-- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/lib/routes/docker-compose-cluster.js b/lib/routes/docker-compose-cluster.js index 21cda2c25..94c63cb67 100644 --- a/lib/routes/docker-compose-cluster.js +++ b/lib/routes/docker-compose-cluster.js @@ -78,7 +78,7 @@ const postRoute = function (req, res, next) { testReporters: testReporters || [], clusterName, parentInputClusterConfigId: parentInputClusterConfigId || '', - shouldNotAutoFork + shouldNotAutoFork: !!shouldNotAutoFork }) const message = 'cluster.create job enqueued' return { json: { message }, status: 202 } diff --git a/unit/models/services/cluster-config-service.js b/unit/models/services/cluster-config-service.js index 1fc83c5c4..6b05b1769 100644 --- a/unit/models/services/cluster-config-service.js +++ b/unit/models/services/cluster-config-service.js @@ -188,6 +188,7 @@ describe('Cluster Config Service Unit Tests', function () { const repoFullName = orgName + '/' + repoName const branchName = 'feature-1' const clusterName = 'api-unit' + const shouldNotAutoFork = true const parsedInput = { repositoryName: clusterName, ownerUsername: orgName, @@ -203,7 +204,7 @@ describe('Cluster Config Service Unit Tests', function () { } const testData = { - triggeredAction, repoFullName, branchName, filePath, isTesting, testReporters, clusterName, parentInputClusterConfigId + triggeredAction, repoFullName, branchName, filePath, isTesting, testReporters, clusterName, parentInputClusterConfigId, shouldNotAutoFork } beforeEach(function (done) { @@ -264,7 +265,7 @@ describe('Cluster Config Service Unit Tests', function () { ClusterConfigService.createFromRunnableConfig, testSessionUser, { results: testParsedContent.results, envFiles: [], files: testParsedContent.files }, - { triggeredAction, repoFullName }, + { triggeredAction, repoFullName, shouldNotAutoFork }, sinon.match({ clusterName, files: testParsedContent.files, @@ -942,7 +943,8 @@ describe('Cluster Config Service Unit Tests', function () { } buildOpts = { isolated: objectId('407f191e810c19729de860e1'), - masterShorthash: 'asdasdsad' + masterShorthash: 'asdasdsad', + shouldNotAutoFork: true } sinon.stub(InstanceService, 'createInstance') done() @@ -993,7 +995,7 @@ describe('Cluster Config Service Unit Tests', function () { isTestReporter, isolated: buildOpts.isolated, isIsolationGroupMaster: false, - shouldNotAutofork: false, + shouldNotAutofork: true, masterPod: false, ipWhitelist: { enabled: false diff --git a/unit/routes/docker-compose-cluster.js b/unit/routes/docker-compose-cluster.js index 6ce090a0f..a182b43c8 100644 --- a/unit/routes/docker-compose-cluster.js +++ b/unit/routes/docker-compose-cluster.js @@ -34,6 +34,7 @@ describe('/docker-compose-cluster', function () { const sessionUserBigPoppaId = 8084808 const parentInputClusterConfigId = 'funk flex' const githubId = sessionUserGithubId + const shouldNotAutoFork = true beforeEach(function (done) { nextStub = sinon.stub() resMock = { @@ -55,7 +56,7 @@ describe('/docker-compose-cluster', function () { createClusterStub = sinon.stub(rabbitMQ, 'createCluster') validateOrBoomStub = sinon.spy(joi, 'validateOrBoomAsync') reqMock = { - body: { repo, branch, filePath, name, parentInputClusterConfigId, githubId }, + body: { repo, branch, filePath, name, parentInputClusterConfigId, githubId, shouldNotAutoFork }, sessionUser: { accounts: { github: { id: sessionUserGithubId } @@ -107,7 +108,8 @@ describe('/docker-compose-cluster', function () { isTesting, parentInputClusterConfigId, testReporters, - clusterName: name + clusterName: name, + shouldNotAutoFork }) }) .asCallback(done) From 2c7fb90ef01c95e29f1edce1b76ca0f3a1bda783 Mon Sep 17 00:00:00 2001 From: Henry Mollman Date: Mon, 19 Jun 2017 12:21:28 -0700 Subject: [PATCH 03/18] Enforce shouldNotAutoFork by default --- lib/models/services/cluster-config-service.js | 2 +- .../models/services/cluster-config-service.js | 52 +++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/lib/models/services/cluster-config-service.js b/lib/models/services/cluster-config-service.js index 21eed43ee..754b150b3 100644 --- a/lib/models/services/cluster-config-service.js +++ b/lib/models/services/cluster-config-service.js @@ -693,7 +693,7 @@ module.exports = class ClusterConfigService { ipWhitelist: { enabled: false }, - shouldNotAutofork: shouldNotAutoFork, + shouldNotAutofork: shouldNotAutoFork !== undefined ? shouldNotAutoFork : true, isolated: buildOpts.isolated, isTesting: testingOpts.isTesting, isTestReporter: testingOpts.isTestReporter diff --git a/unit/models/services/cluster-config-service.js b/unit/models/services/cluster-config-service.js index 6b05b1769..81d49173f 100644 --- a/unit/models/services/cluster-config-service.js +++ b/unit/models/services/cluster-config-service.js @@ -1006,6 +1006,58 @@ describe('Cluster Config Service Unit Tests', function () { }) }) + it('should set shouldNotAutoFork to true by default', () => { + const testParentBuildId = objectId('407f191e810c19729de860ef') + const testParentComposeData = { + env: 'env', + aliases: { + 'dGhyZWUtY2hhbmdpbmctdGhlLWhvc3RuYW1l': { + 'instanceName': 'compose-test-5-1-rethinkdb4', + 'alias': 'three-changing-the-hostname' + } + }, + containerStartCommand: 'containerStartCommand', + name: 'name' + } + const composeData = { + metadata: { + name: 'a1', + isMain: true + }, + instance: testParentComposeData, + build: { + dockerFilePath: 'Nathan219/hello' + } + } + const testInstance = 'build' + buildOpts.shouldNotAutoFork = undefined + InstanceService.createInstance.resolves(testInstance) + + return ClusterConfigService._createInstance(testSessionUser, composeData, testParentBuildId, testingOpts, buildOpts) + .then(instance => { + sinon.assert.calledOnce(InstanceService.createInstance) + sinon.assert.calledWithExactly(InstanceService.createInstance, { + shortName: composeData.metadata.name, + build: testParentBuildId, + aliases: testParentComposeData.aliases, + env: testParentComposeData.env, + containerStartCommand: testParentComposeData.containerStartCommand, + name: buildOpts.masterShorthash + '--' + testParentComposeData.name, + isTesting, + isTestReporter, + isolated: buildOpts.isolated, + isIsolationGroupMaster: false, + shouldNotAutofork: true, + masterPod: false, + ipWhitelist: { + enabled: false + } + }, testSessionUser) + + expect(instance).to.equal(testInstance) + }) + }) + it('should create non-test non-isolated instance', () => { testingOpts.isTesting = false delete buildOpts.isolated From 32ca68d768e972a698fbcc388bbeb7900efd0907 Mon Sep 17 00:00:00 2001 From: Henry Mollman Date: Mon, 19 Jun 2017 18:00:15 -0700 Subject: [PATCH 04/18] Remove ternary operator --- lib/models/services/cluster-config-service.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/models/services/cluster-config-service.js b/lib/models/services/cluster-config-service.js index 754b150b3..6cef26732 100644 --- a/lib/models/services/cluster-config-service.js +++ b/lib/models/services/cluster-config-service.js @@ -676,7 +676,7 @@ module.exports = class ClusterConfigService { static _createInstance (sessionUser, parsedInstanceData, parentBuildId, testingOpts, buildOpts) { const serviceName = keypather.get(parsedInstanceData, 'metadata.name') const isMain = keypather.get(parsedInstanceData, 'metadata.isMain') - const shouldNotAutoFork = isMain ? buildOpts.shouldNotAutoFork : true + const shouldNotAutoFork = !isMain || buildOpts.shouldNotAutoFork const configData = parsedInstanceData.instance const inputInstanceOpts = pick(configData, ['aliases', 'env', 'containerStartCommand', 'name', 'ports']) if (buildOpts.isolated) { From 4a7065f97d668cca5638e62f962bc53c1911beb7 Mon Sep 17 00:00:00 2001 From: Henry Mollman Date: Mon, 19 Jun 2017 18:22:24 -0700 Subject: [PATCH 05/18] Changed to default false 24/7 --- lib/models/services/cluster-config-service.js | 4 ++-- unit/models/services/cluster-config-service.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/models/services/cluster-config-service.js b/lib/models/services/cluster-config-service.js index 6cef26732..023637b77 100644 --- a/lib/models/services/cluster-config-service.js +++ b/lib/models/services/cluster-config-service.js @@ -676,7 +676,7 @@ module.exports = class ClusterConfigService { static _createInstance (sessionUser, parsedInstanceData, parentBuildId, testingOpts, buildOpts) { const serviceName = keypather.get(parsedInstanceData, 'metadata.name') const isMain = keypather.get(parsedInstanceData, 'metadata.isMain') - const shouldNotAutoFork = !isMain || buildOpts.shouldNotAutoFork + const shouldNotAutofork = !isMain || !!buildOpts.shouldNotAutoFork const configData = parsedInstanceData.instance const inputInstanceOpts = pick(configData, ['aliases', 'env', 'containerStartCommand', 'name', 'ports']) if (buildOpts.isolated) { @@ -693,7 +693,7 @@ module.exports = class ClusterConfigService { ipWhitelist: { enabled: false }, - shouldNotAutofork: shouldNotAutoFork !== undefined ? shouldNotAutoFork : true, + shouldNotAutofork, isolated: buildOpts.isolated, isTesting: testingOpts.isTesting, isTestReporter: testingOpts.isTestReporter diff --git a/unit/models/services/cluster-config-service.js b/unit/models/services/cluster-config-service.js index 81d49173f..89c771e6a 100644 --- a/unit/models/services/cluster-config-service.js +++ b/unit/models/services/cluster-config-service.js @@ -1006,7 +1006,7 @@ describe('Cluster Config Service Unit Tests', function () { }) }) - it('should set shouldNotAutoFork to true by default', () => { + it('should set shouldNotAutoFork to false by default', () => { const testParentBuildId = objectId('407f191e810c19729de860ef') const testParentComposeData = { env: 'env', @@ -1047,7 +1047,7 @@ describe('Cluster Config Service Unit Tests', function () { isTestReporter, isolated: buildOpts.isolated, isIsolationGroupMaster: false, - shouldNotAutofork: true, + shouldNotAutofork: false, masterPod: false, ipWhitelist: { enabled: false From ab2fd512aa283ad06e359315c784752a5342e81b Mon Sep 17 00:00:00 2001 From: Nathan219 Date: Thu, 22 Jun 2017 16:53:11 -0700 Subject: [PATCH 06/18] Fixing the merge resolution --- unit/models/services/cluster-config-service.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/unit/models/services/cluster-config-service.js b/unit/models/services/cluster-config-service.js index 91534b503..3f9abf79d 100644 --- a/unit/models/services/cluster-config-service.js +++ b/unit/models/services/cluster-config-service.js @@ -264,7 +264,7 @@ describe('Cluster Config Service Unit Tests', function () { const args = ClusterConfigService.createFromRunnableConfig.getCall(0).args expect(args[0]).to.equal(testSessionUser) expect(args[1]).to.equal(testParsedContent.results) - expect(args[2]).to.equal( { triggeredAction, clusterCreateId, repoFullName }) + expect(args[2]).to.equal( { triggeredAction, clusterCreateId, repoFullName, shouldNotAutoFork }) expect(args[3]).to.equal({ branch: branchName, commit: commitSha, @@ -1007,6 +1007,7 @@ describe('Cluster Config Service Unit Tests', function () { build: testParentBuildId, aliases: testParentComposeData.aliases, env: testParentComposeData.env, + clusterCreateId, containerStartCommand: testParentComposeData.containerStartCommand, name: buildOpts.masterShorthash + '--' + testParentComposeData.name, isTesting, From 6fc49fee268a377fe1c60674a4a455f99025a3ce Mon Sep 17 00:00:00 2001 From: Nathan219 Date: Thu, 22 Jun 2017 18:50:30 -0700 Subject: [PATCH 07/18] add tests Fix merge conflicts! --- lib/models/services/cluster-config-service.js | 1 - .../models/services/cluster-config-service.js | 58 +++++++++++++++++-- 2 files changed, 54 insertions(+), 5 deletions(-) diff --git a/lib/models/services/cluster-config-service.js b/lib/models/services/cluster-config-service.js index aee652080..a4f04a34d 100644 --- a/lib/models/services/cluster-config-service.js +++ b/lib/models/services/cluster-config-service.js @@ -742,7 +742,6 @@ module.exports = class ClusterConfigService { inputInstanceOpts.name ) } - const shouldNotAutofork = const defaultInstanceOpts = { // short name is service name shortName: serviceName, diff --git a/unit/models/services/cluster-config-service.js b/unit/models/services/cluster-config-service.js index 2abb1f0f2..0b613e0ac 100644 --- a/unit/models/services/cluster-config-service.js +++ b/unit/models/services/cluster-config-service.js @@ -1004,8 +1004,7 @@ describe('Cluster Config Service Unit Tests', function () { } const composeData = { metadata: { - name: 'a1', - isMain: true + name: 'a1' }, instance: testParentComposeData, build: { @@ -1056,8 +1055,7 @@ describe('Cluster Config Service Unit Tests', function () { } const composeData = { metadata: { - name: 'a1', - isMain: true + name: 'a2' }, instance: testParentComposeData, build: { @@ -1094,6 +1092,58 @@ describe('Cluster Config Service Unit Tests', function () { }) }) + it('should set shouldNotAutoFork to true by default for main instances', () => { + const testParentBuildId = objectId('407f191e810c19729de860ef') + const testParentComposeData = { + env: 'env', + aliases: { + 'dGhyZWUtY2hhbmdpbmctdGhlLWhvc3RuYW1l': { + 'instanceName': 'compose-test-5-1-rethinkdb4', + 'alias': 'three-changing-the-hostname' + } + }, + containerStartCommand: 'containerStartCommand', + name: 'name' + } + const composeData = { + metadata: { + name: 'a1' + }, + instance: testParentComposeData, + build: { + dockerFilePath: 'Nathan219/hello' + } + } + const testInstance = 'build' + buildOpts.shouldNotAutoFork = undefined + InstanceService.createInstance.resolves(testInstance) + + return ClusterConfigService._createInstance(testSessionUser, composeData, testParentBuildId, testingOpts, buildOpts) + .then(instance => { + sinon.assert.calledOnce(InstanceService.createInstance) + sinon.assert.calledWithExactly(InstanceService.createInstance, { + shortName: composeData.metadata.name, + build: testParentBuildId, + aliases: testParentComposeData.aliases, + env: testParentComposeData.env, + containerStartCommand: testParentComposeData.containerStartCommand, + name: buildOpts.masterShorthash + '--' + testParentComposeData.name, + isTesting, + isTestReporter, + clusterCreateId, + isolated: buildOpts.isolated, + isIsolationGroupMaster: false, + shouldNotAutofork: true, + masterPod: false, + ipWhitelist: { + enabled: false + } + }, testSessionUser) + + expect(instance).to.equal(testInstance) + }) + }) + it('should create non-test non-isolated instance', () => { testingOpts.isTesting = false delete buildOpts.isolated From 6b44958d6eadb2a6231118d7082e960c195a964d Mon Sep 17 00:00:00 2001 From: Nathan219 Date: Wed, 28 Jun 2017 11:24:32 -0700 Subject: [PATCH 08/18] Change the return value to include all of the random names --- lib/routes/docker-compose-cluster.js | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/lib/routes/docker-compose-cluster.js b/lib/routes/docker-compose-cluster.js index f5fe79ed0..b65179116 100644 --- a/lib/routes/docker-compose-cluster.js +++ b/lib/routes/docker-compose-cluster.js @@ -140,19 +140,28 @@ const multiClusterCreate = function (sessionUser, body) { // Now that we have the main instances, we need to create clusters for each one. return Promise.props({ builds: Promise.map(serviceKeys.builds, (buildKey) => { - body.name = generateHashClusterName() - return rabbitMQ.createCluster(makeCreateOptsFromBody(sessionUser, body, buildKey)) + const hash = generateHashClusterName() + body.name = hash + return Promise.try(() => { + return rabbitMQ.createCluster(makeCreateOptsFromBody(sessionUser, body, buildKey)) + }) + .returns({ service: buildKey, hash }) }), externals: Promise.map(serviceKeys.externals, (externalKey) => { - body.name = generateHashClusterName() - return rabbitMQ.createCluster(makeCreateOptsFromBody(sessionUser, body, externalKey)) + const hash = generateHashClusterName() + body.name = hash + return Promise.try(() => { + return rabbitMQ.createCluster(makeCreateOptsFromBody(sessionUser, body, externalKey)) + }) + .returns({ service: externalKey, hash }) }) }) }) - .then((results) => { + .tap((results) => { const buildKeys = results.builds const externals = results.externals - return buildKeys.length + externals.length + results.total = buildKeys.length + externals.length + log.info(`Created ${results.total} clusters`, { results }) }) } @@ -209,7 +218,7 @@ function multiCreateRoute (req, res, next) { return multiClusterCreate(sessionUser, body) .then((results) => { - const message = `${results} cluster.create jobs enqueued` + const message = `${results.total} cluster.create jobs enqueued` return { json: { message, created: results }, status: 202 } }) .asCallback(responseHandler.bind(null, res, next)) From e2dd1ebc2ad80c4e4ec84c804f777bb480f94e7f Mon Sep 17 00:00:00 2001 From: Nathan219 Date: Wed, 28 Jun 2017 11:27:53 -0700 Subject: [PATCH 09/18] FIx tests --- lib/routes/docker-compose-cluster.js | 4 ++-- unit/routes/docker-compose-cluster.js | 10 +++++++--- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/lib/routes/docker-compose-cluster.js b/lib/routes/docker-compose-cluster.js index b65179116..ad7f4dd28 100644 --- a/lib/routes/docker-compose-cluster.js +++ b/lib/routes/docker-compose-cluster.js @@ -145,7 +145,7 @@ const multiClusterCreate = function (sessionUser, body) { return Promise.try(() => { return rabbitMQ.createCluster(makeCreateOptsFromBody(sessionUser, body, buildKey)) }) - .returns({ service: buildKey, hash }) + .return({ service: buildKey, hash }) }), externals: Promise.map(serviceKeys.externals, (externalKey) => { const hash = generateHashClusterName() @@ -153,7 +153,7 @@ const multiClusterCreate = function (sessionUser, body) { return Promise.try(() => { return rabbitMQ.createCluster(makeCreateOptsFromBody(sessionUser, body, externalKey)) }) - .returns({ service: externalKey, hash }) + .return({ service: externalKey, hash }) }) }) }) diff --git a/unit/routes/docker-compose-cluster.js b/unit/routes/docker-compose-cluster.js index f6c565a7b..c2f7efdf5 100644 --- a/unit/routes/docker-compose-cluster.js +++ b/unit/routes/docker-compose-cluster.js @@ -237,9 +237,13 @@ describe('/docker-compose-cluster', function () { sinon.assert.calledOnce(resMock.status) sinon.assert.calledWith(resMock.status, 202) sinon.assert.calledOnce(resMock.json) - sinon.assert.calledWith(resMock.json, - { message: sinon.match.string, created: sinon.match.number } - ) + sinon.assert.calledWith(resMock.json, { + message: sinon.match.string, + created: sinon.match({ + builds: sinon.match.array, + externals: sinon.match.array + }) + }) }) }) }) From e609ef10a2bf1535979be8fd8bbf1da32a583fa0 Mon Sep 17 00:00:00 2001 From: Nathan219 Date: Wed, 28 Jun 2017 14:45:52 -0700 Subject: [PATCH 10/18] Only throw the error that the files are the same if the instance._id matches the one on the AIC --- lib/models/services/cluster-config-service.js | 12 ++++++-- .../models/services/cluster-config-service.js | 29 ++++++++++++++----- 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/lib/models/services/cluster-config-service.js b/lib/models/services/cluster-config-service.js index e1c702f0b..7b081f151 100644 --- a/lib/models/services/cluster-config-service.js +++ b/lib/models/services/cluster-config-service.js @@ -1568,8 +1568,12 @@ module.exports = class ClusterConfigService { instance, githubPushInfo }) log.info('called') - return ClusterConfigService.fetchConfigByInstanceId(instance._id) - .then(clusterConfig => { + return Promise.props({ + aic: AutoIsolationService.fetchAutoIsolationForInstance(instance._id), + clusterConfig: ClusterConfigService.fetchConfigByInstanceId(instance._id) + }) + .then(results => { + const clusterConfig = results.clusterConfig const currentPaths = clusterConfig.files.map(pluck('path')) // We found a cluster, so fetch the current one, and see if it changed return UserService.getByBpId(githubPushInfo.bpUserId) @@ -1590,7 +1594,9 @@ module.exports = class ClusterConfigService { currentPaths }, 'new compose files') const diffedShas = difference(newShas, currentShas) - if (diffedShas.length === 0) { + const aic = results.aic + // We want branches to act like they've changed, so they make their own ICC and AIC + if (diffedShas.length === 0 && instance._id === aic.instance) { throw new InputClusterConfig.NotChangedError({ newShas, currentShas diff --git a/unit/models/services/cluster-config-service.js b/unit/models/services/cluster-config-service.js index 608bb1d49..a4a2b42c2 100644 --- a/unit/models/services/cluster-config-service.js +++ b/unit/models/services/cluster-config-service.js @@ -1725,15 +1725,26 @@ describe('Cluster Config Service Unit Tests', function () { bpUserId, repo: repoFullName } + const aic = { + instance: instanceId + } + const parentAic = { + instance: 'asdasdasdassa' + } + const instance = { + _id: instanceId + } beforeEach(function (done) { sinon.stub(ClusterConfigService, 'fetchConfigByInstanceId').resolves(clusterConfig) + sinon.stub(AutoIsolationService, 'fetchAutoIsolationForInstance').resolves(aic) sinon.stub(UserService, 'getByBpId').resolves(userModel) sinon.stub(ClusterConfigService, 'fetchFilesFromGithub').resolves(changedClusterConfigFiles) done() }) afterEach(function (done) { ClusterConfigService.fetchConfigByInstanceId.restore() + AutoIsolationService.fetchAutoIsolationForInstance.restore() UserService.getByBpId.restore() ClusterConfigService.fetchFilesFromGithub.restore() done() @@ -1742,7 +1753,7 @@ describe('Cluster Config Service Unit Tests', function () { it('should return error if fetchConfigByInstanceId failed', function (done) { const error = new Error('Some error') ClusterConfigService.fetchConfigByInstanceId.rejects(error) - ClusterConfigService.checkIfComposeFilesChanged(instanceId, githubPushInfo) + ClusterConfigService.checkIfComposeFilesChanged(instance, githubPushInfo) .asCallback(function (err) { expect(err).to.exist() expect(err.message).to.equal(error.message) @@ -1752,7 +1763,7 @@ describe('Cluster Config Service Unit Tests', function () { it('should return error if UserService.getByBpId failed', function (done) { const error = new Error('Some error') UserService.getByBpId.rejects(error) - ClusterConfigService.checkIfComposeFilesChanged(instanceId, githubPushInfo) + ClusterConfigService.checkIfComposeFilesChanged(instance, githubPushInfo) .asCallback(function (err) { expect(err).to.exist() expect(err.message).to.equal(error.message) @@ -1762,7 +1773,7 @@ describe('Cluster Config Service Unit Tests', function () { it('should return error if ClusterConfigService.fetchFilesFromGithub failed', function (done) { const error = new Error('Some error') ClusterConfigService.fetchFilesFromGithub.rejects(error) - ClusterConfigService.checkIfComposeFilesChanged(instanceId, githubPushInfo) + ClusterConfigService.checkIfComposeFilesChanged(instance, githubPushInfo) .asCallback(function (err) { expect(err).to.exist() expect(err.message).to.equal(error.message) @@ -1771,14 +1782,13 @@ describe('Cluster Config Service Unit Tests', function () { }) }) describe('success', function () { - it('should run successfully', function (done) { + it('should run successfully', function () { ClusterConfigService.fetchFilesFromGithub.resolves(changedClusterConfigFiles) - ClusterConfigService.checkIfComposeFilesChanged(instanceId, githubPushInfo) - .asCallback(done) + return ClusterConfigService.checkIfComposeFilesChanged(instance, githubPushInfo) }) it('should return InputClusterConfig.NotChangedError if shas match', function (done) { ClusterConfigService.fetchFilesFromGithub.resolves(clusterConfigFiles) - ClusterConfigService.checkIfComposeFilesChanged(instanceId, githubPushInfo) + ClusterConfigService.checkIfComposeFilesChanged(instance, githubPushInfo) .then(function () { done(new Error('Expecting NotChangedError')) }) @@ -1786,6 +1796,11 @@ describe('Cluster Config Service Unit Tests', function () { done() }) }) + it('should not return InputClusterConfig.NotChangedError if aic is parent\'s', function () { + ClusterConfigService.fetchFilesFromGithub.resolves(changedClusterConfigFiles) + AutoIsolationService.fetchAutoIsolationForInstance.resolves(parentAic) + return ClusterConfigService.checkIfComposeFilesChanged(instance, githubPushInfo) + }) }) }) describe('_createAutoIsolationModelsFromClusterInstances', function () { From 4bf6b6a83af5b52a9b02fdd1b1823a48544cb081 Mon Sep 17 00:00:00 2001 From: Nathan219 Date: Wed, 28 Jun 2017 15:56:53 -0700 Subject: [PATCH 11/18] Add to the JSDoc --- lib/routes/docker-compose-cluster.js | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/routes/docker-compose-cluster.js b/lib/routes/docker-compose-cluster.js index ad7f4dd28..418c96ce9 100644 --- a/lib/routes/docker-compose-cluster.js +++ b/lib/routes/docker-compose-cluster.js @@ -119,7 +119,15 @@ const postRoute = function (req, res, next) { * * @param {SessionUser} sessionUser - this user * @param {Object} body - cluster create options - * @resolves {Number} The number of cluster create jobs created + * + * @resolves {Object} results + * {Object[]} results.builds - Clusters created from the owning repo + * {String} results.builds.service - Service name + * {String} results.builds.hash - Random Hash generated for this cluster + * {Object[]} results.externals - Clusters created from github links + * {String} results.externals.service - Service name + * {String} results.externals.hash - Random Hash generated for this cluster + * {Number} results.total - Total number of clusters to be generated */ const multiClusterCreate = function (sessionUser, body) { // First, we need to fetch the file(s), and get all of the mains From 11f31758604a3db62cbca0bf38cd6571ffbbb17d Mon Sep 17 00:00:00 2001 From: Henry Mollman Date: Wed, 28 Jun 2017 17:22:37 -0700 Subject: [PATCH 12/18] Updated logic for shouldNotAutoFork --- lib/models/services/cluster-config-service.js | 2 +- unit/models/services/cluster-config-service.js | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/models/services/cluster-config-service.js b/lib/models/services/cluster-config-service.js index 598061a32..9db85ad12 100644 --- a/lib/models/services/cluster-config-service.js +++ b/lib/models/services/cluster-config-service.js @@ -794,7 +794,7 @@ module.exports = class ClusterConfigService { inputInstanceOpts.name ) } - const shouldNotAutofork = keypather.get(parsedInstanceData, 'metadata.name') !== buildOpts.mainInstanceServiceName && !!buildOpts.shouldNotAutoFork + const shouldNotAutofork = !(keypather.get(parsedInstanceData, 'metadata.name') === buildOpts.mainInstanceServiceName && !buildOpts.shouldNotAutoFork) const defaultInstanceOpts = { // short name is service name shortName: serviceName, diff --git a/unit/models/services/cluster-config-service.js b/unit/models/services/cluster-config-service.js index 6da2fec87..7425f3d28 100644 --- a/unit/models/services/cluster-config-service.js +++ b/unit/models/services/cluster-config-service.js @@ -1027,7 +1027,7 @@ describe('Cluster Config Service Unit Tests', function () { isTestReporter, isolated: buildOpts.isolated, isIsolationGroupMaster: false, - shouldNotAutofork: false, + shouldNotAutofork: true, masterPod: false, ipWhitelist: { enabled: false @@ -1038,7 +1038,7 @@ describe('Cluster Config Service Unit Tests', function () { }) }) - it('should set shouldNotAutoFork to false by default', () => { + it('should set shouldNotAutoFork to true if it is not supplied', () => { const testParentBuildId = objectId('407f191e810c19729de860ef') const testParentComposeData = { env: 'env', @@ -1080,7 +1080,7 @@ describe('Cluster Config Service Unit Tests', function () { clusterCreateId, isolated: buildOpts.isolated, isIsolationGroupMaster: false, - shouldNotAutofork: false, + shouldNotAutofork: true, masterPod: false, ipWhitelist: { enabled: false @@ -1091,7 +1091,7 @@ describe('Cluster Config Service Unit Tests', function () { }) }) - it('should set shouldNotAutoFork to false by default for main instances', () => { + it('should set shouldNotAutoFork to false by default for non-main instances', () => { const testParentBuildId = objectId('407f191e810c19729de860ef') const testParentComposeData = { env: 'env', From ef52adb609e0ff717c3cc554016296328729d425 Mon Sep 17 00:00:00 2001 From: Henry Mollman Date: Thu, 29 Jun 2017 14:34:24 -0700 Subject: [PATCH 13/18] 12.10.3 --- npm-shrinkwrap.json | 2 +- package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json index b7e86149e..6053d4ef3 100644 --- a/npm-shrinkwrap.json +++ b/npm-shrinkwrap.json @@ -1,6 +1,6 @@ { "name": "api", - "version": "12.10.2", + "version": "12.10.3", "dependencies": { "101": { "version": "1.6.2", diff --git a/package.json b/package.json index aa7df39d4..5f55fe765 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "api", "description": "Runnable API Server", - "version": "12.10.2", + "version": "12.10.3", "repository": { "type": "git", "url": "http://github.com/CodeNow/api.git" From cca7df023396d117886a035e8ce4cd21a021f959 Mon Sep 17 00:00:00 2001 From: Nathan219 Date: Thu, 29 Jun 2017 15:00:01 -0700 Subject: [PATCH 14/18] Fix issue where created branches wouldn't update their compose --- lib/models/services/cluster-config-service.js | 5 +++-- lib/routes/instances/index.js | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/models/services/cluster-config-service.js b/lib/models/services/cluster-config-service.js index 7b081f151..6e94b837e 100644 --- a/lib/models/services/cluster-config-service.js +++ b/lib/models/services/cluster-config-service.js @@ -1620,8 +1620,9 @@ module.exports = class ClusterConfigService { /** * If you need to do an updateCluster job, use this method. It checks if the compose file has changed * before creating the job. - * @param {Instance} instance - instance to be updated - * @param {Object} githubPushInfo - parsed githook data + * @param {Instance} instance - instance to be updated + * @param {Object} githubPushInfo - parsed githook data + * @param {String} githubPushInfo.bpUserId - BigPoppa user id * * @resolves {undefined} * diff --git a/lib/routes/instances/index.js b/lib/routes/instances/index.js index d3875ec3c..6887c002d 100644 --- a/lib/routes/instances/index.js +++ b/lib/routes/instances/index.js @@ -504,7 +504,8 @@ module.exports.forkInstance = function (req) { commit: req.body.sha, user: { id: req.sessionUser.accounts.github.id - } + }, + bpUserId: keypather.get(req.sessionUser, 'bigPoppaUser.id') } log.info({ githubPushInfo: githubPushInfo From 63fc5bfa92bacf887153b1e9f4f33bbc7298d56f Mon Sep 17 00:00:00 2001 From: Nathan219 Date: Thu, 29 Jun 2017 15:02:24 -0700 Subject: [PATCH 15/18] toString() --- lib/models/services/cluster-config-service.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/models/services/cluster-config-service.js b/lib/models/services/cluster-config-service.js index 6e94b837e..38fc5f52b 100644 --- a/lib/models/services/cluster-config-service.js +++ b/lib/models/services/cluster-config-service.js @@ -1596,7 +1596,7 @@ module.exports = class ClusterConfigService { const diffedShas = difference(newShas, currentShas) const aic = results.aic // We want branches to act like they've changed, so they make their own ICC and AIC - if (diffedShas.length === 0 && instance._id === aic.instance) { + if (diffedShas.length === 0 && instance._id.toString() === aic.instance.toString()) { throw new InputClusterConfig.NotChangedError({ newShas, currentShas From 6b01e1f62a0d038c327baa012ff2e3d6b500d519 Mon Sep 17 00:00:00 2001 From: Nathan219 Date: Thu, 29 Jun 2017 16:19:14 -0700 Subject: [PATCH 16/18] Fix tests --- unit/routes/instances.js | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/unit/routes/instances.js b/unit/routes/instances.js index 4130f77aa..ae8d0b963 100644 --- a/unit/routes/instances.js +++ b/unit/routes/instances.js @@ -39,6 +39,9 @@ describe('/instances', () => { github: { id: githubId } + }, + bigPoppaUser: { + id: 1 } } } @@ -79,7 +82,8 @@ describe('/instances', () => { commit: 'sha12', user: { id: '1234' - } + }, + bpUserId: mockReq.sessionUser.bigPoppaUser.id }, 'manual' ) @@ -111,7 +115,8 @@ describe('/instances', () => { commit: 'sha12', user: { id: '1234' - } + }, + bpUserId: mockReq.sessionUser.bigPoppaUser.id } ) }) @@ -129,7 +134,8 @@ describe('/instances', () => { commit: 'sha12', user: { id: '1234' - } + }, + bpUserId: mockReq.sessionUser.bigPoppaUser.id } ) }) From e825b4794a98fca57452e02b8533d9851d0ee4cf Mon Sep 17 00:00:00 2001 From: Nathan219 Date: Thu, 29 Jun 2017 16:24:51 -0700 Subject: [PATCH 17/18] 12.10.4 --- npm-shrinkwrap.json | 2 +- package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json index 6053d4ef3..115f17917 100644 --- a/npm-shrinkwrap.json +++ b/npm-shrinkwrap.json @@ -1,6 +1,6 @@ { "name": "api", - "version": "12.10.3", + "version": "12.10.4", "dependencies": { "101": { "version": "1.6.2", diff --git a/package.json b/package.json index 5f55fe765..c68e6bdbb 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "api", "description": "Runnable API Server", - "version": "12.10.3", + "version": "12.10.4", "repository": { "type": "git", "url": "http://github.com/CodeNow/api.git" From 58b97001ee96a271bd4f5ef10127b27cc492bb0e Mon Sep 17 00:00:00 2001 From: Nathan219 Date: Thu, 29 Jun 2017 17:38:00 -0700 Subject: [PATCH 18/18] 12.10.5 --- npm-shrinkwrap.json | 2 +- package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json index 115f17917..664f8f1e7 100644 --- a/npm-shrinkwrap.json +++ b/npm-shrinkwrap.json @@ -1,6 +1,6 @@ { "name": "api", - "version": "12.10.4", + "version": "12.10.5", "dependencies": { "101": { "version": "1.6.2", diff --git a/package.json b/package.json index c68e6bdbb..f6639958a 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "api", "description": "Runnable API Server", - "version": "12.10.4", + "version": "12.10.5", "repository": { "type": "git", "url": "http://github.com/CodeNow/api.git"