Update based on feedback

aws · Feb 19, 2021 · 0ed5b9c · 0ed5b9c
1 parent dea6efd
commit 0ed5b9c
Show file tree

Hide file tree

Showing 4 changed files with 126 additions and 55 deletions.
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
@@ -288,7 +288,7 @@ export abstract class QueueProcessingServiceBase extends CoreConstruct {
 
     // Determine the desired task count (minimum) and maximum scaling capacity
     if (!this.node.tryGetContext(cxapi.ECS_REMOVE_DEFAULT_DESIRED_COUNT)) {
-      this.desiredCount = props.desiredTaskCount != null ? props.desiredTaskCount : 1;
+      this.desiredCount = props.desiredTaskCount ?? 1;
       this.minCapacity = props.minScalingCapacity || this.desiredCount;
       this.maxCapacity = props.maxScalingCapacity || (2 * this.desiredCount);
     } else {

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
@@ -1,4 +1,4 @@
-import { Ec2Service, Ec2TaskDefinition } from '@aws-cdk/aws-ecs';
+import { CfnService, Ec2Service, Ec2TaskDefinition } from '@aws-cdk/aws-ecs';
 import * as cxapi from '@aws-cdk/cx-api';
 import { Construct } from 'constructs';
 import { QueueProcessingServiceBase, QueueProcessingServiceBaseProps } from '../base/queue-processing-service-base';
@@ -101,30 +101,22 @@ 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: this.cluster,
+      taskDefinition: this.taskDefinition,
+      serviceName: props.serviceName,
+      minHealthyPercent: props.minHealthyPercent,
+      maxHealthyPercent: props.maxHealthyPercent,
+      propagateTags: props.propagateTags,
+      enableECSManagedTags: props.enableECSManagedTags,
+      deploymentController: props.deploymentController,
+    });
+
     if (!this.node.tryGetContext(cxapi.ECS_REMOVE_DEFAULT_DESIRED_COUNT)) {
-      this.service = new Ec2Service(this, 'QueueProcessingService', {
-        cluster: this.cluster,
-        desiredCount: this.desiredCount,
-        taskDefinition: this.taskDefinition,
-        serviceName: props.serviceName,
-        minHealthyPercent: props.minHealthyPercent,
-        maxHealthyPercent: props.maxHealthyPercent,
-        propagateTags: props.propagateTags,
-        enableECSManagedTags: props.enableECSManagedTags,
-        deploymentController: props.deploymentController,
-      });
-    } else {
-      this.service = new Ec2Service(this, 'QueueProcessingService', {
-        cluster: this.cluster,
-        taskDefinition: this.taskDefinition,
-        serviceName: props.serviceName,
-        minHealthyPercent: props.minHealthyPercent,
-        maxHealthyPercent: props.maxHealthyPercent,
-        propagateTags: props.propagateTags,
-        enableECSManagedTags: props.enableECSManagedTags,
-        deploymentController: props.deploymentController,
-      });
+      const cfnService = this.service.node.children.find(c => c instanceof CfnService) as CfnService;
+      cfnService.desiredCount = this.desiredCount;
     }
+
     this.configureAutoscalingForService(this.service);
     this.grantPermissionsToService(this.service);
   }

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
@@ -1,5 +1,5 @@
 import * as ec2 from '@aws-cdk/aws-ec2';
-import { FargatePlatformVersion, FargateService, FargateTaskDefinition } from '@aws-cdk/aws-ecs';
+import { CfnService, FargatePlatformVersion, FargateService, FargateTaskDefinition } from '@aws-cdk/aws-ecs';
 import * as cxapi from '@aws-cdk/cx-api';
 import { Construct } from 'constructs';
 import { QueueProcessingServiceBase, QueueProcessingServiceBaseProps } from '../base/queue-processing-service-base';
