feat(autoscaling): CloudFormation init for ASGs #9674

njlynch · 2020-08-13T13:22:11Z

Introduces CloudFormation init (cfn-init) support for autoscaling groups.
This builds on the previous work (#9065) and (#9664) that introduced init
support for instances.

This change also reworks the existing signaling functionality, as this becomes
even more important with cfn-init.

A final change is to export the CloudFormationInit._attach method and related
options.

Credit for 90% of this goes to @rix0rrr; all cfn-init support was pair-
programmed, but the ASG stuff was mostly him.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@rix0rrr

Introduces CloudFormation init (cfn-init) support for autoscaling groups. This builds on the previous work (#9065) and (#9664) that introduced init support for instances. This change also reworks the existing signaling functionality, as this becomes even more important with cfn-init. A final change was to adjust the (internal) interface of the `_attach` method to accept an `ec2.OperatingSystemType` instead of the internal-only `ec2.InitPlatform` type which wasn't exported and unavailable within the asg module. This change is also in the #9664 diff, as both are pending. Credit for 90% of this goes to @rix0rrr; all cfn-init support was pair- programmed, but the ASG stuff was much more heavily done by him.

…scaling

NetaNir

Initial comments

NetaNir · 2020-08-20T20:24:23Z

packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts

+   *
+   * @default true if you configured `signals` on the AutoScalingGroup, false otherwise
+   */
+  readonly waitOnResourceSignals?: boolean;


Is this property necessary? wouldn't providing signal (or not providing it) be sufficient?

Well yes but the point is you can disable it for updates. I don't know why you'd want to, but CFN allows you to do so.

You'll notice the default "does the right thing", so this property is here for completeness and someone's edge case.

NetaNir · 2020-08-21T04:18:58Z

packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts

+/**
+ * Options for rendering UpdatePolicy
+ */
+interface RenderUpdateOptions {


Since this used only in an internal method, does it needs to be an interface?

Maybe not isn't this a nice pattern to just use everywhere?

packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts

NetaNir · 2020-08-21T05:21:51Z

packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts

+}
+
+/**
+ * How existing instances should be updated


This actually control how the auto scaling group and the instance in it will be updated

What does "how the auto scaling group will be updated" even mean? I mean, technically I guess so, but the thing you're interested in will be the instances in the ASG.

packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts

NetaNir · 2020-08-21T05:32:11Z

packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts

+   *
+   * @default - The `timeout` configured for `signals` on the AutoScalingGroup
+   */
+  readonly pauseTime?: Duration;


Should this property also be deprecated in favor of the value in signals?
Add that the max allowed value is 1hr

Again, the default does the right thing but CFN allows you to customize and we'll get yelled at if we don't offer the customization.

Do we need to expose all L1 properties? This one feels very confusing

Erring on the side of caution. Again, it will just use the right default if you don't specify anything.

NetaNir · 2020-08-21T05:35:30Z

packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts

+   */
+  protected doRender(options: SignalsOptions, count?: number): CfnCreationPolicy {
+    const minSuccessfulInstancesPercent = validatePercentage(options.minSuccessPercentage);
+    return {


Seems like the validatePercentage was already called in all calling methods?

Just to be safe I guess?

NetaNir · 2020-08-21T05:41:34Z

packages/@aws-cdk/aws-autoscaling/README.md

+
+* you *must* also specify `signals` to configure how long CloudFormation
+  should wait for the instances to successfully configure themselves.
+* you *should* also specify an `updatePolicy` to configure how instances


Since updatePolicy can be used without configuring cfn init it will be helpful to add a section on it

NetaNir · 2020-08-21T06:12:59Z

packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts

+  /**
+   * ConfigSet to activate
+   *
+   * @default ['default']


What does default mean here?

The configset with the literal name "default".

rix0rrr

After reading this... what is the default UpdatePolicy when you specify a CfnInit configuration?

Feels silly to have it be UpdatePolicy.none(). I think a rollingUpdate() with default settings makes for a good default. If we do that, we might need to add UpdatePolicy.none() to have people be able to disable it.

gitpod-io · 2020-10-26T13:56:18Z

rix0rrr · 2020-10-26T13:59:37Z

Made 2 final tweaks:

updatePolicy is implied if you also specify init (actually we should have strongly discouraged UpdatePolicy=NONE from the start, but backwards compatibility and all that...)
If you specify signals without also specifying init, we used to require you to take care of IAM permissions yourself. That's silly if you are clearly expressing intent, let's just add it. Usage will be rare but it's going to be just that bit smoother for people who are.

Added explanations of updatepolicy/signals as requested.

rix0rrr · 2020-10-26T14:01:13Z

@NetaNir unless you have strong objections we should be merging this. It's been out for too long.

hansterwal · 2020-10-27T09:14:14Z

Hi, In our one of our cloudformation templates we use an ASG and all two launchtemplates. If its production it defaults to launchtemplate high. And for other environments it defaults to launchtemplate low.

We offer a possibility to switch to out templates when necessary using a custom portal.

Does this construct support this?

packages/@aws-cdk/aws-autoscaling/README.md

njlynch · 2020-10-27T15:35:23Z

packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts

@@ -1094,7 +1096,7 @@ export class AutoScalingGroup extends AutoScalingGroupBase implements
   * - Update the instance's CreationPolicy to wait for `cfn-init` to finish
   *   before reporting success.
   */
-  private applyCloudFormationInit(init: ec2.CloudFormationInit, options: ApplyCloudFormationInitOptions = {}) {
+  public applyCloudFormationInit(init: ec2.CloudFormationInit, options: ApplyCloudFormationInitOptions = {}) {


One question now that this is public -- what's the behavior if I supply one init in the constructor and then call this separately? (Or call this twice, with either different or the same init)? Should we error in that case?

I wanna say they just shouldn't do that, but I guess we can add a check on it, sure.

Co-authored-by: Nick Lynch <[email protected]>

rix0rrr · 2020-10-28T08:36:44Z

@hansterwal we don't support LaunchTemplate-based ASGs yet, so no.

What you need is CfnInit.attach(), it will allow you to attach to some resource of your choosing, add to some other UserData of your choosing, etc. All that kind of flexibility is not supported at the top-level API.

Unfortunately we made the lower-level API private. That's a different change though.

EDIT: See #9674.

njlynch

New changes look good to me!

aws-cdk-automation · 2020-10-28T17:38:59Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 31caf05
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2020-10-28T17:39:34Z

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

njlynch requested review from NetaNir and a team August 13, 2020 13:22

mergify bot added the contribution/core This is a PR that came from AWS. label Aug 13, 2020

Expose Init.attach() and AttachInitOptions to be accessible from auto…

2429578

…scaling

SomayaB assigned NetaNir Aug 13, 2020

njlynch added 2 commits August 17, 2020 12:28

Merge remote-tracking branch 'origin/master' into njlynch/cfn-init-asg

221b5db

New formatting rules

3b3acbe

NetaNir reviewed Aug 21, 2020

View reviewed changes

rix0rrr reviewed Aug 21, 2020

View reviewed changes

njlynch mentioned this pull request Aug 24, 2020

feat(ec2/asg): CloudFormation-init support #8788

Closed

NetaNir mentioned this pull request Aug 26, 2020

AutoScaling: switch to Rolling Updates by default #1080

Closed

njlynch mentioned this pull request Oct 13, 2020

[ec2] Resource ID support in CloudFormationInit (e.g. to use cfn-hup) #10791

Closed

1 task

rix0rrr mentioned this pull request Oct 26, 2020

[EC2] ConfigSets not being generated correctly in c# #11021

Closed

rix0rrr added 3 commits October 26, 2020 13:21

Default updatePolicy

7972382

Merge remote-tracking branch 'origin/master' into njlynch/cfn-init-asg

3b1523f

Add docs, make Signals permissions automatic

5e63f18

rix0rrr requested a review from NetaNir October 26, 2020 14:00

njlynch commented Oct 27, 2020

View reviewed changes

Update packages/@aws-cdk/aws-autoscaling/README.md

2874d4a

Co-authored-by: Nick Lynch <[email protected]>

rix0rrr mentioned this pull request Oct 28, 2020

fix(ec2): CfnInit cannot be used with custom constructs #11167

Merged

Add protection against double-attach

8148391

rix0rrr self-requested a review October 28, 2020 09:11

njlynch commented Oct 28, 2020

View reviewed changes

rix0rrr approved these changes Oct 28, 2020

View reviewed changes

Merge remote-tracking branch 'origin/master' into njlynch/cfn-init-asg

31caf05

mergify bot merged commit bdf1d30 into master Oct 28, 2020

mergify bot deleted the njlynch/cfn-init-asg branch October 28, 2020 17:39

enrico-usai mentioned this pull request Mar 2, 2021

CloudFormationInit support for LaunchTemplate #13351

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(autoscaling): CloudFormation init for ASGs #9674

feat(autoscaling): CloudFormation init for ASGs #9674

njlynch commented Aug 13, 2020 •

edited

Loading

NetaNir left a comment

NetaNir Aug 20, 2020

rix0rrr Aug 21, 2020

NetaNir Aug 21, 2020

rix0rrr Aug 21, 2020

NetaNir Aug 21, 2020

rix0rrr Aug 21, 2020

NetaNir Aug 21, 2020

rix0rrr Aug 21, 2020

NetaNir Aug 26, 2020

rix0rrr Oct 26, 2020

NetaNir Aug 21, 2020

rix0rrr Oct 26, 2020

NetaNir Aug 21, 2020

NetaNir Aug 21, 2020

rix0rrr Aug 21, 2020

rix0rrr left a comment

gitpod-io bot commented Oct 26, 2020 •

edited

Loading

rix0rrr commented Oct 26, 2020

rix0rrr commented Oct 26, 2020

hansterwal commented Oct 27, 2020

njlynch Oct 27, 2020

rix0rrr Oct 28, 2020

rix0rrr commented Oct 28, 2020 •

edited

Loading

njlynch left a comment

aws-cdk-automation commented Oct 28, 2020

mergify bot commented Oct 28, 2020

feat(autoscaling): CloudFormation init for ASGs #9674

feat(autoscaling): CloudFormation init for ASGs #9674

Conversation

njlynch commented Aug 13, 2020 • edited Loading

NetaNir left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rix0rrr left a comment

Choose a reason for hiding this comment

gitpod-io bot commented Oct 26, 2020 • edited Loading

rix0rrr commented Oct 26, 2020

rix0rrr commented Oct 26, 2020

hansterwal commented Oct 27, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rix0rrr commented Oct 28, 2020 • edited Loading

njlynch left a comment

Choose a reason for hiding this comment

aws-cdk-automation commented Oct 28, 2020

AWS CodeBuild CI Report

mergify bot commented Oct 28, 2020

njlynch commented Aug 13, 2020 •

edited

Loading

gitpod-io bot commented Oct 26, 2020 •

edited

Loading

rix0rrr commented Oct 28, 2020 •

edited

Loading