feat(ecs-patterns): allow customizing logdriver (aws#3550)

* Update L3 constructs * Address feedback * Address feedback
Pinlv · Aug 6, 2019 · 8ffba4b · 8ffba4b
1 parent 92cb0f9
commit 8ffba4b
Show file tree

Hide file tree

Showing 8 changed files with 169 additions and 11 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
@@ -143,6 +143,13 @@ export interface LoadBalancedServiceBaseProps {
    * @default CloudFormation-generated name
    */
   readonly serviceName?: string;
+
+  /**
+   * The LogDriver to use for logging.
+   *
+   * @default - AwsLogDriver if enableLogging is true
+   */
+  readonly logDriver?: LogDriver;
 }
 
 /**
@@ -178,7 +185,7 @@ export abstract class LoadBalancedServiceBase extends cdk.Construct {
 
     // Create log driver if logging is enabled
     const enableLogging = props.enableLogging !== undefined ? props.enableLogging : true;
-    this.logDriver = enableLogging ? this.createAWSLogDriver(this.node.id) : undefined;
+    this.logDriver = props.logDriver !== undefined ? props.logDriver : enableLogging ? this.createAWSLogDriver(this.node.id) : undefined;
 
     this.assignPublicIp = props.publicTasks !== undefined ? props.publicTasks : false;
     this.desiredCount = props.desiredCount || 1;

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
@@ -1,16 +1,28 @@
 import { ScalingInterval } from '@aws-cdk/aws-applicationautoscaling';
-import { AwsLogDriver, BaseService, ContainerImage, ICluster, LogDriver, Secret } from '@aws-cdk/aws-ecs';
+import { IVpc } from '@aws-cdk/aws-ec2';
+import { AwsLogDriver, BaseService, Cluster, ContainerImage, ICluster, LogDriver, Secret } from '@aws-cdk/aws-ecs';
 import { IQueue, Queue } from '@aws-cdk/aws-sqs';
-import { CfnOutput, Construct } from '@aws-cdk/core';
+import { CfnOutput, Construct, Stack } from '@aws-cdk/core';
 
 /**
  * The properties for the base QueueProcessingEc2Service or QueueProcessingFargateService service.
  */
 export interface QueueProcessingServiceBaseProps {
   /**
-   * The name of the cluster that hosts the service.
+   * The cluster where your service will be deployed
+   * You can only specify either vpc or cluster. Alternatively, you can leave both blank
+   *
+   * @default - create a new cluster; if you do not specify a cluster nor a vpc, a new VPC will be created for you as well
+   */
+  readonly cluster?: ICluster;
+
+  /**
+   * VPC that the cluster instances or tasks are running in
+   * You can only specify either vpc or cluster. Alternatively, you can leave both blank
+   *
+   * @default - use vpc of cluster or create a new one
    */
-  readonly cluster: ICluster;
+  readonly vpc?: IVpc;
 
   /**
    * The image used to start a container.
@@ -87,6 +99,13 @@ export interface QueueProcessingServiceBaseProps {
    * @default [{ upper: 0, change: -1 },{ lower: 100, change: +1 },{ lower: 500, change: +5 }]
    */
   readonly scalingSteps?: ScalingInterval[];
+
+  /**
+   * The LogDriver to use for logging.
+   *
+   * @default AwsLogDriver if enableLogging is true
+   */
+  readonly logDriver?: LogDriver;
 }
 
 /**
@@ -98,6 +117,11 @@ export abstract class QueueProcessingServiceBase extends Construct {
    */
   public readonly sqsQueue: IQueue;
 
+  /**
+   * The cluster where your service will be deployed
+   */
+  public readonly cluster: ICluster;
+
   // Properties that have defaults defined. The Queue Processing Service will handle assigning undefined properties with default
   // values so that derived classes do not need to maintain the same logic.
 
@@ -136,6 +160,11 @@ export abstract class QueueProcessingServiceBase extends Construct {
   constructor(scope: Construct, id: string, props: QueueProcessingServiceBaseProps) {
     super(scope, id);
 
+    if (props.cluster && props.vpc) {
+      throw new Error(`You can only specify either vpc or cluster. Alternatively, you can leave both blank`);
+    }
+    this.cluster = props.cluster || this.getDefaultCluster(this, props.vpc);
+
     // Create the SQS queue if one is not provided
     this.sqsQueue = props.queue !== undefined ? props.queue : new Queue(this, 'EcsProcessingQueue', {});
 
@@ -145,7 +174,11 @@ export abstract class QueueProcessingServiceBase extends Construct {
 
     // Create log driver if logging is enabled
     const enableLogging = props.enableLogging !== undefined ? props.enableLogging : true;
-    this.logDriver = enableLogging ? this.createAWSLogDriver(this.node.id) : undefined;
+    this.logDriver = props.logDriver !== undefined
+                        ? props.logDriver
+                        : enableLogging
+                            ? this.createAWSLogDriver(this.node.id)
+                            : undefined;
 
     // Add the queue name to environment variables
     this.environment = { ...(props.environment || {}), QUEUE_NAME: this.sqsQueue.queueName };
@@ -175,6 +208,13 @@ export abstract class QueueProcessingServiceBase extends Construct {
     });
   }
 
+  protected getDefaultCluster(scope: Construct, vpc?: IVpc): Cluster {
+    // magic string to avoid collision with user-defined constructs
+    const DEFAULT_CLUSTER_ID = `EcsDefaultClusterMnL3mNNYN${vpc ? vpc.node.id : ''}`;
+    const stack = Stack.of(scope);
+    return stack.node.tryFindChild(DEFAULT_CLUSTER_ID) as Cluster || new Cluster(stack, DEFAULT_CLUSTER_ID, { vpc });
+  }
+
   /**
    * Create an AWS Log Driver with the provided streamPrefix
    *

diff --git a/packages/@aws-cdk/aws-ecs-patterns/lib/base/scheduled-task-base.ts b/packages/@aws-cdk/aws-ecs-patterns/lib/base/scheduled-task-base.ts
@@ -1,5 +1,5 @@
 import { Schedule } from "@aws-cdk/aws-applicationautoscaling";
-import { AwsLogDriver, ContainerImage, ICluster, Secret, TaskDefinition } from "@aws-cdk/aws-ecs";
+import { AwsLogDriver, ContainerImage, ICluster, LogDriver, Secret, TaskDefinition } from "@aws-cdk/aws-ecs";
 import { Rule } from "@aws-cdk/aws-events";
 import { EcsTask } from "@aws-cdk/aws-events-targets";
 import { Construct } from "@aws-cdk/core";
@@ -60,6 +60,13 @@ export interface ScheduledTaskBaseProps {
    * @default - No secret environment variables.
    */
   readonly secrets?: { [key: string]: Secret };
+
+  /**
+   * The LogDriver to use for logging.
+   *
+   * @default AwsLogDriver if enableLogging is true
+   */
+  readonly logDriver?: LogDriver;
 }
 
 /**
@@ -75,6 +82,10 @@ export abstract class ScheduledTaskBase extends Construct {
    */
   public readonly desiredTaskCount: number;
   public readonly eventRule: Rule;
+  /**
+   * The AwsLogDriver to use for logging if logging is enabled.
+   */
+  public readonly logDriver?: LogDriver;
 
   /**
    * Constructs a new instance of the ScheduledTaskBase class.
@@ -89,6 +100,10 @@ export abstract class ScheduledTaskBase extends Construct {
     this.eventRule = new Rule(this, 'ScheduledEventRule', {
       schedule: props.schedule,
     });
+
+    this.logDriver = props.logDriver !== undefined
+                        ? props.logDriver
+                        : this.createAWSLogDriver(this.node.id);
   }
 
   /**

diff --git a/packages/@aws-cdk/aws-ecs-patterns/lib/ecs/queue-processing-ecs-service.ts b/packages/@aws-cdk/aws-ecs-patterns/lib/ecs/queue-processing-ecs-service.ts
@@ -72,7 +72,7 @@ export class QueueProcessingEc2Service extends QueueProcessingServiceBase {
     // Create an ECS service with the previously defined Task Definition and configure
     // autoscaling based on cpu utilization and number of messages visible in the SQS queue.
     this.service = new Ec2Service(this, 'QueueProcessingService', {
-      cluster: props.cluster,
+      cluster: this.cluster,
       desiredCount: this.desiredCount,
       taskDefinition
     });

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
@@ -66,7 +66,7 @@ export class ScheduledEc2Task extends ScheduledTaskBase {
       command: props.command,
       environment: props.environment,
       secrets: props.secrets,
-      logging: this.createAWSLogDriver(this.node.id)
+      logging: this.logDriver
     });
 
     this.addTaskDefinitionToEventTarget(this.taskDefinition);

diff --git a/packages/@aws-cdk/aws-ecs-patterns/lib/fargate/queue-processing-fargate-service.ts b/packages/@aws-cdk/aws-ecs-patterns/lib/fargate/queue-processing-fargate-service.ts
@@ -75,7 +75,7 @@ export class QueueProcessingFargateService extends QueueProcessingServiceBase {
     // Create a Fargate service with the previously defined Task Definition and configure
     // autoscaling based on cpu utilization and number of messages visible in the SQS queue.
     this.service = new FargateService(this, 'QueueProcessingFargateService', {
-      cluster: props.cluster,
+      cluster: this.cluster,
       desiredCount: this.desiredCount,
       taskDefinition
     });

diff --git a/packages/@aws-cdk/aws-ecs-patterns/lib/fargate/scheduled-fargate-task.ts b/packages/@aws-cdk/aws-ecs-patterns/lib/fargate/scheduled-fargate-task.ts
@@ -57,7 +57,7 @@ export class ScheduledFargateTask extends ScheduledTaskBase {
       command: props.command,
       environment: props.environment,
       secrets: props.secrets,
-      logging: this.createAWSLogDriver(this.node.id)
+      logging: this.logDriver
     });
 
     this.addTaskDefinitionToEventTarget(this.taskDefinition);

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
@@ -2,6 +2,7 @@ import { expect, haveResource, haveResourceLike } from '@aws-cdk/assert';
 import { Certificate } from '@aws-cdk/aws-certificatemanager';
 import ec2 = require('@aws-cdk/aws-ec2');
 import ecs = require('@aws-cdk/aws-ecs');
+import { AwsLogDriver } from '@aws-cdk/aws-ecs';
 import { PublicHostedZone } from '@aws-cdk/aws-route53';
 import cdk = require('@aws-cdk/core');
 import { Test } from 'nodeunit';
@@ -289,6 +290,101 @@ export = {
       });
     });
 
+    test.done();
+  },
+  'test Fargate loadbalanced construct with optional log driver input'(test: Test) {
+    // GIVEN
+    const stack = new cdk.Stack();
+    const vpc = new ec2.Vpc(stack, 'VPC');
+    const cluster = new ecs.Cluster(stack, 'Cluster', { vpc });
+
+    // WHEN
+    new ecsPatterns.LoadBalancedFargateService(stack, 'Service', {
+      cluster,
+      image: ecs.ContainerImage.fromRegistry('test'),
+      desiredCount: 2,
+      enableLogging: false,
+      environment: {
+        TEST_ENVIRONMENT_VARIABLE1: "test environment variable 1 value",
+        TEST_ENVIRONMENT_VARIABLE2: "test environment variable 2 value"
+      },
+      logDriver: new AwsLogDriver({
+        streamPrefix: "TestStream"
+      })
+    });
+
+    // THEN - stack contains a load balancer and a service
+    expect(stack).to(haveResourceLike('AWS::ECS::TaskDefinition', {
+      ContainerDefinitions: [
+        {
+          Environment: [
+            {
+              Name: "TEST_ENVIRONMENT_VARIABLE1",
+              Value: "test environment variable 1 value"
+            },
+            {
+              Name: "TEST_ENVIRONMENT_VARIABLE2",
+              Value: "test environment variable 2 value"
+            }
+          ],
+          LogConfiguration: {
+            LogDriver: "awslogs",
+            Options: {
+              "awslogs-group": { Ref: "ServiceTaskDefwebLogGroup2A898F61" },
+              "awslogs-stream-prefix": "TestStream",
+              "awslogs-region": { Ref: "AWS::Region" }
+            }
+          },
+        }
+      ]
+    }));
+
+    test.done();
+  },
+  'test Fargate loadbalanced construct with logging enabled'(test: Test) {
+    // GIVEN
+    const stack = new cdk.Stack();
+    const vpc = new ec2.Vpc(stack, 'VPC');
+    const cluster = new ecs.Cluster(stack, 'Cluster', { vpc });
+
+    // WHEN
+    new ecsPatterns.LoadBalancedFargateService(stack, 'Service', {
+      cluster,
+      image: ecs.ContainerImage.fromRegistry('test'),
+      desiredCount: 2,
+      enableLogging: true,
+      environment: {
+        TEST_ENVIRONMENT_VARIABLE1: "test environment variable 1 value",
+        TEST_ENVIRONMENT_VARIABLE2: "test environment variable 2 value"
+      }
+    });
+
+    // THEN - stack contains a load balancer and a service
+    expect(stack).to(haveResourceLike('AWS::ECS::TaskDefinition', {
+      ContainerDefinitions: [
+        {
+          Environment: [
+            {
+              Name: "TEST_ENVIRONMENT_VARIABLE1",
+              Value: "test environment variable 1 value"
+            },
+            {
+              Name: "TEST_ENVIRONMENT_VARIABLE2",
+              Value: "test environment variable 2 value"
+            }
+          ],
+          LogConfiguration: {
+            LogDriver: "awslogs",
+            Options: {
+              "awslogs-group": { Ref: "ServiceTaskDefwebLogGroup2A898F61" },
+              "awslogs-stream-prefix": "Service",
+              "awslogs-region": { Ref: "AWS::Region" }
+            }
+          },
+        }
+      ]
+    }));
+
     test.done();
   },
 };