From 8ffba4b04f5086b33b46aa87fe59a7fe7845ff01 Mon Sep 17 00:00:00 2001 From: Piradeep Kandasamy Date: Tue, 6 Aug 2019 14:12:46 -0700 Subject: [PATCH] feat(ecs-patterns): allow customizing logdriver (#3550) * Update L3 constructs * Address feedback * Address feedback --- .../lib/base/load-balanced-service-base.ts | 9 +- .../lib/base/queue-processing-service-base.ts | 50 +++++++++- .../lib/base/scheduled-task-base.ts | 17 +++- .../lib/ecs/queue-processing-ecs-service.ts | 2 +- .../lib/ecs/scheduled-ecs-task.ts | 2 +- .../queue-processing-fargate-service.ts | 2 +- .../lib/fargate/scheduled-fargate-task.ts | 2 +- .../aws-ecs-patterns/test/ec2/test.l3s.ts | 96 +++++++++++++++++++ 8 files changed, 169 insertions(+), 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 index 635aab7943841..056a3d936b339 100644 --- 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 index a6f06ec7fae6d..efaeaf0373e85 100644 --- 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 index 152ec4e70f066..f1ada22e17382 100644 --- 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 index 4eef0d356e748..a455abe0f855d 100644 --- 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 index e819c6fdd306f..b5365616cbdd5 100644 --- 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 index 1f78eb295e62a..27647cd4815fb 100644 --- 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 index b2e1b5f059e69..0ff7f087e96a1 100644 --- 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 index e4b4cd3e266a7..39a1edf8c42cc 100644 --- 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(); }, };