feat(autoscaling): support for throughput on GP3 volumes #22441
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
|
|
github-actions
bot
added
the
beginning-contributor
aws-cdk-automation
previously requested changes
Oct 10, 2022
csumpter
changed the title
csumpter
force-pushed
the
csumpter/asg-ebs-throughput
branch
2 times, most recently
from
db11b67
to
78ef388
Compare
aws-cdk-automation
dismissed
their stale review
✅ Updated pull request passes all PRLinter validations. Dissmissing previous PRLinter review.
corymhall
previously requested changes
Oct 21, 2022
|
This PR has been in the MERGE CONFLICTS state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week. |
Adds validation on ec2 and autoscaling volumes to ensure that that the throughput to iops ratio.
csumpter
force-pushed
the
csumpter/asg-ebs-throughput
branch
from
78ef388
to
220dfba
Compare
mergify
bot
dismissed
corymhall’s stale review
Pull request has been modified.
|
Hello aws-cdk team! This PR is ready for review at your leisure. Please let me know if there are any additional updates you would like to see. Thank you! |
mrgrain
changed the title
mrgrain
changed the title
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
1 task
1 task
mergify bot
pushed a commit
that referenced
this pull request
Aug 29, 2024
### Issue # (if applicable) Fixes #24341. ### Reason for this change You currently can't specify `throughput` on LaunchTemplate EBS volumes. ### Description of changes This support was simply not included in ec2 when it was added to autoscaling in #22441. I have copied that PR's implementation implementation to ec2 and similarly adapted its tests. ### Description of how you validated changes Unit and integration tests. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
xazhao
pushed a commit
to xazhao/aws-cdk
that referenced
this pull request
Sep 12, 2024
### Issue # (if applicable) Fixes aws#24341. ### Reason for this change You currently can't specify `throughput` on LaunchTemplate EBS volumes. ### Description of changes This support was simply not included in ec2 when it was added to autoscaling in aws#22441. I have copied that PR's implementation implementation to ec2 and similarly adapted its tests. ### Description of how you validated changes Unit and integration tests. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
This was referenced Sep 26, 2024
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.
Adds support for the
throughputproperty on GP3 volumes in both autoscaling and EC2 packages since their volume implementations are separate per the comment https://github.com/aws/aws-cdk/blob/master/packages/@aws-cdk/aws-autoscaling/lib/volume.ts#L1.Change includes unit test coverage, integration test coverage on the autoscaling changes, and validation of the inputq to throughput. I was on the fence about whether to include the validation for synth time checking since as far as I can tell validation is not performed consistently across the CDK at synth time. Happy to modify that behavior pending reviewer feedback.
It was not obvious to me at first pass where similar integration test coverage that was added to the autoscaling package could be added to the EC2 package, so if the reviewer would like to see integration tests for GP3 volumes on in the EC2 package, would you mind providing a hint as to where that test might fit in the existing test paradigm?
Happy to break this change into two PRs -- one for EC2 and one for autoscaling.
closes: #16213
All Submissions:
Adding new Unconventional Dependencies:
New Features
yarn integto deploy the infrastructure and generate the snapshot (i.e.yarn integwithout--dry-run)?By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license