Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .agents/skills/openshell-cli/cli-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ Provision or start a cluster (local or remote).
| `--port <PORT>` | 8080 | Host port mapped to gateway |
| `--gateway-host <HOST>` | none | Override gateway host in metadata |
| `--recreate` | false | Destroy and recreate from scratch if a gateway already exists (skips interactive prompt) |
| `--gpu` | false | Force NVIDIA GPU passthrough |
| `--no-gpu` | false | Disable automatic NVIDIA GPU passthrough detection |

### `openshell gateway stop`

Expand Down
9 changes: 6 additions & 3 deletions architecture/gateway-single-node.md
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,8 @@ When environment variables are set, the entrypoint modifies the HelmChart manife

GPU support is part of the single-node gateway bootstrap path rather than a separate architecture.

- `openshell gateway start --gpu` threads GPU device options through `crates/openshell-cli`, `crates/openshell-bootstrap`, and `crates/openshell-bootstrap/src/docker.rs`.
- `openshell gateway start` auto-detects GPU support and threads GPU device options through `crates/openshell-cli`, `crates/openshell-bootstrap`, and `crates/openshell-bootstrap/src/docker.rs`. Users can force passthrough with `--gpu` or disable auto-detection with `--no-gpu`.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Question: Does the gateway still need GPU support? Does the new architecture not delegate this to the driver?

- Auto-detection enables passthrough when Docker reports NVIDIA CDI devices. For local non-CDI hosts, it also enables passthrough when `/dev/nvidia*` devices exist and Docker reports the NVIDIA runtime. Remote legacy-runtime hosts still require explicit `--gpu`.
- When enabled, the cluster container is created with Docker `DeviceRequests`. The injection mechanism is selected based on whether CDI is enabled on the daemon (`SystemInfo.CDISpecDirs` via `GET /info`):
- **CDI enabled** (daemon reports non-empty `CDISpecDirs`): CDI device injection — `driver="cdi"` with `nvidia.com/gpu=all`. Specs are expected to be pre-generated on the host (e.g. automatically by the `nvidia-cdi-refresh.service` or manually via `nvidia-ctk generate`).
- **CDI not enabled**: `--gpus all` device request — `driver="nvidia"`, `count=-1`, which relies on the NVIDIA Container Runtime hook.
Expand All @@ -317,9 +318,11 @@ Host GPU drivers & NVIDIA Container Toolkit
└─ Pods: request nvidia.com/gpu in resource limits (CDI injection — no runtimeClassName needed)
```

### `--gpu` flag
### GPU flags

The `--gpu` flag on `gateway start` enables GPU passthrough. OpenShell auto-selects CDI when enabled on the daemon and falls back to Docker's NVIDIA GPU request path (`--gpus all`) otherwise.
`gateway start` enables GPU passthrough automatically when it detects NVIDIA GPU support. The `--gpu` flag forces GPU passthrough even when auto-detection does not find a device. The `--no-gpu` flag disables auto-detection.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Question: Does the --no-gpu disable auto detection or disable GPU support?


OpenShell auto-selects CDI when enabled on the daemon and falls back to Docker's NVIDIA GPU request path (`--gpus all`) otherwise.

Device injection uses CDI (`deviceListStrategy: cdi-cri`): the device plugin injects devices via direct CDI device requests in the CRI. Sandbox pods only need `nvidia.com/gpu: 1` in their resource limits, and GPU pods do not set `runtimeClassName`.

Expand Down
2 changes: 1 addition & 1 deletion architecture/gateway.md
Original file line number Diff line number Diff line change
Expand Up @@ -606,7 +606,7 @@ The gateway reaches the sandbox exclusively through the supervisor-initiated `Co

The Docker driver (`crates/openshell-driver-docker/src/lib.rs`) is an in-process compute backend for local standalone gateways. It creates one Docker container per sandbox, labels each container with `openshell.ai/managed-by=openshell`, `openshell.ai/sandbox-id`, `openshell.ai/sandbox-name`, and `openshell.ai/sandbox-namespace`, and bind-mounts a Linux `openshell-sandbox` supervisor binary into the container.

