aws-cdk
aws-cdk copied to clipboard
feat(ec2): support throughput on LaunchTemplate EBS volumes
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 and DESIGN GUIDELINES
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license
The build failure:
[4/4] Building fresh packages...
error /codebuild/output/src3794156442/src/github.com/aws/aws-cdk/node_modules/@lerna/create/node_modules/nx, /codebuild/output/src3794156442/src/github.com/aws/aws-cdk/node_modules/@nx/devkit/node_modules/nx, /codebuild/output/src3794156442/src/github.com/aws/aws-cdk/node_modules/lerna/node_modules/nx: Command failed.
Exit code: 135
Command: node ./bin/post-install
Arguments:
Directory: /codebuild/output/src3794156442/src/github.com/aws/aws-cdk/node_modules/lerna/node_modules/nx
Output:
Bus error (core dumped)
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.
[Container] 2024/07/01 02:44:29.448680 Command did not exit successfully /bin/bash ./build.sh --ci exit status 135
[Container] 2024/07/01 02:44:29.451865 Phase complete: BUILD State: FAILED
This is nowhere near the first spurious build failure I've seen in just my third PR to this project, and the problems are not limited to this main CI job: https://github.com/aws/aws-cdk/actions/runs/9737293345/job/26869186116?pr=30716 has also failed for reasons unclear.
It has proven to be challenging to contribute to aws-cdk. Contributors need to pass through an underspecified battery of PR label transitions in order that their PR be brought to a human being's attention so that it may be reviewed and merged. These transitions are automated and gated by CI success. But here is a PR where two of the gating CI jobs have spuriously failed, with no way for me to initiate a retry except by, I guess, rewording my commit message so that I get a different commit hash.
And if contributors don't teach themselves to do this song and dance then their PR will get closed by automation after a period of inactivity, as happened to this changeset in #30317.
Suggestions to maintainers:
- Please specify the actual process for PR review so that contributors understand what is expected of them. Having to spend substantial time trying to infer the process based upon my interactions with flakey CI jobs and occasionally maintainers is discouraging. https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md#getting-a-review-from-a-maintainer does not specify: a. What to do if you can't get this project's flakey CI jobs to pass. b. What an exemption request is. c. What to do if your exemption requests are being ignored. I ended up having to repeatedly ping maintainers I thought could help me on #30317, to no avail. I hope this process was nowhere near as embarrassing to them as it was to me.
- If you cannot produce reliable CI, please stop gating PR review progression on it.
- The automated "close stale PRs" job leaves something to be desired. On #30317, this job closed the PR. I successfully begged a maintainer to reopen it. The job then quickly closed it again. (I gave up for a week, and am now back with a new PR.) Having been reopened by a maintainer, the job should not have quickly closed the PR again.
In aggregate, you have crafted a fully automated, drawn out process for contributors to be (incorrectly, I think) ignored. Ignoring contributors is your prerogative as maintainers, but you could be achieving such outcomes with much less effort than this.
Well, thanks, I guess.
This PR cannot be merged because it has conflicts. Please resolve them. The PR will be considered stale and closed if it remains in an unmergeable state.
https://github.com/aws/aws-cdk/pull/30716#issuecomment-2259539502 the log file is completely empty?
Maybe I missed it, but I only saw the validation and definition of the property
throughput, but I don't see where you assign it toCfnVolume?
https://github.com/aws/aws-cdk/pull/30716/files#diff-c1fd5c618720a81ee00a0360ca37364d5d5281c596a5bc8f63edb637607b6d54R64
This is not using CfnVolume. It's using a (nested) property of LaunchTemplate: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-ec2-launchtemplate-ebs.html
https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_ec2.CfnLaunchTemplate.EbsProperty.html#throughput.
Maybe I missed it, but I only saw the validation and definition of the property
throughput, but I don't see where you assign it toCfnVolume?https://github.com/aws/aws-cdk/pull/30716/files#diff-c1fd5c618720a81ee00a0360ca37364d5d5281c596a5bc8f63edb637607b6d54R64
This is not using
CfnVolume. It's using a (nested) property of LaunchTemplate: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-ec2-launchtemplate-ebs.html https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_ec2.CfnLaunchTemplate.EbsProperty.html#throughput.
Ah I see, I missed that line of code. Thanks for the clarification.
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).
AWS CodeBuild CI Report
- CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
- Commit ID: 37e72916a1c9d7c44774e01ccb981ea2cb277d76
- Result: SUCCEEDED
- Build Logs (available for 30 days)
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository
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).
Comments on closed issues and PRs are hard for our team to see. If you need help, please open a new issue that references this one.