From 22815d3e79408cd28aba92b9bef444f56030bcd4 Mon Sep 17 00:00:00 2001 From: Kris Hicks Date: Wed, 15 Jul 2026 10:34:38 -0700 Subject: [PATCH] fix(gateway): probe Docker socket during driver auto-detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, Docker was auto-detected when the CLI was installed or a candidate Unix socket existed. Neither check verified that the Docker API was responsive. A similar check was done when auto-detecting Podman in the past, but was replaced in 1f07bf04 with a probe of candidate Podman sockets instead. This change applies the functional API probing approach introduced for Podman in 1f07bf04 to Docker. It also makes Docker driver initialization use the same socket-selection mechanism as Docker auto-detection instead of Bollard’s local defaults. This means the previously auto-detectable Docker socket paths $HOME/.docker/run/docker.sock and $XDG_RUNTIME_DIR/docker.sock will actually be usable. When no working compute driver can be auto-detected, the gateway exits early with a message saying as much: > configuration error: no compute driver configured and auto-detection found no > suitable driver; set --drivers or OPENSHELL_DRIVERS to kubernetes, podman, > docker, or vm This makes for a better user experience when installing OpenShell without an available supported compute driver. Signed-off-by: Kris Hicks --- architecture/compute-runtimes.md | 5 + crates/openshell-core/src/config.rs | 129 +++++++++++++++--- crates/openshell-driver-docker/README.md | 6 + crates/openshell-driver-docker/src/lib.rs | 24 +++- crates/openshell-server/src/cli.rs | 2 +- .../src/compute/driver_config.rs | 15 ++ docs/reference/gateway-config.mdx | 1 + docs/reference/sandbox-compute-drivers.mdx | 4 +- 8 files changed, 164 insertions(+), 22 deletions(-) diff --git a/architecture/compute-runtimes.md b/architecture/compute-runtimes.md index d5d0912e23..cda2e7f813 100644 --- a/architecture/compute-runtimes.md +++ b/architecture/compute-runtimes.md @@ -26,6 +26,11 @@ The capability RPC reports driver identity, version, and the default sandbox image used by the gateway. GPU availability stays driver-local and is validated when a sandbox create request asks for GPU resources. +The gateway auto-detects Podman or Docker only after a candidate Unix socket +answers an API ping. Docker driver startup uses the same candidate selection as +Docker auto-detection instead of independently applying the client library's +socket defaults. + ## Runtime Summary | Runtime | Best fit | Sandbox boundary | Notes | diff --git a/crates/openshell-core/src/config.rs b/crates/openshell-core/src/config.rs index 36a2a4ec64..c9f8a86b21 100644 --- a/crates/openshell-core/src/config.rs +++ b/crates/openshell-core/src/config.rs @@ -12,7 +12,6 @@ use std::net::SocketAddr; #[cfg(unix)] use std::os::unix::fs::FileTypeExt; use std::path::{Path, PathBuf}; -use std::process::Command; use std::str::FromStr; use std::time::Duration; @@ -163,7 +162,7 @@ pub fn detect_driver() -> Option { return Some(ComputeDriverKind::Podman); } - // Docker: check if the CLI is available or a local Docker socket exists. + // Docker: check for a reachable local API socket. if is_docker_available() { return Some(ComputeDriverKind::Docker); } @@ -171,14 +170,6 @@ pub fn detect_driver() -> Option { None } -/// Check if a binary is available on the system PATH. -fn is_binary_available(name: &str) -> bool { - Command::new(name) - .arg("--version") - .output() - .is_ok_and(|output| output.status.success()) -} - fn is_podman_available() -> bool { podman_socket_candidates() .iter() @@ -228,13 +219,18 @@ fn podman_socket_candidates_from_env( } fn is_docker_available() -> bool { - is_binary_available("docker") || docker_socket_available() + detect_docker_socket().is_some() } -fn docker_socket_available() -> bool { - docker_socket_candidates() +pub fn detect_docker_socket() -> Option { + detect_docker_socket_from_candidates(&docker_socket_candidates()) +} + +fn detect_docker_socket_from_candidates(candidates: &[PathBuf]) -> Option { + candidates .iter() - .any(|path| is_unix_socket(path)) + .find(|path| docker_socket_responds(path)) + .cloned() } fn docker_socket_candidates() -> Vec { @@ -277,6 +273,15 @@ fn podman_socket_responds(path: &Path) -> bool { }) } +#[cfg(unix)] +fn docker_socket_responds(path: &Path) -> bool { + unix_socket_http_ping(path, |response| { + http_response_is_success(response) + && contains_ascii(response, b"Api-Version:") + && !contains_ascii(response, b"Libpod-Api-Version:") + }) +} + #[cfg(unix)] fn unix_socket_http_ping(path: &Path, accepts_response: impl FnOnce(&[u8]) -> bool) -> bool { const PROBE_TIMEOUT: Duration = Duration::from_secs(1); @@ -350,6 +355,12 @@ fn podman_socket_responds(path: &Path) -> bool { false } +#[cfg(not(unix))] +fn docker_socket_responds(path: &Path) -> bool { + let _ = path; + false +} + /// Server configuration. /// /// Built programmatically in [`crate::Config::new`] and the gateway CLI from @@ -960,9 +971,9 @@ mod tests { use super::{ ComputeDriverKind, Config, DEFAULT_SERVICE_ROUTING_DOMAIN, GatewayInterceptorBindingPolicy, GatewayInterceptorConfig, GatewayInterceptorFailurePolicy, GatewayJwtConfig, - GatewayProviderProfileSourceConfig, detect_driver, docker_host_unix_socket_path, - is_unix_socket, normalize_compute_driver_name, podman_socket_candidates_from_env, - podman_socket_responds, + GatewayProviderProfileSourceConfig, detect_docker_socket_from_candidates, detect_driver, + docker_host_unix_socket_path, docker_socket_responds, is_unix_socket, + normalize_compute_driver_name, podman_socket_candidates_from_env, podman_socket_responds, }; #[cfg(unix)] use std::io::{Read as _, Write as _}; @@ -1239,6 +1250,90 @@ mod tests { handle.join().expect("probe server exits"); } + #[cfg(unix)] + #[test] + fn docker_socket_probe_accepts_successful_ping_response() { + let temp_dir = tempfile::tempdir().expect("create temp dir"); + let socket_path = temp_dir.path().join("docker.sock"); + let listener = UnixListener::bind(&socket_path).expect("bind docker socket"); + + let handle = std::thread::spawn(move || { + let (mut stream, _) = listener.accept().expect("accept docker probe"); + let mut request = [0_u8; 128]; + let n = stream.read(&mut request).expect("read docker probe"); + assert!(request[..n].starts_with(b"GET /_ping HTTP/1.1\r\n")); + stream + .write_all( + b"HTTP/1.1 200 OK\r\nApi-Version: 1.51\r\nDocker-Experimental: false\r\nContent-Length: 2\r\n\r\nOK", + ) + .expect("write docker ping response"); + }); + + assert!(docker_socket_responds(&socket_path)); + handle.join().expect("probe server exits"); + } + + #[cfg(unix)] + #[test] + fn docker_socket_probe_rejects_podman_ping_response() { + let temp_dir = tempfile::tempdir().expect("create temp dir"); + let socket_path = temp_dir.path().join("podman.sock"); + let listener = UnixListener::bind(&socket_path).expect("bind podman socket"); + + let handle = std::thread::spawn(move || { + let (mut stream, _) = listener.accept().expect("accept docker probe"); + let mut request = [0_u8; 128]; + let n = stream.read(&mut request).expect("read docker probe"); + assert!(request[..n].starts_with(b"GET /_ping HTTP/1.1\r\n")); + stream + .write_all( + b"HTTP/1.1 200 OK\r\nLibpod-Api-Version: 5.8.2\r\nContent-Length: 2\r\n\r\nOK", + ) + .expect("write podman ping response"); + }); + + assert!(!docker_socket_responds(&socket_path)); + handle.join().expect("probe server exits"); + } + + #[cfg(unix)] + #[test] + fn docker_socket_probe_rejects_inactive_socket() { + let temp_dir = tempfile::tempdir().expect("create temp dir"); + let socket_path = temp_dir.path().join("docker.sock"); + let listener = UnixListener::bind(&socket_path).expect("bind docker socket"); + drop(listener); + + assert!(is_unix_socket(&socket_path)); + assert!(!docker_socket_responds(&socket_path)); + } + + #[cfg(unix)] + #[test] + fn docker_socket_detection_returns_the_responsive_candidate() { + let temp_dir = tempfile::tempdir().expect("create temp dir"); + let inactive_path = temp_dir.path().join("inactive.sock"); + let inactive_listener = UnixListener::bind(&inactive_path).expect("bind inactive socket"); + drop(inactive_listener); + + let responsive_path = temp_dir.path().join("responsive.sock"); + let listener = UnixListener::bind(&responsive_path).expect("bind responsive socket"); + let handle = std::thread::spawn(move || { + let (mut stream, _) = listener.accept().expect("accept docker probe"); + let mut request = [0_u8; 128]; + let _ = stream.read(&mut request).expect("read docker probe"); + stream + .write_all(b"HTTP/1.1 200 OK\r\nApi-Version: 1.51\r\nContent-Length: 2\r\n\r\nOK") + .expect("write docker ping response"); + }); + + assert_eq!( + detect_docker_socket_from_candidates(&[inactive_path, responsive_path.clone(),]), + Some(responsive_path) + ); + handle.join().expect("probe server exits"); + } + #[test] fn podman_socket_candidates_include_env_runtime_and_home_paths() { let candidates = podman_socket_candidates_from_env( diff --git a/crates/openshell-driver-docker/README.md b/crates/openshell-driver-docker/README.md index 82e5eea4b7..e17791e747 100644 --- a/crates/openshell-driver-docker/README.md +++ b/crates/openshell-driver-docker/README.md @@ -6,6 +6,12 @@ The driver manages sandbox containers through the local Docker daemon with the `bollard` client. It is intended for developer environments where Docker is already available and running Kubernetes would be unnecessary. +The driver connects to `[openshell.drivers.docker].socket_path` when configured. +Otherwise, it uses the first standard local Docker socket that responds to an +API ping, which is the same selection mechanism used by gateway auto-detection. +An explicitly selected Docker driver falls back to `/var/run/docker.sock` when +no candidate responds. + ## Runtime Model The gateway runs as a host process. The Docker driver creates one container per diff --git a/crates/openshell-driver-docker/src/lib.rs b/crates/openshell-driver-docker/src/lib.rs index 06d575a1e1..97a0f60839 100644 --- a/crates/openshell-driver-docker/src/lib.rs +++ b/crates/openshell-driver-docker/src/lib.rs @@ -93,6 +93,11 @@ pub trait SupervisorReadiness: Send + Sync + 'static { #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] #[serde(default, deny_unknown_fields)] pub struct DockerComputeConfig { + /// Docker API Unix socket. When unset, use the socket selected by gateway + /// auto-detection, falling back to `/var/run/docker.sock` for an explicitly + /// configured Docker driver. + pub socket_path: Option, + /// Default OCI image for sandboxes. pub default_image: String, @@ -145,6 +150,7 @@ pub struct DockerComputeConfig { impl Default for DockerComputeConfig { fn default() -> Self { Self { + socket_path: None, default_image: openshell_core::image::default_sandbox_image(), image_pull_policy: String::new(), sandbox_namespace: "default".to_string(), @@ -307,8 +313,22 @@ impl DockerComputeDriver { docker_config: &DockerComputeConfig, supervisor_readiness: Arc, ) -> CoreResult { - let docker = Docker::connect_with_local_defaults() - .map_err(|err| Error::execution(format!("failed to create Docker client: {err}")))?; + let socket_path = docker_config + .socket_path + .clone() + .or_else(openshell_core::config::detect_docker_socket) + .unwrap_or_else(|| PathBuf::from("/var/run/docker.sock")); + let socket_path_str = socket_path.to_str().ok_or_else(|| { + Error::config(format!( + "Docker socket path is not valid UTF-8: {}", + socket_path.display() + )) + })?; + let docker = + Docker::connect_with_socket(socket_path_str, 120, bollard::API_DEFAULT_VERSION) + .map_err(|err| { + Error::execution(format!("failed to create Docker client: {err}")) + })?; let version = docker.version().await.map_err(|err| { Error::execution(format!("failed to query Docker daemon version: {err}")) })?; diff --git a/crates/openshell-server/src/cli.rs b/crates/openshell-server/src/cli.rs index 7260fc1fdf..269b9f40e9 100644 --- a/crates/openshell-server/src/cli.rs +++ b/crates/openshell-server/src/cli.rs @@ -100,7 +100,7 @@ struct RunArgs { /// `kubernetes,podman`. The configuration format is future-proofed for /// multiple drivers, but the gateway currently requires exactly one. /// When unset, the gateway auto-detects the driver based on the runtime - /// environment (Kubernetes → Podman → Docker CLI or socket). VM is never + /// environment (Kubernetes → Podman → Docker). VM is never /// auto-detected and requires explicit configuration. #[arg( long, diff --git a/crates/openshell-server/src/compute/driver_config.rs b/crates/openshell-server/src/compute/driver_config.rs index 294cec0ce7..fa44cd3a38 100644 --- a/crates/openshell-server/src/compute/driver_config.rs +++ b/crates/openshell-server/src/compute/driver_config.rs @@ -321,6 +321,21 @@ enable_bind_mounts = true assert!(cfg.enable_bind_mounts); } + #[test] + fn docker_config_reads_socket_path_from_driver_table() { + let file: config_file::ConfigFile = toml::from_str( + r#" +[openshell.drivers.docker] +socket_path = "/tmp/docker.sock" +"#, + ) + .expect("valid config"); + + let cfg = docker_config_from_context(test_context(Some(&file))).expect("docker config"); + + assert_eq!(cfg.socket_path, Some(PathBuf::from("/tmp/docker.sock"))); + } + #[test] fn remote_driver_config_reads_socket_path_from_named_table() { let file: config_file::ConfigFile = toml::from_str( diff --git a/docs/reference/gateway-config.mdx b/docs/reference/gateway-config.mdx index 6ed2bae858..b0efa87e40 100644 --- a/docs/reference/gateway-config.mdx +++ b/docs/reference/gateway-config.mdx @@ -287,6 +287,7 @@ log_level = "info" compute_drivers = ["docker"] [openshell.drivers.docker] +socket_path = "/var/run/docker.sock" default_image = "ghcr.io/nvidia/openshell/sandbox:latest" # Docker vocabulary: Always | IfNotPresent | Never. Empty behaves like IfNotPresent. image_pull_policy = "IfNotPresent" diff --git a/docs/reference/sandbox-compute-drivers.mdx b/docs/reference/sandbox-compute-drivers.mdx index 03a9b564f0..a0167ae72a 100644 --- a/docs/reference/sandbox-compute-drivers.mdx +++ b/docs/reference/sandbox-compute-drivers.mdx @@ -25,7 +25,7 @@ Reserved built-in values are `docker`, `podman`, `kubernetes`, and `vm`. Non-reserved names select an extension driver and require a `socket_path` in `[openshell.drivers.]`. -When `compute_drivers` is unset, the gateway auto-detects Kubernetes, then Podman, then Docker by CLI availability or a local Unix socket. The VM driver is never auto-detected; configure it explicitly with `compute_drivers = ["vm"]` or set `OPENSHELL_DRIVERS=vm` in the launch environment. +When `compute_drivers` is unset, the gateway auto-detects Kubernetes, then Podman, then Docker. Local container runtimes must respond to an API probe before the gateway selects them. The VM driver is never auto-detected; configure it explicitly with `compute_drivers = ["vm"]` or set `OPENSHELL_DRIVERS=vm` in the launch environment. Common gateway options: @@ -115,7 +115,7 @@ The gateway talks to the Docker daemon to create sandbox containers. Docker is a For maintainer-level implementation details, refer to the [Docker driver README](https://github.com/NVIDIA/OpenShell/blob/main/crates/openshell-driver-docker/README.md). -Select Docker with `compute_drivers = ["docker"]` in `[openshell.gateway]`. Configure Docker driver values such as `grpc_endpoint`, `network_name`, `supervisor_bin`, `supervisor_image`, `image_pull_policy`, `ssh_socket_path`, `sandbox_pids_limit`, and `guest_tls_*` in `[openshell.drivers.docker]`. +Select Docker with `compute_drivers = ["docker"]` in `[openshell.gateway]`. Configure Docker driver values such as `socket_path`, `grpc_endpoint`, `network_name`, `supervisor_bin`, `supervisor_image`, `image_pull_policy`, `ssh_socket_path`, `sandbox_pids_limit`, and `guest_tls_*` in `[openshell.drivers.docker]`. When `socket_path` is unset, the driver uses the same responsive local socket selected by auto-detection. An explicitly selected Docker driver falls back to `/var/run/docker.sock` when no candidate responds. For GPU-backed Docker sandboxes, configure Docker CDI before starting the gateway so OpenShell can detect the daemon capability.