Merge pull request #2036 from CodeNow/SAN-6464-make-all-iccs-unique

Ensure all ICCs and AICs are unique

lib/models/services/cluster-config-service.js

@@ -1569,8 +1569,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)
@@ -1591,7 +1595,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.toString() === aic.instance.toString()) {
               throw new InputClusterConfig.NotChangedError({
                 newShas,
                 currentShas
@@ -1615,8 +1621,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}
    *

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

unit/models/services/cluster-config-service.js

@@ -1831,15 +1831,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()
@@ -1848,7 +1859,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)
@@ -1858,7 +1869,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)
@@ -1868,7 +1879,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)
@@ -1877,21 +1888,25 @@ 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'))
           })
           .catch(InputClusterConfig.NotChangedError, 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 () {

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
             }
           )
         })