refactor(ecs): update LogDriver to new bind pattern (#2990)

Update LogDriver bind in accordance with how we do binds in the rest of the library. BREAKING CHANGES: * **ecs**: `AwsLogDriver` no longer takes construct properties.
aws · Jun 21, 2019 · ad72271 · ad72271
1 parent b662188
commit ad72271
Show file tree

Hide file tree

Showing 24 changed files with 143 additions and 109 deletions.
diff --git a/packages/@aws-cdk/aws-ecs-patterns/lib/base/load-balanced-service-base.ts b/packages/@aws-cdk/aws-ecs-patterns/lib/base/load-balanced-service-base.ts
@@ -185,6 +185,6 @@ export abstract class LoadBalancedServiceBase extends cdk.Construct {
   }
 
   private createAWSLogDriver(prefix: string): ecs.AwsLogDriver {
-    return new ecs.AwsLogDriver(this, 'Logging', { streamPrefix: prefix });
+    return new ecs.AwsLogDriver({ streamPrefix: prefix });
   }
 }
diff --git a/packages/@aws-cdk/aws-ecs-patterns/lib/base/queue-processing-service-base.ts b/packages/@aws-cdk/aws-ecs-patterns/lib/base/queue-processing-service-base.ts
@@ -153,6 +153,6 @@ export abstract class QueueProcessingServiceBase extends cdk.Construct {
    * @param prefix the Cloudwatch logging prefix
    */
   private createAWSLogDriver(prefix: string): ecs.AwsLogDriver {
-    return new ecs.AwsLogDriver(this, 'ProcessingContainerLogging', { streamPrefix: prefix });
+    return new ecs.AwsLogDriver({ streamPrefix: prefix });
   }
 }
diff --git a/packages/@aws-cdk/aws-ecs-patterns/lib/ecs/scheduled-ecs-task.ts b/packages/@aws-cdk/aws-ecs-patterns/lib/ecs/scheduled-ecs-task.ts
@@ -94,7 +94,7 @@ export class ScheduledEc2Task extends cdk.Construct {
       cpu: props.cpu,
       command: props.command,
       environment: props.environment,
-      logging: new ecs.AwsLogDriver(this, 'ScheduledTaskLogging', { streamPrefix: this.node.id })
+      logging: new ecs.AwsLogDriver({ streamPrefix: this.node.id })
     });
 
     // Use Ec2TaskEventRuleTarget as the target of the EventRule

diff --git a/packages/@aws-cdk/aws-ecs-patterns/test/ec2/integ.scheduled-ecs-task.lit.expected.json b/packages/@aws-cdk/aws-ecs-patterns/test/ec2/integ.scheduled-ecs-task.lit.expected.json
@@ -664,7 +664,7 @@
               "LogDriver": "awslogs",
               "Options": {
                 "awslogs-group": {
-                  "Ref": "ScheduledEc2TaskScheduledTaskLoggingLogGroupDBEF2BF3"
+                  "Ref": "ScheduledEc2TaskScheduledTaskDefScheduledContainerLogGroupA85E11E6"
                 },
                 "awslogs-stream-prefix": "ScheduledEc2Task",
                 "awslogs-region": {
@@ -740,7 +740,7 @@
               "Effect": "Allow",
               "Resource": {
                 "Fn::GetAtt": [
-                  "ScheduledEc2TaskScheduledTaskLoggingLogGroupDBEF2BF3",
+                  "ScheduledEc2TaskScheduledTaskDefScheduledContainerLogGroupA85E11E6",
                   "Arn"
                 ]
               }
@@ -826,7 +826,7 @@
         ]
       }
     },
-    "ScheduledEc2TaskScheduledTaskLoggingLogGroupDBEF2BF3": {
+    "ScheduledEc2TaskScheduledTaskDefScheduledContainerLogGroupA85E11E6": {
       "Type": "AWS::Logs::LogGroup",
       "DeletionPolicy": "Retain"
     },
@@ -868,4 +868,4 @@
       "Default": "/aws/service/ecs/optimized-ami/amazon-linux/recommended/image_id"
     }
   }
