revert: "fix(ecs): removed explicit addition of ecs deployment type w…

…hen circuit breaker is enabled (#22328)" (#22418) This reverts commit 635129c. Per comment #16126 (comment) this will introduce a breaking change. ---- ### All Submissions: * [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
aws · Oct 10, 2022 · 0f002e2 · 0f002e2
1 parent 6e0acb2
commit 0f002e2
Show file tree

Hide file tree

Showing 14 changed files with 40 additions and 96 deletions.
diff --git a/packages/@aws-cdk/aws-ecs-patterns/test/ec2/l3s.test.ts b/packages/@aws-cdk/aws-ecs-patterns/test/ec2/l3s.test.ts
@@ -1573,6 +1573,9 @@ test('ALB with circuit breaker', () => {
         Rollback: true,
       },
     },
+    DeploymentController: {
+      Type: 'ECS',
+    },
   });
 });
 
@@ -1607,6 +1610,9 @@ test('NLB with circuit breaker', () => {
         Rollback: true,
       },
     },
+    DeploymentController: {
+      Type: 'ECS',
+    },
   });
 });
 

diff --git a/packages/@aws-cdk/aws-ecs-patterns/test/ec2/queue-processing-ecs-service.test.ts b/packages/@aws-cdk/aws-ecs-patterns/test/ec2/queue-processing-ecs-service.test.ts
@@ -301,6 +301,9 @@ testDeprecated('test ECS queue worker service construct - with optional props',
     },
     LaunchType: 'EC2',
     ServiceName: 'ecs-test-service',
+    DeploymentController: {
+      Type: 'ECS',
+    },
     PlacementConstraints: [{ Type: 'memberOf', Expression: 'attribute:ecs.instance-type =~ m5a.*' }],
     PlacementStrategies: [{ Field: 'instanceId', Type: 'spread' }, { Field: 'CPU', Type: 'binpack' }, { Type: 'random' }],
   });

diff --git a/...fargate/alb-fargate-service-https.integ.snapshot/aws-ecs-integ-alb-fg-https.template.json b/...fargate/alb-fargate-service-https.integ.snapshot/aws-ecs-integ-alb-fg-https.template.json
@@ -741,11 +741,7 @@
     },
     "DeploymentConfiguration": {
      "MaximumPercent": 200,
-     "MinimumHealthyPercent": 50,
-     "DeploymentCircuitBreaker": {
-      "Enable": true,
-      "Rollback": true
-     }
+     "MinimumHealthyPercent": 50
     },
     "EnableECSManagedTags": true,
     "EnableExecuteCommand": true,

diff --git a/...-load-balanced-fargate-service.integ.snapshot/aws-ecs-integ-circuit-breaker.template.json b/...-load-balanced-fargate-service.integ.snapshot/aws-ecs-integ-circuit-breaker.template.json
@@ -634,6 +634,9 @@
      "MaximumPercent": 200,
      "MinimumHealthyPercent": 50
     },
+    "DeploymentController": {
+     "Type": "ECS"
+    },
     "EnableECSManagedTags": false,
     "HealthCheckGracePeriodSeconds": 60,
     "LaunchType": "FARGATE",

diff --git a/...terns/test/fargate/circuit-breaker-load-balanced-fargate-service.integ.snapshot/tree.json b/...terns/test/fargate/circuit-breaker-load-balanced-fargate-service.integ.snapshot/tree.json
@@ -1121,6 +1121,9 @@
                             "rollback": true
                           }
                         },
+                        "deploymentController": {
+                          "type": "ECS"
+                        },
                         "enableEcsManagedTags": false,
                         "healthCheckGracePeriodSeconds": 60,
                         "launchType": "FARGATE",

diff --git a/...aker-queue-processing-fargate-service.integ.snapshot/aws-ecs-patterns-queue.template.json b/...aker-queue-processing-fargate-service.integ.snapshot/aws-ecs-patterns-queue.template.json
@@ -621,6 +621,9 @@
      "MaximumPercent": 200,
      "MinimumHealthyPercent": 50
     },
+    "DeploymentController": {
+     "Type": "ECS"
+    },
     "EnableECSManagedTags": false,
     "LaunchType": "FARGATE",
     "NetworkConfiguration": {

diff --git a/...ns/test/fargate/circuit-breaker-queue-processing-fargate-service.integ.snapshot/tree.json b/...ns/test/fargate/circuit-breaker-queue-processing-fargate-service.integ.snapshot/tree.json
@@ -1107,6 +1107,9 @@
                             "rollback": true
                           }
                         },
