From 566d2b9bf0d6095ea8559f9e8736d6ac405005ab Mon Sep 17 00:00:00 2001 From: John Jones Date: Wed, 12 Jun 2024 13:49:17 -0700 Subject: [PATCH 1/3] [INFREQ-643] Warn on RunTask failures --- index.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/index.js b/index.js index 8ce4752..eb58ddd 100644 --- a/index.js +++ b/index.js @@ -74,6 +74,10 @@ module.exports = function (options, cb) { return taskRunner.runPromisified(params); }) .then((taskDefinition) => { + if (taskDefinition.failures) { + throw new Error("ECS RunTask returned failure messages", { cause: taskDefinition.failures }); + } + const taskArn = taskDefinition.tasks[0].taskArn; const taskId = taskArn.substring(taskArn.lastIndexOf('/') + 1); const formatter = new FormatStream(); From 737aaf4e2b17f58e70aca1f0909860808909a597 Mon Sep 17 00:00:00 2001 From: John Jones Date: Wed, 12 Jun 2024 19:34:12 -0700 Subject: [PATCH 2/3] add extra logging and unit tests for this --- .eslintrc.json | 2 +- index.js | 2 + package.json | 2 +- test/index.js | 111 +++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 115 insertions(+), 2 deletions(-) create mode 100644 test/index.js diff --git a/.eslintrc.json b/.eslintrc.json index 8c6a277..eb6c782 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -6,7 +6,7 @@ }, "extends": "eslint:recommended", "parserOptions": { - "ecmaVersion": 6, + "ecmaVersion": 8, "sourceType": "script" }, "rules": { diff --git a/index.js b/index.js index eb58ddd..9964c40 100644 --- a/index.js +++ b/index.js @@ -75,6 +75,8 @@ module.exports = function (options, cb) { }) .then((taskDefinition) => { if (taskDefinition.failures) { + console.error("Task failed to launch:") + console.error(JSON.stringify(taskDefinition.failures)) throw new Error("ECS RunTask returned failure messages", { cause: taskDefinition.failures }); } diff --git a/package.json b/package.json index 5291a53..038f1f3 100644 --- a/package.json +++ b/package.json @@ -10,7 +10,7 @@ "ecs-task-runner": "./bin/ecs-task-runner" }, "scripts": { - "test": "NODE_ENV=test ./node_modules/.bin/mocha --recursive" + "test": "NODE_ENV=test ./node_modules/.bin/mocha --parallel --recursive" }, "repository": { "type": "git", diff --git a/test/index.js b/test/index.js new file mode 100644 index 0000000..7f8400c --- /dev/null +++ b/test/index.js @@ -0,0 +1,111 @@ +'use strict' + +const { mockClient } = require('aws-sdk-client-mock'); +const { ECS, DescribeTaskDefinitionCommand, RunTaskCommand } = require("@aws-sdk/client-ecs"); +const { CloudWatchLogs, GetLogEventsCommand } = require("@aws-sdk/client-cloudwatch-logs"); +const expect = require('expect.js'); +const { promisify } = require('node:util'); +const index = promisify(require('../index')); + +describe('index', function () { + const cwlMock = mockClient(CloudWatchLogs); + const ecsMock = mockClient(ECS); + afterEach(() => { + cwlMock.reset(); + ecsMock.reset(); + }); + + it('should do the thing', async function () { + const options = { + taskDefinitionArn: 'task-definition.arn', + containerName: 'meow' + }; + + ecsMock.on(DescribeTaskDefinitionCommand).callsFake(async params => { + expect(params.taskDefinition).to.eql(options.taskDefinitionArn) + + return { + taskDefinition: { + taskDefinitionArn: options.taskDefinitionArn, + containerDefinitions: [{ + name: options.containerName, + logConfiguration: { + logDriver: 'awslogs', + options: { 'awslogs-group': '', 'awslogs-stream-prefix': '' }, + } + }], + } + } + }); + + // thread the randon EOF through to kill the stream + let eofSet; + const eof = new Promise(r => { + eofSet = r; + }) + ecsMock.on(RunTaskCommand).callsFake(async params => { + expect(params.taskDefinition).to.eql(options.taskDefinitionArn) + + eofSet(params.overrides.containerOverrides[0].command[2].split(' ')[9]) + + return { + tasks: [{ + taskArn: '' + }] + } + }); + + cwlMock.on(GetLogEventsCommand).callsFake(async _params => { + return { + events: [{ + timestamp: 1477346285562, + message: Buffer.from(await eof).toString('base64'), + }] + }; + }); + + // if this returns without crashing we're gucci + return index(options); + }); + + + it('should warn us when a task fails to launch', async function () { + const options = { + taskDefinitionArn: 'task-definition.arn', + containerName: 'meow' + }; + + ecsMock.on(DescribeTaskDefinitionCommand).callsFake(async params => { + expect(params.taskDefinition).to.eql(options.taskDefinitionArn) + + return { + taskDefinition: { + taskDefinitionArn: options.taskDefinitionArn, + containerDefinitions: [{ + name: options.containerName, + logConfiguration: { + logDriver: 'awslogs', + options: { 'awslogs-group': '', 'awslogs-stream-prefix': '' }, + } + }], + } + } + }); + + const reason = 'ECS is haunted' + ecsMock.on(RunTaskCommand).callsFake(async params => { + expect(params.taskDefinition).to.eql(options.taskDefinitionArn) + + return { + failures: [{ reason }] + } + }); + + try { + await index(options); + expect().fail("App should have thrown an error about ECS returning errors") + } catch (err) { + expect(err.cause[0].reason).to.eql(reason) + } + }); +}); From 50fdb1cfc57eeb7ecfb2e2923fceb00abbb90f67 Mon Sep 17 00:00:00 2001 From: John Jones Date: Thu, 13 Jun 2024 11:59:29 -0700 Subject: [PATCH 3/3] PR feedback, lint, and fix weird behavior with mocha and aws-sdk-client-mock --- index.js | 2 -- package.json | 2 +- test/format-transform-stream.js | 2 +- test/index.js | 11 ++++++----- test/log-stream.js | 6 ++++-- test/taskrunner.js | 10 ++++++---- 6 files changed, 18 insertions(+), 15 deletions(-) diff --git a/index.js b/index.js index 9964c40..eb58ddd 100644 --- a/index.js +++ b/index.js @@ -75,8 +75,6 @@ module.exports = function (options, cb) { }) .then((taskDefinition) => { if (taskDefinition.failures) { - console.error("Task failed to launch:") - console.error(JSON.stringify(taskDefinition.failures)) throw new Error("ECS RunTask returned failure messages", { cause: taskDefinition.failures }); } diff --git a/package.json b/package.json index 038f1f3..5291a53 100644 --- a/package.json +++ b/package.json @@ -10,7 +10,7 @@ "ecs-task-runner": "./bin/ecs-task-runner" }, "scripts": { - "test": "NODE_ENV=test ./node_modules/.bin/mocha --parallel --recursive" + "test": "NODE_ENV=test ./node_modules/.bin/mocha --recursive" }, "repository": { "type": "git", diff --git a/test/format-transform-stream.js b/test/format-transform-stream.js index 95611df..a62de23 100644 --- a/test/format-transform-stream.js +++ b/test/format-transform-stream.js @@ -1,6 +1,6 @@ 'use strict' -const expect = require('expect.js'); +const expect = require('expect.js'); const FormatTransformStream = require('../lib/format-transform-stream'); describe('FormatTransformStream', function() { diff --git a/test/index.js b/test/index.js index 7f8400c..69e2b52 100644 --- a/test/index.js +++ b/test/index.js @@ -8,11 +8,11 @@ const { promisify } = require('node:util'); const index = promisify(require('../index')); describe('index', function () { - const cwlMock = mockClient(CloudWatchLogs); - const ecsMock = mockClient(ECS); - afterEach(() => { - cwlMock.reset(); - ecsMock.reset(); + let cwlMock; + let ecsMock; + beforeEach(() => { + cwlMock = mockClient(CloudWatchLogs); + ecsMock = mockClient(ECS); }); it('should do the thing', async function () { @@ -46,6 +46,7 @@ describe('index', function () { ecsMock.on(RunTaskCommand).callsFake(async params => { expect(params.taskDefinition).to.eql(options.taskDefinitionArn) + // Grab the `xxx` from ..."TASK FINISHED: $(echo -n xxx | base64),"... eofSet(params.overrides.containerOverrides[0].command[2].split(' ')[9]) return { diff --git a/test/log-stream.js b/test/log-stream.js index f92c9eb..3261288 100644 --- a/test/log-stream.js +++ b/test/log-stream.js @@ -7,8 +7,10 @@ const LogsStream = require('../lib/log-stream'); describe('LogStream', function () { this.timeout(5000); - const cwlMock = mockClient(CloudWatchLogs); - afterEach(() => { cwlMock.reset(); }); + let cwlMock ; + beforeEach(() => { + cwlMock = mockClient(CloudWatchLogs); + }); const options = { logGroup: 'yee', diff --git a/test/taskrunner.js b/test/taskrunner.js index 26f9ed0..5bfa70b 100644 --- a/test/taskrunner.js +++ b/test/taskrunner.js @@ -1,7 +1,7 @@ 'use strict' const { mockClient } = require('aws-sdk-client-mock'); -const { ECS, StartTaskCommand, StopTaskCommand, RunTaskCommand } = require("@aws-sdk/client-ecs"); +const { ECS, StopTaskCommand, RunTaskCommand } = require("@aws-sdk/client-ecs"); const expect = require('expect.js'); const taskRunner = require('../lib/taskrunner'); @@ -21,8 +21,10 @@ describe('TaskRunner', function () { }) describe('#run', function () { - const ecsMock = mockClient(ECS); - afterEach(() => { ecsMock.reset(); }); + let ecsMock; + beforeEach(() => { + ecsMock = mockClient(ECS); + }); it('should make a call to AWS.ECS with correct arguments not including env', function (done) { const options = { @@ -83,7 +85,7 @@ describe('TaskRunner', function () { describe('#stop', function () { const ecsMock = mockClient(ECS); - afterEach(() => { ecsMock.reset(); }); + beforeEach(() => { ecsMock.reset(); }); it('should make a call to AWS.ECS with correct arguments', function (done) { const options = {