-
Notifications
You must be signed in to change notification settings - Fork 4k
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
feat(autoscaling): health check configuration #3390
Changes from 4 commits
5944005
ef18ded
837f97f
1978b98
a68f32c
50f9e06
430812c
e254ac6
10a41d0
621c076
0b8a6a7
5cf98c4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -156,6 +156,13 @@ export interface CommonAutoScalingGroupProps { | |
* @default none | ||
*/ | ||
readonly spotPrice?: string; | ||
|
||
/** | ||
* Configuration for health checks | ||
* | ||
* @default - HealthCheckConfiguration with defaults. | ||
*/ | ||
readonly healthCheck?: HealthCheck; | ||
} | ||
|
||
/** | ||
|
@@ -444,7 +451,9 @@ export class AutoScalingGroup extends AutoScalingGroupBase implements | |
], | ||
} | ||
], | ||
vpcZoneIdentifier: subnetIds | ||
vpcZoneIdentifier: subnetIds, | ||
healthCheckType: props.healthCheck && props.healthCheck.type, | ||
healthCheckGracePeriod: props.healthCheck && props.healthCheck.gracePeriod, | ||
}; | ||
|
||
if (!hasPublic && props.associatePublicIpAddress) { | ||
|
@@ -673,6 +682,61 @@ export enum ScalingProcess { | |
ADD_TO_LOAD_BALANCER = 'AddToLoadBalancer' | ||
} | ||
|
||
/** | ||
* EC2 Heath check options | ||
*/ | ||
export interface Ec2HealthCheckOptions { | ||
/** | ||
* Specified the time Auto Scaling waits before checking the health status of an EC2 instance that has come into service | ||
* | ||
* @default - | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am assuming no grace is the default? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, like I said in the original PR, the CloudFormation doc isn't explicit, but the API reference says the duration is 0 seconds. |
||
*/ | ||
readonly grace?: Duration; | ||
} | ||
|
||
/** | ||
* ELB Heath check options | ||
*/ | ||
export interface ElbHealthCheckOptions { | ||
/** | ||
* Specified the time Auto Scaling waits before checking the health status of an EC2 instance that has come into service | ||
* | ||
* This option is required for ELB health checks. | ||
*/ | ||
readonly grace: Duration; | ||
} | ||
|
||
/** | ||
* Health check settings | ||
*/ | ||
export class HealthCheck { | ||
/** | ||
* Use EC2 for health checks | ||
* | ||
* @param options EC2 health check options | ||
*/ | ||
public static ec2(options: Ec2HealthCheckOptions = {}): HealthCheck { | ||
return new HealthCheck(HealthCheckType.EC2, options.grace && options.grace.toSeconds()); | ||
} | ||
|
||
/** | ||
* Use ELB for health checks. | ||
* It considers the instance unhealthy if it fails either the EC2 status checks or the load balancer health checks. | ||
* | ||
* @param options ELB health check options | ||
*/ | ||
public static elb(options: ElbHealthCheckOptions): HealthCheck { | ||
return new HealthCheck(HealthCheckType.ELB, options.grace.toSeconds()); | ||
} | ||
|
||
private constructor(public readonly type: string, public readonly gracePeriod?: number) { } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could be a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
} | ||
|
||
export enum HealthCheckType { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No need to export this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
EC2 = 'EC2', | ||
ELB = 'ELB', | ||
} | ||
|
||
/** | ||
* Render the rolling update configuration into the appropriate object | ||
*/ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -397,6 +397,28 @@ export = { | |
test.done(); | ||
}, | ||
|
||
'can configure health check'(test: Test) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. add a test for ec2 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
// GIVEN | ||
const stack = new cdk.Stack(undefined, 'MyStack', { env: { region: 'us-east-1', account: '1234' } }); | ||
const vpc = mockVpc(stack); | ||
|
||
// WHEN | ||
new autoscaling.AutoScalingGroup(stack, 'MyFleet', { | ||
instanceType: ec2.InstanceType.of(ec2.InstanceClass.M4, ec2.InstanceSize.MICRO), | ||
machineImage: new ec2.AmazonLinuxImage(), | ||
vpc, | ||
healthCheck: autoscaling.HealthCheck.elb({grace: cdk.Duration.minutes(15)}) | ||
}); | ||
|
||
// THEN | ||
expect(stack).to(haveResourceLike("AWS::AutoScaling::AutoScalingGroup", { | ||
HealthCheckType: 'ELB', | ||
HealthCheckGracePeriod: 900 | ||
})); | ||
|
||
test.done(); | ||
}, | ||
|
||
'can add Security Group to Fleet'(test: Test) { | ||
// GIVEN | ||
const stack = new cdk.Stack(undefined, 'MyStack', { env: { region: 'us-east-1', account: '1234' } }); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If
healthCheck
is not specified, then there are no health checks, right? Fix the default description.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the health check is mandatory. The CloudFormation documentation says EC2 is the default, and the API reference says the value is required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it, so we should say that EC2 with no grace period is the default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done