feat(aws): add marketplace RHEL AI image support#863
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
🚧 Files skipped from review as they are similar to previous changes (8)
📝 WalkthroughSummary by CodeRabbit
WalkthroughChangesRHEL AI Marketplace provisioning
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant RHELAICLI
participant RHELAIAction
participant AWSAMI
participant SpotAllocation
Operator->>RHELAICLI: select --marketplace
RHELAICLI->>RHELAIAction: create Marketplace request
RHELAIAction->>AWSAMI: query public Marketplace AMI
RHELAIAction->>SpotAllocation: pass AMI owner and visibility
SpotAllocation->>AWSAMI: filter regions by AMI offer
AWSAMI-->>RHELAIAction: return AMI or subscription error
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.3.8)tkn/infra-aws-rhel-ai.yamlTraceback (most recent call last): tkn/template/infra-aws-rhel-ai.yamlTraceback (most recent call last): Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
31dedc6 to
d8ec785
Compare
|
Not tested because RHEL AI only allows 3 instance types and I couldn't get actual machine in any of 5 regions (tried bot h spot and on-demand).
|
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/provider/aws/action/rhel-ai/rhelai.go (1)
176-256: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
listMarketplaceVersionsreturns raw AMI names instead of extracted version strings.Unlike the non-marketplace
ListVersions, which strips the accelerator prefix and arch/qualifier suffixes to yield a plain version (e.g.,3.4.0),listMarketplaceVersionsdedups on and returns the full AMI name (seen[*img.Name]). Theseenmap is otherwise pointless since raw AMI names are already unique — this strongly suggests the version-extraction step was omitted. Any consumer feeding a listed value back into--versionfor--marketplacewould produce a malformed AMI name pattern inmarketplaceAMIName. Also note this branch (line 180-183) skips thecuda/rocmaccelerator validation that the non-marketplace path performs.Please extract the version substring (mirroring the prefix/suffix trimming used in
ListVersions) before dedup/sort.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/provider/aws/action/rhel-ai/rhelai.go` around lines 176 - 256, Update listMarketplaceVersions to validate the normalized accelerator as ListVersions does, then extract each AMI’s plain version by removing the marketplace name prefix and architecture/qualifier suffixes before inserting it into seen. Deduplicate and sort the extracted versions, ensuring returned values can be passed back to marketplaceAMIName as version arguments.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/provider/aws/action/rhel-ai/rhelai.go`:
- Line 136: Update checkMarketplaceAMIExists to validate the exact resolved
amiName rather than deriving a wildcard with marketplaceAMIName(accelerator,
nil). Pass the version-pinned amiName computed by Create() at both call sites,
including the on-demand/non-spot path, while preserving the existing region and
architecture checks.
- Around line 119-130: Propagate the selected amiName to allocArgs.AMIName for
both marketplace and non-marketplace flows in the allocation setup. Keep the
marketplace-specific AMIOwner and AMIPublic assignments conditional, while
ensuring allocation.Allocation receives the AMI name so downstream spot region
filtering remains active.
- Line 29: Separate the RHEL AI version-presence signal from its value in the
CLI argument flow, and pass that explicit signal or an optional version into the
provider instead of comparing args.Version with RhelAIVersionDefault. Update the
relevant RHEL AI action symbols to distinguish an explicitly supplied version
such as 3.0.0 from an omitted version, preserve wildcard/latest fallback only
when absent, and remove the dependency on cmd/mapt/cmd/params.
---
Outside diff comments:
In `@pkg/provider/aws/action/rhel-ai/rhelai.go`:
- Around line 176-256: Update listMarketplaceVersions to validate the normalized
accelerator as ListVersions does, then extract each AMI’s plain version by
removing the marketplace name prefix and architecture/qualifier suffixes before
inserting it into seen. Deduplicate and sort the extracted versions, ensuring
returned values can be passed back to marketplaceAMIName as version arguments.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 91807123-5fd4-432d-b88e-180ae0a9fc12
📒 Files selected for processing (9)
cmd/mapt/cmd/aws/hosts/rhelai.gopkg/provider/aws/action/rhel-ai/constants.gopkg/provider/aws/action/rhel-ai/rhelai.gopkg/provider/aws/data/ami.gopkg/provider/aws/data/spot.gopkg/provider/aws/modules/allocation/allocation.gopkg/provider/aws/modules/spot/stack.gotkn/infra-aws-rhel-ai.yamltkn/template/infra-aws-rhel-ai.yaml
Enable provisioning RHEL AI instances from AWS Marketplace AMIs using
the --marketplace flag, matching the existing Azure marketplace support.
Marketplace AMIs are discovered by name pattern and owner ID (679593333241),
using the naming convention rhel-ai-{accelerator}-*-prod-usfzkgvxvt65w.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Pavol Pitonak <ppitonak@redhat.com>
d8ec785 to
a6b56ec
Compare
Enable provisioning RHEL AI instances from AWS Marketplace AMIs using the --marketplace flag, matching the existing Azure marketplace support.
Uses EC2 product-code filter (prod-usfzkgvxvt65w) to discover marketplace AMIs without requiring additional SDK dependencies. Handles marketplace- specific spot pricing product description and subscription error messages.