search

aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.66k stars 3.92k forks source link

(ecs): (Ecs Service now Depends on Task Definition forcing Update to Existing Services) #25777

Open jsunico opened 1 year ago

jsunico commented 1 year ago

Describe the bug

I'm upgrading from CDK version 2.25.0 to 2.81.0. With no other changes on the stack aside from the upgrade, I'm getting an error in Cloudformation:

Resource handler returned message: "Invalid request provided: UpdateService error: Unable to update task definition on services with a CODE_DEPLOY deployment controller. Use AWS CodeDeploy to trigger a new deployment. (Service: AmazonECS; Status Code: 400; Error Code: InvalidParameterException; Request ID: xxxxxxxxx Proxy: null)" (RequestToken: XXXXXXXXXX, HandlerErrorCode: InvalidRequest)

I have compared the template from the Cloudformation and the local template built using the upgraded version and I can see the addition of the task definition in the DependsOn.

   "DependsOn": [
    "ApiLoadBalancerBlueTargetListenerTargetBlueRule1180D001",
    "ApiTaskDefinition51EA709E"
   ],

I have narrowed down which version this was introduced. It seems to occur since version 2.50.0. I believe this line in particular was causing the fargate service to update. However, as I'm using DeploymentControllerType.CODE_DEPLOY this causes the above error for already created services.

https://github.com/aws/aws-cdk/pull/22295/files#diff-becce6466790cd3cc81807ab64dd5f4ef85eed0285509d1ae43b381f24aefddaR469

Expected Behavior

Dependencies are not added if not required.

Current Behavior

Task definition is added as dependency on the fargate service forcing updates on existing resources.

Reproduction Steps

Build and deploy using v2.25.0

    const fargateService = new FargateService(this, 'ApiFargateService', {
      assignPublicIp: false,
      cluster: fargateCluster,
      deploymentController: {
        type: DeploymentControllerType.CODE_DEPLOY
      },
      capacityProviderStrategies: [{
        capacityProvider: 'FARGATE_SPOT',
        base: 1,
        weight: 100
      }],
      desiredCount: 0,
      healthCheckGracePeriod: Duration.seconds(10),
      maxHealthyPercent: 200,
      minHealthyPercent: 100,
      platformVersion: FargatePlatformVersion.LATEST,
      securityGroups: [fargateTaskSecurityGroup],
      serviceName: 'my-service',
      taskDefinition: ecsTaskDefinition,
    })

Build and deploy using 2.50.0

Possible Solution

If not required, can we remove the task definition from the list of the fargate service dependencies?

Additional Information/Context

No response

CDK CLI Version

2.50.0

Framework Version

No response

Node.js Version

v16.20.0

OS

Ubuntu 20.04.5 LTS (WSL)

Language

Typescript

Language Version

Typescript ~4.6.4

Other information

No response

pahud commented 1 year ago

From I can see in the aws-codedeploy README of #22295

When the CODE_DEPLOY deployment controller is used, the ECS service cannot be deployed with a new task definition version through CloudFormation.

Looks like you are trying to update the ecs service with CDK with CODE_DEPLOY deployment controller? Can you share more details about that?

jsunico commented 1 year ago

Hi @pahud, the update was an unintentional side effect of the change introduced in v2.50.0. From my end, I only upgraded my dependencies and tried to deploy.

Upon further inspection, the new CDK version adds a dependency into the fargate service triggering an update.

Here's my template in the Cloudformation dashboard:

