Skip to content

feat(api)!: model CPU and memory as typed compute resource requirements - #3012

Open
elezar wants to merge 7 commits into
mainfrom
feat/2838-portable-compute-requirements/elezar
Open

feat(api)!: model CPU and memory as typed compute resource requirements#3012
elezar wants to merge 7 commits into
mainfrom
feat/2838-portable-compute-requirements/elezar

Conversation

@elezar

@elezar elezar commented Aug 28, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Summary

  • Adds typed CPU and memory requirements to the public and compute-driver ResourceRequirements messages.
  • Uses the same public ResourceRequirements message for inline sandbox specs and reusable SandboxWorkloadConfig.resources (field 3), removing the duplicate SandboxResources and template-only GPU SDK types.
  • Keeps the historical inline SandboxTemplate.resources field as platform-native passthrough and rejects legacy CPU/memory entries there with migration guidance.
  • Updates the CLI, gateway resolution, built-in drivers, Rust/Go/Python/TypeScript SDKs, generated bindings, tests, examples, and documentation.

Related Issue

Part of #2838.

Changes

  • proto/openshell.proto: models portable CPU, memory, and GPU requirements once under ResourceRequirements; reusable workload templates reuse that message without renaming or renumbering their resources field.
  • proto/compute_driver.proto: carries the corresponding typed requirements at the existing driver sandbox-spec boundary; no additional driver template message was added.
  • CLI and Python template builders populate cpu.limit and memory.limit; Rust and TypeScript expose the same shared shape.
  • Gateway template resolution clones typed resource requirements directly into the persisted sandbox spec.
  • Kubernetes, Docker, and Podman consume typed CPU/memory requirements. VM and MXC reject unsupported typed CPU/memory requirements explicitly.
  • Go SDK template types and converters reuse ResourceRequirements; generated Go protobuf bindings were regenerated. TypeScript and Python generated bindings remain build outputs regenerated by their build tasks.
  • Documentation distinguishes portable ResourceRequirements from the historical inline platform-native SandboxTemplate.resources passthrough.

Compatibility

This is an intentional breaking API change. Existing reusable-template clients using the removed SandboxResources wire shape must regenerate bindings and send ResourceRequirements (cpu.limit, memory.limit, and gpu). The outer SandboxWorkloadConfig.resources field name and field number remain unchanged.

The SDK impact is recorded in language-specific breaking commits, ordered Rust, Go, Python, then TypeScript:

  • Rust removes the SandboxResources alias and exposes typed CPU and memory requirements.
  • Go changes SandboxWorkloadConfig.Resources to ResourceRequirements and removes template-only resource types.
  • Python preserves the high-level builder arguments, but raw protobuf callers must use the new nested fields.
  • TypeScript removes the curated SandboxResources export and uses the shared generated type.

Testing

  • Focused Rust tests: cargo test -p openshell-server -p openshell-cli -p openshell-sdk
  • Go SDK full CI: mise run go:ci
  • TypeScript SDK full CI: mise run sdk:ts:ci (105 tests)
  • Python tests: mise run test:python (152 tests)
  • Python lint and type checking
  • Repository pre-commit: mise run pre-commit

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Unit and integration tests updated
  • SDK bindings and examples updated
  • Published and architecture documentation updated

@copy-pr-bot

copy-pr-bot Bot commented Aug 28, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@elezar elezar added this to the OpenShell 0.1.0 milestone Aug 28, 2026
@elezar

elezar commented Aug 28, 2026

Copy link
Copy Markdown
Member Author

/ok-to-test cd3f905

@elezar
elezar force-pushed the feat/2838-portable-compute-requirements/elezar branch 2 times, most recently from f3e21a8 to 18df2b1 Compare September 1, 2026 09:18
@elezar
elezar force-pushed the feat/2838-portable-compute-requirements/elezar branch from 18df2b1 to 449d763 Compare September 8, 2026 13:39
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown

🌿 Preview your docs: https://nvidia-preview-pr-3012.docs.buildwithfern.com/openshell

