From 08be54a43e4cf9c5f999aa5fdc3f2bf3d5f0d431 Mon Sep 17 00:00:00 2001 From: Eric Mc Sween Date: Wed, 2 Sep 2020 16:45:49 -0400 Subject: [PATCH 01/16] Decaf cleanup: remove unnecessary Array.from() --- services/clsi/app/js/DockerRunner.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/services/clsi/app/js/DockerRunner.js b/services/clsi/app/js/DockerRunner.js index 723453922f..f719ebd50e 100644 --- a/services/clsi/app/js/DockerRunner.js +++ b/services/clsi/app/js/DockerRunner.js @@ -8,7 +8,6 @@ // Fix any style issues and re-enable lint. /* * decaffeinate suggestions: - * DS101: Remove unnecessary use of Array.from * DS102: Remove unnecessary code created because of implicit returns * DS103: Rewrite code to no longer use __guard__ * DS205: Consider reworking code to avoid use of IIFEs @@ -77,7 +76,7 @@ module.exports = DockerRunner = { const volumes = {} volumes[directory] = '/compile' - command = Array.from(command).map((arg) => + command = command.map((arg) => __guardMethod__(arg.toString(), 'replace', (o) => o.replace('$COMPILE_DIR', '/compile') ) @@ -177,7 +176,7 @@ module.exports = DockerRunner = { _callback = function (error, output) {} } const callback = function (...args) { - _callback(...Array.from(args || [])) + _callback(...args) // Only call the callback once return (_callback = function () {}) } @@ -538,7 +537,7 @@ module.exports = DockerRunner = { _callback = function (error, exitCode) {} } const callback = function (...args) { - _callback(...Array.from(args || [])) + _callback(...args) // Only call the callback once return (_callback = function () {}) } @@ -668,7 +667,7 @@ module.exports = DockerRunner = { return callback(error) } const jobs = [] - for (const container of Array.from(containers || [])) { + for (const container of containers) { ;((container) => DockerRunner.examineOldContainer(container, function ( err, From 99648341e236af78a18838abf1328eaf12fd4f29 Mon Sep 17 00:00:00 2001 From: Eric Mc Sween Date: Wed, 2 Sep 2020 16:58:41 -0400 Subject: [PATCH 02/16] Decaf cleanup: remove unnecessary returns --- services/clsi/app/js/DockerRunner.js | 89 +++++++++++++--------------- 1 file changed, 42 insertions(+), 47 deletions(-) diff --git a/services/clsi/app/js/DockerRunner.js b/services/clsi/app/js/DockerRunner.js index f719ebd50e..8ea3c62ccd 100644 --- a/services/clsi/app/js/DockerRunner.js +++ b/services/clsi/app/js/DockerRunner.js @@ -8,7 +8,6 @@ // Fix any style issues and re-enable lint. /* * decaffeinate suggestions: - * DS102: Remove unnecessary code created because of implicit returns * DS103: Rewrite code to no longer use __guard__ * DS205: Consider reworking code to avoid use of IIFEs * DS207: Consider shorter variations of null checks @@ -120,13 +119,11 @@ module.exports = DockerRunner = { { err: error, project_id }, 'error running container so destroying and retrying' ) - return DockerRunner.destroyContainer(name, null, true, function ( - error - ) { + DockerRunner.destroyContainer(name, null, true, function (error) { if (error != null) { return callback(error) } - return DockerRunner._runAndWaitForContainer( + DockerRunner._runAndWaitForContainer( options, volumes, timeout, @@ -134,12 +131,13 @@ module.exports = DockerRunner = { ) }) } else { - return callback(error, output) + callback(error, output) } }) + // pass back the container name to allow it to be killed return name - }, // pass back the container name to allow it to be killed + }, kill(container_id, callback) { if (callback == null) { @@ -147,7 +145,7 @@ module.exports = DockerRunner = { } logger.log({ container_id }, 'sending kill signal to container') const container = dockerode.getContainer(container_id) - return container.kill(function (error) { + container.kill(function (error) { if ( error != null && __guardMethod__( @@ -164,9 +162,9 @@ module.exports = DockerRunner = { } if (error != null) { logger.error({ err: error, container_id }, 'error killing container') - return callback(error) + callback(error) } else { - return callback() + callback() } }) }, @@ -178,7 +176,7 @@ module.exports = DockerRunner = { const callback = function (...args) { _callback(...args) // Only call the callback once - return (_callback = function () {}) + _callback = function () {} } const { name } = options @@ -189,7 +187,7 @@ module.exports = DockerRunner = { const callbackIfFinished = function () { if (streamEnded && containerReturned) { - return callback(null, output) + callback(null, output) } } @@ -199,10 +197,10 @@ module.exports = DockerRunner = { } output = _output streamEnded = true - return callbackIfFinished() + callbackIfFinished() } - return DockerRunner.startContainer( + DockerRunner.startContainer( options, volumes, attachStreamHandler, @@ -211,7 +209,7 @@ module.exports = DockerRunner = { return callback(error) } - return DockerRunner.waitForContainer(name, timeout, function ( + DockerRunner.waitForContainer(name, timeout, function ( error, exitCode ) { @@ -237,7 +235,7 @@ module.exports = DockerRunner = { (x) => (x.SecurityOpt = null) ) // small log line logger.log({ err, exitCode, options }, 'docker container has exited') - return callbackIfFinished() + callbackIfFinished() }) } ) @@ -364,7 +362,7 @@ module.exports = DockerRunner = { }, startContainer(options, volumes, attachStreamHandler, callback) { - return LockManager.runWithLock( + LockManager.runWithLock( options.name, (releaseLock) => // Check that volumes exist before starting the container. @@ -375,7 +373,7 @@ module.exports = DockerRunner = { if (err != null) { return releaseLock(err) } - return DockerRunner._startContainer( + DockerRunner._startContainer( options, volumes, attachStreamHandler, @@ -405,13 +403,13 @@ module.exports = DockerRunner = { if (!(stats != null ? stats.isDirectory() : undefined)) { return cb(DockerRunner.ERR_NOT_DIRECTORY) } - return cb() + cb() }) const jobs = [] for (const vol in volumes) { ;((vol) => jobs.push((cb) => checkVolume(vol, cb)))(vol) } - return async.series(jobs, callback) + async.series(jobs, callback) }, _startContainer(options, volumes, attachStreamHandler, callback) { @@ -429,7 +427,7 @@ module.exports = DockerRunner = { if (error != null) { return callback(error) } - return startExistingContainer() + startExistingContainer() }) var startExistingContainer = () => DockerRunner.attachToContainer( @@ -439,37 +437,37 @@ module.exports = DockerRunner = { if (error != null) { return callback(error) } - return container.start(function (error) { + container.start(function (error) { if ( error != null && (error != null ? error.statusCode : undefined) !== 304 ) { // already running - return callback(error) + callback(error) } else { - return callback() + callback() } }) } ) - return container.inspect(function (error, stats) { + container.inspect(function (error, stats) { if ((error != null ? error.statusCode : undefined) === 404) { - return createAndStartContainer() + createAndStartContainer() } else if (error != null) { logger.err( { container_name: name, error }, 'unable to inspect container to start' ) - return callback(error) + callback(error) } else { - return startExistingContainer() + startExistingContainer() } }) }, attachToContainer(containerId, attachStreamHandler, attachStartCallback) { const container = dockerode.getContainer(containerId) - return container.attach({ stdout: 1, stderr: 1, stream: 1 }, function ( + container.attach({ stdout: 1, stderr: 1, stream: 1 }, function ( error, stream ) { @@ -495,7 +493,7 @@ module.exports = DockerRunner = { return } if (this.data.length < MAX_OUTPUT) { - return (this.data += data) + this.data += data } else { logger.error( { @@ -506,7 +504,7 @@ module.exports = DockerRunner = { `${name} exceeds max size` ) this.data += `(...truncated at ${MAX_OUTPUT} chars...)` - return (this.overflowed = true) + this.overflowed = true } } // kill container if too much output @@ -526,7 +524,7 @@ module.exports = DockerRunner = { ) ) - return stream.on('end', () => + stream.on('end', () => attachStreamHandler(null, { stdout: stdout.data, stderr: stderr.data }) ) }) @@ -539,7 +537,7 @@ module.exports = DockerRunner = { const callback = function (...args) { _callback(...args) // Only call the callback once - return (_callback = function () {}) + _callback = function () {} } const container = dockerode.getContainer(containerId) @@ -551,11 +549,11 @@ module.exports = DockerRunner = { { container_id: containerId }, 'timeout reached, killing container' ) - return container.kill(function () {}) + container.kill(function () {}) }, timeout) logger.log({ container_id: containerId }, 'waiting for docker container') - return container.wait(function (error, res) { + container.wait(function (error, res) { if (error != null) { clearTimeout(timeoutId) logger.error( @@ -568,14 +566,14 @@ module.exports = DockerRunner = { logger.log({ containerId }, 'docker container timed out') error = DockerRunner.ERR_TIMED_OUT error.timedout = true - return callback(error) + callback(error) } else { clearTimeout(timeoutId) logger.log( { container_id: containerId, exitCode: res.StatusCode }, 'docker container returned' ) - return callback(null, res.StatusCode) + callback(null, res.StatusCode) } }) }, @@ -590,7 +588,7 @@ module.exports = DockerRunner = { if (callback == null) { callback = function (error) {} } - return LockManager.runWithLock( + LockManager.runWithLock( containerName, (releaseLock) => DockerRunner._destroyContainer( @@ -608,7 +606,7 @@ module.exports = DockerRunner = { } logger.log({ container_id: containerId }, 'destroying docker container') const container = dockerode.getContainer(containerId) - return container.remove({ force: shouldForce === true }, function (error) { + container.remove({ force: shouldForce === true }, function (error) { if ( error != null && (error != null ? error.statusCode : undefined) === 404 @@ -627,7 +625,7 @@ module.exports = DockerRunner = { } else { logger.log({ container_id: containerId }, 'destroyed container') } - return callback(error) + callback(error) }) }, @@ -652,17 +650,14 @@ module.exports = DockerRunner = { { containerName: name, created, now, age, maxAge, ttl }, 'checking whether to destroy container' ) - return callback(null, name, container.Id, ttl) + callback(null, name, container.Id, ttl) }, destroyOldContainers(callback) { if (callback == null) { callback = function (error) {} } - return dockerode.listContainers({ all: true }, function ( - error, - containers - ) { + dockerode.listContainers({ all: true }, function (error, containers) { if (error != null) { return callback(error) } @@ -679,7 +674,7 @@ module.exports = DockerRunner = { // strip the / prefix // the LockManager uses the plain container name name = name.slice(1) - return jobs.push((cb) => + jobs.push((cb) => DockerRunner.destroyContainer(name, id, false, () => cb()) ) } @@ -687,7 +682,7 @@ module.exports = DockerRunner = { } // Ignore errors because some containers get stuck but // will be destroyed next time - return async.series(jobs, callback) + async.series(jobs, callback) }) }, From f650da867524708be2abb761ba041e7b61903302 Mon Sep 17 00:00:00 2001 From: Eric Mc Sween Date: Wed, 2 Sep 2020 17:06:35 -0400 Subject: [PATCH 03/16] Decaf cleanup: remove __guard__ --- services/clsi/app/js/DockerRunner.js | 43 ++++++---------------------- 1 file changed, 9 insertions(+), 34 deletions(-) diff --git a/services/clsi/app/js/DockerRunner.js b/services/clsi/app/js/DockerRunner.js index 8ea3c62ccd..4c7ae2a59d 100644 --- a/services/clsi/app/js/DockerRunner.js +++ b/services/clsi/app/js/DockerRunner.js @@ -8,7 +8,6 @@ // Fix any style issues and re-enable lint. /* * decaffeinate suggestions: - * DS103: Rewrite code to no longer use __guard__ * DS205: Consider reworking code to avoid use of IIFEs * DS207: Consider shorter variations of null checks * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md @@ -28,10 +27,9 @@ const _ = require('lodash') logger.info('using docker runner') const usingSiblingContainers = () => - __guard__( - Settings != null ? Settings.path : undefined, - (x) => x.sandboxedCompilesHostDir - ) != null + Settings != null && + Settings.path != null && + Settings.path.sandboxedCompilesHostDir != null let containerMonitorTimeout let containerMonitorInterval @@ -76,9 +74,7 @@ module.exports = DockerRunner = { volumes[directory] = '/compile' command = command.map((arg) => - __guardMethod__(arg.toString(), 'replace', (o) => - o.replace('$COMPILE_DIR', '/compile') - ) + arg.toString().replace('$COMPILE_DIR', '/compile') ) if (image == null) { ;({ image } = Settings.clsi.docker) @@ -148,11 +144,8 @@ module.exports = DockerRunner = { container.kill(function (error) { if ( error != null && - __guardMethod__( - error != null ? error.message : undefined, - 'match', - (o) => o.match(/Cannot kill container .* is not running/) - ) + error.message != null && + error.message.match(/Cannot kill container .* is not running/) ) { logger.warn( { err: error, container_id }, @@ -230,10 +223,9 @@ module.exports = DockerRunner = { return callback(err) } containerReturned = true - __guard__( - options != null ? options.HostConfig : undefined, - (x) => (x.SecurityOpt = null) - ) // small log line + if (options != null && options.HostConfig != null) { + options.HostConfig.SecurityOpt = null + } logger.log({ err, exitCode, options }, 'docker container has exited') callbackIfFinished() }) @@ -718,20 +710,3 @@ module.exports = DockerRunner = { } DockerRunner.startContainerMonitor() - -function __guard__(value, transform) { - return typeof value !== 'undefined' && value !== null - ? transform(value) - : undefined -} -function __guardMethod__(obj, methodName, transform) { - if ( - typeof obj !== 'undefined' && - obj !== null && - typeof obj[methodName] === 'function' - ) { - return transform(obj, methodName) - } else { - return undefined - } -} From 32f0bbe266b2ae9e85118b42511ab9a9169dd124 Mon Sep 17 00:00:00 2001 From: Eric Mc Sween Date: Wed, 2 Sep 2020 17:16:24 -0400 Subject: [PATCH 04/16] Decaf cleanup: remove IIFEs --- services/clsi/app/js/DockerRunner.js | 54 +++++++++++----------------- 1 file changed, 20 insertions(+), 34 deletions(-) diff --git a/services/clsi/app/js/DockerRunner.js b/services/clsi/app/js/DockerRunner.js index 4c7ae2a59d..f2ffc58a9d 100644 --- a/services/clsi/app/js/DockerRunner.js +++ b/services/clsi/app/js/DockerRunner.js @@ -8,7 +8,6 @@ // Fix any style issues and re-enable lint. /* * decaffeinate suggestions: - * DS205: Consider reworking code to avoid use of IIFEs * DS207: Consider shorter variations of null checks * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md */ @@ -278,23 +277,11 @@ module.exports = DockerRunner = { NetworkDisabled: true, Memory: 1024 * 1024 * 1024 * 1024, // 1 Gb User: Settings.clsi.docker.user, - Env: (() => { - const result = [] - for (key in env) { - value = env[key] - result.push(`${key}=${value}`) - } - return result - })(), // convert the environment hash to an array + Env: Object.entries(env).map(([key, value]) => `${key}=${value}`), HostConfig: { - Binds: (() => { - const result1 = [] - for (hostVol in volumes) { - dockerVol = volumes[hostVol] - result1.push(`${hostVol}:${dockerVol}`) - } - return result1 - })(), + Binds: Object.entries(volumes).map( + ([hostVol, dockerVol]) => `${hostVol}:${dockerVol}` + ), LogConfig: { Type: 'none', Config: {} }, Ulimits: [ { @@ -399,7 +386,7 @@ module.exports = DockerRunner = { }) const jobs = [] for (const vol in volumes) { - ;((vol) => jobs.push((cb) => checkVolume(vol, cb)))(vol) + jobs.push((cb) => checkVolume(vol, cb)) } async.series(jobs, callback) }, @@ -655,22 +642,21 @@ module.exports = DockerRunner = { } const jobs = [] for (const container of containers) { - ;((container) => - DockerRunner.examineOldContainer(container, function ( - err, - name, - id, - ttl - ) { - if (name.slice(0, 9) === '/project-' && ttl <= 0) { - // strip the / prefix - // the LockManager uses the plain container name - name = name.slice(1) - jobs.push((cb) => - DockerRunner.destroyContainer(name, id, false, () => cb()) - ) - } - }))(container) + DockerRunner.examineOldContainer(container, function ( + err, + name, + id, + ttl + ) { + if (name.slice(0, 9) === '/project-' && ttl <= 0) { + // strip the / prefix + // the LockManager uses the plain container name + name = name.slice(1) + jobs.push((cb) => + DockerRunner.destroyContainer(name, id, false, () => cb()) + ) + } + }) } // Ignore errors because some containers get stuck but // will be destroyed next time From ee4c08868c8ff0a25d1545bc6d3803baa56652fe Mon Sep 17 00:00:00 2001 From: Eric Mc Sween Date: Thu, 3 Sep 2020 14:43:04 -0400 Subject: [PATCH 05/16] Decaf cleanup: remove default callbacks --- services/clsi/app/js/DockerRunner.js | 37 +++++----------------------- 1 file changed, 6 insertions(+), 31 deletions(-) diff --git a/services/clsi/app/js/DockerRunner.js b/services/clsi/app/js/DockerRunner.js index f2ffc58a9d..fbb851f8f3 100644 --- a/services/clsi/app/js/DockerRunner.js +++ b/services/clsi/app/js/DockerRunner.js @@ -50,9 +50,6 @@ module.exports = DockerRunner = { callback ) { let name - if (callback == null) { - callback = function (error, output) {} - } if (usingSiblingContainers()) { const _newPath = Settings.path.sandboxedCompilesHostDir logger.log( @@ -135,9 +132,6 @@ module.exports = DockerRunner = { }, kill(container_id, callback) { - if (callback == null) { - callback = function (error) {} - } logger.log({ container_id }, 'sending kill signal to container') const container = dockerode.getContainer(container_id) container.kill(function (error) { @@ -162,9 +156,6 @@ module.exports = DockerRunner = { }, _runAndWaitForContainer(options, volumes, timeout, _callback) { - if (_callback == null) { - _callback = function (error, output) {} - } const callback = function (...args) { _callback(...args) // Only call the callback once @@ -366,9 +357,6 @@ module.exports = DockerRunner = { // Check that volumes exist and are directories _checkVolumes(options, volumes, callback) { - if (callback == null) { - callback = function (error, containerName) {} - } if (usingSiblingContainers()) { // Server Pro, with sibling-containers active, skip checks return callback(null) @@ -392,9 +380,6 @@ module.exports = DockerRunner = { }, _startContainer(options, volumes, attachStreamHandler, callback) { - if (callback == null) { - callback = function (error, output) {} - } callback = _.once(callback) const { name } = options @@ -510,9 +495,6 @@ module.exports = DockerRunner = { }, waitForContainer(containerId, timeout, _callback) { - if (_callback == null) { - _callback = function (error, exitCode) {} - } const callback = function (...args) { _callback(...args) // Only call the callback once @@ -564,9 +546,6 @@ module.exports = DockerRunner = { // async exception, but if you delete by id it just does a normal // error callback. We fall back to deleting by name if no id is // supplied. - if (callback == null) { - callback = function (error) {} - } LockManager.runWithLock( containerName, (releaseLock) => @@ -580,9 +559,6 @@ module.exports = DockerRunner = { }, _destroyContainer(containerId, shouldForce, callback) { - if (callback == null) { - callback = function (error) {} - } logger.log({ container_id: containerId }, 'destroying docker container') const container = dockerode.getContainer(containerId) container.remove({ force: shouldForce === true }, function (error) { @@ -614,9 +590,6 @@ module.exports = DockerRunner = { Settings.clsi.docker.maxContainerAge || (oneHour = 60 * 60 * 1000), examineOldContainer(container, callback) { - if (callback == null) { - callback = function (error, name, id, ttl) {} - } const name = container.Name || (container.Names != null ? container.Names[0] : undefined) @@ -633,9 +606,6 @@ module.exports = DockerRunner = { }, destroyOldContainers(callback) { - if (callback == null) { - callback = function (error) {} - } dockerode.listContainers({ all: true }, function (error, containers) { if (error != null) { return callback(error) @@ -677,7 +647,12 @@ module.exports = DockerRunner = { const randomDelay = Math.floor(Math.random() * 5 * 60 * 1000) containerMonitorTimeout = setTimeout(() => { containerMonitorInterval = setInterval( - () => DockerRunner.destroyOldContainers(), + () => + DockerRunner.destroyOldContainers((err) => { + if (err) { + logger.error({ err }, 'failed to destroy old containers') + } + }), (oneHour = 60 * 60 * 1000) ) }, randomDelay) From c52d7d8f02d4ad1a1c77829bfb46594bfceb5a32 Mon Sep 17 00:00:00 2001 From: Eric Mc Sween Date: Thu, 3 Sep 2020 14:56:27 -0400 Subject: [PATCH 06/16] Decaf cleanup: simplify null checks --- services/clsi/app/js/DockerRunner.js | 32 ++++++---------------------- 1 file changed, 7 insertions(+), 25 deletions(-) diff --git a/services/clsi/app/js/DockerRunner.js b/services/clsi/app/js/DockerRunner.js index fbb851f8f3..9725fe9331 100644 --- a/services/clsi/app/js/DockerRunner.js +++ b/services/clsi/app/js/DockerRunner.js @@ -4,13 +4,6 @@ no-return-assign, no-unused-vars, */ -// TODO: This file was created by bulk-decaffeinate. -// Fix any style issues and re-enable lint. -/* - * decaffeinate suggestions: - * DS207: Consider shorter variations of null checks - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ let DockerRunner, oneHour const Settings = require('settings-sharelatex') const logger = require('logger-sharelatex') @@ -286,10 +279,7 @@ module.exports = DockerRunner = { } } - if ( - (Settings.path != null ? Settings.path.synctexBinHostPath : undefined) != - null - ) { + if (Settings.path != null && Settings.path.synctexBinHostPath != null) { options.HostConfig.Binds.push( `${Settings.path.synctexBinHostPath}:/opt/synctex:ro` ) @@ -367,7 +357,7 @@ module.exports = DockerRunner = { if (err != null) { return cb(err) } - if (!(stats != null ? stats.isDirectory() : undefined)) { + if (!stats.isDirectory()) { return cb(DockerRunner.ERR_NOT_DIRECTORY) } cb() @@ -402,20 +392,17 @@ module.exports = DockerRunner = { return callback(error) } container.start(function (error) { - if ( - error != null && - (error != null ? error.statusCode : undefined) !== 304 - ) { - // already running + if (error != null && error.statusCode !== 304) { callback(error) } else { + // already running callback() } }) } ) container.inspect(function (error, stats) { - if ((error != null ? error.statusCode : undefined) === 404) { + if (error != null && error.statusCode === 404) { createAndStartContainer() } else if (error != null) { logger.err( @@ -562,10 +549,7 @@ module.exports = DockerRunner = { logger.log({ container_id: containerId }, 'destroying docker container') const container = dockerode.getContainer(containerId) container.remove({ force: shouldForce === true }, function (error) { - if ( - error != null && - (error != null ? error.statusCode : undefined) === 404 - ) { + if (error != null && error.statusCode === 404) { logger.warn( { err: error, container_id: containerId }, 'container not found, continuing' @@ -590,9 +574,7 @@ module.exports = DockerRunner = { Settings.clsi.docker.maxContainerAge || (oneHour = 60 * 60 * 1000), examineOldContainer(container, callback) { - const name = - container.Name || - (container.Names != null ? container.Names[0] : undefined) + const name = container.Name || (container.Names && container.Names[0]) const created = container.Created * 1000 // creation time is returned in seconds const now = Date.now() const age = now - created From 2584586ba2b7a6b65d517a476f762ce8c62120c3 Mon Sep 17 00:00:00 2001 From: Eric Mc Sween Date: Thu, 3 Sep 2020 14:58:37 -0400 Subject: [PATCH 07/16] Decaf cleanup: camel-case variables --- services/clsi/app/js/DockerRunner.js | 52 +++++++++++----------------- 1 file changed, 21 insertions(+), 31 deletions(-) diff --git a/services/clsi/app/js/DockerRunner.js b/services/clsi/app/js/DockerRunner.js index 9725fe9331..f9977d8090 100644 --- a/services/clsi/app/js/DockerRunner.js +++ b/services/clsi/app/js/DockerRunner.js @@ -1,5 +1,4 @@ /* eslint-disable - camelcase, handle-callback-err, no-return-assign, no-unused-vars, @@ -33,7 +32,7 @@ module.exports = DockerRunner = { ERR_TIMED_OUT: new Error('container timed out'), run( - project_id, + projectId, command, directory, image, @@ -90,18 +89,18 @@ module.exports = DockerRunner = { compileGroup ) const fingerprint = DockerRunner._fingerprintContainer(options) - options.name = name = `project-${project_id}-${fingerprint}` + options.name = name = `project-${projectId}-${fingerprint}` // logOptions = _.clone(options) // logOptions?.HostConfig?.SecurityOpt = "secomp used, removed in logging" - logger.log({ project_id }, 'running docker container') + logger.log({ projectId }, 'running docker container') DockerRunner._runAndWaitForContainer(options, volumes, timeout, function ( error, output ) { if (error && error.statusCode === 500) { logger.log( - { err: error, project_id }, + { err: error, projectId }, 'error running container so destroying and retrying' ) DockerRunner.destroyContainer(name, null, true, function (error) { @@ -124,9 +123,9 @@ module.exports = DockerRunner = { return name }, - kill(container_id, callback) { - logger.log({ container_id }, 'sending kill signal to container') - const container = dockerode.getContainer(container_id) + kill(containerId, callback) { + logger.log({ containerId }, 'sending kill signal to container') + const container = dockerode.getContainer(containerId) container.kill(function (error) { if ( error != null && @@ -134,13 +133,13 @@ module.exports = DockerRunner = { error.message.match(/Cannot kill container .* is not running/) ) { logger.warn( - { err: error, container_id }, + { err: error, containerId }, 'container not running, continuing' ) error = null } if (error != null) { - logger.error({ err: error, container_id }, 'error killing container') + logger.error({ err: error, containerId }, 'error killing container') callback(error) } else { callback() @@ -424,7 +423,7 @@ module.exports = DockerRunner = { ) { if (error != null) { logger.error( - { err: error, container_id: containerId }, + { err: error, containerId }, 'error attaching to container' ) return attachStartCallback(error) @@ -432,7 +431,7 @@ module.exports = DockerRunner = { attachStartCallback() } - logger.log({ container_id: containerId }, 'attached to container') + logger.log({ containerId }, 'attached to container') const MAX_OUTPUT = 1024 * 1024 // limit output to 1MB const createStringOutputStream = function (name) { @@ -448,7 +447,7 @@ module.exports = DockerRunner = { } else { logger.error( { - container_id: containerId, + containerId, length: this.data.length, maxLen: MAX_OUTPUT }, @@ -470,7 +469,7 @@ module.exports = DockerRunner = { stream.on('error', (err) => logger.error( - { err, container_id: containerId }, + { err, containerId }, 'error reading from container stream' ) ) @@ -493,21 +492,15 @@ module.exports = DockerRunner = { let timedOut = false const timeoutId = setTimeout(function () { timedOut = true - logger.log( - { container_id: containerId }, - 'timeout reached, killing container' - ) + logger.log({ containerId }, 'timeout reached, killing container') container.kill(function () {}) }, timeout) - logger.log({ container_id: containerId }, 'waiting for docker container') + logger.log({ containerId }, 'waiting for docker container') container.wait(function (error, res) { if (error != null) { clearTimeout(timeoutId) - logger.error( - { err: error, container_id: containerId }, - 'error waiting for container' - ) + logger.error({ err: error, containerId }, 'error waiting for container') return callback(error) } if (timedOut) { @@ -518,7 +511,7 @@ module.exports = DockerRunner = { } else { clearTimeout(timeoutId) logger.log( - { container_id: containerId, exitCode: res.StatusCode }, + { containerId, exitCode: res.StatusCode }, 'docker container returned' ) callback(null, res.StatusCode) @@ -546,23 +539,20 @@ module.exports = DockerRunner = { }, _destroyContainer(containerId, shouldForce, callback) { - logger.log({ container_id: containerId }, 'destroying docker container') + logger.log({ containerId }, 'destroying docker container') const container = dockerode.getContainer(containerId) container.remove({ force: shouldForce === true }, function (error) { if (error != null && error.statusCode === 404) { logger.warn( - { err: error, container_id: containerId }, + { err: error, containerId }, 'container not found, continuing' ) error = null } if (error != null) { - logger.error( - { err: error, container_id: containerId }, - 'error destroying container' - ) + logger.error({ err: error, containerId }, 'error destroying container') } else { - logger.log({ container_id: containerId }, 'destroyed container') + logger.log({ containerId }, 'destroyed container') } callback(error) }) From 44bf38d6dbe6c71e7dcd640f5cdc5721f5d5a4ed Mon Sep 17 00:00:00 2001 From: Eric Mc Sween Date: Thu, 3 Sep 2020 15:05:13 -0400 Subject: [PATCH 08/16] Decaf cleanup: convert async function to sync The examineOldContainer() function doesn't need to use callbacks since it only does synchronous work. --- services/clsi/app/js/DockerRunner.js | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/services/clsi/app/js/DockerRunner.js b/services/clsi/app/js/DockerRunner.js index f9977d8090..cfe8d1166a 100644 --- a/services/clsi/app/js/DockerRunner.js +++ b/services/clsi/app/js/DockerRunner.js @@ -1,5 +1,4 @@ /* eslint-disable - handle-callback-err, no-return-assign, no-unused-vars, */ @@ -574,7 +573,7 @@ module.exports = DockerRunner = { { containerName: name, created, now, age, maxAge, ttl }, 'checking whether to destroy container' ) - callback(null, name, container.Id, ttl) + return { name, id: container.Id, ttl } }, destroyOldContainers(callback) { @@ -584,21 +583,15 @@ module.exports = DockerRunner = { } const jobs = [] for (const container of containers) { - DockerRunner.examineOldContainer(container, function ( - err, - name, - id, - ttl - ) { - if (name.slice(0, 9) === '/project-' && ttl <= 0) { - // strip the / prefix - // the LockManager uses the plain container name - name = name.slice(1) - jobs.push((cb) => - DockerRunner.destroyContainer(name, id, false, () => cb()) - ) - } - }) + const { name, id, ttl } = DockerRunner.examineOldContainer(container) + if (name.slice(0, 9) === '/project-' && ttl <= 0) { + // strip the / prefix + // the LockManager uses the plain container name + const plainName = name.slice(1) + jobs.push((cb) => + DockerRunner.destroyContainer(plainName, id, false, () => cb()) + ) + } } // Ignore errors because some containers get stuck but // will be destroyed next time From 4905e7db2ccdd4bde4585a808dd90915fdd9f7db Mon Sep 17 00:00:00 2001 From: Eric Mc Sween Date: Thu, 3 Sep 2020 15:30:07 -0400 Subject: [PATCH 09/16] Decaf cleanup: unused vars --- services/clsi/app/js/DockerRunner.js | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/services/clsi/app/js/DockerRunner.js b/services/clsi/app/js/DockerRunner.js index cfe8d1166a..3e635cdb5e 100644 --- a/services/clsi/app/js/DockerRunner.js +++ b/services/clsi/app/js/DockerRunner.js @@ -1,8 +1,4 @@ -/* eslint-disable - no-return-assign, - no-unused-vars, -*/ -let DockerRunner, oneHour +let DockerRunner const Settings = require('settings-sharelatex') const logger = require('logger-sharelatex') const Docker = require('dockerode') @@ -14,6 +10,7 @@ const fs = require('fs') const Path = require('path') const _ = require('lodash') +const ONE_HOUR_IN_MS = 60 * 60 * 1000 logger.info('using docker runner') const usingSiblingContainers = () => @@ -559,8 +556,7 @@ module.exports = DockerRunner = { // handle expiry of docker containers - MAX_CONTAINER_AGE: - Settings.clsi.docker.maxContainerAge || (oneHour = 60 * 60 * 1000), + MAX_CONTAINER_AGE: Settings.clsi.docker.maxContainerAge || ONE_HOUR_IN_MS, examineOldContainer(container, callback) { const name = container.Name || (container.Names && container.Names[0]) @@ -618,7 +614,7 @@ module.exports = DockerRunner = { logger.error({ err }, 'failed to destroy old containers') } }), - (oneHour = 60 * 60 * 1000) + ONE_HOUR_IN_MS ) }, randomDelay) }, From a282bccd4838d609613a663cc7c050ae40b98cc8 Mon Sep 17 00:00:00 2001 From: Eric Mc Sween Date: Thu, 3 Sep 2020 15:34:19 -0400 Subject: [PATCH 10/16] Do not instantiate errors at module load time This prevents the right stack trace from being captured. --- services/clsi/app/js/DockerRunner.js | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/services/clsi/app/js/DockerRunner.js b/services/clsi/app/js/DockerRunner.js index 3e635cdb5e..c422f57267 100644 --- a/services/clsi/app/js/DockerRunner.js +++ b/services/clsi/app/js/DockerRunner.js @@ -22,11 +22,6 @@ let containerMonitorTimeout let containerMonitorInterval module.exports = DockerRunner = { - ERR_NOT_DIRECTORY: new Error('not a directory'), - ERR_TERMINATED: new Error('terminated'), - ERR_EXITED: new Error('exited'), - ERR_TIMED_OUT: new Error('container timed out'), - run( projectId, command, @@ -190,13 +185,13 @@ module.exports = DockerRunner = { } if (exitCode === 137) { // exit status from kill -9 - err = DockerRunner.ERR_TERMINATED + err = new Error('terminated') err.terminated = true return callback(err) } if (exitCode === 1) { // exit status from chktex - err = DockerRunner.ERR_EXITED + err = new Error('exited') err.code = exitCode return callback(err) } @@ -353,7 +348,7 @@ module.exports = DockerRunner = { return cb(err) } if (!stats.isDirectory()) { - return cb(DockerRunner.ERR_NOT_DIRECTORY) + return cb(new Error('not a directory')) } cb() }) @@ -501,7 +496,7 @@ module.exports = DockerRunner = { } if (timedOut) { logger.log({ containerId }, 'docker container timed out') - error = DockerRunner.ERR_TIMED_OUT + error = new Error('container timed out') error.timedout = true callback(error) } else { From 30a44eddedabf756ee1fc6962a975ffe5bf27dfa Mon Sep 17 00:00:00 2001 From: Eric Mc Sween Date: Thu, 3 Sep 2020 15:50:12 -0400 Subject: [PATCH 11/16] Decaf cleanup: simplify variable declarations --- services/clsi/app/js/DockerRunner.js | 41 ++++++++++++---------------- 1 file changed, 17 insertions(+), 24 deletions(-) diff --git a/services/clsi/app/js/DockerRunner.js b/services/clsi/app/js/DockerRunner.js index c422f57267..f4e2659800 100644 --- a/services/clsi/app/js/DockerRunner.js +++ b/services/clsi/app/js/DockerRunner.js @@ -1,4 +1,3 @@ -let DockerRunner const Settings = require('settings-sharelatex') const logger = require('logger-sharelatex') const Docker = require('dockerode') @@ -21,7 +20,7 @@ const usingSiblingContainers = () => let containerMonitorTimeout let containerMonitorInterval -module.exports = DockerRunner = { +const DockerRunner = { run( projectId, command, @@ -32,7 +31,6 @@ module.exports = DockerRunner = { compileGroup, callback ) { - let name if (usingSiblingContainers()) { const _newPath = Settings.path.sandboxedCompilesHostDir logger.log( @@ -49,14 +47,13 @@ module.exports = DockerRunner = { ) } - const volumes = {} - volumes[directory] = '/compile' + const volumes = { [directory]: '/compile' } command = command.map((arg) => arg.toString().replace('$COMPILE_DIR', '/compile') ) if (image == null) { - ;({ image } = Settings.clsi.docker) + image = Settings.clsi.docker.image } if ( @@ -80,7 +77,8 @@ module.exports = DockerRunner = { compileGroup ) const fingerprint = DockerRunner._fingerprintContainer(options) - options.name = name = `project-${projectId}-${fingerprint}` + const name = `project-${projectId}-${fingerprint}` + options.name = name // logOptions = _.clone(options) // logOptions?.HostConfig?.SecurityOpt = "secomp used, removed in logging" @@ -179,19 +177,18 @@ module.exports = DockerRunner = { error, exitCode ) { - let err if (error != null) { return callback(error) } if (exitCode === 137) { // exit status from kill -9 - err = new Error('terminated') + const err = new Error('terminated') err.terminated = true return callback(err) } if (exitCode === 1) { // exit status from chktex - err = new Error('exited') + const err = new Error('exited') err.code = exitCode return callback(err) } @@ -199,7 +196,7 @@ module.exports = DockerRunner = { if (options != null && options.HostConfig != null) { options.HostConfig.SecurityOpt = null } - logger.log({ err, exitCode, options }, 'docker container has exited') + logger.log({ exitCode, options }, 'docker container has exited') callbackIfFinished() }) } @@ -214,13 +211,11 @@ module.exports = DockerRunner = { environment, compileGroup ) { - let m, year - let key, value, hostVol, dockerVol const timeoutInSeconds = timeout / 1000 const dockerVolumes = {} - for (hostVol in volumes) { - dockerVol = volumes[hostVol] + for (const hostVol in volumes) { + const dockerVol = volumes[hostVol] dockerVolumes[dockerVol] = {} if (volumes[hostVol].slice(-3).indexOf(':r') === -1) { @@ -231,17 +226,14 @@ module.exports = DockerRunner = { // merge settings and environment parameter const env = {} for (const src of [Settings.clsi.docker.env, environment || {}]) { - for (key in src) { - value = src[key] + for (const key in src) { + const value = src[key] env[key] = value } } // set the path based on the image year - if ((m = image.match(/:([0-9]+)\.[0-9]+/))) { - year = m[1] - } else { - year = '2014' - } + const match = image.match(/:([0-9]+)\.[0-9]+/) + const year = match ? match[1] : '2014' env.PATH = `/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/local/texlive/${year}/bin/x86_64-linux/` const options = { Cmd: command, @@ -296,8 +288,7 @@ module.exports = DockerRunner = { Settings.clsi.docker.compileGroupConfig[compileGroup] ) { const override = Settings.clsi.docker.compileGroupConfig[compileGroup] - let key - for (key in override) { + for (const key in override) { _.set(options, key, override[key]) } } @@ -627,3 +618,5 @@ module.exports = DockerRunner = { } DockerRunner.startContainerMonitor() + +module.exports = DockerRunner From a853950a995d7a415c092efe5f723ecc39533d8f Mon Sep 17 00:00:00 2001 From: Eric Mc Sween Date: Thu, 3 Sep 2020 15:50:45 -0400 Subject: [PATCH 12/16] Fix container monitor cleanup function The intent here is clearly to clear both the timeout and the interval. --- services/clsi/app/js/DockerRunner.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/clsi/app/js/DockerRunner.js b/services/clsi/app/js/DockerRunner.js index f4e2659800..33000d7ba6 100644 --- a/services/clsi/app/js/DockerRunner.js +++ b/services/clsi/app/js/DockerRunner.js @@ -611,8 +611,8 @@ const DockerRunner = { containerMonitorTimeout = undefined } if (containerMonitorInterval) { - clearInterval(containerMonitorTimeout) - containerMonitorTimeout = undefined + clearInterval(containerMonitorInterval) + containerMonitorInterval = undefined } } } From 5cd889038e4820c49aee1fffe560382a5054329c Mon Sep 17 00:00:00 2001 From: Eric Mc Sween Date: Thu, 3 Sep 2020 15:52:09 -0400 Subject: [PATCH 13/16] Use _.once() instead of ad hoc implementation --- services/clsi/app/js/DockerRunner.js | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/services/clsi/app/js/DockerRunner.js b/services/clsi/app/js/DockerRunner.js index 33000d7ba6..e355f95067 100644 --- a/services/clsi/app/js/DockerRunner.js +++ b/services/clsi/app/js/DockerRunner.js @@ -137,12 +137,7 @@ const DockerRunner = { }, _runAndWaitForContainer(options, volumes, timeout, _callback) { - const callback = function (...args) { - _callback(...args) - // Only call the callback once - _callback = function () {} - } - + const callback = _.once(_callback) const { name } = options let streamEnded = false @@ -463,11 +458,7 @@ const DockerRunner = { }, waitForContainer(containerId, timeout, _callback) { - const callback = function (...args) { - _callback(...args) - // Only call the callback once - _callback = function () {} - } + const callback = _.once(_callback) const container = dockerode.getContainer(containerId) From 67f4a6eeebd0500c1f7f17a5259c540dfc0b9795 Mon Sep 17 00:00:00 2001 From: Eric Mc Sween Date: Thu, 3 Sep 2020 15:58:16 -0400 Subject: [PATCH 14/16] Decaf cleanup: normalize functions Use function keyword for declarations and arrow functions for callbacks. --- services/clsi/app/js/DockerRunner.js | 109 ++++++++++++++------------- 1 file changed, 57 insertions(+), 52 deletions(-) diff --git a/services/clsi/app/js/DockerRunner.js b/services/clsi/app/js/DockerRunner.js index e355f95067..d357a241d0 100644 --- a/services/clsi/app/js/DockerRunner.js +++ b/services/clsi/app/js/DockerRunner.js @@ -12,10 +12,13 @@ const _ = require('lodash') const ONE_HOUR_IN_MS = 60 * 60 * 1000 logger.info('using docker runner') -const usingSiblingContainers = () => - Settings != null && - Settings.path != null && - Settings.path.sandboxedCompilesHostDir != null +function usingSiblingContainers() { + return ( + Settings != null && + Settings.path != null && + Settings.path.sandboxedCompilesHostDir != null + ) +} let containerMonitorTimeout let containerMonitorInterval @@ -83,30 +86,32 @@ const DockerRunner = { // logOptions = _.clone(options) // logOptions?.HostConfig?.SecurityOpt = "secomp used, removed in logging" logger.log({ projectId }, 'running docker container') - DockerRunner._runAndWaitForContainer(options, volumes, timeout, function ( - error, - output - ) { - if (error && error.statusCode === 500) { - logger.log( - { err: error, projectId }, - 'error running container so destroying and retrying' - ) - DockerRunner.destroyContainer(name, null, true, function (error) { - if (error != null) { - return callback(error) - } - DockerRunner._runAndWaitForContainer( - options, - volumes, - timeout, - callback + DockerRunner._runAndWaitForContainer( + options, + volumes, + timeout, + (error, output) => { + if (error && error.statusCode === 500) { + logger.log( + { err: error, projectId }, + 'error running container so destroying and retrying' ) - }) - } else { - callback(error, output) + DockerRunner.destroyContainer(name, null, true, (error) => { + if (error != null) { + return callback(error) + } + DockerRunner._runAndWaitForContainer( + options, + volumes, + timeout, + callback + ) + }) + } else { + callback(error, output) + } } - }) + ) // pass back the container name to allow it to be killed return name @@ -115,7 +120,7 @@ const DockerRunner = { kill(containerId, callback) { logger.log({ containerId }, 'sending kill signal to container') const container = dockerode.getContainer(containerId) - container.kill(function (error) { + container.kill((error) => { if ( error != null && error.message != null && @@ -144,13 +149,13 @@ const DockerRunner = { let containerReturned = false let output = {} - const callbackIfFinished = function () { + function callbackIfFinished() { if (streamEnded && containerReturned) { callback(null, output) } } - const attachStreamHandler = function (error, _output) { + function attachStreamHandler(error, _output) { if (error != null) { return callback(error) } @@ -163,15 +168,12 @@ const DockerRunner = { options, volumes, attachStreamHandler, - function (error, containerId) { + (error, containerId) => { if (error != null) { return callback(error) } - DockerRunner.waitForContainer(name, timeout, function ( - error, - exitCode - ) { + DockerRunner.waitForContainer(name, timeout, (error, exitCode) => { if (error != null) { return callback(error) } @@ -305,7 +307,7 @@ const DockerRunner = { // When a container is started with volume pointing to a // non-existent directory then docker creates the directory but // with root ownership. - DockerRunner._checkVolumes(options, volumes, function (err) { + DockerRunner._checkVolumes(options, volumes, (err) => { if (err != null) { return releaseLock(err) } @@ -329,7 +331,7 @@ const DockerRunner = { } const checkVolume = (path, cb) => - fs.stat(path, function (err, stats) { + fs.stat(path, (err, stats) => { if (err != null) { return cb(err) } @@ -352,22 +354,24 @@ const DockerRunner = { logger.log({ container_name: name }, 'starting container') const container = dockerode.getContainer(name) - const createAndStartContainer = () => - dockerode.createContainer(options, function (error, container) { + function createAndStartContainer() { + dockerode.createContainer(options, (error, container) => { if (error != null) { return callback(error) } startExistingContainer() }) - var startExistingContainer = () => + } + + function startExistingContainer() { DockerRunner.attachToContainer( options.name, attachStreamHandler, - function (error) { + (error) => { if (error != null) { return callback(error) } - container.start(function (error) { + container.start((error) => { if (error != null && error.statusCode !== 304) { callback(error) } else { @@ -377,7 +381,9 @@ const DockerRunner = { }) } ) - container.inspect(function (error, stats) { + } + + container.inspect((error, stats) => { if (error != null && error.statusCode === 404) { createAndStartContainer() } else if (error != null) { @@ -394,10 +400,7 @@ const DockerRunner = { attachToContainer(containerId, attachStreamHandler, attachStartCallback) { const container = dockerode.getContainer(containerId) - container.attach({ stdout: 1, stderr: 1, stream: 1 }, function ( - error, - stream - ) { + container.attach({ stdout: 1, stderr: 1, stream: 1 }, (error, stream) => { if (error != null) { logger.error( { err: error, containerId }, @@ -411,7 +414,7 @@ const DockerRunner = { logger.log({ containerId }, 'attached to container') const MAX_OUTPUT = 1024 * 1024 // limit output to 1MB - const createStringOutputStream = function (name) { + function createStringOutputStream(name) { return { data: '', overflowed: false, @@ -463,14 +466,16 @@ const DockerRunner = { const container = dockerode.getContainer(containerId) let timedOut = false - const timeoutId = setTimeout(function () { + const timeoutId = setTimeout(() => { timedOut = true logger.log({ containerId }, 'timeout reached, killing container') - container.kill(function () {}) + container.kill((err) => { + logger.warn({ err, containerId }, 'failed to kill container') + }) }, timeout) logger.log({ containerId }, 'waiting for docker container') - container.wait(function (error, res) { + container.wait((error, res) => { if (error != null) { clearTimeout(timeoutId) logger.error({ err: error, containerId }, 'error waiting for container') @@ -514,7 +519,7 @@ const DockerRunner = { _destroyContainer(containerId, shouldForce, callback) { logger.log({ containerId }, 'destroying docker container') const container = dockerode.getContainer(containerId) - container.remove({ force: shouldForce === true }, function (error) { + container.remove({ force: shouldForce === true }, (error) => { if (error != null && error.statusCode === 404) { logger.warn( { err: error, containerId }, @@ -550,7 +555,7 @@ const DockerRunner = { }, destroyOldContainers(callback) { - dockerode.listContainers({ all: true }, function (error, containers) { + dockerode.listContainers({ all: true }, (error, containers) => { if (error != null) { return callback(error) } From 1c13f6fe94a84c4bdcfaa78be7167d8b5fb26e32 Mon Sep 17 00:00:00 2001 From: Eric Mc Sween Date: Thu, 3 Sep 2020 17:10:24 -0400 Subject: [PATCH 15/16] Mount /home/tex in an anonymous volume When we mount the container's root filesystem as read-only, mount an anonymous volume in /home/tex so that it's writable. Our TeX Live images have cached content in /home/tex. This content will automatically get copied by Docker into this anonymous volume. --- services/clsi/app/js/DockerRunner.js | 3 ++- services/clsi/test/unit/js/DockerRunnerTests.js | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/services/clsi/app/js/DockerRunner.js b/services/clsi/app/js/DockerRunner.js index d357a241d0..0ff8109585 100644 --- a/services/clsi/app/js/DockerRunner.js +++ b/services/clsi/app/js/DockerRunner.js @@ -277,6 +277,7 @@ const DockerRunner = { if (Settings.clsi.docker.Readonly) { options.HostConfig.ReadonlyRootfs = true options.HostConfig.Tmpfs = { '/tmp': 'rw,noexec,nosuid,size=65536k' } + options.Volumes['/home/tex'] = {} } // Allow per-compile group overriding of individual settings @@ -519,7 +520,7 @@ const DockerRunner = { _destroyContainer(containerId, shouldForce, callback) { logger.log({ containerId }, 'destroying docker container') const container = dockerode.getContainer(containerId) - container.remove({ force: shouldForce === true }, (error) => { + container.remove({ force: shouldForce === true, v: true }, (error) => { if (error != null && error.statusCode === 404) { logger.warn( { err: error, containerId }, diff --git a/services/clsi/test/unit/js/DockerRunnerTests.js b/services/clsi/test/unit/js/DockerRunnerTests.js index 553b20f3ad..ac68c75079 100644 --- a/services/clsi/test/unit/js/DockerRunnerTests.js +++ b/services/clsi/test/unit/js/DockerRunnerTests.js @@ -802,7 +802,7 @@ describe('DockerRunner', function () { (err) => { this.fakeContainer.remove.callCount.should.equal(1) this.fakeContainer.remove - .calledWith({ force: true }) + .calledWithMatch({ force: true }) .should.equal(true) return done() } @@ -816,7 +816,7 @@ describe('DockerRunner', function () { (err) => { this.fakeContainer.remove.callCount.should.equal(1) this.fakeContainer.remove - .calledWith({ force: false }) + .calledWithMatch({ force: false }) .should.equal(true) return done() } From 14e1e02a681764839b1e697caf813cdf2cabbfdd Mon Sep 17 00:00:00 2001 From: Eric Mc Sween Date: Fri, 4 Sep 2020 11:34:08 -0400 Subject: [PATCH 16/16] Test anonymous volumes are removed with containers --- services/clsi/test/unit/js/DockerRunnerTests.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/clsi/test/unit/js/DockerRunnerTests.js b/services/clsi/test/unit/js/DockerRunnerTests.js index ac68c75079..517179a962 100644 --- a/services/clsi/test/unit/js/DockerRunnerTests.js +++ b/services/clsi/test/unit/js/DockerRunnerTests.js @@ -802,7 +802,7 @@ describe('DockerRunner', function () { (err) => { this.fakeContainer.remove.callCount.should.equal(1) this.fakeContainer.remove - .calledWithMatch({ force: true }) + .calledWith({ force: true, v: true }) .should.equal(true) return done() } @@ -816,7 +816,7 @@ describe('DockerRunner', function () { (err) => { this.fakeContainer.remove.callCount.should.equal(1) this.fakeContainer.remove - .calledWithMatch({ force: false }) + .calledWith({ force: false, v: true }) .should.equal(true) return done() }