- **Create**: Pulls or validates the sandbox image according to `sandbox_image_pull_policy`, creates a labeled container, mounts the supervisor binary and optional TLS material, and starts the container with the supervisor as entrypoint.
- **Create**: Pulls or validates the sandbox image according to `sandbox_image_pull_policy`, creates a labeled container, mounts the supervisor binary and optional TLS material, and starts the container with the supervisor as entrypoint. When the sandbox spec requests GPU and Docker exposes NVIDIA CDI devices or the NVIDIA runtime, the driver adds a Docker `DeviceRequest` for those GPUs.
- **List/Get/Watch**: Reads labeled containers in the configured sandbox namespace and derives driver-native sandbox status from Docker state plus supervisor relay readiness.
- **Stop**: Stops the matching labeled container without deleting it.
- **Delete**: Force-removes the matching labeled container.
Expand Down
140 changes: 137 additions & 3 deletions crates/openshell-bootstrap/src/docker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@ use bollard::API_DEFAULT_VERSION;
use bollard::Docker;
use bollard::errors::Error as BollardError;
use bollard::models::{
ContainerCreateBody, DeviceRequest, EndpointSettings, HostConfig, HostConfigCgroupnsModeEnum,
NetworkConnectRequest, NetworkCreateRequest, NetworkDisconnectRequest, PortBinding,
RestartPolicy, RestartPolicyNameEnum, VolumeCreateRequest,
ContainerCreateBody, DeviceInfo, DeviceRequest, EndpointSettings, HostConfig,
HostConfigCgroupnsModeEnum, NetworkConnectRequest, NetworkCreateRequest,
NetworkDisconnectRequest, PortBinding, RestartPolicy, RestartPolicyNameEnum, Runtime,
SystemInfo, VolumeCreateRequest,
};
use bollard::query_parameters::{
CreateContainerOptions, CreateImageOptions, InspectContainerOptions, InspectNetworkOptions,
Expand All @@ -19,6 +20,7 @@ use bollard::query_parameters::{
};
use futures::StreamExt;
use miette::{IntoDiagnostic, Result, WrapErr};
use openshell_core::config::CDI_GPU_DEVICE_ALL;
use std::collections::HashMap;

const REGISTRY_NAMESPACE_DEFAULT: &str = "openshell";
Expand Down Expand Up @@ -64,6 +66,92 @@ pub(crate) fn resolve_gpu_device_ids(gpu: &[String], cdi_enabled: bool) -> Vec<S
}
}

/// Detect concrete GPU device IDs for automatic gateway GPU enablement.
///
/// Auto-detection is intentionally stricter than explicit `--gpu`: CDI is
/// selected only when Docker reports NVIDIA CDI devices, while the legacy
/// NVIDIA runtime path is selected only when the local host exposes NVIDIA
/// device files.
pub(crate) fn auto_detect_gpu_device_ids(
info: Option<&SystemInfo>,
local_nvidia_devices_present: bool,
) -> Vec<String> {
let Some(info) = info else {
return Vec::new();
};

let cdi_device_ids = nvidia_cdi_device_ids(info);
if !cdi_device_ids.is_empty() {
return cdi_device_ids;
}

if local_nvidia_devices_present && docker_info_has_nvidia_runtime(info) {
return vec!["legacy".to_string()];
}

Vec::new()
}

pub(crate) fn docker_info_cdi_enabled(info: Option<&SystemInfo>) -> bool {
info.and_then(|info| info.cdi_spec_dirs.as_ref())
.is_some_and(|dirs| !dirs.is_empty())
}

pub(crate) fn local_nvidia_devices_present() -> bool {
["/dev/nvidia0", "/dev/nvidiactl", "/proc/driver/nvidia/gpus"]
.iter()
.any(|path| std::path::Path::new(path).exists())
}

fn nvidia_cdi_device_ids(info: &SystemInfo) -> Vec<String> {
let Some(devices) = info.discovered_devices.as_ref() else {
return Vec::new();
};

let mut ids = devices
.iter()
.filter_map(nvidia_cdi_device_id)
.collect::<Vec<_>>();
ids.sort();
ids.dedup();

if ids.iter().any(|id| id == CDI_GPU_DEVICE_ALL) {
vec![CDI_GPU_DEVICE_ALL.to_string()]
} else {
ids
}
}

fn nvidia_cdi_device_id(device: &DeviceInfo) -> Option<String> {
let id = device.id.as_ref()?;
if id.contains("nvidia.com/gpu=")
|| (id.contains("/gpu=") && device.source.as_deref().is_some_and(contains_nvidia))
{
return Some(id.clone());
}
None
}

