fix(autoscaling): every deployment resets capacity #5507
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If `DesiredCapacity` is specified in the CloudFormation template, on every deployment the capacity of the AutoScalingGroup is reset to that number, even if the group had been scaled out at that point. The solution is to leave DesiredCapacity empty, in which case it will remain untouched during a deployment. Previously, CDK would use some logic to always calculate a DesiredCapacity for you, even if you left the `desiredCapacity` property unset, leading to the undesirable behavior--which frankly represents an availability risk. Now, if you don't specify `desiredCapacity`, we won't set `DesiredCapacity` either, avoiding the availability risk that we introduced beforehand. In fact, if you *do* set `desiredCapacity`, we will warn you that you probably shouldn't using a construct warning. Fixes #5215, closes #5208. BREAKING CHANGE: AutoScalingGroups without `desiredCapacity` are now initially scaled to their minimum capacity (instead of their maximum capaciety).
rix0rrr
requested review from
RomainMuller,
eladb,
MrArnoldPalmer,
NetaNir,
nija-at,
piradeepk and
iamhopaul123
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
piradeepk
previously requested changes
Dec 20, 2019
| * If this is set to a number, every deployment will reset the amount of | ||
| * instances to this number. It is recommended to leave this value blank. | ||
| * | ||
| * @default minCapacity, and leave unchanged during deployment |
There was a problem hiding this comment.
Is this accurate, it doesn't look like you're defaulting desiredCapacity
There was a problem hiding this comment.
This describes the end-to-end behavior of what will happen if you leave the value empty.
If you do not specify a desired capacity, the default is the minimum size of the group.
There was a problem hiding this comment.
Add this link in the docstring
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
eladb
approved these changes
Dec 23, 2019
| * If this is set to a number, every deployment will reset the amount of | ||
| * instances to this number. It is recommended to leave this value blank. | ||
| * | ||
| * @default minCapacity, and leave unchanged during deployment |
There was a problem hiding this comment.
Add this link in the docstring
| if (max < desired) { | ||
| throw new Error(`Should have desiredCapacity (${desired}) <= maxCapacity (${max})`); | ||
| } | ||
| }); | ||
|
|
||
| if (desiredCapacity !== undefined) { | ||
| this.node.addWarning(`desiredCapacity has been configured. Be aware this will reset the size of your AutoScalingGroup on every deployment.`); |
There was a problem hiding this comment.
Provider a link to a github issue in the help message so people can dive deeper/comment.
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Thank you for contributing! Your pull request is now being automatically merged. |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Thank you for contributing! Your pull request is now being automatically merged. |
eladb
pushed a commit
that referenced
this pull request
Dec 24, 2019
* fix(autoscaling): every deployment resets capacity If `DesiredCapacity` is specified in the CloudFormation template, on every deployment the capacity of the AutoScalingGroup is reset to that number, even if the group had been scaled out at that point. The solution is to leave DesiredCapacity empty, in which case it will remain untouched during a deployment. Previously, CDK would use some logic to always calculate a DesiredCapacity for you, even if you left the `desiredCapacity` property unset, leading to the undesirable behavior--which frankly represents an availability risk. Now, if you don't specify `desiredCapacity`, we won't set `DesiredCapacity` either, avoiding the availability risk that we introduced beforehand. In fact, if you *do* set `desiredCapacity`, we will warn you that you probably shouldn't using a construct warning. Fixes #5215, closes #5208. BREAKING CHANGE: AutoScalingGroups without `desiredCapacity` are now initially scaled to their minimum capacity (instead of their maximum capaciety). * Add links Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
eladb
pushed a commit
that referenced
this pull request
Jan 5, 2020
As described in #5215, `desiredCapacity` is not the recommended way to configure an auto scaling group since it will cause the ASG to reset the number of nodes in every CloudFormation deployment. Since EKS's default capacity uses `desiredCapacity` instead of `minCapacity`, as of #5507 this would emit a warning: "desiredCapacity has been configured. Be aware this will reset the size of your AutoScalingGroup on every deployment". This change modifies the behavior of the default capacity such that it will configure the ASG using `minCapacity` instead of `desiredCapacity` as recommended by ASG. Fixes #5650
mergify bot
added a commit
that referenced
this pull request
Jan 6, 2020
…ern (#5651) * fix(eks): default capacity uses desiredCapacity which is an anti-pattern As described in #5215, `desiredCapacity` is not the recommended way to configure an auto scaling group since it will cause the ASG to reset the number of nodes in every CloudFormation deployment. Since EKS's default capacity uses `desiredCapacity` instead of `minCapacity`, as of #5507 this would emit a warning: "desiredCapacity has been configured. Be aware this will reset the size of your AutoScalingGroup on every deployment". This change modifies the behavior of the default capacity such that it will configure the ASG using `minCapacity` instead of `desiredCapacity` as recommended by ASG. Fixes #5650 * Update integ.eks-cluster.defaults.expected.json Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If
DesiredCapacityis specified in the CloudFormation template, on everydeployment the capacity of the AutoScalingGroup is reset to that number,
even if the group had been scaled out at that point. The solution is to
leave DesiredCapacity empty, in which case it will remain untouched
during a deployment.
Previously, CDK would use some logic to always calculate a
DesiredCapacity for you, even if you left the
desiredCapacitypropertyunset, leading to the undesirable behavior--which frankly represents
an availability risk.
Now, if you don't specify
desiredCapacity, we won't setDesiredCapacityeither, avoiding the availability risk that weintroduced beforehand. In fact, if you do set
desiredCapacity, wewill warn you that you probably shouldn't using a construct warning.
Fixes #5215, closes #5208.
BREAKING CHANGE: AutoScalingGroups without
desiredCapacityare nowinitially scaled to their minimum capacity (instead of their maximum
capaciety).
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license