@@ -131,38 +131,26 @@ 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: this.cluster,
+      taskDefinition: this.taskDefinition,
+      serviceName: props.serviceName,
+      minHealthyPercent: props.minHealthyPercent,
+      maxHealthyPercent: props.maxHealthyPercent,
+      propagateTags: props.propagateTags,
+      enableECSManagedTags: props.enableECSManagedTags,
+      platformVersion: props.platformVersion,
+      deploymentController: props.deploymentController,
+      securityGroups: props.securityGroups,
+      vpcSubnets: props.taskSubnets,
+      assignPublicIp: props.assignPublicIp,
+    });
+
     if (!this.node.tryGetContext(cxapi.ECS_REMOVE_DEFAULT_DESIRED_COUNT)) {
-      this.service = new FargateService(this, 'QueueProcessingFargateService', {
-        cluster: this.cluster,
-        desiredCount: this.desiredCount,
-        taskDefinition: this.taskDefinition,
-        serviceName: props.serviceName,
-        minHealthyPercent: props.minHealthyPercent,
-        maxHealthyPercent: props.maxHealthyPercent,
-        propagateTags: props.propagateTags,
-        enableECSManagedTags: props.enableECSManagedTags,
-        platformVersion: props.platformVersion,
-        deploymentController: props.deploymentController,
-        securityGroups: props.securityGroups,
-        vpcSubnets: props.taskSubnets,
-        assignPublicIp: props.assignPublicIp,
-      });
-    } else {
-      this.service = new FargateService(this, 'QueueProcessingFargateService', {
-        cluster: this.cluster,
-        taskDefinition: this.taskDefinition,
-        serviceName: props.serviceName,
-        minHealthyPercent: props.minHealthyPercent,
-        maxHealthyPercent: props.maxHealthyPercent,
-        propagateTags: props.propagateTags,
-        enableECSManagedTags: props.enableECSManagedTags,
-        platformVersion: props.platformVersion,
-        deploymentController: props.deploymentController,
-        securityGroups: props.securityGroups,
-        vpcSubnets: props.taskSubnets,
-        assignPublicIp: props.assignPublicIp,
-      });
+      const cfnService = this.service.node.children.find(c => c instanceof CfnService) as CfnService;
+      cfnService.desiredCount = this.desiredCount;
     }
+
     this.configureAutoscalingForService(this.service);
     this.grantPermissionsToService(this.service);
   }

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
@@ -227,6 +227,97 @@ export = {
     test.done();
   },
 
+  'test Fargate queue worker service construct - without desiredCount specified'(test: Test) {
+    // GIVEN
+    const stack = new cdk.Stack();
+    const vpc = new ec2.Vpc(stack, 'VPC');
+    const cluster = new ecs.Cluster(stack, 'Cluster', { vpc });
+    cluster.addCapacity('DefaultAutoScalingGroup', { instanceType: new ec2.InstanceType('t2.micro') });
+    const queue = new sqs.Queue(stack, 'fargate-test-queue', {
+      queueName: 'fargate-test-sqs-queue',
+    });
+
+    // WHEN
+    new ecsPatterns.QueueProcessingFargateService(stack, 'Service', {
+      cluster,
+      memoryLimitMiB: 512,
+      image: ecs.ContainerImage.fromRegistry('test'),
+      command: ['-c', '4', 'amazon.com'],
+      enableLogging: false,
+      environment: {
+        TEST_ENVIRONMENT_VARIABLE1: 'test environment variable 1 value',
+        TEST_ENVIRONMENT_VARIABLE2: 'test environment variable 2 value',
+      },
+      queue,
+      maxScalingCapacity: 5,
+      minScalingCapacity: 2,
+      minHealthyPercent: 60,
+      maxHealthyPercent: 150,
+      serviceName: 'fargate-test-service',
+      family: 'fargate-task-family',
+      platformVersion: ecs.FargatePlatformVersion.VERSION1_4,
+      deploymentController: {
+        type: ecs.DeploymentControllerType.CODE_DEPLOY,
+      },
+    });
+
+    // THEN - QueueWorker is of FARGATE launch type, an SQS queue is created and all optional properties are set.
+    expect(stack).to(haveResource('AWS::ECS::Service', {
+      DeploymentConfiguration: {
+        MinimumHealthyPercent: 60,
+        MaximumPercent: 150,
+      },
+      LaunchType: 'FARGATE',
+      ServiceName: 'fargate-test-service',
+      PlatformVersion: ecs.FargatePlatformVersion.VERSION1_4,
+      DeploymentController: {
+        Type: 'CODE_DEPLOY',
+      },
+    }));
+
+    expect(stack).to(haveResource('AWS::ApplicationAutoScaling::ScalableTarget', {
+      MaxCapacity: 5,
+      MinCapacity: 2,
+    }));
+
+    expect(stack).to(haveResource('AWS::SQS::Queue', { QueueName: 'fargate-test-sqs-queue' }));
+
+    expect(stack).to(haveResourceLike('AWS::ECS::TaskDefinition', {
+      ContainerDefinitions: [
+        {
+          Command: [
+            '-c',
+            '4',
+            'amazon.com',
+          ],
+          Environment: [
+            {
+              Name: 'TEST_ENVIRONMENT_VARIABLE1',
+              Value: 'test environment variable 1 value',
+            },
+            {
+              Name: 'TEST_ENVIRONMENT_VARIABLE2',
+              Value: 'test environment variable 2 value',
+            },
+            {
+              Name: 'QUEUE_NAME',
+              Value: {
+                'Fn::GetAtt': [
+                  'fargatetestqueue28B43841',
+                  'QueueName',
+                ],
+              },
+            },
+          ],
+          Image: 'test',
+        },
+      ],
+      Family: 'fargate-task-family',
+    }));
+
+    test.done();
+  },
+
   'test Fargate queue worker service construct - with optional props'(test: Test) {
     // GIVEN
     const stack = new cdk.Stack();