Skip to content

Commit 9af4d04

Browse files
committed
refactor(gpu): use resource requirements for GPU requests
Signed-off-by: Evan Lezar <elezar@nvidia.com>
1 parent a7188c7 commit 9af4d04

20 files changed

Lines changed: 460 additions & 291 deletions

File tree

architecture/compute-runtimes.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,9 @@ through the driver configuration. The Helm chart defaults sandbox agents to
4545
`Unconfined` so runtime/default AppArmor profiles do not block supervisor
4646
network namespace setup on AppArmor-enabled nodes.
4747

48+
GPU requests enter the driver layer through
49+
`SandboxSpec.resource_requirements.gpu`.
50+
4851
VM runtime state paths are derived only from driver-validated sandbox IDs
4952
matching `[A-Za-z0-9._-]{1,128}`. The gateway-owned VM driver socket uses a
5053
private `run/` directory plus Unix peer UID/PID checks. Standalone

crates/openshell-cli/src/run.rs

Lines changed: 76 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -39,18 +39,18 @@ use openshell_core::proto::{
3939
GetClusterInferenceRequest, GetDraftHistoryRequest, GetDraftPolicyRequest,
4040
GetGatewayConfigRequest, GetProviderProfileRequest, GetProviderRefreshStatusRequest,
4141
GetProviderRequest, GetSandboxConfigRequest, GetSandboxLogsRequest,
42-
GetSandboxPolicyStatusRequest, GetSandboxRequest, GetServiceRequest, GpuRequestSpec,
42+
GetSandboxPolicyStatusRequest, GetSandboxRequest, GetServiceRequest, GpuResourceRequirement,
4343
HealthRequest, ImportProviderProfilesRequest, LintProviderProfilesRequest,
4444
ListProviderProfilesRequest, ListProvidersRequest, ListSandboxPoliciesRequest,
4545
ListSandboxProvidersRequest, ListSandboxesRequest, ListServicesRequest, PlatformEvent,
4646
PolicySource, PolicyStatus, Provider, ProviderCredentialRefreshStatus,
4747
ProviderCredentialRefreshStrategy, ProviderProfile, ProviderProfileDiagnostic,
4848
ProviderProfileImportItem, RejectDraftChunkRequest, RevokeSshSessionRequest,
49-
RotateProviderCredentialRequest, Sandbox, SandboxPhase, SandboxPolicy, SandboxSpec,
50-
SandboxTemplate, ServiceEndpointResponse, SetClusterInferenceRequest, SettingScope,
51-
SettingValue, TcpForwardFrame, TcpForwardInit, TcpRelayTarget, UpdateConfigRequest,
52-
UpdateProviderRequest, WatchSandboxRequest, exec_sandbox_event, setting_value,
53-
tcp_forward_init,
49+
RotateProviderCredentialRequest, Sandbox, SandboxPhase, SandboxPolicy,
50+
SandboxResourceRequirements, SandboxSpec, SandboxTemplate, ServiceEndpointResponse,
51+
SetClusterInferenceRequest, SettingScope, SettingValue, TcpForwardFrame, TcpForwardInit,
52+
TcpRelayTarget, UpdateConfigRequest, UpdateProviderRequest, WatchSandboxRequest,
53+
exec_sandbox_event, setting_value, tcp_forward_init,
5454
};
5555
use openshell_core::settings::{self, SettingValueKind};
5656
use openshell_core::{ObjectId, ObjectName};
@@ -1801,11 +1801,6 @@ pub async fn sandbox_create(
18011801
}
18021802
None => None,
18031803
};
1804-
let requested_gpu = gpu
1805-
|| gpu_device.is_some_and(|device_id| !device_id.is_empty())
1806-
|| gpu_count.is_some()
1807-
|| image.as_deref().is_some_and(image_requests_gpu);
1808-
18091804
let providers_v2_enabled = gateway_providers_v2_enabled(&mut client).await?;
18101805
let inferred_types: Vec<String> = if providers_v2_enabled {
18111806
Vec::new()
@@ -1825,6 +1820,11 @@ pub async fn sandbox_create(
18251820
let driver_config = driver_config_json
18261821
.map(parse_driver_config_json)
18271822
.transpose()?;
1823+
let resource_requirements =
1824+
resource_requirements_from_cli(image.as_deref(), gpu, gpu_device, gpu_count);
1825+
let requested_gpu = resource_requirements
1826+
.as_ref()
1827+
.is_some_and(|requirements| requirements.gpu.is_some());
18281828

18291829
let template = if image.is_some() || resource_limits.is_some() || driver_config.is_some() {
18301830
Some(SandboxTemplate {
@@ -1839,7 +1839,7 @@ pub async fn sandbox_create(
18391839

18401840
let request = CreateSandboxRequest {
18411841
spec: Some(SandboxSpec {
1842-
gpu: gpu_request_from_cli(requested_gpu, gpu_device, gpu_count),
1842+
resource_requirements,
18431843
policy,
18441844
providers: configured_providers,
18451845
template,
@@ -2274,17 +2274,26 @@ pub async fn sandbox_create(
22742274
}
22752275
}
22762276

2277-
fn gpu_request_from_cli(
2278-
requested_gpu: bool,
2277+
fn resource_requirements_from_cli(
2278+
image: Option<&str>,
2279+
gpu: bool,
22792280
gpu_device: Option<&str>,
22802281
gpu_count: Option<u32>,
2281-
) -> Option<GpuRequestSpec> {
2282-
requested_gpu.then(|| GpuRequestSpec {
2283-
device_id: gpu_device
2284-
.filter(|device_id| !device_id.is_empty())
2285-
.map(|device_id| vec![device_id.to_string()])
2286-
.unwrap_or_default(),
2287-
count: gpu_count,
2282+
) -> Option<SandboxResourceRequirements> {
2283+
let device_ids = gpu_device
2284+
.filter(|device_id| !device_id.is_empty())
2285+
.map(|device_id| vec![device_id.to_string()])
2286+
.unwrap_or_default();
2287+
let requested_gpu = gpu
2288+
|| gpu_count.is_some()
2289+
|| !device_ids.is_empty()
2290+
|| image.is_some_and(image_requests_gpu);
2291+
2292+
requested_gpu.then_some(SandboxResourceRequirements {
2293+
gpu: Some(GpuResourceRequirement {
2294+
device_ids,
2295+
count: gpu_count,
2296+
}),
22882297
})
22892298
}
22902299

@@ -7537,15 +7546,14 @@ mod tests {
75377546
dockerfile_sources_supported_for_gateway, format_endpoint, format_gateway_select_header,
75387547
format_gateway_select_items, format_provider_attachment_table, gateway_add,
75397548
gateway_auth_label, gateway_env_override_warning, gateway_select_with, gateway_type_label,
7540-
git_sync_files, gpu_request_from_cli, http_health_check, image_requests_gpu,
7541-
import_local_package_mtls_bundle, inferred_provider_type, mtls_certs_exist_for_gateway,
7542-
package_managed_tls_dirs,
7549+
git_sync_files, http_health_check, image_requests_gpu, import_local_package_mtls_bundle,
7550+
inferred_provider_type, mtls_certs_exist_for_gateway, package_managed_tls_dirs,
75437551
parse_cli_setting_value, parse_credential_expiry_cli_value, parse_credential_expiry_pairs,
75447552
parse_credential_pairs, parse_driver_config_json, plaintext_gateway_is_remote,
75457553
progress_step_from_metadata, provider_profile_allows_refresh_bootstrap,
75467554
provisioning_timeout_message, ready_false_condition_message, refresh_status_header,
7547-
refresh_status_row, resolve_from, sandbox_should_persist, sandbox_upload_plan,
7548-
service_expose_status_error, service_url_for_gateway,
7555+
refresh_status_row, resolve_from, resource_requirements_from_cli, sandbox_should_persist,
7556+
sandbox_upload_plan, service_expose_status_error, service_url_for_gateway,
75497557
};
75507558
use crate::TEST_ENV_LOCK;
75517559
use hyper::StatusCode;
@@ -8075,43 +8083,64 @@ mod tests {
80758083
}
80768084

80778085
#[test]
8078-
fn gpu_request_from_cli_uses_presence_with_empty_device_ids_for_default_gpu() {
8079-
let request =
8080-
gpu_request_from_cli(true, None, None).expect("gpu request should be present");
8086+
fn resource_requirements_from_cli_uses_presence_for_default_gpu() {
8087+
let requirements = resource_requirements_from_cli(None, true, None, None)
8088+
.expect("resource requirements should be present");
8089+
let gpu = requirements.gpu.expect("GPU requirement should be present");
80818090

8082-
assert!(request.device_id.is_empty());
8083-
assert_eq!(request.count, None);
8091+
assert!(gpu.device_ids.is_empty());
8092+
assert_eq!(gpu.count, None);
80848093
}
80858094

80868095
#[test]
8087-
fn gpu_request_from_cli_maps_gpu_device_to_one_device_id() {
8088-
let request = gpu_request_from_cli(true, Some("0000:2d:00.0"), None)
8089-
.expect("gpu request should be present");
8096+
fn resource_requirements_from_cli_maps_gpu_device_to_one_device_id() {
8097+
let requirements = resource_requirements_from_cli(None, false, Some("0000:2d:00.0"), None)
8098+
.expect("resource requirements should be present");
8099+
let gpu = requirements.gpu.expect("GPU requirement should be present");
80908100

8091-
assert_eq!(request.device_id, vec!["0000:2d:00.0"]);
8092-
assert_eq!(request.count, None);
8101+
assert_eq!(gpu.device_ids, vec!["0000:2d:00.0"]);
8102+
assert_eq!(gpu.count, None);
80938103
}
80948104

80958105
#[test]
8096-
fn gpu_request_from_cli_maps_gpu_count() {
8097-
let request = gpu_request_from_cli(true, None, Some(2)).expect("gpu request should exist");
8106+
fn resource_requirements_from_cli_maps_gpu_count() {
8107+
let requirements = resource_requirements_from_cli(None, false, None, Some(2))
8108+
.expect("requirements should exist");
8109+
let gpu = requirements.gpu.expect("GPU requirement should be present");
80988110

8099-
assert!(request.device_id.is_empty());
8100-
assert_eq!(request.count, Some(2));
8111+
assert!(gpu.device_ids.is_empty());
8112+
assert_eq!(gpu.count, Some(2));
81018113
}
81028114

81038115
#[test]
8104-
fn gpu_request_from_cli_preserves_device_and_gpu_count_for_gateway_validation() {
8105-
let request = gpu_request_from_cli(true, Some("nvidia.com/gpu=0"), Some(2))
8106-
.expect("gpu request should exist");
8116+
fn resource_requirements_from_cli_preserves_device_and_gpu_count_for_gateway_validation() {
8117+
let requirements =
8118+
resource_requirements_from_cli(None, false, Some("nvidia.com/gpu=0"), Some(2))
8119+
.expect("requirements should exist");
8120+
let gpu = requirements.gpu.expect("GPU requirement should be present");
81078121

8108-
assert_eq!(request.device_id, vec!["nvidia.com/gpu=0"]);
8109-
assert_eq!(request.count, Some(2));
8122+
assert_eq!(gpu.device_ids, vec!["nvidia.com/gpu=0"]);
8123+
assert_eq!(gpu.count, Some(2));
81108124
}
81118125

81128126
#[test]
8113-
fn gpu_request_from_cli_omits_gpu_request_when_not_requested() {
8114-
assert!(gpu_request_from_cli(false, Some("0"), None).is_none());
8127+
fn resource_requirements_from_cli_omits_gpu_request_when_not_requested() {
8128+
assert!(resource_requirements_from_cli(None, false, None, None).is_none());
8129+
}
8130+
8131+
#[test]
8132+
fn resource_requirements_from_cli_infers_gpu_from_image() {
8133+
let requirements = resource_requirements_from_cli(
8134+
Some("ghcr.io/nvidia/openshell-community/sandboxes/nvidia-gpu:latest"),
8135+
false,
8136+
None,
8137+
None,
8138+
)
8139+
.expect("resource requirements should be present");
8140+
let gpu = requirements.gpu.expect("GPU requirement should be present");
8141+
8142+
assert!(gpu.device_ids.is_empty());
8143+
assert_eq!(gpu.count, None);
81158144
}
81168145

81178146
#[test]

crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -788,6 +788,7 @@ async fn sandbox_create_keeps_command_sessions_by_default() {
788788
None,
789789
None,
790790
None,
791+
None,
791792
&[],
792793
None,
793794
None,
@@ -907,6 +908,7 @@ async fn sandbox_create_sends_driver_config_json() {
907908
None,
908909
None,
909910
None,
911+
None,
910912
Some(r#"{"kubernetes":{"pod":{"priority_class_name":"batch-low"}}}"#),
911913
None,
912914
&[],
@@ -982,6 +984,7 @@ async fn sandbox_create_sends_gpu_device_request_without_gpu_flag() {
982984
None,
983985
None,
984986
None,
987+
None,
985988
&[],
986989
None,
987990
None,
@@ -999,10 +1002,11 @@ async fn sandbox_create_sends_gpu_device_request_without_gpu_flag() {
9991002
let gpu = requests[0]
10001003
.spec
10011004
.as_ref()
1002-
.and_then(|spec| spec.gpu.as_ref())
1005+
.and_then(|spec| spec.resource_requirements.as_ref())
1006+
.and_then(|requirements| requirements.gpu.as_ref())
10031007
.expect("GPU request should be sent");
10041008

1005-
assert_eq!(gpu.device_id, vec!["nvidia.com/gpu=0"]);
1009+
assert_eq!(gpu.device_ids, vec!["nvidia.com/gpu=0"]);
10061010
assert_eq!(gpu.count, None);
10071011
}
10081012

@@ -1020,14 +1024,15 @@ async fn sandbox_create_sends_gpu_count_request() {
10201024
Some("gpu-count"),
10211025
None,
10221026
"openshell",
1023-
None,
1027+
&[],
10241028
true,
10251029
false,
10261030
None,
10271031
Some(2),
10281032
None,
10291033
None,
10301034
None,
1035+
None,
10311036
&[],
10321037
None,
10331038
None,
@@ -1045,10 +1050,11 @@ async fn sandbox_create_sends_gpu_count_request() {
10451050
let gpu = requests[0]
10461051
.spec
10471052
.as_ref()
1048-
.and_then(|spec| spec.gpu.as_ref())
1053+
.and_then(|spec| spec.resource_requirements.as_ref())
1054+
.and_then(|requirements| requirements.gpu.as_ref())
10491055
.expect("GPU request should be sent");
10501056

1051-
assert!(gpu.device_id.is_empty());
1057+
assert!(gpu.device_ids.is_empty());
10521058
assert_eq!(gpu.count, Some(2));
10531059
}
10541060

@@ -1075,6 +1081,7 @@ async fn sandbox_create_does_not_infer_command_providers_when_v2_enabled() {
10751081
None,
10761082
None,
10771083
None,
1084+
None,
10781085
&[],
10791086
None,
10801087
None,
@@ -1133,6 +1140,7 @@ async fn sandbox_create_returns_vm_error_without_waiting_for_timeout() {
11331140
None,
11341141
None,
11351142
None,
1143+
None,
11361144
&[],
11371145
None,
11381146
None,
@@ -1187,6 +1195,7 @@ async fn sandbox_create_keeps_waiting_while_vm_progress_arrives() {
11871195
None,
11881196
None,
11891197
None,
1198+
None,
11901199
&[],
11911200
None,
11921201
None,
@@ -1233,6 +1242,7 @@ async fn sandbox_create_times_out_when_only_logs_arrive() {
12331242
None,
12341243
None,
12351244
None,
1245+
None,
12361246
&[],
12371247
None,
12381248
None,
@@ -1275,6 +1285,7 @@ async fn sandbox_create_deletes_command_sessions_with_no_keep() {
12751285
None,
12761286
None,
12771287
None,
1288+
None,
12781289
&[],
12791290
None,
12801291
None,
@@ -1321,6 +1332,7 @@ async fn sandbox_create_deletes_shell_sessions_with_no_keep() {
13211332
None,
13221333
None,
13231334
None,
1335+
None,
13241336
&[],
13251337
None,
13261338
None,
@@ -1367,6 +1379,7 @@ async fn sandbox_create_keeps_sandbox_with_hidden_keep_flag() {
13671379
None,
13681380
None,
13691381
None,
1382+
None,
13701383
&[],
13711384
None,
13721385
None,
@@ -1413,6 +1426,7 @@ async fn sandbox_create_keeps_sandbox_with_forwarding() {
14131426
None,
14141427
None,
14151428
None,
1429+
None,
14161430
&[],
14171431
None,
14181432
Some(openshell_core::forward::ForwardSpec::new(forward_port)),

0 commit comments

Comments
 (0)