Skip to content

Conversation

snese
Copy link

@snese snese commented Sep 9, 2025

Issue #34921

Closes #34921.

Reason for this change

The CDK LaunchTemplate construct was missing the CapacityReservationSpecification parameter that exists in the CloudFormation specification. This prevented users from utilizing EC2 capacity reservations through CDK, causing job failures for instance types with poor on-demand availability (like p4d.24xlarge for GPU workloads). Users were forced to use workarounds with addPropertyOverride() that bypass CDK's type safety.

Description of changes

Added comprehensive Capacity Reservation support to the LaunchTemplate construct following CDK L2 construct patterns and design guidelines:

L2 Construct Implementation:

  • New interface: ICapacityReservation - represents a Capacity Reservation reference
  • New class: CapacityReservation with static factory methods:
    • fromId(capacityReservationId) - reference by Capacity Reservation ID
    • fromResourceGroupArn(resourceGroupArn) - reference by resource group ARN
  • New enum: CapacityReservationPreference with OPEN, NONE, and CAPACITY_RESERVATIONS_ONLY values

Flat Property Structure (following CDK Design Guidelines):

  • capacityReservation?: ICapacityReservation - direct property on LaunchTemplateProps
  • capacityReservationPreference?: CapacityReservationPreference - direct property on LaunchTemplateProps
  • No artificial nesting - properties are flat and discoverable

Key Features:

  • Type-safe factory methods following CDK patterns
  • Direct 1:1 mapping to CloudFormation AWS::EC2::LaunchTemplate.CapacityReservationSpecification
  • Backward compatible - properties omitted from CloudFormation when not specified
  • Comprehensive JSDoc documentation with AWS service links
  • Clear default behavior documentation

Example Usage:

// Reference by Capacity Reservation ID
new ec2.LaunchTemplate(this, 'Template', {
  machineImage: ec2.MachineImage.latestAmazonLinux2023(),
  capacityReservation: ec2.CapacityReservation.fromId('cr-1234567890abcdef0'),
  capacityReservationPreference: ec2.CapacityReservationPreference.OPEN,
});

// Reference by resource group ARN
new ec2.LaunchTemplate(this, 'Template', {
  machineImage: ec2.MachineImage.latestAmazonLinux2023(),
  capacityReservation: ec2.CapacityReservation.fromResourceGroupArn('arn:aws:resource-groups:...'),
  capacityReservationPreference: ec2.CapacityReservationPreference.CAPACITY_RESERVATIONS_ONLY,
});

Describe any new or updated permissions being added

N/A - This change only adds configuration properties that map to existing CloudFormation functionality. No new IAM permissions are required.

Description of how you validated changes

  • Unit tests: 7 comprehensive unit tests covering all scenarios:
    • All enum values (OPEN, NONE, CAPACITY_RESERVATIONS_ONLY)
    • Capacity Reservation by ID
    • Capacity Reservation by resource group ARN
    • Combined reservation + preference
    • Backward compatibility (property omission when not specified)
    • CloudFormation template generation validation
  • Integration tests: TypeScript integration test with 3 comprehensive scenarios
  • Build verification: All TypeScript compilation, linting, and JSII compilation passes
  • Regression testing: All 96 existing LaunchTemplate unit tests pass (100% pass rate)

Reviewer Feedback Addressed

This implementation addresses all feedback from @kaizencc and @rix0rrr:

  • ✅ Created proper L2 constructs (ICapacityReservation, CapacityReservation class)
  • ✅ Flattened property structure (no artificial nesting)
  • ✅ Factory methods following CDK patterns (fromId, fromResourceGroupArn)
  • ✅ TypeScript integration test (not JavaScript)
  • ✅ Clear documentation of default behaviors
  • ✅ Removed unnecessary fixture references

Checklist


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

@github-actions github-actions bot added the beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK label Sep 9, 2025
@aws-cdk-automation aws-cdk-automation requested a review from a team September 9, 2025 05:14
@github-actions github-actions bot added effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2 labels Sep 9, 2025
Copy link
Collaborator

@aws-cdk-automation aws-cdk-automation left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(This review is outdated)

@snese snese changed the title feat(aws-ec2): add CapacityReservationSpecification support to Launch… feat(ec2): add CapacityReservationSpecification support to Launch… Sep 9, 2025
@pahud pahud marked this pull request as draft September 9, 2025 11:18
@pahud pahud self-assigned this Sep 9, 2025
@aws-cdk-automation aws-cdk-automation dismissed their stale review September 9, 2025 11:37

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

pahud
pahud previously requested changes Sep 11, 2025
@mergify mergify bot dismissed pahud’s stale review September 14, 2025 12:55

Pull request has been modified.

pahud
pahud previously requested changes Sep 19, 2025
Copy link
Contributor