fn docker_info_has_nvidia_runtime(info: &SystemInfo) -> bool {
info.runtimes.as_ref().is_some_and(|runtimes| {
runtimes
.iter()
.any(|(name, runtime)| is_nvidia_runtime(name, runtime))
})
}

fn is_nvidia_runtime(name: &str, runtime: &Runtime) -> bool {
contains_nvidia(name)
|| runtime
.path
.as_deref()
.is_some_and(|path| path.contains("nvidia-container-runtime"))
}

fn contains_nvidia(value: &str) -> bool {
value.to_ascii_lowercase().contains("nvidia")
}

const REGISTRY_MODE_EXTERNAL: &str = "external";

fn env_non_empty(key: &str) -> Option<String> {
Expand Down Expand Up @@ -1466,4 +1554,50 @@ mod tests {
];
assert_eq!(resolve_gpu_device_ids(&multi, true), multi);
}

#[test]
fn auto_detect_gpu_prefers_reported_nvidia_cdi_devices() {
let info = SystemInfo {
discovered_devices: Some(vec![DeviceInfo {
source: Some("cdi".to_string()),
id: Some("nvidia.com/gpu=all".to_string()),
}]),
runtimes: Some(HashMap::from([("nvidia".to_string(), Runtime::default())])),
..Default::default()
};

assert_eq!(
auto_detect_gpu_device_ids(Some(&info), false),
vec!["nvidia.com/gpu=all".to_string()]
);
}

#[test]
fn auto_detect_gpu_uses_legacy_runtime_only_when_local_devices_exist() {
let info = SystemInfo {
runtimes: Some(HashMap::from([("nvidia".to_string(), Runtime::default())])),
..Default::default()
};

assert_eq!(
auto_detect_gpu_device_ids(Some(&info), false),
Vec::<String>::new()
);
assert_eq!(
auto_detect_gpu_device_ids(Some(&info), true),
vec!["legacy".to_string()]
);
}

#[test]
fn docker_info_cdi_enabled_requires_cdi_dirs() {
assert!(!docker_info_cdi_enabled(None));
assert!(!docker_info_cdi_enabled(Some(&SystemInfo::default())));

let info = SystemInfo {
cdi_spec_dirs: Some(vec!["/etc/cdi".to_string()]),
..Default::default()
};
assert!(docker_info_cdi_enabled(Some(&info)));
}
}
41 changes: 29 additions & 12 deletions crates/openshell-bootstrap/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,9 @@ pub struct DeployOptions {
/// - `["auto"]` — resolved at deploy time: CDI if enabled on the daemon, else the non-CDI fallback
/// - `[cdi-ids…]` — CDI DeviceRequest with the given device IDs
pub gpu: Vec<String>,
/// Detect NVIDIA GPU support during deploy and enable passthrough when no
/// explicit GPU device IDs were supplied.
pub gpu_auto_detect: bool,
Comment on lines +122 to +124
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Question: Is there a reason for a separate value? Does an "auto" element in the gpu list not already do this?

/// When true, destroy any existing gateway resources before deploying.
/// When false, an existing gateway is left as-is and deployment is
/// skipped (the caller is responsible for prompting the user first).
Expand All @@ -138,6 +141,7 @@ impl DeployOptions {
registry_username: None,
registry_token: None,
gpu: vec![],
gpu_auto_detect: false,
recreate: false,
}
}
Expand Down Expand Up @@ -202,6 +206,13 @@ impl DeployOptions {
self
}

/// Enable or disable automatic GPU passthrough detection.
#[must_use]
pub fn with_gpu_auto_detect(mut self, auto_detect: bool) -> Self {
self.gpu_auto_detect = auto_detect;
self
}

/// Set whether to destroy and recreate existing gateway resources.
#[must_use]
pub fn with_recreate(mut self, recreate: bool) -> Self {
Expand Down Expand Up @@ -270,7 +281,8 @@ where
let disable_gateway_auth = options.disable_gateway_auth;
let registry_username = options.registry_username;
let registry_token = options.registry_token;
let gpu = options.gpu;
let mut gpu = options.gpu;
let gpu_auto_detect = options.gpu_auto_detect;
let recreate = options.recreate;

// Wrap on_log in Arc<Mutex<>> so we can share it with pull_remote_image
Expand All @@ -296,17 +308,22 @@ where
(preflight.docker, None)
};

