From 8e5f26e78cb615bb6d818556bae0897f0badc235 Mon Sep 17 00:00:00 2001 From: elad-codefresh <82316166+elad-codefresh@users.noreply.github.com> Date: Sun, 20 Jun 2021 13:18:05 +0300 Subject: [PATCH] CR-4891-new (#74) * follow true * no tests * with tests * bump * bremoved debug-purpose changes * and now after save... * without delay * test fix * removed redundant logs * removed redundant comment * removed comma * removed redundant comma * without q.all * with q.all --- lib/ContainerLogger.js | 30 +++--- lib/logger.js | 60 ++++++------ service.yaml | 2 +- test/ContainerLogger.unit.spec.js | 154 +++++++++++++++--------------- 4 files changed, 123 insertions(+), 123 deletions(-) diff --git a/lib/ContainerLogger.js b/lib/ContainerLogger.js index 060e2a1..2fb2770 100644 --- a/lib/ContainerLogger.js +++ b/lib/ContainerLogger.js @@ -1,7 +1,7 @@ -const EventEmitter = require('events'); -const Q = require('q'); -const logger = require('cf-logs').Logger('codefresh:containerLogger'); -const CFError = require('cf-errors'); +const EventEmitter = require('events'); +const Q = require('q'); +const logger = require('cf-logs').Logger('codefresh:containerLogger'); +const CFError = require('cf-errors'); const { Transform } = require('stream'); const { LoggerStrategy } = require('./enums'); @@ -16,15 +16,15 @@ class ContainerLogger extends EventEmitter { loggerStrategy }) { super(); - this.containerId = containerId; - this.containerInterface = containerInterface; - this.stepLogger = stepLogger; - this.loggerStrategy = loggerStrategy; - this.tty = false; - this.logSizeLimit = logSizeLimit; - this.logSize = 0; + this.containerId = containerId; + this.containerInterface = containerInterface; + this.stepLogger = stepLogger; + this.loggerStrategy = loggerStrategy; + this.tty = false; + this.logSizeLimit = logSizeLimit; + this.logSize = 0; this.isWorkflowLogSizeExceeded = isWorkflowLogSizeExceeded; - this.stepFinished = false; + this.stepFinished = false; this.finishedStreams = 0; this.handledStreams = 0; } @@ -91,9 +91,9 @@ class ContainerLogger extends EventEmitter { _getLogsStrategyStream() { return Q.all([ Q.ninvoke(this.containerInterface, 'logs', { - follow: 1, - stdout: 1, - stderr: 1 + follow: true, + stdout: true, + stderr: true }) ]); } diff --git a/lib/logger.js b/lib/logger.js index cc06600..cd14df5 100644 --- a/lib/logger.js +++ b/lib/logger.js @@ -1,18 +1,18 @@ -const fs = require('fs'); -const { EventEmitter } = require('events'); -const _ = require('lodash'); -const Q = require('q'); -const Docker = require('dockerode'); -const DockerEvents = require('docker-events'); -const bodyParser = require('body-parser'); -const CFError = require('cf-errors'); -const logger = require('cf-logs').Logger('codefresh:containerLogger'); -const { TaskLogger } = require('@codefresh-io/task-logger'); -const express = require('express'); +const fs = require('fs'); +const { EventEmitter } = require('events'); +const _ = require('lodash'); +const Q = require('q'); +const Docker = require('dockerode'); +const DockerEvents = require('docker-events'); +const bodyParser = require('body-parser'); +const CFError = require('cf-errors'); +const logger = require('cf-logs').Logger('codefresh:containerLogger'); +const { TaskLogger } = require('@codefresh-io/task-logger'); +const express = require('express'); const { ContainerStatus } = require('./enums'); const { LoggerStrategy } = require('./enums'); const { ContainerHandlingStatus } = require('./enums'); -const ContainerLogger = require('./ContainerLogger'); +const ContainerLogger = require('./ContainerLogger'); const initialState = { pid: process.pid, status: 'init', lastLogsDate: new Date(), failedHealthChecks: [], restartCounter: 0, containers: {} @@ -27,13 +27,13 @@ class Logger { buildFinishedPromise, showProgress, }) { - this.taskLoggerConfig = taskLoggerConfig; - this.loggerId = loggerId; + this.taskLoggerConfig = taskLoggerConfig; + this.loggerId = loggerId; this.findExistingContainers = findExistingContainers === 'true'; - this.logSizeLimit = logSizeLimit; - this.containerLoggers = []; - this.logSize = 0; - this.taskLogger = undefined; + this.logSizeLimit = logSizeLimit; + this.containerLoggers = []; + this.logSize = 0; + this.taskLogger = undefined; this.buildFinishedPromise = buildFinishedPromise || Q.resolve(); this.finishedContainers = 0; this.finishedContainersEmitter = new EventEmitter(); @@ -48,7 +48,7 @@ class Logger { // console.log('Using /var/run/docker.sock'); } - this.docker = new Docker({ + this.docker = new Docker({ socketPath: dockerSockPath, }); this._readState(); @@ -131,11 +131,11 @@ class Logger { this.state = _.omit(JSON.parse(fs.readFileSync(filePath, 'utf8'), ['containers', 'pid'])); this.state.containers = {}; this.state.pid = process.pid; - let restartCounter = _.get(this.state, 'restartCounter', 0); + let restartCounter = _.get(this.state, 'restartCounter', 0); restartCounter++; this.state.restartCounter = restartCounter; } else { - this.state = initialState; + this.state = initialState; } } @@ -153,7 +153,7 @@ class Logger { * @param disableLog */ _writeNewState(disableLog = false) { - const filePath = `${__dirname}/state.json`; + const filePath = `${__dirname}/state.json`; const currentState = JSON.stringify(this.state); fs.writeFile(filePath, currentState, (err) => { if (err) { @@ -187,15 +187,15 @@ class Logger { * @param newContainer */ async _handleContainer(container) { // jshint ignore:line - const containerId = container.Id || container.id; - const containerStatus = container.Status || container.status; - const receivedLoggerId = _.get(container, 'Labels', _.get(container, 'Actor.Attributes'))['io.codefresh.logger.id']; - const runCreationLogic = _.get(container, 'Labels', _.get(container, 'Actor.Attributes'))['io.codefresh.runCreationLogic']; - const stepName = _.get(container, 'Labels', _.get(container, 'Actor.Attributes'))['io.codefresh.logger.stepName']; + const containerId = container.Id || container.id; + const containerStatus = container.Status || container.status; + const receivedLoggerId = _.get(container, 'Labels', _.get(container, 'Actor.Attributes'))['io.codefresh.logger.id']; + const runCreationLogic = _.get(container, 'Labels', _.get(container, 'Actor.Attributes'))['io.codefresh.runCreationLogic']; + const stepName = _.get(container, 'Labels', _.get(container, 'Actor.Attributes'))['io.codefresh.logger.stepName']; const receivedLogSizeLimit = _.get(container, 'Labels', _.get(container, 'Actor.Attributes'))['io.codefresh.logger.logSizeLimit']; - const loggerStrategy = _.get(container, 'Labels', _.get(container, 'Actor.Attributes'))['io.codefresh.logger.strategy']; + const loggerStrategy = _.get(container, 'Labels', _.get(container, 'Actor.Attributes'))['io.codefresh.logger.strategy']; if (!containerId) { logger.error(`Not handling container because id is missing`); @@ -248,7 +248,7 @@ class Logger { const logSizeLimit = receivedLogSizeLimit ? (parseInt(receivedLogSizeLimit, 10) * 1000000) : undefined; const containerInterface = this.docker.getContainer(containerId); - const containerLogger = new ContainerLogger({ + const containerLogger = new ContainerLogger({ containerId, containerInterface, stepLogger, @@ -275,8 +275,8 @@ class Logger { } _updateMissingLogs() { - const resolvedCalls = _.get(this, 'state.logsStatus.resolvedCalls', 0); const writeCalls = _.get(this, 'state.logsStatus.writeCalls', 0); + const resolvedCalls = _.get(this, 'state.logsStatus.resolvedCalls', 0); const rejectedCalls = _.get(this, 'state.logsStatus.rejectedCalls', 0); _.set(this, 'state.logsStatus.missingLogs', writeCalls - resolvedCalls - rejectedCalls); diff --git a/service.yaml b/service.yaml index ced61b7..bda5afc 100644 --- a/service.yaml +++ b/service.yaml @@ -1 +1 @@ -version: 1.4.7 +version: 1.4.8 \ No newline at end of file diff --git a/test/ContainerLogger.unit.spec.js b/test/ContainerLogger.unit.spec.js index 65f545e..354b7a4 100644 --- a/test/ContainerLogger.unit.spec.js +++ b/test/ContainerLogger.unit.spec.js @@ -1,13 +1,13 @@ 'use strict'; -const Q = require('q'); -const chai = require('chai'); -const expect = chai.expect; -const sinon = require('sinon'); +const Q = require('q'); +const chai = require('chai'); +const expect = chai.expect; +const sinon = require('sinon'); const sinonChai = require('sinon-chai'); chai.use(sinonChai); const ContainerLogger = require('../lib/ContainerLogger'); -const LoggerStrategy = require('../lib/enums').LoggerStrategy; +const LoggerStrategy = require('../lib/enums').LoggerStrategy; const { EventEmitter } = require('events'); const { Writable, Readable, PassThrough } = require('stream'); @@ -42,7 +42,7 @@ describe('Container Logger tests', () => { } }; - const containerId = 'containerId'; + const containerId = 'containerId'; const containerInterface = { inspect: (callback) => { callback(null, containerInspect); @@ -58,9 +58,9 @@ describe('Container Logger tests', () => { const stepLogger = { write: sinon.spy() }; - const loggerStrategy = LoggerStrategy.LOGS; + const loggerStrategy = LoggerStrategy.LOGS; - const containerLogger = new ContainerLogger({ + const containerLogger = new ContainerLogger({ containerId, containerInterface, stepLogger, loggerStrategy }); containerLogger._logMessage = sinon.spy(); @@ -96,7 +96,7 @@ describe('Container Logger tests', () => { } }; - const containerId = 'containerId'; + const containerId = 'containerId'; const containerInterface = { inspect: (callback) => { callback(null, containerInspect); @@ -112,9 +112,9 @@ describe('Container Logger tests', () => { const stepLogger = { write: sinon.spy() }; - const loggerStrategy = LoggerStrategy.LOGS; + const loggerStrategy = LoggerStrategy.LOGS; - const containerLogger = new ContainerLogger({ + const containerLogger = new ContainerLogger({ containerId, containerInterface, stepLogger, loggerStrategy }); containerLogger._logMessage = sinon.spy(); @@ -133,7 +133,7 @@ describe('Container Logger tests', () => { Tty: false } }; - const stream = { + const stream = { on: (event, callback) => { if (event === 'end') { callback(); @@ -141,7 +141,7 @@ describe('Container Logger tests', () => { } }; - const containerId = 'containerId'; + const containerId = 'containerId'; const containerInterface = { inspect: (callback) => { callback(null, containerInspect); @@ -150,12 +150,12 @@ describe('Container Logger tests', () => { callback(null, stream); } }; - const firebaseLogger = {}; + const firebaseLogger = {}; const firebaseLastUpdate = {}; const firebaseMetricsLogSize = {}; - const loggerStrategy = LoggerStrategy.LOGS; + const loggerStrategy = LoggerStrategy.LOGS; - const containerLogger = new ContainerLogger({ + const containerLogger = new ContainerLogger({ containerId, containerInterface, firebaseLogger, firebaseLastUpdate, firebaseMetricsLogSize, loggerStrategy }); containerLogger._logMessageToFirebase = sinon.spy(); @@ -168,21 +168,21 @@ describe('Container Logger tests', () => { describe('negative', () => { it('should fail in case the provided strategy is not supported', (done) => { - const containerInspect = { + const containerInspect = { Config: { Tty: true } }; - const containerId = 'containerId'; + const containerId = 'containerId'; const containerInterface = { inspect: (callback) => { callback(null, containerInspect); } }; - const firebaseLogger = {}; + const firebaseLogger = {}; const firebaseLastUpdate = {}; const firebaseMetricsLogSize = {}; - const loggerStrategy = 'non-existing-strategy'; + const loggerStrategy = 'non-existing-strategy'; const containerLogger = new ContainerLogger({ containerId, containerInterface, firebaseLogger, firebaseLastUpdate, firebaseMetricsLogSize, loggerStrategy @@ -199,15 +199,15 @@ describe('Container Logger tests', () => { }); it('should fail in case inspect of the container failes', (done) => { - const containerId = 'containerId'; + const containerId = 'containerId'; const containerInterface = { inspect: (callback) => { callback(new Error('inspect error')); } }; - const firebaseLogger = {}; + const firebaseLogger = {}; const firebaseLastUpdate = {}; - const loggerStrategy = LoggerStrategy.LOGS; + const loggerStrategy = LoggerStrategy.LOGS; const containerLogger = new ContainerLogger({ containerId, containerInterface, firebaseLogger, firebaseLastUpdate, loggerStrategy @@ -222,13 +222,13 @@ describe('Container Logger tests', () => { }); it('should fail in case strategy is attach and attach failed', (done) => { - const containerInspect = { + const containerInspect = { Config: { Tty: true } }; let receivedAttachOptions = []; - const containerId = 'containerId'; + const containerId = 'containerId'; const containerInterface = { inspect: (callback) => { callback(null, containerInspect); @@ -238,10 +238,10 @@ describe('Container Logger tests', () => { callback(new Error('attach error')); } }; - const firebaseLogger = {}; + const firebaseLogger = {}; const firebaseLastUpdate = {}; const firebaseMetricsLogSize = {}; - const loggerStrategy = LoggerStrategy.ATTACH; + const loggerStrategy = LoggerStrategy.ATTACH; const containerLogger = new ContainerLogger({ containerId, containerInterface, firebaseLogger, firebaseLastUpdate, firebaseMetricsLogSize, loggerStrategy @@ -261,13 +261,13 @@ describe('Container Logger tests', () => { }); it('should fail in case strategy is logs and logs failed', (done) => { - const containerInspect = { + const containerInspect = { Config: { Tty: true } }; let receivedLogsOptions = []; - const containerId = 'containerId'; + const containerId = 'containerId'; const containerInterface = { inspect: (callback) => { callback(null, containerInspect); @@ -277,10 +277,10 @@ describe('Container Logger tests', () => { callback(new Error('logs error')); } }; - const firebaseLogger = {}; + const firebaseLogger = {}; const firebaseLastUpdate = {}; const firebaseMetricsLogSize = {}; - const loggerStrategy = LoggerStrategy.LOGS; + const loggerStrategy = LoggerStrategy.LOGS; const containerLogger = new ContainerLogger({ containerId, containerInterface, firebaseLogger, firebaseLastUpdate, firebaseMetricsLogSize, loggerStrategy @@ -290,7 +290,7 @@ describe('Container Logger tests', () => { return Q.reject(new Error('should have failed')); }, (err) => { receivedLogsOptions.forEach((options) => { - expect(options.follow).to.equal(1); + expect(options.follow).to.equal(true); }); expect(err.toString()).to.contain('logs error'); }) @@ -305,10 +305,10 @@ describe('Container Logger tests', () => { it('should log message to firebase', () => { - const containerId = 'containerId'; + const containerId = 'containerId'; const containerInterface = {}; - const loggerStrategy = LoggerStrategy.LOGS; + const loggerStrategy = LoggerStrategy.LOGS; const isWorkflowLogSizeExceeded = sinon.spy(() => { return false; }); @@ -326,10 +326,10 @@ describe('Container Logger tests', () => { it('should log error to firebase with red decoration', () => { - const containerId = 'containerId'; + const containerId = 'containerId'; const containerInterface = {}; - const loggerStrategy = LoggerStrategy.LOGS; + const loggerStrategy = LoggerStrategy.LOGS; const isWorkflowLogSizeExceeded = sinon.spy(() => { return false; }); @@ -347,11 +347,11 @@ describe('Container Logger tests', () => { }); it('should not log message to firebase in case limit of step reached', () => { - const containerId = 'containerId'; + const containerId = 'containerId'; const containerInterface = {}; - const loggerStrategy = LoggerStrategy.LOGS; - const logSizeLimit = 1000; + const loggerStrategy = LoggerStrategy.LOGS; + const logSizeLimit = 1000; const isWorkflowLogSizeExceeded = sinon.spy(() => { return false; }); @@ -375,11 +375,11 @@ describe('Container Logger tests', () => { }); it('should not log message to firebase in case limit of workflow reached and step not yet', () => { - const containerId = 'containerId'; + const containerId = 'containerId'; const containerInterface = {}; - const loggerStrategy = LoggerStrategy.LOGS; - const logSizeLimit = 1000; + const loggerStrategy = LoggerStrategy.LOGS; + const logSizeLimit = 1000; const isWorkflowLogSizeExceeded = sinon.spy(() => { return true; }); @@ -403,11 +403,11 @@ describe('Container Logger tests', () => { }); it('should print warning message to user in case of reaching the limit of step', () => { - const containerId = 'containerId'; + const containerId = 'containerId'; const containerInterface = {}; - const loggerStrategy = LoggerStrategy.LOGS; - const logSizeLimit = 1000; + const loggerStrategy = LoggerStrategy.LOGS; + const logSizeLimit = 1000; const isWorkflowLogSizeExceeded = sinon.spy(() => { return false; }); @@ -431,11 +431,11 @@ describe('Container Logger tests', () => { }); it('should print warning message to user in case of reaching the limit of workflow', () => { - const containerId = 'containerId'; + const containerId = 'containerId'; const containerInterface = {}; - const loggerStrategy = LoggerStrategy.LOGS; - const logSizeLimit = 1000; + const loggerStrategy = LoggerStrategy.LOGS; + const logSizeLimit = 1000; const isWorkflowLogSizeExceeded = sinon.spy(() => { return true; }); @@ -459,11 +459,11 @@ describe('Container Logger tests', () => { }); it('should not print the warning message more than once', () => { - const containerId = 'containerId'; + const containerId = 'containerId'; const containerInterface = {}; - const loggerStrategy = LoggerStrategy.LOGS; - const logSizeLimit = 1000; + const loggerStrategy = LoggerStrategy.LOGS; + const logSizeLimit = 1000; const isWorkflowLogSizeExceeded = sinon.spy(() => { return true; }); @@ -488,11 +488,11 @@ describe('Container Logger tests', () => { }); it('should print error messages even if reached the limit', () => { - const containerId = 'containerId'; + const containerId = 'containerId'; const containerInterface = {}; - const loggerStrategy = LoggerStrategy.LOGS; - const logSizeLimit = 1000; + const loggerStrategy = LoggerStrategy.LOGS; + const logSizeLimit = 1000; const isWorkflowLogSizeExceeded = sinon.spy(() => { return false; }); @@ -516,11 +516,11 @@ describe('Container Logger tests', () => { }); it('should emit an event each time a log is registered', () => { - const containerId = 'containerId'; + const containerId = 'containerId'; const containerInterface = {}; - const loggerStrategy = LoggerStrategy.LOGS; - const logSizeLimit = 1000; + const loggerStrategy = LoggerStrategy.LOGS; + const logSizeLimit = 1000; const isWorkflowLogSizeExceeded = sinon.spy(() => { return false; }); @@ -549,12 +549,12 @@ describe('Container Logger tests', () => { describe('log size', () => { it('log size should stay 0 in case of not passing logSizeLimit param', () => { - const containerId = 'containerId'; + const containerId = 'containerId'; const containerInterface = {}; - const pushSpy = sinon.spy(); - const setSpy = sinon.spy(); - const firebaseLogger = { + const pushSpy = sinon.spy(); + const setSpy = sinon.spy(); + const firebaseLogger = { push: pushSpy }; const firebaseLastUpdate = { @@ -567,7 +567,7 @@ describe('Container Logger tests', () => { }; }) }; - const loggerStrategy = LoggerStrategy.LOGS; + const loggerStrategy = LoggerStrategy.LOGS; const containerLogger = new ContainerLogger({ containerId, containerInterface, firebaseLogger, firebaseLastUpdate, firebaseMetricsLogs, loggerStrategy @@ -577,12 +577,12 @@ describe('Container Logger tests', () => { it('log size should be 0 before any messages', () => { - const containerId = 'containerId'; + const containerId = 'containerId'; const containerInterface = {}; - const pushSpy = sinon.spy(); - const setSpy = sinon.spy(); - const firebaseLogger = { + const pushSpy = sinon.spy(); + const setSpy = sinon.spy(); + const firebaseLogger = { push: pushSpy }; const firebaseLastUpdate = { @@ -595,8 +595,8 @@ describe('Container Logger tests', () => { }; }) }; - const loggerStrategy = LoggerStrategy.LOGS; - const logSizeLimit = 1000; + const loggerStrategy = LoggerStrategy.LOGS; + const logSizeLimit = 1000; const containerLogger = new ContainerLogger({ containerId, containerInterface, firebaseLogger, firebaseLastUpdate, firebaseMetricsLogs, loggerStrategy, logSizeLimit @@ -606,11 +606,11 @@ describe('Container Logger tests', () => { it('log size should increase after adding a message', () => { - const containerId = 'containerId'; + const containerId = 'containerId'; const containerInterface = {}; - const loggerStrategy = LoggerStrategy.LOGS; - const logSizeLimit = 1000; + const loggerStrategy = LoggerStrategy.LOGS; + const logSizeLimit = 1000; const isWorkflowLogSizeExceeded = sinon.spy(() => { return false; }); @@ -634,11 +634,11 @@ describe('Container Logger tests', () => { it('total log size should be updated in firebase', () => { - const containerId = 'containerId'; + const containerId = 'containerId'; const containerInterface = {}; - const loggerStrategy = LoggerStrategy.LOGS; - const logSizeLimit = 1000; + const loggerStrategy = LoggerStrategy.LOGS; + const logSizeLimit = 1000; const isWorkflowLogSizeExceeded = sinon.spy(() => { return false; }); @@ -700,7 +700,7 @@ describe('Container Logger tests', () => { containerLogger.once('end', () => { endEventCalled = true; }); containerLogger._logMessage = sinon.spy(); await containerLogger.start(); - + expect(containerLogger.handledStreams).to.be.equal(2); expect(containerLogger.finishedStreams).to.be.equal(0); expect(endEventCalled).to.be.false; @@ -745,7 +745,7 @@ describe('Container Logger tests', () => { containerLogger.once('end', () => { endEventCalled = true; }); containerLogger._logMessage = sinon.spy(); await containerLogger.start(); - + expect(containerLogger.handledStreams).to.be.equal(1); expect(containerLogger.finishedStreams).to.be.equal(0); expect(endEventCalled).to.be.false; @@ -832,7 +832,7 @@ describe('Container Logger tests', () => { containerLogger._logMessage = sinon.spy(); await containerLogger.start(); await Q.delay(20); // incase the piping is not finished - + expect(stepLogger.writeStream).to.have.been.calledOnce; expect(containerLogger.handledStreams).to.be.equal(2); expect(transformSpies[0].pipe).to.have.been.calledOnceWith(writableSpy, { end: false });