Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(aws-ecs-patterns): Some service patterns still inline desired count to 1 when not present #12990

Closed
dastbe opened this issue Feb 11, 2021 · 2 comments · Fixed by #13130
Closed
Assignees
Labels
@aws-cdk/aws-ecs-patterns Related to ecs-patterns library bug This issue is a bug. in-progress This issue is being actively worked on.

Comments

@dastbe
Copy link
Contributor

dastbe commented Feb 11, 2021

Previous changes were made to make desired count truly optional through to cloudformation to make autoscaling work more consistently for create and update. However, this change was not applied to all patterns (example)

Reproduction Steps

Use ApplicationLoadBalancedEc2Service and do not set a desired count. This will result in a desired count of 1 in the template.

What did you expect to happen?

The final template should have no desired count set.

What actually happened?

The desired count was set to 1.

Environment

Cut this on behalf of someone else, but I've provided references to the issue in cdk itself

  • CDK CLI Version : n/a
  • Framework Version: n/a
  • Node.js Version: n/a
  • OS : n/a
  • Language (Version): n/a

Other


This is 🐛 Bug Report

@dastbe dastbe added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Feb 11, 2021
@peterwoodworth peterwoodworth added the @aws-cdk/aws-ecs-patterns Related to ecs-patterns library label Feb 11, 2021
@peterwoodworth peterwoodworth added investigating This issue is being investigated and/or work is in progress to resolve the issue. and removed needs-triage This issue or PR still needs to be triaged. labels Feb 11, 2021
@piradeepk piradeepk added in-progress This issue is being actively worked on. and removed investigating This issue is being investigated and/or work is in progress to resolve the issue. labels Feb 12, 2021
@piradeepk
Copy link
Contributor

piradeepk commented Feb 18, 2021

@dastbe Thanks for reporting this issue. There is a PR out to fix this issue across all patterns, but in the meantime, you would be able to override the current construct using the ConstructNode escape hatch. This will allow users to override the current desiredCount behaviour to generate the cfn template without the desiredCount specified.

For instance, if you have a service defined like so:

export class TestingStack extends cdk.Stack {
  constructor(scope: cdk.App, id: string, props?: cdk.StackProps) {
    super(scope, id, props);

    const vpc = new ec2.Vpc(this, 'MyVpc', { maxAzs: 2 });
    const cluster = new ecs.Cluster(this, 'Cluster', { vpc });

    const albFargateService = new ecs_patterns.ApplicationLoadBalancedFargateService(this, "FargateService", {
      cluster,
      taskImageOptions: {
        image: ecs.ContainerImage.fromRegistry("amazon/amazon-ecs-sample"),
      },
    });
  }
}

const app = new cdk.App();
new TestingStack(app, 'Bonjour');

You can override the desiredCount set in the ApplicationLoadBalancedFargateService construct by adding the following:

const cfnService = albFargateService.service.node.children.find(c => c instanceof ecs.CfnService) as ecs.CfnService;
cfnService.desiredCount = undefined;

Based on the original app, the new app would look like the following:

export class TestingStack extends cdk.Stack {
  constructor(scope: cdk.App, id: string, props?: cdk.StackProps) {
    super(scope, id, props);

    const vpc = new ec2.Vpc(this, 'MyVpc', { maxAzs: 2 });
    const cluster = new ecs.Cluster(this, 'Cluster', { vpc });

    const albFargateService = new ecs_patterns.ApplicationLoadBalancedFargateService(this, "FargateService", {
      cluster,
      taskImageOptions: {
        image: ecs.ContainerImage.fromRegistry("amazon/amazon-ecs-sample"),
      },
    });

    const cfnService = albFargateService.service.node.children.find(c => c instanceof ecs.CfnService) as ecs.CfnService;
    cfnService.desiredCount = undefined;
  }
}

const app = new cdk.App();
new TestingStack(app, 'Bonjour');

@mergify mergify bot closed this as completed in #13130 Feb 25, 2021
mergify bot pushed a commit that referenced this issue Feb 25, 2021
…aviour (under feature flag) (#13130)

AWS CloudFormation previously required a desiredCount to be specified to create an Amazon ECS Service. They have recently removed that required field, and now provide the following default behaviour, if the desired count is not specified.
* when creating a service, the service is created and starts up with a desired count of 1.
* when updating a service, the service is updated and starts up with the same desired count as it had prior to the deployment.

The AWS-CDK currently adds a default desiredCount of 1 when creating or updating an Amazon ECS service, and this could lead to issues when a service has scaled up due to autoscaling. When updating a service using the AWS-CDK, the service would then be scaled down to the original desiredCount, leading to potential outages.

This change introduces a new feature flag `REMOVE_DEFAULT_DESIRED_COUNT` to flip the default behaviour of setting the desiredCount for any service created using any of the following ECS Patterns constructs. Without the feature flag set, the desiredCount of any service created is automatically defaulted to 1 (which is the current behaviour). With the feature flag set (to true), the default will be removed and the desiredCount will only be passed to the service if it is passed in as input.

* ApplicationLoadBalancedEc2Service
* ApplicationLoadBalancedFargateService
* NetworkLoadBalancedEc2Service
* NetworkLoadBalancedFargateService
* QueueProcessingEc2Service
* QueueProcessingFargateService

**BREAKING CHANGE:** the desiredCount property stored on the above constructs will be optional, allowing them to be undefined. This is enabled through the `@aws-cdk/aws-ecs-patterns:removeDefaultDesiredCount` feature flag. We would recommend all aws-cdk users to set the `REMOVE_DEFAULT_DESIRED_COUNT` flag to true for all of their existing applications. 

Fixes: #12990

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-ecs-patterns Related to ecs-patterns library bug This issue is a bug. in-progress This issue is being actively worked on.
Projects
None yet
3 participants