Skip to content

Commit

Permalink
feat(applicationautoscaling): validate evaluationPeriods and `datap…
Browse files Browse the repository at this point in the history
…ointsToAlarm` for step scaling policy (#28880)

This PR adds validations for `evaluationPeriods` and `datapointsToAlarm` for step scaling policy. It is based on [the PR for datapointsToAlarm in autoscaling step scaling policy](#28792).

Check the following cases:
- `evaluationPeriods` < 1
- `datapointsToAlarm` is set, and
  - `evaluationPeriods` is not set
  - `datapointsToAlarm` < 1
  - `evaluationPeriods` < `datapointsToAlarm`

These validations also consider whether those parameters are tokens or not.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
go-to-k authored and SankyRed committed Feb 8, 2024
1 parent c148af0 commit 2078944
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ export interface BasicStepScalingPolicyProps {
*
* Only has meaning if `evaluationPeriods != 1`.
*
* @default `evaluationPeriods`
* @default - Same as `evaluationPeriods`
*/
readonly datapointsToAlarm?: number;

Expand Down Expand Up @@ -117,8 +117,22 @@ export class StepScalingPolicy extends Construct {
throw new Error(`'scalingSteps' can have at most 40 steps, got ${props.scalingSteps.length}`);
}

if (props.datapointsToAlarm !== undefined && props.datapointsToAlarm < 1) {
throw new RangeError(`datapointsToAlarm cannot be less than 1, got: ${props.datapointsToAlarm}`);
if (props.evaluationPeriods !== undefined && !cdk.Token.isUnresolved(props.evaluationPeriods) && props.evaluationPeriods < 1) {
throw new Error(`evaluationPeriods cannot be less than 1, got: ${props.evaluationPeriods}`);
}
if (props.datapointsToAlarm !== undefined) {
if (props.evaluationPeriods === undefined) {
throw new Error('evaluationPeriods must be set if datapointsToAlarm is set');
}
if (!cdk.Token.isUnresolved(props.datapointsToAlarm) && props.datapointsToAlarm < 1) {
throw new Error(`datapointsToAlarm cannot be less than 1, got: ${props.datapointsToAlarm}`);
}
if (!cdk.Token.isUnresolved(props.datapointsToAlarm)
&& !cdk.Token.isUnresolved(props.evaluationPeriods)
&& props.evaluationPeriods < props.datapointsToAlarm
) {
throw new Error(`datapointsToAlarm must be less than or equal to evaluationPeriods, got datapointsToAlarm: ${props.datapointsToAlarm}, evaluationPeriods: ${props.evaluationPeriods}`);
}
}

const adjustmentType = props.adjustmentType || AdjustmentType.CHANGE_IN_CAPACITY;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,26 @@ describe('step scaling policy', () => {

});

test('step scaling with invalid evaluation period throws error', () => {
// GIVEN
const stack = new cdk.Stack();
const target = createScalableTarget(stack);

// THEN
expect(() => {
target.scaleOnMetric('Tracking', {
metric: new cloudwatch.Metric({ namespace: 'Test', metricName: 'Metric', statistic: 'p99' }),
scalingSteps: [
{ upper: 0, change: -1 },
{ lower: 100, change: +1 },
{ lower: 500, change: +5 },
],
evaluationPeriods: 0,
metricAggregationType: appscaling.MetricAggregationType.MAXIMUM,
});
}).toThrow(/evaluationPeriods cannot be less than 1, got: 0/);
});

test('step scaling with evaluation period & data points to alarm configured', () => {
// GIVEN
const stack = new cdk.Stack();
Expand Down Expand Up @@ -274,6 +294,47 @@ describe('step scaling policy', () => {
}).toThrow('datapointsToAlarm cannot be less than 1, got: 0');
});

test('step scaling with datapointsToAlarm is greater than evaluationPeriods throws error', () => {
// GIVEN
const stack = new cdk.Stack();
const target = createScalableTarget(stack);

// THEN
expect(() => {
target.scaleOnMetric('Tracking', {
metric: new cloudwatch.Metric({ namespace: 'Test', metricName: 'Metric', statistic: 'p99' }),
scalingSteps: [
{ upper: 0, change: -1 },
{ lower: 100, change: +1 },
{ lower: 500, change: +5 },
],
evaluationPeriods: 10,
datapointsToAlarm: 15,
metricAggregationType: appscaling.MetricAggregationType.MAXIMUM,
});
}).toThrow(/datapointsToAlarm must be less than or equal to evaluationPeriods, got datapointsToAlarm: 15, evaluationPeriods: 10/);
});

test('step scaling with datapointsToAlarm without evaluationPeriods throws error', () => {
// GIVEN
const stack = new cdk.Stack();
const target = createScalableTarget(stack);

// THEN
expect(() => {
target.scaleOnMetric('Tracking', {
metric: new cloudwatch.Metric({ namespace: 'Test', metricName: 'Metric', statistic: 'p99' }),
scalingSteps: [
{ upper: 0, change: -1 },
{ lower: 100, change: +1 },
{ lower: 500, change: +5 },
],
datapointsToAlarm: 15,
metricAggregationType: appscaling.MetricAggregationType.MAXIMUM,
});
}).toThrow(/evaluationPeriods must be set if datapointsToAlarm is set/);
});

test('scalingSteps must have at least 2 steps', () => {
// GIVEN
const stack = new cdk.Stack();
Expand Down

0 comments on commit 2078944

Please sign in to comment.