+                        "deploymentController": {
+                          "type": "ECS"
+                        },
                         "enableEcsManagedTags": false,
                         "launchType": "FARGATE",
                         "networkConfiguration": {

diff --git a/packages/@aws-cdk/aws-ecs-patterns/test/fargate/integ.alb-fargate-service-https.ts b/packages/@aws-cdk/aws-ecs-patterns/test/fargate/integ.alb-fargate-service-https.ts
@@ -27,7 +27,6 @@ new ApplicationLoadBalancedFargateService(stack, 'myService', {
     zoneName: 'example.com.',
   }),
   redirectHTTP: true,
-  circuitBreaker: { rollback: true },
 });
 
 new integ.IntegTest(app, 'albFargateServiceTest', {

diff --git a/packages/@aws-cdk/aws-ecs-patterns/test/fargate/load-balanced-fargate-service.test.ts b/packages/@aws-cdk/aws-ecs-patterns/test/fargate/load-balanced-fargate-service.test.ts
@@ -451,6 +451,9 @@ test('setting ALB circuitBreaker works', () => {
         Rollback: true,
       },
     },
+    DeploymentController: {
+      Type: 'ECS',
+    },
   });
 });
 
@@ -474,6 +477,9 @@ test('setting NLB circuitBreaker works', () => {
         Rollback: true,
       },
     },
+    DeploymentController: {
+      Type: 'ECS',
+    },
   });
 });
 
@@ -1159,4 +1165,4 @@ test('NetworkLoadBalancedFargateService multiple capacity provider strategies ar
       },
     ]),
   });
-});
+});
diff --git a/packages/@aws-cdk/aws-ecs-patterns/test/fargate/queue-processing-fargate-service.test.ts b/packages/@aws-cdk/aws-ecs-patterns/test/fargate/queue-processing-fargate-service.test.ts
@@ -446,6 +446,9 @@ testDeprecated('test Fargate queue worker service construct - with optional prop
     LaunchType: 'FARGATE',
     ServiceName: 'fargate-test-service',
     PlatformVersion: ecs.FargatePlatformVersion.VERSION1_4,
+    DeploymentController: {
+      Type: 'ECS',
+    },
   });
 
   Template.fromStack(stack).hasResourceProperties('AWS::SQS::Queue', { QueueName: 'fargate-test-sqs-queue' });

diff --git a/packages/@aws-cdk/aws-ecs/lib/base/base-service.ts b/packages/@aws-cdk/aws-ecs/lib/base/base-service.ts
@@ -449,7 +449,9 @@ export abstract class BaseService extends Resource
       },
       propagateTags: propagateTagsFromSource === PropagatedTagSource.NONE ? undefined : props.propagateTags,
       enableEcsManagedTags: props.enableECSManagedTags ?? false,
-      deploymentController: props.deploymentController,
+      deploymentController: props.circuitBreaker ? {
+        type: DeploymentControllerType.ECS,
+      } : props.deploymentController,
       launchType: launchType,
       enableExecuteCommand: props.enableExecuteCommand,
       capacityProviderStrategy: props.capacityProviderStrategies,
@@ -463,9 +465,7 @@ export abstract class BaseService extends Resource
     if (props.deploymentController?.type === DeploymentControllerType.EXTERNAL) {
       Annotations.of(this).addWarning('taskDefinition and launchType are blanked out when using external deployment controller.');
     }
