From d3ff214b887c2e9cdc63abddf6edeedfd9627d04 Mon Sep 17 00:00:00 2001 From: Tim Alby Date: Thu, 11 Jun 2020 12:50:43 +0200 Subject: [PATCH] partially revert "[DockerRunner] fix metric incrementing and error logging" This reverts commits: - 2b2fcca39ce8dee0fdc0c342aa0d6c822592bcec - 9e82ab0890c5cc8c7fb95362c3f7edbcaad0cf29 - e3da458b376871c3ce72d6984d14bf1ee668b04b --- services/clsi/app/js/DockerRunner.js | 36 +++++------- .../clsi/test/unit/js/DockerRunnerTests.js | 58 ------------------- 2 files changed, 13 insertions(+), 81 deletions(-) diff --git a/services/clsi/app/js/DockerRunner.js b/services/clsi/app/js/DockerRunner.js index c50087d018..3f90c81123 100644 --- a/services/clsi/app/js/DockerRunner.js +++ b/services/clsi/app/js/DockerRunner.js @@ -26,7 +26,6 @@ const LockManager = require('./DockerLockManager') const fs = require('fs') const Path = require('path') const _ = require('underscore') -const metrics = require('metrics-sharelatex') logger.info('using docker runner') @@ -412,28 +411,19 @@ module.exports = DockerRunner = { }) } ) - var inspectContainer = isRetry => - container.inspect(function(error, stats) { - if ((error != null ? error.statusCode : undefined) === 404) { - return createAndStartContainer() - } else if (error != null) { - if (error.message.match(/EPIPE/)) { - if (!isRetry) { - metrics.inc('container-inspect-epipe-retry') - return inspectContainer(true) - } - metrics.inc('container-inspect-epipe-error') - } - logger.err( - { container_name: name, error }, - 'unable to inspect container to start' - ) - return callback(error) - } else { - return startExistingContainer() - } - }) - inspectContainer(false) + return container.inspect(function(error, stats) { + if ((error != null ? error.statusCode : undefined) === 404) { + return createAndStartContainer() + } else if (error != null) { + logger.err( + { container_name: name, error }, + 'unable to inspect container to start' + ) + return callback(error) + } else { + return startExistingContainer() + } + }) }, attachToContainer(containerId, attachStreamHandler, attachStartCallback) { diff --git a/services/clsi/test/unit/js/DockerRunnerTests.js b/services/clsi/test/unit/js/DockerRunnerTests.js index 1e44daffb4..aa45e88da1 100644 --- a/services/clsi/test/unit/js/DockerRunnerTests.js +++ b/services/clsi/test/unit/js/DockerRunnerTests.js @@ -36,7 +36,6 @@ describe('DockerRunner', function() { 'logger-sharelatex': (this.logger = { log: sinon.stub(), error: sinon.stub(), - err: sinon.stub(), info: sinon.stub(), warn: sinon.stub() }), @@ -388,63 +387,6 @@ describe('DockerRunner', function() { }) }) - describe('when inspect always fails with EPIPE error', function() { - beforeEach(function() { - this.error = new Error('write EPIPE') - this.container.inspect = sinon.stub().yields(this.error) - this.container.start = sinon.stub().yields() - - this.DockerRunner.startContainer( - this.options, - this.volumes, - () => {}, - this.callback - ) - }) - - it('should retry once', function() { - sinon.assert.callOrder( - this.container.inspect, - this.container.inspect, - this.callback - ) - }) - - it('should call back with error', function() { - sinon.assert.calledWith(this.callback, this.error) - }) - }) - - describe('when inspect fails once with EPIPE error', function() { - beforeEach(function() { - this.container.inspect = sinon.stub() - this.container.inspect.onFirstCall().yields(new Error('write EPIPE')) - this.container.inspect.onSecondCall().yields() - this.container.start = sinon.stub().yields() - - this.DockerRunner.startContainer( - this.options, - this.volumes, - () => {}, - this.callback - ) - }) - - it('should retry once and start container', function() { - sinon.assert.callOrder( - this.container.inspect, - this.container.inspect, - this.DockerRunner.attachToContainer, - this.container.start, - this.callback - ) - }) - - it('should call back without error', function() { - sinon.assert.calledWith(this.callback, null) - }) - }) - describe('when the container does not exist', function() { beforeEach(function() { const exists = false