fix(1839): Store exact build start time in redis hash (#79)

* fix(1839): Add enqueueTime to buildConfig * fix(1839): Add test * Update versions * Add timeoutConfigs queue and funcs * added more jsdoc * feat(1839): fix tests and remove console log
screwdriver-cd · Jan 16, 2020 · f8799e7 · f8799e7
1 parent 2b093bb
commit f8799e7
Show file tree

Hide file tree

Showing 2 changed files with 250 additions and 7 deletions.
diff --git a/index.js b/index.js
@@ -14,6 +14,7 @@ const FuseBox = fuses.box;
 const EXPIRE_TIME = 1800; // 30 mins
 const RETRY_LIMIT = 3;
 const RETRY_DELAY = 5;
+const DEFAULT_BUILD_TIMEOUT = 90;
 
 class ExecutorQueue extends Executor {
     /**
@@ -47,6 +48,7 @@ class ExecutorQueue extends Executor {
         this.tokenGen = null;
         this.userTokenGen = null;
         this.pipelineFactory = config.pipelineFactory;
+        this.timeoutQueue = `${this.prefix}timeoutConfigs`;
 
         const redisConnection = Object.assign({}, config.redisConnection, { pkg: 'ioredis' });
 
@@ -417,6 +419,78 @@ class ExecutorQueue extends Executor {
         return this.redisBreaker.runCommand('hdel', this.frozenBuildTable, config.jobId);
     }
 
+    /**
+     * Adds start time of a build to timeout queue
+     * @method status
+     * @param  {Object} config               Configuration
+     * @param  {String} config.buildId       Unique ID for a build
+     * @param  {String} config.startTime     Start time fo build
+     * @param  {String} config.buildStatus     Status of build
+     * @return {Promise}
+     */
+    async _startTimer(config) {
+        try {
+            await this.connect();
+            const {
+                buildId,
+                jobId,
+                buildStatus,
+                startTime
+            } = config;
+
+            if (buildStatus === 'RUNNING') {
+                const buildTimeout = hoek.reach(config, 'annotations>screwdriver.cd/timeout',
+                    { separator: '>' });
+                const timeout = parseInt(buildTimeout || DEFAULT_BUILD_TIMEOUT, 10);
+
+                const data = await this.redisBreaker.runCommand('hget', this.timeoutQueue, buildId);
+
+                if (data) {
+                    return Promise.resolve();
+                }
+
+                return await this.redisBreaker.runCommand('hset', this.timeoutQueue, buildId,
+                    JSON.stringify({
+                        jobId,
+                        startTime,
+                        timeout
+                    }));
+            }
+
+            return Promise.resolve();
+        } catch (err) {
+            logger.error(`Error occurred while saving to timeout queue ${err}`);
+
+            return Promise.resolve();
+        }
+    }
+
+    /**
+     * Removes start time info key from timeout queue
+     * @method status
+     * @param  {Object} config               Configuration
+     * @param  {String} config.buildId       Unique ID for a build
+     * @return {Promise}
+     */
+    async _stopTimer(config) {
+        try {
+            await this.connect();
+
+            const data = await this.redisBreaker.runCommand('hget', this.timeoutQueue,
+                config.buildId);
+
+            if (!data) {
+                return Promise.resolve();
+            }
+
+            return await this.redisBreaker.runCommand('hdel', this.timeoutQueue, config.buildId);
+        } catch (err) {
+            logger.error(`Error occurred while removing from timeout queue ${err}`);
+
+            return Promise.resolve();
+        }
+    }
+
     /**
      * Starts a new build in an executor
      * @async  _start

diff --git a/test/index.test.js b/test/index.test.js
@@ -96,7 +96,8 @@ describe('index test', () => {
             hdel: sinon.stub().yieldsAsync(),
             hset: sinon.stub().yieldsAsync(),
             set: sinon.stub().yieldsAsync(),
-            expire: sinon.stub().yieldsAsync()
+            expire: sinon.stub().yieldsAsync(),
+            hget: sinon.stub().yieldsAsync()
         };
         redisConstructorMock = sinon.stub().returns(redisMock);
         cronMock = {
@@ -462,12 +463,14 @@ describe('index test', () => {
             });
         });
 
-        it('enqueues a build and caches the config', () => executor.start(testConfig).then(() => {
-            assert.calledTwice(queueMock.connect);
-            assert.calledWith(redisMock.hset, 'buildConfigs', buildId,
-                JSON.stringify(testConfig));
-            assert.calledWith(queueMock.enqueue, 'builds', 'start', [partialTestConfigToString]);
-        }));
+        it('enqueues a build and caches the config', () =>
+            executor.start(testConfig).then(() => {
+                assert.calledTwice(queueMock.connect);
+                assert.calledWith(redisMock.hset, 'buildConfigs', buildId,
+                    JSON.stringify(testConfig));
+                assert.calledWith(queueMock.enqueue, 'builds', 'start',
+                    [partialTestConfigToString]);
+            }));
 
         it('doesn\'t call connect if there\'s already a connection', () => {
             queueMock.connection.connected = true;
@@ -642,4 +645,170 @@ describe('index test', () => {
             });
         });
     });
+
+    describe('_stopTimer', () => {
+        it('does not reject if it can\'t establish a connection', async () => {
+            queueMock.connect.rejects(new Error('couldn\'t connect'));
+            try {
+                await executor.stopTimer({});
+            } catch (err) {
+                assert.fail('Should not get here');
+            }
+        });
+
+        it('removes a key from redis for the specified buildId if it exists', async () => {
+            const dateNow = Date.now();
+            const isoTime = (new Date(dateNow)).toISOString();
+            const sandbox = sinon.sandbox.create({
+                useFakeTimers: false
+            });
+
+            const timerConfig = {
+                buildId,
+                jobId,
+                startTime: isoTime
+            };
+
+            sandbox.useFakeTimers(dateNow);
+            redisMock.hget.withArgs('timeoutConfigs', buildId).yieldsAsync(null, {
+                buildId,
+                jobId,
+                startTime: isoTime
+            });
+
+            await executor.stopTimer(timerConfig);
+
+            assert.calledOnce(queueMock.connect);
+            assert.calledWith(redisMock.hdel, 'timeoutConfigs', buildId);
+            sandbox.restore();
+        });
+
+        it('hdel is not called if buildId does not exist in cache', async () => {
+            redisMock.hget.withArgs('timeoutConfigs', buildId)
+                .yieldsAsync(null, null);
+
+            await executor.stopTimer(testConfig);
+            assert.calledOnce(queueMock.connect);
+            assert.notCalled(redisMock.hdel);
+        });
+    });
+
+    describe('_startTimer', () => {
+        it('does not reject if it can\'t establish a connection', async () => {
+            queueMock.connect.rejects(new Error('couldn\'t connect'));
+            try {
+                await executor.startTimer({});
+            } catch (err) {
+                assert.fail('Should not get here');
+            }
+        });
+
+        it('adds a timeout key if status is RUNNING and caches the config', async () => {
+            const dateNow = Date.now();
+            const isoTime = (new Date(dateNow)).toISOString();
+            const sandbox = sinon.sandbox.create({
+                useFakeTimers: false
+            });
+
+            const timerConfig = {
+                buildId,
+                jobId,
+                buildStatus: 'RUNNING',
+                startTime: isoTime
+            };
+
+            sandbox.useFakeTimers(dateNow);
+            redisMock.hget.yieldsAsync(null, null);
+            await executor.startTimer(timerConfig);
+            assert.calledOnce(queueMock.connect);
+            assert.calledWith(redisMock.hset, 'timeoutConfigs', buildId,
+                JSON.stringify({
+                    jobId,
+                    startTime: isoTime,
+                    timeout: 90
+                }));
+            sandbox.restore();
+        });
+
+        it('does not add a timeout key if status is !RUNNING', async () => {
+            const dateNow = Date.now();
+            const isoTime = (new Date(dateNow)).toISOString();
+            const sandbox = sinon.sandbox.create({
+                useFakeTimers: false
+            });
+
+            const timerConfig = {
+                buildId,
+                jobId,
+                buildStatus: 'QUEUED',
+                startTime: isoTime
+            };
+
+            sandbox.useFakeTimers(dateNow);
+            redisMock.hget.yieldsAsync(null, null);
+
+            await executor.startTimer(timerConfig);
+            assert.calledOnce(queueMock.connect);
+            assert.notCalled(redisMock.hset);
+            sandbox.restore();
+        });
+
+        it('does not add a timeout key if buildId already exists', async () => {
+            const dateNow = Date.now();
+            const isoTime = (new Date(dateNow)).toISOString();
+            const sandbox = sinon.sandbox.create({
+                useFakeTimers: false
+            });
+
+            const timerConfig = {
+                buildId,
+                jobId,
+                buildStatus: 'QUEUED',
+                startTime: isoTime
+            };
+
+            sandbox.useFakeTimers(dateNow);
+            redisMock.hget.withArgs('timeoutConfigs', buildId).yieldsAsync({
+                jobId,
+                startTime: isoTime,
+                timeout: 90
+            });
+
+            await executor.startTimer(timerConfig);
+            assert.calledOnce(queueMock.connect);
+            assert.notCalled(redisMock.hset);
+            sandbox.restore();
+        });
+
+        it('adds a timeout config with specific timeout when annotations present',
+            async () => {
+                const dateNow = Date.now();
+                const isoTime = (new Date(dateNow)).toISOString();
+                const sandbox = sinon.sandbox.create({
+                    useFakeTimers: false
+                });
+
+                const timerConfig = {
+                    buildId,
+                    jobId,
+                    buildStatus: 'RUNNING',
+                    startTime: isoTime,
+                    annotations: {
+                        'screwdriver.cd/timeout': 5
+                    }
+                };
+
+                sandbox.useFakeTimers(dateNow);
+                redisMock.hget.yieldsAsync(null, null);
+                await executor.startTimer(timerConfig);
+                assert.calledOnce(queueMock.connect);
+                assert.calledWith(redisMock.hset, 'timeoutConfigs', buildId,
+                    JSON.stringify({
+                        jobId,
+                        startTime: isoTime,
+                        timeout: 5
+                    }));
+                sandbox.restore();
+            });
+    });
 });