fix: support vcpu in modal

[kerthcet]

What this PR does / why we need it

Which issue(s) this PR fixes

Fixes #

Special notes for your reviewer

Does this PR introduce a user-facing change?

Summary by CodeRabbit

• Bug Fixes
  • Modal CPU resource requests and hourly pricing now reflect physical cores, with positive values rounded down to a minimum of 0.125 cores.
  • Memory reservations now round up to the next whole MiB. Missing resource requests fall back to limits, while missing limits remain uncapped.
• Documentation
  • Updated AWS pricing examples to show rates of $20.839 and $55.040 per hour.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 7393be78-2334-433b-b346-05ee04cf1ae9

📥 Commits

Reviewing files that changed from the base of the PR and between e7ed3fa and f2312a5.

⛔ Files ignored due to path filters: 1

• pkg/provider/catalog/data/aws.csv is excluded by !**/*.csv

📒 Files selected for processing: 6

• pkg/provider/catalog/data/pricing.go
• pkg/provider/modal/modal.go
• pkg/provider/modal/modal_test.go
• pkg/provider/pricing.go
• pkg/util/resources.go
• pkg/util/resources_test.go

🚧 Files skipped from review as they are similar to previous changes: 2

• pkg/provider/pricing.go
• pkg/provider/catalog/data/pricing.go

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

Pod resource extraction now reports requests and limits separately and rounds memory up to whole MiB. Modal converts CPU vCPUs to physical cores for sandbox resource mapping and CPU pricing. AWS p5.48xlarge price references and the .gitignore also change.

Changes: Modal resource reservations

Layer / File(s)	Summary
Extract and round pod resources pkg/util/resources.go, pkg/util/resources_test.go	PodResources returns first-container requests and limits separately, with requests falling back to limits. Memory values round up to whole MiB. PodReservation uses the request values with limit fallback.
Map pod resources to Modal pkg/provider/modal/modal.go, pkg/provider/modal/modal_test.go	Modal uses util.PodResources for sandbox requests and limits. Positive CPU values convert from vCPUs to physical cores and floor at 0.125 cores. CPU pricing uses the same conversion.

AWS p5.48xlarge pricing

Layer / File(s)	Summary
Update AWS price references pkg/provider/pricing.go, pkg/provider/catalog/base.go, pkg/provider/catalog/data/pricing.go, pkg/provider/catalog/catalog_test.go	The Spot and OnDemand examples change to $20.839 and $55.040. The catalog test expects $55.040 for the whole-instance rate. The CPU and memory cost comment describes the rates as a floor.

HTML ignore rule

Layer / File(s)	Summary
Ignore HTML files .gitignore	Adds an ignore rule for files matching *.html.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant Pod
  participant PodResources as util.PodResources
  participant Modal as Modal resource mapping
  Pod->>PodResources: Provide first-container requests and limits
  PodResources-->>Modal: Return separate request and limit Resources
  Modal->>Modal: Convert CPU vCPUs to physical cores

Loading

Merge Risk: ⚪ Minimal · up to f2312

The revised price expectation matches the catalog, and no merge-blocking resource-mapping issue is established. Merge after normal checks.

Security architecture risk: 🔵 Low · up to f2312

Pricing and provisioning now use consistent CPU units, and requests remain separate from limits. An extreme-value memory conversion can nevertheless produce negative resource sizes and price estimates. Provider-side handling of these edge cases remains unverified; no broader identity, credential, or isolation exposure was established.

Retained concerns

• Low · architecture · observed: The new whole-MiB ceiling conversion can overflow a positive byte count near MaxInt64 into a negative request or limit. For MaxInt64 bytes, its arithmetic produces -8796093022207 MiB. Unlike the base division-only conversion, this breaks the nonnegative resource contract: GPU pricing can compute a negative rate, while provisioning forwards negative memory fields. Provider rejection versus other handling is unresolved; an effective resource-cap bypass or financial loss is not established.

Security review details

Security Blast Radius

• inferred — The identified edge case starts with workload-controlled memory quantities and reaches that workload's Modal sandbox specification and NodeClaim price calculation. Reachability requires permission to submit a Pod that passes deployment admission and quota controls. No new IAM authority, credential access, or cross-tenant data access was established.

Security Findings and Attack Paths

• observed — The introduced arithmetic defect propagates negative memory into GPU pricing and outbound sandbox fields. CPU-only pricing rejects nonpositive memory, which limits that pricing path. External rejection or omission of negative sandbox fields remains unknown, so this assessment does not establish a working cap-bypass attack.

Trust Boundaries and Controls

• observed — The inspected changes do not replace claim identity, registry-authentication checks, or outbound-policy fields with resource-derived data. Claim tags, resolved registry authentication, egress allowlists, and timeouts continue through the existing provisioning boundary.

Resilience and Maintainability Implications

• observed — Claim-based retry recovery, context propagation, creation followed by credential minting, and explicit termination remain unchanged. A failure after remote creation can already leave a sandbox for claim-based recovery; this behavior predates the PR, and the resource conversion does not change its identity or cleanup dispatch.

Hardening Proposals

• proposed — Use overflow-safe quotient-and-remainder rounding and reject unrepresentable resource values before pricing or remote creation. This would preserve the positive-size and nonzero-cap invariants at the shared contract boundary.
• proposed — Make provider-granularity policy explicit for hard caps: reject unsupported ceilings or explicitly authorize their widening, rather than treating reservation rounding as sufficient evidence that a hard ceiling is preserved.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 68.75% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 9 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly summarizes the main change: adding vCPU support to the Modal provider. It is concise and specific.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.

[kerthcet]

/lgtm
/kind bug

[InftyAI-Agent]

Approved: PR has both lgtm and approved labels

[InftyAI-Agent]

Approved: PR has both lgtm and approved labels