@pahud pahud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aws-cdk-testing/framework-integ:   UNCHANGED  triggers/test/integ.triggers 1.349s
@aws-cdk-testing/framework-integ: Snapshot Results: 
@aws-cdk-testing/framework-integ: Tests:    1 failed, 1265 total
@aws-cdk-testing/framework-integ: Failed: /codebuild/output/src3595677729/src/actions-runner/_work/aws-cdk/aws-cdk/packages/@aws-cdk-testing/framework-integ/test/aws-ec2/test/integ.launch-template-capacity-reservation.js
@aws-cdk-testing/framework-integ: Error: Some tests failed!
@aws-cdk-testing/framework-integ: To re-run failed tests run: integ-runner --update-on-failed
@aws-cdk-testing/framework-integ:     at main (/codebuild/output/src3595677729/src/actions-runner/_work/aws-cdk/aws-cdk/node_modules/@aws-cdk/integ-runner/lib/index.js:10626:13)
@aws-cdk-testing/framework-integ: Error: integ-runner exited with error code 1
@aws-cdk-testing/framework-integ: Tests failed. Total time (2m59.8s) | integ-runner (2m13.9s) | /codebuild/output/src3595677729/src/actions-runner/_work/aws-cdk/aws-cdk/node_modules/jest/bin/jest.js (44.7s)
@aws-cdk-testing/framework-integ: !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
@aws-cdk-testing/framework-integ: 
@aws-cdk-testing/framework-integ: error Command failed with exit code 1.

let's re-run the integ tests

@mergify mergify bot dismissed pahud’s stale review September 19, 2025 14:18

Pull request has been modified.

@pahud pahud marked this pull request as ready for review September 19, 2025 17:07
@pahud pahud removed their assignment Sep 19, 2025
Copy link
Contributor

@pahud pahud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Requesting the team for another review.

@aws-cdk-automation aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Sep 19, 2025
Copy link
Contributor

@kaizencc kaizencc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR @snese. I think we're skirting around actually modeling new L2s and I'm confused why. That seems like the better approach to me that is more maintainable and will follow CDK conventions better.

One more thing: the title is cutoff and should not have the word add.


You can specify [EC2 Capacity Reservations](https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/ec2-capacity-reservations.html) for your launch template to ensure that EC2 capacity is reserved for your instances:

```ts fixture=with-vpc
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: why do we need fixture=with-vpc?

* The ARN of the Capacity Reservation resource group in which to run the instance
* @default - No Capacity Reservation resource group
*/
readonly capacityReservationResourceGroupArn?: string;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we rarely model things as Arn in CDK. This represents a place where we can uplift the existing CFN spec and modle capacityReservationResourceGroup directly.

The issue is that if we model capacityReservationResourceGroup in the future, then it wouldn't match the normal CDK pattern of supplying a IInterface to connect constructs

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

after doing a ton of research it looks like CFN does not even have a capacityReservationResourceGroup. But even then, we need to model this as an L2 with the static function fromResourceGroupArn so that we still don't model this as a raw arn.

// Fields not yet implemented:
// ==========================
// https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-ec2-launchtemplate-launchtemplatedata-capacityreservationspecification.html
// Will require creating an L2 for AWS::EC2::CapacityReservation
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this comment sums it up: to support this we need to create an L2 for AWS::ECR::CapacityReservation

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you've modeled all the individual components of AWS::ECR::CapacityReservation why not just create it as an L2 also?

then the property becomes

readonly capacityReservation?: ICapacityReservation;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why .lit.ts? this is a legacy way of providing examples in the readme.

@@ -0,0 +1,63 @@
const cdk = require('aws-cdk-lib');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be a TypeScript file, not a .js file.

Comment on lines 12 to 15
capacityReservationSpecification: {
capacityReservationPreference: ec2.CapacityReservationPreference.OPEN,
capacityReservationTarget: {
capacityReservationId: 'cr-1234567890abcdef0',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going code blind from all the times that the words capacityReservation shows up here. Why is this not:

capacityReservation: CapacityReservation.fromId(this, 'Reservation', 'cr-1234567890abcdef0'),
capacityReservationPreference: ec2.CapacityReservationPreference.OPEN,

* Capacity Reservation specification
* @default - No capacity reservation specification
*/
readonly capacityReservationSpecification?: LaunchTemplateCapacityReservationSpecification;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This goes against our design guidelines:

Do not introduce artificial nesting for props. It hinders discoverability and makes it cumbersome to use in some languages (like Java) [awslint:props-flat].

https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md#flat

* Information about the target Capacity Reservation or Capacity Reservation group
* @default - No target specified
*/
readonly capacityReservationTarget?: LaunchTemplateCapacityReservationTarget;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't target be required if you specify a capacity reservation?

I can now specify:

capacityReservationSpecification: { }

What does that mean?

export interface LaunchTemplateCapacityReservationSpecification {
/**
* Indicates the instance's Capacity Reservation preferences
* @default - No preference specified
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Surely there is an implied default here. Tell us what the default is.

Describe the default value or default behavior, even if it's not CDK that controls the default. For example, if an absent value does not get rendered into the template and it's ultimately the AWS service that determines the default behavior, we still describe it in our documentation.

https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md#defaults

"No XXX specified" is not useful to say. Here's what you're telling a user:

"If you don't specify a capacityReservationPreference, then no capacityReservationPreference is specified."

I think we can trust our customers to figure that out for themselves. What they want to know is what happens when they don't specify a certain thing.

@aws-cdk-automation aws-cdk-automation removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Oct 14, 2025
@snese snese force-pushed the feat-34921-capacity-reservation-specification branch from 12659f7 to 8132748 Compare October 15, 2025 09:52
@mergify mergify bot dismissed stale reviews from kaizencc and rix0rrr October 15, 2025 09:53

Pull request has been modified.

snese added 5 commits October 15, 2025 21:04
…Template construct

- Add LaunchTemplateCapacityReservationSpecification and LaunchTemplateCapacityReservationTarget interfaces
- Add CapacityReservationPreference enum with OPEN and NONE values
- Add optional capacityReservationSpecification property to LaunchTemplateProps
- Map property to existing CloudFormation L1 constructs
- Add 7 comprehensive unit tests covering all scenarios
- Maintain backward compatibility with optional property

Closes aws#34921
…ReservationSpecification

- Add capacity reservation documentation to aws-ec2 README with usage examples
- Create integration test with snapshot for capacity reservation functionality
- Fix PR title scope from aws-ec2 to ec2 per CDK automation requirements
…fication

- Add integ.launch-template-capacity-reservation.lit.ts in main package
- Follows existing pattern of .lit.ts integration tests
- Tests both capacity reservation ID and resource group ARN scenarios
…late construct

- Add LaunchTemplateCapacityReservationSpecification interface
- Add LaunchTemplateCapacityReservationTarget interface
- Add CapacityReservationPreference enum with OPEN, NONE, CAPACITY_RESERVATIONS_ONLY values
- Add optional capacityReservationSpecification property to LaunchTemplateProps
- Implement CloudFormation mapping to AWS::EC2::LaunchTemplate.CapacityReservationSpecification
- Add comprehensive unit tests covering all property combinations
- Add README documentation with usage examples
- Maintain backward compatibility - property omitted when not specified

Closes aws#34921
…ecification

- Add comprehensive integration test for CapacityReservationSpecification
- Test covers all capacity reservation preference options (open, none, capacity-reservations-only)
- Test includes capacity reservation target with both ID and resource group ARN
- Validates CloudFormation template generation for capacity reservation properties
- Ensures backward compatibility when capacity reservation is not specified

Addresses aws#34921
…at properties

- Created ICapacityReservation interface and CapacityReservation class with static factory methods
- Flattened property structure: capacityReservation and capacityReservationPreference as direct properties
- Removed artificial nesting (capacityReservationSpecification)
- Changed integration test from .js to .ts format
- Improved documentation with actual default behavior descriptions
- Removed unnecessary fixture=with-vpc from README examples

Addresses feedback from @kaizencc and @rix0rrr:
- Proper L2 construct modeling instead of raw interfaces
- Follows CDK Design Guidelines (no artificial nesting)
- Factory methods: fromId() and fromResourceGroupArn()
- TypeScript integration test instead of JavaScript
- Clear documentation of default behaviors
@snese snese force-pushed the feat-34921-capacity-reservation-specification branch from 8132748 to 63a82f8 Compare October 15, 2025 13:05
@snese
Copy link
Author

snese commented Oct 15, 2025

Reviewer Feedback Addressed

I've updated the implementation to address all feedback from @kaizencc and @rix0rrr. The changes implement proper L2 constructs with flat properties following CDK Design Guidelines.

Key Changes Made:

  1. Created proper L2 constructs (addresses @kaizencc feedback):

    • ICapacityReservation interface
    • CapacityReservation class with static factory methods: fromId() and fromResourceGroupArn()
  2. Flattened property structure (addresses @rix0rrr feedback):

    • Removed artificial nesting (capacityReservationSpecification)
    • Added direct properties: capacityReservation and capacityReservationPreference
    • Follows CDK Design Guidelines: "Do not introduce artificial nesting for props"
  3. TypeScript integration test (addresses @rix0rrr feedback):

    • Changed from .js to .ts format
    • Removed .lit.ts legacy format
  4. Improved documentation (addresses @rix0rrr feedback):

    • Clear default behavior descriptions (not just "no XXX specified")
    • Explains actual AWS service behavior when properties are omitted
  5. Removed unnecessary fixtures (addresses @kaizencc feedback):

    • Removed fixture=with-vpc from README examples

Example Usage (New API):

// Reference by Capacity Reservation ID
new ec2.LaunchTemplate(this, 'Template', {
  machineImage: ec2.MachineImage.latestAmazonLinux2023(),
  capacityReservation: ec2.CapacityReservation.fromId('cr-1234567890abcdef0'),
  capacityReservationPreference: ec2.CapacityReservationPreference.OPEN,
});

This is much cleaner than the previous nested structure and follows the exact pattern suggested by @rix0rrr.

Test Results:

  • All 96 unit tests passing (100% pass rate)
  • Integration test ready in TypeScript format
  • No breaking changes
  • Full backward compatibility

Ready for re-review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

aws-ec2: CapacityReservationSpecification parameter missing in LaunchTemplate construct

5 participants