-}
+}
diff --git a/packages/@aws-cdk/aws-ecs-patterns/test/ec2/test.l3s.ts b/packages/@aws-cdk/aws-ecs-patterns/test/ec2/test.l3s.ts
@@ -119,7 +119,7 @@ export = {
           LogConfiguration: {
             LogDriver: "awslogs",
             Options: {
-              "awslogs-group": { Ref: "ServiceLoggingLogGroupC3D6A581" },
+              "awslogs-group": { Ref: "ServiceTaskDefwebLogGroup2A898F61" },
               "awslogs-stream-prefix": "Service",
               "awslogs-region": { Ref: "AWS::Region" }
             }
@@ -175,7 +175,7 @@ export = {
           LogConfiguration: {
             LogDriver: "awslogs",
             Options: {
-              "awslogs-group": { Ref: "ServiceLoggingLogGroupC3D6A581" },
+              "awslogs-group": { Ref: "ServiceTaskDefwebLogGroup2A898F61" },
               "awslogs-stream-prefix": "Service",
               "awslogs-region": { Ref: "AWS::Region" }
             }

diff --git a/packages/@aws-cdk/aws-ecs-patterns/test/ec2/test.queue-processing-ecs-service.ts b/packages/@aws-cdk/aws-ecs-patterns/test/ec2/test.queue-processing-ecs-service.ts
@@ -47,7 +47,7 @@ export = {
             LogDriver: "awslogs",
             Options: {
               "awslogs-group": {
-                Ref: "ServiceProcessingContainerLoggingLogGroupF40B9C5D"
+                Ref: "ServiceQueueProcessingTaskDefQueueProcessingContainerLogGroupD52338D1"
               },
               "awslogs-stream-prefix": "Service",
               "awslogs-region": {

diff --git a/packages/@aws-cdk/aws-ecs-patterns/test/ec2/test.scheduled-ecs-task.ts b/packages/@aws-cdk/aws-ecs-patterns/test/ec2/test.scheduled-ecs-task.ts
@@ -49,7 +49,7 @@ export = {
             LogDriver: "awslogs",
             Options: {
               "awslogs-group": {
-                Ref: "ScheduledEc2TaskScheduledTaskLoggingLogGroupDBEF2BF3"
+                Ref: "ScheduledEc2TaskScheduledTaskDefScheduledContainerLogGroupA85E11E6"
               },
               "awslogs-stream-prefix": "ScheduledEc2Task",
               "awslogs-region": {
@@ -126,7 +126,7 @@ export = {
             LogDriver: "awslogs",
             Options: {
               "awslogs-group": {
-                Ref: "ScheduledEc2TaskScheduledTaskLoggingLogGroupDBEF2BF3"
+                Ref: "ScheduledEc2TaskScheduledTaskDefScheduledContainerLogGroupA85E11E6"
               },
               "awslogs-stream-prefix": "ScheduledEc2Task",
               "awslogs-region": {
@@ -174,7 +174,7 @@ export = {
             LogDriver: "awslogs",
             Options: {
               "awslogs-group": {
-                Ref: "ScheduledEc2TaskScheduledTaskLoggingLogGroupDBEF2BF3"
+                Ref: "ScheduledEc2TaskScheduledTaskDefScheduledContainerLogGroupA85E11E6"
               },
               "awslogs-stream-prefix": "ScheduledEc2Task",
               "awslogs-region": {
@@ -228,7 +228,7 @@ export = {
             LogDriver: "awslogs",
             Options: {
               "awslogs-group": {
-                Ref: "ScheduledEc2TaskScheduledTaskLoggingLogGroupDBEF2BF3"
+                Ref: "ScheduledEc2TaskScheduledTaskDefScheduledContainerLogGroupA85E11E6"
               },
               "awslogs-stream-prefix": "ScheduledEc2Task",
               "awslogs-region": {

diff --git a/packages/@aws-cdk/aws-ecs-patterns/test/fargate/integ.asset-image.expected.json b/packages/@aws-cdk/aws-ecs-patterns/test/fargate/integ.asset-image.expected.json
@@ -346,7 +346,7 @@
     "ClusterEB0386A7": {
       "Type": "AWS::ECS::Cluster"
     },
-    "FargateServiceLoggingLogGroup9B16742A": {
+    "FargateServiceTaskDefwebLogGroup71FAF541": {
       "Type": "AWS::Logs::LogGroup",
       "DeletionPolicy": "Retain"
     },
@@ -589,7 +589,7 @@
               "LogDriver": "awslogs",
               "Options": {
                 "awslogs-group": {
-                  "Ref": "FargateServiceLoggingLogGroup9B16742A"
+                  "Ref": "FargateServiceTaskDefwebLogGroup71FAF541"
                 },
                 "awslogs-stream-prefix": "FargateService",
                 "awslogs-region": {
@@ -739,7 +739,7 @@
               "Effect": "Allow",
               "Resource": {
                 "Fn::GetAtt": [
-                  "FargateServiceLoggingLogGroup9B16742A",
+                  "FargateServiceTaskDefwebLogGroup71FAF541",
                   "Arn"
                 ]
               }

diff --git a/packages/@aws-cdk/aws-ecs-patterns/test/fargate/integ.executionrole.expected.json b/packages/@aws-cdk/aws-ecs-patterns/test/fargate/integ.executionrole.expected.json
@@ -399,7 +399,7 @@
               "Effect": "Allow",
               "Resource": {
                 "Fn::GetAtt": [
-                  "L3LoggingLogGroupBD1F02DD",
+                  "L3TaskDefwebLogGroupC6E4A38A",
                   "Arn"
                 ]
               }
@@ -554,7 +554,7 @@
               "LogDriver": "awslogs",
               "Options": {
                 "awslogs-group": {
-                  "Ref": "L3LoggingLogGroupBD1F02DD"
+                  "Ref": "L3TaskDefwebLogGroupC6E4A38A"
                 },
                 "awslogs-stream-prefix": "L3",
                 "awslogs-region": {
@@ -596,7 +596,7 @@
         "Volumes": []
       }
     },
-    "L3LoggingLogGroupBD1F02DD": {
+    "L3TaskDefwebLogGroupC6E4A38A": {
       "Type": "AWS::Logs::LogGroup",
       "DeletionPolicy": "Retain"
     },

diff --git a/packages/@aws-cdk/aws-ecs-patterns/test/fargate/integ.l3.expected.json b/packages/@aws-cdk/aws-ecs-patterns/test/fargate/integ.l3.expected.json
@@ -485,7 +485,7 @@
               "LogDriver": "awslogs",
               "Options": {
                 "awslogs-group": {
-                  "Ref": "L3LoggingLogGroupBD1F02DD"
+                  "Ref": "L3TaskDefwebLogGroupC6E4A38A"
                 },
                 "awslogs-stream-prefix": "L3",
                 "awslogs-region": {
@@ -567,7 +567,7 @@
               "Effect": "Allow",
               "Resource": {
                 "Fn::GetAtt": [
-                  "L3LoggingLogGroupBD1F02DD",
+                  "L3TaskDefwebLogGroupC6E4A38A",
                   "Arn"
                 ]
               }
@@ -583,7 +583,7 @@
         ]
       }
     },
-    "L3LoggingLogGroupBD1F02DD": {
+    "L3TaskDefwebLogGroupC6E4A38A": {
       "Type": "AWS::Logs::LogGroup",
       "DeletionPolicy": "Retain"
     },

diff --git a/packages/@aws-cdk/aws-ecs-patterns/test/fargate/test.queue-processing-fargate-service.ts b/packages/@aws-cdk/aws-ecs-patterns/test/fargate/test.queue-processing-fargate-service.ts
@@ -47,7 +47,7 @@ export = {
             LogDriver: "awslogs",
             Options: {
               "awslogs-group": {
-                Ref: "ServiceProcessingContainerLoggingLogGroupF40B9C5D"
+                Ref: "ServiceQueueProcessingTaskDefQueueProcessingContainerLogGroupD52338D1"
               },
               "awslogs-stream-prefix": "Service",
               "awslogs-region": {

diff --git a/packages/@aws-cdk/aws-ecs/lib/container-definition.ts b/packages/@aws-cdk/aws-ecs/lib/container-definition.ts
@@ -4,7 +4,7 @@ import { NetworkMode, TaskDefinition } from './base/task-definition';
 import { ContainerImage, ContainerImageConfig } from './container-image';
 import { CfnTaskDefinition } from './ecs.generated';
 import { LinuxParameters } from './linux-parameters';
-import { LogDriver } from './log-drivers/log-driver';
+import { LogDriver, LogDriverConfig } from './log-drivers/log-driver';
 
 export interface ContainerDefinitionOptions {
   /**
@@ -249,6 +249,8 @@ export class ContainerDefinition extends cdk.Construct {
 
   private readonly imageConfig: ContainerImageConfig;
 
+  private readonly logDriverConfig?: LogDriverConfig;
+
   constructor(scope: cdk.Construct, id: string, private readonly props: ContainerDefinitionProps) {
     super(scope, id);
     this.essential = props.essential !== undefined ? props.essential : true;
@@ -257,7 +259,9 @@ export class ContainerDefinition extends cdk.Construct {
     this.linuxParameters = props.linuxParameters;
 
     this.imageConfig = props.image.bind(this, this);
-    if (props.logging) { props.logging.bind(this); }
+    if (props.logging) {
+      this.logDriverConfig = props.logging.bind(this, this);
+    }
     props.taskDefinition._linkContainer(this);
   }
 
@@ -410,7 +414,7 @@ export class ContainerDefinition extends cdk.Construct {
       user: this.props.user,
       volumesFrom: this.volumesFrom.map(renderVolumeFrom),
       workingDirectory: this.props.workingDirectory,
-      logConfiguration: this.props.logging && this.props.logging.renderLogDriver(),
+      logConfiguration: this.logDriverConfig,
       environment: this.props.environment && renderKV(this.props.environment, 'name', 'value'),
       extraHosts: this.props.extraHosts && renderKV(this.props.extraHosts, 'hostname', 'ipAddress'),
       healthCheck: this.props.healthCheck && renderHealthCheck(this.props.healthCheck),

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
@@ -1,9 +1,7 @@
 import logs = require('@aws-cdk/aws-logs');
-import cdk = require('@aws-cdk/cdk');
-import { Stack } from '@aws-cdk/cdk';
+import { Construct, Stack } from '@aws-cdk/cdk';
 import { ContainerDefinition } from '../container-definition';
-import { CfnTaskDefinition } from '../ecs.generated';
-import { LogDriver } from "./log-driver";
+import { LogDriver, LogDriverConfig } from "./log-driver";
 
 /**
  * Properties for defining a new AWS Log Driver
@@ -67,38 +65,35 @@ export interface AwsLogDriverProps {
 export class AwsLogDriver extends LogDriver {
   /**
    * The log group that the logs will be sent to
+   *
+   * Only available after the LogDriver has been bound to a ContainerDefinition.
    */
-  public readonly logGroup: logs.ILogGroup;
+  public logGroup?: logs.ILogGroup;
 
-  constructor(scope: cdk.Construct, id: string, private readonly props: AwsLogDriverProps) {
-    super(scope, id);
+  constructor(private readonly props: AwsLogDriverProps) {
+    super();
 
     if (props.logGroup && props.logRetention) {
       throw new Error('Cannot specify both `logGroup` and `logRetentionDays`.');
     }
-
-    this.logGroup = props.logGroup || new logs.LogGroup(this, 'LogGroup', {
-        retention: props.logRetention || Infinity,
-    });
   }
 
   /**
    * Called when the log driver is configured on a container
    */
-  public bind(containerDefinition: ContainerDefinition): void {
+  public bind(scope: Construct, containerDefinition: ContainerDefinition): LogDriverConfig {
+    this.logGroup = this.props.logGroup || new logs.LogGroup(scope, 'LogGroup', {
+        retention: this.props.logRetention || Infinity,
+    });
+
     this.logGroup.grantWrite(containerDefinition.taskDefinition.obtainExecutionRole());
-  }
 
-  /**
-   * Return the log driver CloudFormation JSON
-   */
-  public renderLogDriver(): CfnTaskDefinition.LogConfigurationProperty {
     return {
       logDriver: 'awslogs',
       options: removeEmpty({
         'awslogs-group': this.logGroup.logGroupName,
         'awslogs-stream-prefix': this.props.streamPrefix,
-        'awslogs-region': Stack.of(this).region,
+        'awslogs-region': Stack.of(containerDefinition).region,
         'awslogs-datetime-format': this.props.datetimeFormat,
         'awslogs-multiline-pattern': this.props.multilinePattern,
       }),

diff --git a/packages/@aws-cdk/aws-ecs/lib/log-drivers/log-driver.ts b/packages/@aws-cdk/aws-ecs/lib/log-drivers/log-driver.ts
@@ -1,18 +1,35 @@
-import cdk = require('@aws-cdk/cdk');
+import { Construct } from '@aws-cdk/cdk';
 import { ContainerDefinition } from '../container-definition';
-import { CfnTaskDefinition } from '../ecs.generated';
+import { AwsLogDriver, AwsLogDriverProps } from './aws-log-driver';
 
 /**
  * Base class for log drivers
  */
-export abstract class LogDriver extends cdk.Construct {
+export abstract class LogDriver {
   /**
-   * Return the log driver CloudFormation JSON
+   * Create an AWS Logs logdriver
    */
-  public abstract renderLogDriver(): CfnTaskDefinition.LogConfigurationProperty;
+  public static awsLogs(props: AwsLogDriverProps): LogDriver {
+    return new AwsLogDriver(props);
+  }
 
   /**
    * Called when the log driver is configured on a container
    */
-  public abstract bind(containerDefinition: ContainerDefinition): void;
+  public abstract bind(scope: Construct, containerDefinition: ContainerDefinition): LogDriverConfig;
 }
+
+/**
+ * Configuration to create a log driver from
+ */
+export interface LogDriverConfig {
+  /**
+   * Name of the log driver to use
+   */
+  readonly logDriver: string;
+
+  /**
+   * Log-driver specific option set
+   */
+  readonly options?: { [key: string]: string };
+}