"ApiFargateServiceXXXXXX": {
   "Type": "AWS::ECS::Service",
   "Properties": {
    "CapacityProviderStrategy": [
     {
      "Base": 1,
      "CapacityProvider": "FARGATE_SPOT",
      "Weight": 100
     }
    ],
    "Cluster": "fargate-cluster-XXXXXX",
    "DeploymentConfiguration": {
     "MaximumPercent": 200,
     "MinimumHealthyPercent": 100
    },
    "DeploymentController": {
     "Type": "CODE_DEPLOY"
    },
    "DesiredCount": 0,
    "EnableECSManagedTags": false,
    "HealthCheckGracePeriodSeconds": 10,
    "LoadBalancers": [
     {
      "ContainerName": "XXXXXX",
      "ContainerPort": 3333,
      "TargetGroupArn": {
       "Ref": "BlueTargetGroupXXXXXX"
      }
     }
    ],
    "NetworkConfiguration": {
     "AwsvpcConfiguration": {
      "AssignPublicIp": "DISABLED",
      "SecurityGroups": [
       {
        "Fn::GetAtt": [
         "ApiSecurityGroupXXXXXX",
         "GroupId"
        ]
       }
      ],
      "Subnets": [
       "subnet-XXXXXX",
       "subnet-XXXXXX",
       "subnet-XXXXXX"
      ]
     }
    },
    "PlatformVersion": "LATEST",
    "ServiceName": "XXXXXX",
    "TaskDefinition": {
     "Ref": "ApiTaskDefinitionXXXXXX"
    }
   },
   "DependsOn": [
    "ApiLoadBalancerBlueTargetListenerTargetBlueRuleXXXXXX"
   ],
   "Metadata": {
    "aws:cdk:path": "XXXXXX/XXXXXX/ApiFargateService/Service"
   }

This is the one compiled/built locally using version 2.50.0:

"ApiFargateServiceXXXXXX": {
   "Type": "AWS::ECS::Service",
   "Properties": {
    "CapacityProviderStrategy": [
     {
      "Base": 1,
      "CapacityProvider": "FARGATE_SPOT",
      "Weight": 100
     }
    ],
    "Cluster": "XXXXXX",
    "DeploymentConfiguration": {
     "MaximumPercent": 200,
     "MinimumHealthyPercent": 100
    },
    "DeploymentController": {
     "Type": "CODE_DEPLOY"
    },
    "DesiredCount": 0,
    "EnableECSManagedTags": false,
    "HealthCheckGracePeriodSeconds": 10,
    "LoadBalancers": [
     {
      "ContainerName": "XXXXXX",
      "ContainerPort": 3333,
      "TargetGroupArn": {
       "Ref": "BlueTargetGroupXXXXXX"
      }
     }
    ],
    "NetworkConfiguration": {
     "AwsvpcConfiguration": {
      "AssignPublicIp": "DISABLED",
      "SecurityGroups": [
       {
        "Fn::GetAtt": [
         "ApiSecurityGroupXXXXXX",
         "GroupId"
        ]
       }
      ],
      "Subnets": [
       "subnet-XXXXXX",
       "subnet-XXXXXX",
       "subnet-XXXXXX"
      ]
     }
    },
    "PlatformVersion": "LATEST",
    "ServiceName": "XXXXXX",
    "TaskDefinition": "XXXXXX"
   },
   "DependsOn": [
    "ApiLoadBalancerBlueTargetListenerTargetBlueRuleXXXXXX",
    "ApiTaskDefinitionXXXXXX"
   ],
   "Metadata": {
    "aws:cdk:path": "XXXXXX/XXXXXX/ApiFargateService/Service"
   }
  }

Notice the DependsOn property.

These are the lines of codes that was introduced in v2.50.0 that is now adding dependency in the fargate service.

https://github.com/aws/aws-cdk/blob/1d7a9a80b08d41ce8759bed9286adaa8259c2bc8/packages/aws-cdk-lib/aws-ecs/lib/base/base-service.ts#L573-L579

Hope this helps. Thank you.

kichik commented 1 year ago

This happened again with #25840 and 2.87.0.

const l1service = service.node.defaultChild as ecs.CfnService;
l1service.addPropertyDeletionOverride('DeploymentConfiguration.Alarms');
jorgemaroto-eb commented 1 year ago

having the same issue when we've updated the version of aws-cdk from 2.86.0 to 2.87.0

SinBirb commented 1 year ago

Hi, we also have this issue, because we are using DeploymentControllerType.CODE_DEPLOY. Did anyone find a work-around?

Can you possibly add a feature flag to disable this new feature? Otherwise, we can not upgrade our CDK version for our existing infrastructure.

pahud commented 1 year ago

@SinBirb

I am not sure if the dependency is necessary

https://github.com/aws/aws-cdk/blame/1d7a9a80b08d41ce8759bed9286adaa8259c2bc8/packages/aws-cdk-lib/aws-ecs/lib/base/base-service.ts#L578

As a workaround, I guess you probably can remove it with removeDependency if you really have to?

vishnus17 commented 1 year ago

I found a fix for this by doing const cfnService = service.node.defaultChild as ecs.CfnService; cfnService.addOverride('Properties.TaskDefinition', service.taskDefinition.taskDefinitionArn);

I was able to upgrade to CDK 2.95.0 by adding this below my ECS service construct.

jsunico commented 1 month ago

@SinBirb

I am not sure if the dependency is necessary

https://github.com/aws/aws-cdk/blame/1d7a9a80b08d41ce8759bed9286adaa8259c2bc8/packages/aws-cdk-lib/aws-ecs/lib/base/base-service.ts#L578

As a workaround, I guess you probably can remove it with removeDependency if you really have to?

Hi @pahud, I tried this workaround but it doesn't seem to work. Could you point out if I'm doing something wrong, please?

    const l1FargateService = this.fargateService.node.defaultChild as CfnService;
    l1FargateService.removeDependency(this.ecsTaskDefinition.node.defaultChild as CfnTaskDefinition)
jsunico commented 1 month ago

It seems like the dependency is being added on the node level rather than the defaultChild level. Are the two technically the same? There is currently no removeDependency in the node object.

In the addOverride option, I can:

    const l1FargateService = this.fargateService.node.defaultChild as CfnService;
    l1FargateService.addOverride('DependsOn', [])

But the problem is this removes all the dependencies. My Cloudformation has already one declared dependency and doing this will still cause a force update on the fargate service.

SinBirb commented 2 weeks ago

I found a fix for this by doing const cfnService = service.node.defaultChild as ecs.CfnService; cfnService.addOverride('Properties.TaskDefinition', service.taskDefinition.taskDefinitionArn);

I was able to upgrade to CDK 2.95.0 by adding this below my ECS service construct.

@vishnus17 Thank you, that worked for us for CDK 2.163.1.