From fa37ed865ae98c36e483c18c730c3b25291164bc Mon Sep 17 00:00:00 2001 From: mserranom Date: Thu, 20 Feb 2020 17:24:28 +0100 Subject: [PATCH] added container monitor cleanup to fix hanging tests --- services/clsi/app/js/DockerRunner.js | 31 ++++++++++++++----- services/clsi/npm-shrinkwrap.json | 6 ---- services/clsi/package.json | 1 - .../test/unit/js/DockerLockManagerTests.js | 1 - .../clsi/test/unit/js/DockerRunnerTests.js | 5 ++- 5 files changed, 27 insertions(+), 17 deletions(-) diff --git a/services/clsi/app/js/DockerRunner.js b/services/clsi/app/js/DockerRunner.js index 5ac234b7e0..393ce3df37 100644 --- a/services/clsi/app/js/DockerRunner.js +++ b/services/clsi/app/js/DockerRunner.js @@ -35,6 +35,9 @@ const usingSiblingContainers = () => x => x.sandboxedCompilesHostDir ) != null +let containerMonitorTimeout +let containerMonitorInterval + module.exports = DockerRunner = { ERR_NOT_DIRECTORY: new Error('not a directory'), ERR_TERMINATED: new Error('terminated'), @@ -646,17 +649,29 @@ module.exports = DockerRunner = { { maxAge: DockerRunner.MAX_CONTAINER_AGE }, 'starting container expiry' ) + + // guarantee only one monitor is running + DockerRunner.stopContainerMonitor() + // randomise the start time const randomDelay = Math.floor(Math.random() * 5 * 60 * 1000) - return setTimeout( - () => - setInterval( - () => DockerRunner.destroyOldContainers(), - (oneHour = 60 * 60 * 1000) - ), + containerMonitorTimeout = setTimeout(() => { + containerMonitorInterval = setInterval( + () => DockerRunner.destroyOldContainers(), + (oneHour = 60 * 60 * 1000) + ) + }, randomDelay) + }, - randomDelay - ) + stopContainerMonitor() { + if (containerMonitorTimeout) { + clearTimeout(containerMonitorTimeout) + containerMonitorTimeout = undefined + } + if (containerMonitorInterval) { + clearInterval(containerMonitorTimeout) + containerMonitorTimeout = undefined + } } } diff --git a/services/clsi/npm-shrinkwrap.json b/services/clsi/npm-shrinkwrap.json index 40fed3c24f..05e00cd79a 100644 --- a/services/clsi/npm-shrinkwrap.json +++ b/services/clsi/npm-shrinkwrap.json @@ -1536,12 +1536,6 @@ "resolved": "https://registry.npmjs.org/coffee-script/-/coffee-script-1.6.0.tgz", "integrity": "sha1-gIs5bhEPU9AhoZpO8fZb4OjjX6M=" }, - "coffeescript": { - "version": "1.6.0", - "resolved": "https://registry.npmjs.org/coffeescript/-/coffeescript-1.6.0.tgz", - "integrity": "sha1-bdTeHrYveE2MjYCWdVLLpUf/2d4=", - "dev": true - }, "color-convert": { "version": "1.9.3", "resolved": "https://registry.npmjs.org/color-convert/-/color-convert-1.9.3.tgz", diff --git a/services/clsi/package.json b/services/clsi/package.json index c38621aa23..87403ca5bd 100644 --- a/services/clsi/package.json +++ b/services/clsi/package.json @@ -44,7 +44,6 @@ "babel-eslint": "^10.0.3", "bunyan": "^0.22.1", "chai": "~1.8.1", - "coffeescript": "1.6.0", "eslint": "^6.6.0", "eslint-config-prettier": "^6.10.0", "eslint-config-standard": "^14.1.0", diff --git a/services/clsi/test/unit/js/DockerLockManagerTests.js b/services/clsi/test/unit/js/DockerLockManagerTests.js index 177d7a21f4..bc13c5ade5 100644 --- a/services/clsi/test/unit/js/DockerLockManagerTests.js +++ b/services/clsi/test/unit/js/DockerLockManagerTests.js @@ -12,7 +12,6 @@ const SandboxedModule = require('sandboxed-module') const sinon = require('sinon') require('chai').should() -require('coffee-script') const modulePath = require('path').join( __dirname, '../../../app/js/DockerLockManager' diff --git a/services/clsi/test/unit/js/DockerRunnerTests.js b/services/clsi/test/unit/js/DockerRunnerTests.js index 597c5d309d..d17d906dc7 100644 --- a/services/clsi/test/unit/js/DockerRunnerTests.js +++ b/services/clsi/test/unit/js/DockerRunnerTests.js @@ -17,7 +17,6 @@ const SandboxedModule = require('sandboxed-module') const sinon = require('sinon') require('chai').should() const { expect } = require('chai') -require('coffee-script') const modulePath = require('path').join( __dirname, '../../../app/js/DockerRunner' @@ -89,6 +88,10 @@ describe('DockerRunner', function() { return (this.Settings.clsi.docker.env = { PATH: 'mock-path' }) }) + afterEach(function() { + this.DockerRunner.stopContainerMonitor() + }) + describe('run', function() { beforeEach(function(done) { this.DockerRunner._getContainerOptions = sinon