@elezar
elezar force-pushed the feat/2838-portable-compute-requirements/elezar branch 3 times, most recently from beaa621 to 90eb97b Compare September 9, 2026 10:28
@elezar
elezar marked this pull request as ready for review September 9, 2026 11:58
@elezar
feat(api)!: unify portable sandbox resource requirements
e2a5051 
Model CPU, memory, and GPU through one typed ResourceRequirements envelope across inline sandbox specs, reusable workload templates, and compute drivers. Update the CLI, gateway translation, and built-in drivers to consume the unified model.

BREAKING CHANGE: CPU and memory move to typed resource requirements, and SandboxWorkloadConfig.resources now uses ResourceRequirements instead of SandboxResources.

Part of #2838

Signed-off-by: Evan Lezar <elezar@nvidia.com>
@elezar
feat(sdk/rust)!: align portable resource requirements
81bc891 
Expose the shared typed ResourceRequirements model for inline and reusable-template workloads and remove the template-only SandboxResources alias.

BREAKING CHANGE: SandboxResources is removed; reusable workload templates now use ResourceRequirements with typed CPU and memory fields.

Part of #2838

Signed-off-by: Evan Lezar <elezar@nvidia.com>
@elezar
feat(sdk/go)!: align portable resource requirements
bcb8d34 
Use ResourceRequirements for both inline and reusable-template sandbox workloads, update deep-copy converters and fakes, and regenerate the Go protobuf binding.

BREAKING CHANGE: SandboxWorkloadConfig.Resources now uses ResourceRequirements, and the template-only SandboxResources and SandboxGPURequirements types are removed.

Part of #2838

Signed-off-by: Evan Lezar <elezar@nvidia.com>
@elezar
feat(sdk/python)!: align portable resource requirements
742e7e1 
Update reusable workload template construction and tests for the shared ResourceRequirements protobuf shape while preserving the high-level builder arguments.

BREAKING CHANGE: Raw reusable-template protobuf callers must use resources.cpu.limit and resources.memory.limit instead of string fields.

Part of #2838

Signed-off-by: Evan Lezar <elezar@nvidia.com>
@elezar
feat(sdk/typescript)!: align portable resource requirements
bf1a441 
Use the shared generated ResourceRequirements shape for reusable workload templates and update the curated exports and tests.

BREAKING CHANGE: SandboxResources is no longer exported, and reusable-template CPU and memory values use typed limit objects.

Part of #2838

Signed-off-by: Evan Lezar <elezar@nvidia.com>
@elezar
fix(server): validate portable resource requirements
3b706af 
Validate typed CPU and memory quantities and reject legacy CPU or memory entries in the historical inline template resource passthrough with migration guidance.

Part of #2838

Signed-off-by: Evan Lezar <elezar@nvidia.com>
@elezar
docs: explain unified portable resource requirements
75f6162 
Document the shared inline and reusable-template resource model, driver behavior, migration from legacy template resource sections, and updated SDK examples.

Part of #2838

Signed-off-by: Evan Lezar <elezar@nvidia.com>
@elezar
elezar force-pushed the feat/2838-portable-compute-requirements/elezar branch from 90eb97b to 75f6162 Compare September 10, 2026 06:57
sjenning
sjenning reviewed Sep 10, 2026
View reviewed changes
Comment thread proto/compute_driver.proto
// Driver CPU resource requirements.
message CpuResourceRequirements {
// CPU limit for the sandbox workload (e.g. "500m", "2").
string limit = 1;

@sjenning sjenning Sep 10, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It seems that there is no way to specify resource requests on cpu/memory, only limits. Typically, one does not want to specify a CPU limit because it needlessly throttles a pod that could otherwise use the idle CPU on the node.

Comment thread crates/openshell-driver-kubernetes/src/driver.rs
apply("requests", "cpu", cpu_request);
apply("requests", "memory", memory_request);
if let Some(cpu) = requirements.cpu.as_ref() {
apply("limits", "cpu", &cpu.limit);

@sjenning sjenning Sep 10, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We lost fidelity here where limits on the API are used to set both limits and requests

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

Reviewers

@sjenning sjenning sjenning left review comments
@mrunalp mrunalp Awaiting requested review from mrunalp mrunalp is a code owner
@derekwaynecarr derekwaynecarr Awaiting requested review from derekwaynecarr derekwaynecarr is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

OpenShell 0.1.0

Development

Successfully merging this pull request may close these issues.

2 participants

@elezar @sjenning