feat(ecs): allow to specify log retention for aws log driver (#2511)

Allow to specify a log retention when using the AWS Log Driver with an automatically created group. Also add tests for the driver.
aws · May 10, 2019 · 1feda0c · 1feda0c
1 parent 6cf4bd3
commit 1feda0c
Show file tree

Hide file tree

Showing 7 changed files with 97 additions and 16 deletions.
diff --git a/packages/@aws-cdk/aws-ecs/lib/log-drivers/aws-log-driver.ts b/packages/@aws-cdk/aws-ecs/lib/log-drivers/aws-log-driver.ts
@@ -27,6 +27,14 @@ export interface AwsLogDriverProps {
    */
   readonly logGroup?: logs.ILogGroup;
 
+  /**
+   * The number of days log events are kept in CloudWatch Logs when the log
+   * group is automatically created by this construct.
+   *
+   * @default logs never expire
+   */
+  readonly logRetentionDays?: logs.RetentionDays;
+
   /**
    * This option defines a multiline start pattern in Python strftime format.
    *
@@ -57,8 +65,13 @@ export class AwsLogDriver extends LogDriver {
 
   constructor(scope: cdk.Construct, id: string, private readonly props: AwsLogDriverProps) {
     super(scope, id);
+
+    if (props.logGroup && props.logRetentionDays) {
+      throw new Error('Cannot specify both `logGroup` and `logRetentionDays`.');
+    }
+
     this.logGroup = props.logGroup || new logs.LogGroup(this, 'LogGroup', {
-        retentionDays: 365,
+        retentionDays: props.logRetentionDays || Infinity,
     });
   }
 

diff --git a/packages/@aws-cdk/aws-ecs/test/ec2/integ.event-task.lit.expected.json b/packages/@aws-cdk/aws-ecs/test/ec2/integ.event-task.lit.expected.json
@@ -1027,9 +1027,6 @@
     },
     "TaskLoggingLogGroupC7E938D4": {
       "Type": "AWS::Logs::LogGroup",
-      "Properties": {
-        "RetentionInDays": 365
-      },
       "DeletionPolicy": "Retain"
     },
     "Rule4C995B7F": {

diff --git a/packages/@aws-cdk/aws-ecs/test/fargate/integ.asset-image.expected.json b/packages/@aws-cdk/aws-ecs/test/fargate/integ.asset-image.expected.json
@@ -904,9 +904,6 @@
     },
     "FargateServiceLoggingLogGroup9B16742A": {
       "Type": "AWS::Logs::LogGroup",
-      "Properties": {
-        "RetentionInDays": 365
-      },
       "DeletionPolicy": "Retain"
     },
     "FargateServiceECC8084D": {

diff --git a/packages/@aws-cdk/aws-ecs/test/fargate/integ.l3.expected.json b/packages/@aws-cdk/aws-ecs/test/fargate/integ.l3.expected.json
@@ -585,9 +585,6 @@
     },
     "L3LoggingLogGroupBD1F02DD": {
       "Type": "AWS::Logs::LogGroup",
-      "Properties": {
-        "RetentionInDays": 365
-      },
       "DeletionPolicy": "Retain"
     },
     "L3Service616D5A93": {

diff --git a/packages/@aws-cdk/aws-ecs/test/test.aws-log-driver.ts b/packages/@aws-cdk/aws-ecs/test/test.aws-log-driver.ts
@@ -0,0 +1,83 @@
+import { expect, haveResource } from '@aws-cdk/assert';
+import logs = require('@aws-cdk/aws-logs');
+import cdk = require('@aws-cdk/cdk');
+import { Test } from 'nodeunit';
+import ecs = require('../lib');
+
+export = {
+  'create an aws log driver'(test: Test) {
+    // GIVEN
+    const stack = new cdk.Stack();
+
+    // WHEN
+    const driver = new ecs.AwsLogDriver(stack, 'Log', {
+      datetimeFormat: 'format',
+      logRetentionDays: logs.RetentionDays.OneMonth,
+      multilinePattern: 'pattern',
+      streamPrefix: 'hello'
+    });
+
+    // THEN
+    expect(stack).to(haveResource('AWS::Logs::LogGroup', {
+      RetentionInDays: logs.RetentionDays.OneMonth
+    }));
+
+    test.deepEqual(
+      stack.node.resolve(driver.renderLogDriver()),
+      {
+        logDriver: 'awslogs',
+        options: {
+          'awslogs-group': { Ref: 'LogLogGroup427F779C' },
+          'awslogs-stream-prefix': 'hello',
+          'awslogs-region': { Ref: 'AWS::Region' },
+          'awslogs-datetime-format': 'format',
+          'awslogs-multiline-pattern': 'pattern'
+        }
+      }
+    );
+
+    test.done();
+  },
+
+  'with a defined log group'(test: Test) {
+    // GIVEN
+    const stack = new cdk.Stack();
+    const logGroup = new logs.LogGroup(stack, 'LogGroup');
+
+    // WHEN
+    const driver = new ecs.AwsLogDriver(stack, 'Log', {
+      logGroup,
+      streamPrefix: 'hello'
+    });
+
+    // THEN
+    test.deepEqual(
+      stack.node.resolve(driver.renderLogDriver()),
+      {
+        logDriver: 'awslogs',
+        options: {
+          'awslogs-group': { Ref: 'LogGroupF5B46931' },
+          'awslogs-stream-prefix': 'hello',
+          'awslogs-region': { Ref: 'AWS::Region' }
+        }
+      }
+    );
+
+    test.done();
+  },
+
+  'throws when specifying log retention and log group'(test: Test) {
+    // GIVEN
+    const stack = new cdk.Stack();
+    const logGroup = new logs.LogGroup(stack, 'LogGroup');
+
+    // THEN
+    test.throws(() => new ecs.AwsLogDriver(stack, 'Log', {
+      logGroup,
+      logRetentionDays: logs.RetentionDays.FiveDays,
+      streamPrefix: 'hello'
+    }), /`logGroup`.*`logRetentionDays`/);
+
+    test.done();
+  }
+};
diff --git a/packages/@aws-cdk/aws-stepfunctions-tasks/test/integ.ec2-task.expected.json b/packages/@aws-cdk/aws-stepfunctions-tasks/test/integ.ec2-task.expected.json
@@ -835,9 +835,6 @@
     },
     "TaskLoggingLogGroupC7E938D4": {
       "Type": "AWS::Logs::LogGroup",
-      "Properties": {
-        "RetentionInDays": 365
-      },
       "DeletionPolicy": "Retain"
     },
     "StateMachineRoleB840431D": {

diff --git a/packages/@aws-cdk/aws-stepfunctions-tasks/test/integ.fargate-task.expected.json b/packages/@aws-cdk/aws-stepfunctions-tasks/test/integ.fargate-task.expected.json
@@ -457,9 +457,6 @@
     },
     "TaskLoggingLogGroupC7E938D4": {
       "Type": "AWS::Logs::LogGroup",
-      "Properties": {
-        "RetentionInDays": 365
-      },
       "DeletionPolicy": "Retain"
     },
     "FargateTaskSecurityGroup0BBB27CB": {