// CDI is considered enabled when the daemon reports at least one CDI spec
// directory via `GET /info` (`SystemInfo.CDISpecDirs`). An empty list or
// missing field means CDI is not configured and we fall back to the legacy
// NVIDIA `DeviceRequest` (driver="nvidia"). Detection is best-effort —
// failure to query daemon info is non-fatal.
let cdi_supported = target_docker
.info()
.await
.ok()
.and_then(|info| info.cdi_spec_dirs)
.is_some_and(|dirs| !dirs.is_empty());
// GPU discovery is best-effort. Explicit `--gpu` still uses the legacy
// CDI-enabled check below, while auto-detection only enables GPU when the
// daemon reports NVIDIA CDI devices or the local host has NVIDIA devices
// plus the NVIDIA Docker runtime.
let docker_info = target_docker.info().await.ok();
let cdi_supported = docker::docker_info_cdi_enabled(docker_info.as_ref());
if gpu_auto_detect && gpu.is_empty() {
let detected_gpu = docker::auto_detect_gpu_device_ids(
docker_info.as_ref(),
remote_opts.is_none() && docker::local_nvidia_devices_present(),
);
if !detected_gpu.is_empty() {
log("[status] Detected NVIDIA GPU support".to_string());
gpu = detected_gpu;
}
}

// If an existing gateway is found, decide how to proceed:
// - recreate: destroy everything and start fresh
Expand Down
36 changes: 36 additions & 0 deletions crates/openshell-cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -821,6 +821,14 @@ enum GatewayCommands {
/// (`--gpus all`) otherwise.
#[arg(long)]
gpu: bool,

/// Disable automatic NVIDIA GPU passthrough detection.
///
/// By default, `gateway start` enables GPU passthrough when Docker
/// reports NVIDIA CDI devices, or when the local host exposes NVIDIA
/// devices and the NVIDIA Docker runtime is configured.
#[arg(long, conflicts_with = "gpu")]
no_gpu: bool,
},

/// Stop the gateway (preserves state).
Expand Down Expand Up @@ -1733,12 +1741,14 @@ async fn main() -> Result<()> {
registry_username,
registry_token,
gpu,
no_gpu,
} => {
let gpu = if gpu {
vec!["auto".to_string()]
} else {
vec![]
};
let gpu_auto_detect = !no_gpu && gpu.is_empty();
run::gateway_admin_deploy(
&name,
remote.as_deref(),
Expand All @@ -1751,6 +1761,7 @@ async fn main() -> Result<()> {
registry_username.as_deref(),
registry_token.as_deref(),
gpu,
gpu_auto_detect,
)
.await?;
}
Expand Down Expand Up @@ -3408,6 +3419,31 @@ mod tests {
}
}

#[test]
fn gateway_start_parses_no_gpu_flag() {
let cli = Cli::try_parse_from(["openshell", "gateway", "start", "--no-gpu"])
.expect("gateway start --no-gpu should parse");

match cli.command {
Some(Commands::Gateway {
command: Some(GatewayCommands::Start { no_gpu, gpu, .. }),
}) => {
assert!(no_gpu);
assert!(!gpu);
}
other => panic!("expected gateway start command, got: {other:?}"),
}
}

#[test]
fn gateway_start_rejects_gpu_and_no_gpu_together() {
let result = Cli::try_parse_from(["openshell", "gateway", "start", "--gpu", "--no-gpu"]);
assert!(
result.is_err(),
"gateway start should reject conflicting GPU flags"
);
}

// ── sandbox create arg-shape tests ───────────────────────────────────

/// Verify that `sandbox create --name <value>` still parses as a named
Expand Down
2 changes: 2 additions & 0 deletions crates/openshell-cli/src/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1436,6 +1436,7 @@ pub async fn gateway_admin_deploy(
registry_username: Option<&str>,
registry_token: Option<&str>,
gpu: Vec<String>,
gpu_auto_detect: bool,
) -> Result<()> {
let location = if remote.is_some() { "remote" } else { "local" };

Expand Down Expand Up @@ -1489,6 +1490,7 @@ pub async fn gateway_admin_deploy(
.with_disable_tls(disable_tls)
.with_disable_gateway_auth(disable_gateway_auth)
.with_gpu(gpu)
.with_gpu_auto_detect(gpu_auto_detect)
.with_recreate(recreate);
if let Some(opts) = remote_opts {
options = options.with_remote(opts);
Expand Down
Loading
Loading