Skip to content

Commit

Permalink
feat(elasticloadbalancingv2): health check interval greater than time…
Browse files Browse the repository at this point in the history
…out (#29075)

Closes #29062.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
msambol authored Mar 8, 2024
1 parent 2dbb381 commit 576d034
Show file tree
Hide file tree
Showing 5 changed files with 96 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -280,9 +280,6 @@ export class NetworkTargetGroup extends TargetGroupBase implements INetworkTarge
if (timeoutSeconds < lowHealthCheckTimeout || timeoutSeconds > highHealthCheckTimeout) {
ret.push(`Health check timeout '${timeoutSeconds}' not supported. Must be a number between ${lowHealthCheckTimeout} and ${highHealthCheckTimeout}.`);
}
if (healthCheck.interval && healthCheck.interval.toSeconds() < timeoutSeconds) {
ret.push(`Health check timeout '${timeoutSeconds}' must not be greater than the interval '${healthCheck.interval.toSeconds()}'`);
}
}

return ret;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,7 @@ export abstract class TargetGroupBase extends Construct implements ITargetGroup
this.targetGroupName = this.resource.attrTargetGroupName;
this.defaultPort = additionalProps.port;

this.node.addValidation({ validate: () => this.validateHealthCheck() });
this.node.addValidation({ validate: () => this.validateTargetGroup() });
}

Expand Down Expand Up @@ -351,6 +352,22 @@ export abstract class TargetGroupBase extends Construct implements ITargetGroup

return ret;
}

protected validateHealthCheck(): string[] {
const ret = new Array<string>();

const intervalSeconds = this.healthCheck.interval?.toSeconds();
const timeoutSeconds = this.healthCheck.timeout?.toSeconds();

if (intervalSeconds && timeoutSeconds) {
if (intervalSeconds < timeoutSeconds) {
// < instead of <= for backwards compatibility, see discussion in https://github.com/aws/aws-cdk/pull/26031
ret.push('Health check interval must be greater than or equal to the timeout; received interval ' +
`${intervalSeconds}, timeout ${timeoutSeconds}.`);
}
}
return ret;
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -549,6 +549,45 @@ describe('tests', () => {
}).toThrow('Healthcheck interval 1 minute must be greater than the timeout 2 minute');
});

test('Throws error for health check interval less than timeout', () => {
const app = new cdk.App();
const stack = new cdk.Stack(app, 'Stack');
const vpc = new ec2.Vpc(stack, 'Vpc');

new elbv2.ApplicationTargetGroup(stack, 'TargetGroup', {
vpc,
port: 80,
healthCheck: {
interval: cdk.Duration.seconds(10),
timeout: cdk.Duration.seconds(20),
},
});

expect(() => {
app.synth();
}).toThrow('Health check interval must be greater than or equal to the timeout; received interval 10, timeout 20.');
});

// for backwards compatibility these can be equal, see discussion in https://github.com/aws/aws-cdk/pull/26031
test('Throws error for health check interval less than timeout', () => {
const app = new cdk.App();
const stack = new cdk.Stack(app, 'Stack');
const vpc = new ec2.Vpc(stack, 'Vpc');

new elbv2.ApplicationTargetGroup(stack, 'TargetGroup', {
vpc,
port: 80,
healthCheck: {
interval: cdk.Duration.seconds(10),
timeout: cdk.Duration.seconds(20),
},
});

expect(() => {
app.synth();
}).toThrow('Health check interval must be greater than or equal to the timeout; received interval 10, timeout 20.');
});

test('imported targetGroup has targetGroupName', () => {
// GIVEN
const app = new cdk.App();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,7 @@ describe('tests', () => {
// THEN
const validationErrors: string[] = targetGroup.node.validate();
expect(validationErrors).toEqual([
"Health check timeout '40' must not be greater than the interval '30'",
'Health check interval must be greater than or equal to the timeout; received interval 30, timeout 40.',
]);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,45 @@ describe('tests', () => {
}).toThrow(/Health check interval '3' not supported. Must be between 5 and 300./);
});

test('Throws error for health check interval less than timeout', () => {
const app = new cdk.App();
const stack = new cdk.Stack(app, 'Stack');
const vpc = new ec2.Vpc(stack, 'Vpc');

new elbv2.NetworkTargetGroup(stack, 'Group', {
vpc,
port: 80,
healthCheck: {
interval: cdk.Duration.seconds(10),
timeout: cdk.Duration.seconds(20),
},
});

expect(() => {
app.synth();
}).toThrow('Health check interval must be greater than or equal to the timeout; received interval 10, timeout 20.');
});

// for backwards compatibility these can be equal, see discussion in https://github.com/aws/aws-cdk/pull/26031
test('No error for health check interval == timeout', () => {
const app = new cdk.App();
const stack = new cdk.Stack(app, 'Stack');
const vpc = new ec2.Vpc(stack, 'Vpc');

new elbv2.NetworkTargetGroup(stack, 'Group', {
vpc,
port: 80,
healthCheck: {
interval: cdk.Duration.seconds(10),
timeout: cdk.Duration.seconds(10),
},
});

expect(() => {
app.synth();
}).not.toThrow();
});

test('targetGroupName unallowed: more than 32 characters', () => {
// GIVEN
const app = new cdk.App();
Expand Down

0 comments on commit 576d034

Please sign in to comment.