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

• Updates
  • Modal CPU reservations and pricing now use physical-core units, converting Kubernetes vCPUs appropriately and applying a 0.125-core minimum to positive values. CPU and memory limits are used as reservations when present; otherwise, requests are used.
  • Memory reservations now round up to the nearest MiB, including values below 1 MiB.
  • Updated the listed hourly prices for AWS p5.48xlarge instances.

[Copilot]

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

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 25 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: a8e9177e-5a80-4ff0-81df-c3ed6c4a646d

📥 Commits

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

📒 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

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 733f26bb-6a5c-475c-ae9f-e95a04df7a3e

📥 Commits

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

⛔ Files ignored due to path filters (1)

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

📒 Files selected for processing (7)

• pkg/provider/catalog/data/pricing.go
• pkg/provider/modal/client.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/catalog/data/pricing.go
• pkg/provider/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

Walkthrough

Modal resource reservations now prefer limits, round memory up to whole MiB, and convert CPU values from vCPUs to physical cores. AWS pricing references and the HTML ignore rule are also updated.

Changes

Modal resource reservations

Layer / File(s)	Summary
Select and round pod reservations pkg/util/resources.go, pkg/util/resources_test.go	PodReservation selects limits before requests and rounds memory up to whole MiB. Tests cover precedence and rounding.
Map reservations to Modal resources pkg/provider/modal/modal.go, pkg/provider/modal/client.go, pkg/provider/modal/modal_test.go	Modal maps reserved CPU values to physical cores, with positive values floored at 0.125. SandboxSpec no longer carries separate CPU and memory limit fields. Sandbox limits use reservation values, and CPU pricing uses the same conversion. Tests cover resource mapping and pricing.

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	Pricing comments and documentation use the revised Spot and OnDemand prices of $20.839 and $55.040. The embedded catalog test expects the revised $55.040 whole-instance rate.

HTML ignore rule

Layer / File(s)	Summary
Ignore HTML files .gitignore	The ignore rules now exclude files matching *.html.

Priority: ⬇️ Low

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

Change: Bug fix

Merge Risk: ⚪ Minimal · up to e7ed3

The resource sizing and pricing changes are consistent, and the AWS price expectation matches the embedded data. No merge-blocking issue was established; merge after normal checks pass.

Security Architecture Review

Security architecture risk: 🔵 Low · up to e7ed3

Newly created sandboxes use matching resource values for pricing and enforcement. A narrow upgrade or recovery case can price an older sandbox using the new conversion without changing its allocation. No new authorization, credential, or network exposure was established, but deployed resource policies and external compatibility remain unverified.

Retained concerns

• Low · architecture · inferred: The new pricing contract is not bound to the reservation policy that created an adopted sandbox. For example, the base maps a Pod with equal 4-CPU requests and limits to 4 Modal physical cores; the head prices that Pod as 2 physical cores. If the sandbox survives an upgrade before its rate is persisted, adoption retains its allocation while recordPrice stores and freezes the new, lower CPU component. This is conditional recovery and accounting-control drift, not a demonstrated authorization bypass. Already-priced legacy claims retain their rates.

Security review details

Security Blast Radius

• inferred — A caller permitted to create workloads can select resource quantities that affect a Modal sandbox's allocation and attributed cost. The changed path does not itself grant additional Kubernetes or Modal authority. Maximum aggregate exposure depends on deployed resource and admission policies, which were not established; no new cross-tenant reachability was demonstrated.

Trust Boundaries and Controls

• observed — Generated Pods still traverse Kubernetes API admission. The inspected Pod webhook adds scheduling metadata rather than CPU or memory ceilings, so it is not evidence of resource-budget enforcement. Modal creation continues to receive the existing resolved environment, encrypted ports, placement, and outbound-policy parameters; the creation diff changes resource limit assignments, not those controls.

Hardening Proposals

• proposed — Bind the persisted rate to the reservation policy used for the actual sandbox, including adopted instances. Alternatively, define an explicit legacy-workload drain policy before changing reservation semantics. Either approach would make cross-version recovery accounting deterministic.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

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

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the primary change: Modal now supports vCPU-based resource handling.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ 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.