-    if (props.circuitBreaker && props.deploymentController?.type !== DeploymentControllerType.ECS) {
-      Annotations.of(this).addError('Deployment circuit breaker requires the ECS deployment controller.');
-    }
+
     this.serviceArn = this.getResourceArnAttribute(this.resource.ref, {
       service: 'ecs',
       resource: 'service',

diff --git a/packages/@aws-cdk/aws-ecs/test/ec2/ec2-service.test.ts b/packages/@aws-cdk/aws-ecs/test/ec2/ec2-service.test.ts
@@ -1201,36 +1201,6 @@ describe('ec2 service', () => {
 
     });
 
-
-    test('add warning to annotations if circuitBreaker is specified with a non-ECS DeploymentControllerType', () => {
-      // GIVEN
-      const stack = new cdk.Stack();
-      const vpc = new ec2.Vpc(stack, 'MyVpc', {});
-      const cluster = new ecs.Cluster(stack, 'EcsCluster', { vpc });
-      addDefaultCapacityProvider(cluster, stack, vpc);
-      const taskDefinition = new ecs.Ec2TaskDefinition(stack, 'Ec2TaskDef');
-
-      taskDefinition.addContainer('web', {
-        image: ecs.ContainerImage.fromRegistry('amazon/amazon-ecs-sample'),
-        memoryLimitMiB: 512,
-      });
-
-      const service = new ecs.Ec2Service(stack, 'Ec2Service', {
-        cluster,
-        taskDefinition,
-        deploymentController: {
-          type: DeploymentControllerType.EXTERNAL,
-        },
-        circuitBreaker: { rollback: true },
-      });
-
-      // THEN
-      expect(service.node.metadata[0].data).toEqual('taskDefinition and launchType are blanked out when using external deployment controller.');
-      expect(service.node.metadata[1].data).toEqual('Deployment circuit breaker requires the ECS deployment controller.');
-
-    });
-
-
     test('errors if daemon and desiredCount both specified', () => {
       // GIVEN
       const stack = new cdk.Stack();

diff --git a/packages/@aws-cdk/aws-ecs/test/external/external-service.test.ts b/packages/@aws-cdk/aws-ecs/test/external/external-service.test.ts
@@ -5,7 +5,7 @@ import * as elbv2 from '@aws-cdk/aws-elasticloadbalancingv2';
 import * as cloudmap from '@aws-cdk/aws-servicediscovery';
 import * as cdk from '@aws-cdk/core';
 import * as ecs from '../../lib';
-import { DeploymentControllerType, LaunchType } from '../../lib/base/base-service';
+import { LaunchType } from '../../lib/base/base-service';
 import { addDefaultCapacityProvider } from '../util';
 
 describe('external service', () => {
@@ -520,34 +520,7 @@ describe('external service', () => {
       containerPort: 8000,
     })).toThrow('Cloud map service association is not supported for an external service');
 
-
-  });
-
-  test('add warning to annotations if circuitBreaker is specified with a non-ECS DeploymentControllerType', () => {
-    // GIVEN
-    const stack = new cdk.Stack();
-    const vpc = new ec2.Vpc(stack, 'MyVpc', {});
-    const cluster = new ecs.Cluster(stack, 'EcsCluster', { vpc });
-    addDefaultCapacityProvider(cluster, stack, vpc);
-    const taskDefinition = new ecs.ExternalTaskDefinition(stack, 'TaskDef');
-
-    taskDefinition.addContainer('web', {
-      image: ecs.ContainerImage.fromRegistry('amazon/amazon-ecs-sample'),
-      memoryLimitMiB: 512,
-    });
-
-    const service = new ecs.ExternalService(stack, 'ExternalService', {
-      cluster,
-      taskDefinition,
-      deploymentController: {
-        type: DeploymentControllerType.EXTERNAL,
-      },
-      circuitBreaker: { rollback: true },
-    });
-
     // THEN
-    expect(service.node.metadata[0].data).toEqual('taskDefinition and launchType are blanked out when using external deployment controller.');
-    expect(service.node.metadata[1].data).toEqual('Deployment circuit breaker requires the ECS deployment controller.');
 
   });
 });
diff --git a/packages/@aws-cdk/aws-ecs/test/fargate/fargate-service.test.ts b/packages/@aws-cdk/aws-ecs/test/fargate/fargate-service.test.ts
@@ -657,33 +657,6 @@ describe('fargate service', () => {
       });
     });
 
-    test('add warning to annotations if circuitBreaker is specified with a non-ECS DeploymentControllerType', () => {
-      // GIVEN
-      const stack = new cdk.Stack();
-      const vpc = new ec2.Vpc(stack, 'MyVpc', {});
-      const cluster = new ecs.Cluster(stack, 'EcsCluster', { vpc });
-      const taskDefinition = new ecs.FargateTaskDefinition(stack, 'FargateTaskDef');
-
-      taskDefinition.addContainer('web', {
-        image: ecs.ContainerImage.fromRegistry('amazon/amazon-ecs-sample'),
-      });
-
-      const service = new ecs.FargateService(stack, 'FargateService', {
-        cluster,
-        taskDefinition,
-        deploymentController: {
-          type: DeploymentControllerType.EXTERNAL,
-        },
-        circuitBreaker: { rollback: true },
-      });
-
-      // THEN
-      expect(service.node.metadata[0].data).toEqual('taskDefinition and launchType are blanked out when using external deployment controller.');
-      expect(service.node.metadata[1].data).toEqual('Deployment circuit breaker requires the ECS deployment controller.');
-
-    });
-
-
     test('errors when no container specified on task definition', () => {
       // GIVEN
       const stack = new cdk.Stack();
@@ -2443,6 +2416,9 @@ describe('fargate service', () => {
             Rollback: true,
           },
         },
+        DeploymentController: {
+          Type: ecs.DeploymentControllerType.ECS,
+        },
       });
     });