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-autoscaling: Step scaling properties expose unsupported cooldown property #29779

Closed
lucajone opened this issue Apr 10, 2024 · 2 comments · Fixed by #30150
Closed

aws-autoscaling: Step scaling properties expose unsupported cooldown property #29779

lucajone opened this issue Apr 10, 2024 · 2 comments · Fixed by #30150
Assignees
Labels
@aws-cdk/aws-autoscaling Related to Amazon EC2 Auto Scaling bug This issue is a bug. effort/small Small work item – less than a day of effort p2

Comments

@lucajone
Copy link

Describe the bug

The StepScalingPolicyProps interface and related StepScalingActionProps interface expose a cooldown property, which if specified becomes the cooldown property of a CfnScalingPolicy here:

cooldown: props.cooldown && props.cooldown.toSeconds().toString(),

However, cooldown is not a supported property for step scaling policies. The documentation for the AWS::AutoScaling::ScalingPolicy resource says:

Valid only if the policy type is SimpleScaling. For more information, see Scaling cooldowns for Amazon EC2 Auto Scaling in the Amazon EC2 Auto Scaling User Guide.

Specifying this property has no effect and can cause CloudFormation deployment errors in some regions (such as ap-southeast-4).

Expected Behavior

Since this property has no effect and can cause deployment errors, it should not be possible to specify for step scaling policies and should be deprecated.

Current Behavior

The property can be specified and results in an AWS::AutoScaling::ScalingPolicy with PolicyType set to StepScaling and the Cooldown property present. This is ineffectual or can cause the following error on deployment, depending on the region:

Property Cooldown cannot be specified.

Reproduction Steps

The following application reproduces the deployment error when deployed to ap-southeast-4.

import { App, Duration, Stack, StackProps } from 'aws-cdk-lib';
import { AutoScalingGroup, } from 'aws-cdk-lib/aws-autoscaling';
import { AmazonLinuxCpuType, InstanceClass, InstanceSize, InstanceType, MachineImage, Vpc } from 'aws-cdk-lib/aws-ec2';
import { Construct } from 'constructs';
import { Metric } from 'aws-cdk-lib/aws-cloudwatch';

export class ReproStack extends Stack {
  constructor(scope: Construct, id: string, props?: StackProps) {
    super(scope, id, props);

    const vpc = new Vpc(this, "VPC");
    const asg = new AutoScalingGroup(this, "ASG", {
      vpc,
      machineImage: MachineImage.latestAmazonLinux2023({ cpuType: AmazonLinuxCpuType.ARM_64 }),
      instanceType: InstanceType.of(InstanceClass.T4G, InstanceSize.MICRO),
    });

    const metric = new Metric({
      namespace: "Repro",
      metricName: "Metric",
    });

    asg.scaleOnMetric("MetricScale", {
      metric,
      scalingSteps: [
        { upper: 10, change: -5 },
        { lower: 50, change: 5 }
      ],
      cooldown: Duration.minutes(5),
    })
  }
}

const app = new App();
new ReproStack(app, "Repro");

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.136.1

Framework Version

No response

Node.js Version

v18.0.0

OS

macOS

Language

TypeScript

Language Version

5.4.3

Other information

(Amazon ticket D120666119 has some more detail.)

@lucajone lucajone added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Apr 10, 2024
@github-actions github-actions bot added the @aws-cdk/aws-autoscaling Related to Amazon EC2 Auto Scaling label Apr 10, 2024
@khushail khushail added the investigating This issue is being investigated and/or work is in progress to resolve the issue. label Apr 11, 2024
@khushail khushail self-assigned this Apr 11, 2024
@khushail khushail removed the needs-triage This issue or PR still needs to be triaged. label Apr 11, 2024
@khushail
Copy link
Contributor

khushail commented Apr 11, 2024

Hi @lucajone thanks for reporting this. I implemented this in us-east-1 and it succeeded. I wonder though it would have any impact. You are absolutely right in saying exposing this property as multiple interfaces won't serve the purpose as its exclusively for 'SimpleScaling` policy type.

Marking it as appropriate.

@khushail khushail added p2 effort/small Small work item – less than a day of effort and removed investigating This issue is being investigated and/or work is in progress to resolve the issue. labels Apr 11, 2024
@GavinZZ GavinZZ assigned GavinZZ and unassigned khushail May 10, 2024
@mergify mergify bot closed this as completed in #30150 May 12, 2024
mergify bot pushed a commit that referenced this issue May 12, 2024
…30150)

### Issue # (if applicable)

Closes #29779 

### Reason for this change

`cooldown` cannot be set on step scaling and can cause deployment failure in certain regions.

### Description of changes

Deprecate the property and ignore the value. Give a warning.

### Description of how you validated changes

Unit test added.

### Checklist
- [ ] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
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-autoscaling Related to Amazon EC2 Auto Scaling bug This issue is a bug. effort/small Small work item – less than a day of effort p2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants