Skip to content

feat: emr instance fleet priority allocation#35731

Open
Daniel-ZA wants to merge 6 commits intoaws:mainfrom
Daniel-ZA:feat/emr-instance-fleet-priority-allocation
Open

feat: emr instance fleet priority allocation#35731
Daniel-ZA wants to merge 6 commits intoaws:mainfrom
Daniel-ZA:feat/emr-instance-fleet-priority-allocation

Conversation

@Daniel-ZA
Copy link
Copy Markdown

@Daniel-ZA Daniel-ZA commented Oct 13, 2025

Issue # (if applicable)

Closes #35710

Reason for this change

The CDK's OnDemandAllocationStrategy enum only includes LOWEST_PRICE, but AWS EMR supports a prioritized allocation strategy that allows users to specify instance type priorities. This enhancement aligns the CDK with the full EMR API capabilities.

Description of changes

  1. Add PRIORITIZED = 'prioritized' to the OnDemandAllocationStrategy enum
  2. Add an optional priority?: number field to InstanceTypeConfigProperty
  3. Update the InstanceTypeConfigPropertyToJson function to map the priority field to the CloudFormation output
  4. Added comprehensive unit tests covering enum values, priority field usage, and CloudFormation template generation
  5. Added integration test demonstrating real-world usage with multiple instance types and priorities

Describe any new or updated permissions being added

No new IAM permissions are required. This change only extends existing EMR cluster creation functionality with additional configuration options.

Description of how you validated changes

Unit tests: Added 3 tests covering enum values, priority field functionality, and CloudFormation output validation
Integration test: Created end-to-end test with realistic EMR cluster configuration using prioritized allocation strategy
Manual validation: Verified CloudFormation templates generate correctly with AllocationStrategy: "prioritized" and Priority: N fields
Backward compatibility: Confirmed existing tests continue to pass without modification

Checklist


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

CDK Developer added 2 commits October 14, 2025 11:34
… strategy

- Add PRIORITIZED option to OnDemandAllocationStrategy enum
- Add optional priority field to InstanceTypeConfigProperty interface
- Update JSON conversion to include priority in CloudFormation output
- Add comprehensive unit and integration tests

Enables users to specify instance type priorities for EMR instance fleets
when using the prioritized allocation strategy, aligning CDK with AWS EMR API.
@aws-cdk-automation aws-cdk-automation requested a review from a team October 13, 2025 22:44
@github-actions github-actions bot added 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 labels Oct 13, 2025
@Daniel-ZA Daniel-ZA changed the title Feat/emr instance fleet priority allocation feat: emr instance fleet priority allocation Oct 13, 2025
Copy link
Copy Markdown
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)

@aws-cdk-automation
Copy link
Copy Markdown
Collaborator

This PR has been in the CHANGES REQUESTED state for 3 weeks, and looks abandoned. Note that PRs with failing linting check or builds are not reviewed, please ensure your build is passing

To prevent automatic closure:

  • Resume work on the PR
  • OR request an exemption by adding a comment containing 'Exemption Request' with justification e.x "Exemption Request: "
  • OR request clarification by adding a comment containing 'Clarification Request' with a question e.x "Clarification Request: "

This PR will automatically close in 14 days if no action is taken.

@aws-cdk-automation aws-cdk-automation added pr-linter/exemption-requested The contributor has requested an exemption to the PR Linter feedback. and removed pr-linter/exemption-requested The contributor has requested an exemption to the PR Linter feedback. labels Nov 4, 2025
…te cluster with prioritized instance fleet

- Add integration test snapshot for integ.emr-create-cluster-with-prioritized-instance-fleet
- Generated CloudFormation templates and test assets
- Validates EMR cluster creation with prioritized instance fleet configuration
@aws-cdk-automation aws-cdk-automation dismissed their stale review November 4, 2025 02:34

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

@pahud pahud added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Nov 11, 2025
@pahud
Copy link
Copy Markdown
Contributor

pahud commented Nov 11, 2025

This PR is now in the needs-community-review queue. Once it receives an approving community review, it will move to the needs-maintainer-review stage for final approval. Feel free to continue improve this PR when necessary. Make sure CI passes with no conflicts after each commit. Thank you.

@pahud
Copy link
Copy Markdown
Contributor

pahud commented Apr 14, 2026

What I like:
Minimal, surgical changes — enum addition, optional priority field, and serialization update. Zero breaking changes.
Unit tests cover enum values, priority field usage, and CloudFormation template output.
Integration test with a realistic prioritized instance fleet scenario.
README updated with a clear code example.

Minor nits (non-blocking):
The priority JSDoc could include a @see link to the EMR InstanceTypeConfig API reference for discoverability.
A few snapshot files are missing a trailing newline (No newline at end of file).

Suggestion for consideration:
There's currently no CDK-level validation to warn users when allocationStrategy is PRIORITIZED but priority is not set on instanceTypeConfigs. EMR will reject it at deploy time, but an early validation (or at minimum a note in the JSDoc) could save users a round-trip. This could also be a follow-up — not necessarily a blocker for this PR.

Overall this looks good to me. Approving for community review. 👍

@pahud pahud added pr/needs-maintainer-review This PR needs a review from a Core Team Member and removed pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. labels Apr 14, 2026
@aws-cdk-automation aws-cdk-automation added pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. and removed pr/needs-maintainer-review This PR needs a review from a Core Team Member labels Apr 14, 2026
@gjurova gjurova added the p1 label Apr 17, 2026
@gjurova gjurova removed the p2 label Apr 17, 2026
@aws-cdk-automation aws-cdk-automation added pr/needs-maintainer-review This PR needs a review from a Core Team Member and removed pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. labels Apr 17, 2026
@vishaalmehrishi vishaalmehrishi self-assigned this Apr 17, 2026
… EMR InstanceTypeConfig API reference for discoverability.

Co-authored-by: Vishaal Mehrishi <mehrishi@amazon.nl>
@mergify mergify bot dismissed vishaalmehrishi’s stale review April 17, 2026 23:04

Pull request has been modified.

Add CDK-level validations per reviewer feedback:
- Validate priority is non-negative in InstanceTypeConfigPropertyToJson
- Validate priority values are only used with PRIORITIZED allocation strategy
- Update PRIORITIZED enum JSDoc to document priority requirement

Co-authored-by: Vishaal Mehrishi <mehrishi@amazon.nl>
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. p1 pr/needs-maintainer-review This PR needs a review from a Core Team Member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

(EmrCreateCluster.OnDemandAllocationStrategy): Add support for PRIORITIZED allocation strategy and Priority property in EmrCreateCluster

5 participants