Skip to content

Commit cf4decc

Browse files
authored
fix(gateway): probe Docker socket during driver auto-detection (#2303)
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 1f07bf0 with a probe of candidate Podman sockets instead. This change applies the functional API probing approach introduced for Podman in 1f07bf0 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 <khicks@nvidia.com>
1 parent 008193a commit cf4decc

7 files changed

Lines changed: 159 additions & 22 deletions

File tree

crates/openshell-core/src/config.rs

Lines changed: 112 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ use std::net::SocketAddr;
1212
#[cfg(unix)]
1313
use std::os::unix::fs::FileTypeExt;
1414
use std::path::{Path, PathBuf};
15-
use std::process::Command;
1615
use std::str::FromStr;
1716
use std::time::Duration;
1817

@@ -163,22 +162,14 @@ pub fn detect_driver() -> Option<ComputeDriverKind> {
163162
return Some(ComputeDriverKind::Podman);
164163
}
165164

166-
// Docker: check if the CLI is available or a local Docker socket exists.
165+
// Docker: check for a reachable local API socket.
167166
if is_docker_available() {
168167
return Some(ComputeDriverKind::Docker);
169168
}
170169

171170
None
172171
}
173172

174-
/// Check if a binary is available on the system PATH.
175-
fn is_binary_available(name: &str) -> bool {
176-
Command::new(name)
177-
.arg("--version")
178-
.output()
179-
.is_ok_and(|output| output.status.success())
180-
}
181-
182173
fn is_podman_available() -> bool {
183174
podman_socket_candidates()
184175
.iter()
@@ -228,13 +219,18 @@ fn podman_socket_candidates_from_env(
228219
}
229220

230221
fn is_docker_available() -> bool {
231-
is_binary_available("docker") || docker_socket_available()
222+
detect_docker_socket().is_some()
232223
}
233224

234-
fn docker_socket_available() -> bool {
235-
docker_socket_candidates()
225+
pub fn detect_docker_socket() -> Option<PathBuf> {
226+
detect_docker_socket_from_candidates(&docker_socket_candidates())
227+
}
228+
229+
fn detect_docker_socket_from_candidates(candidates: &[PathBuf]) -> Option<PathBuf> {
230+
candidates
236231
.iter()
237-
.any(|path| is_unix_socket(path))
232+
.find(|path| docker_socket_responds(path))
233+
.cloned()
238234
}
239235

240236
fn docker_socket_candidates() -> Vec<PathBuf> {
@@ -277,6 +273,15 @@ fn podman_socket_responds(path: &Path) -> bool {
277273
})
278274
}
279275

276+
#[cfg(unix)]
277+
fn docker_socket_responds(path: &Path) -> bool {
278+
unix_socket_http_ping(path, |response| {
279+
http_response_is_success(response)
280+
&& contains_ascii(response, b"Api-Version:")
281+
&& !contains_ascii(response, b"Libpod-Api-Version:")
282+
})
283+
}
284+
280285
#[cfg(unix)]
281286
fn unix_socket_http_ping(path: &Path, accepts_response: impl FnOnce(&[u8]) -> bool) -> bool {
282287
const PROBE_TIMEOUT: Duration = Duration::from_secs(1);
@@ -350,6 +355,12 @@ fn podman_socket_responds(path: &Path) -> bool {
350355
false
351356
}
352357

358+
#[cfg(not(unix))]
359+
fn docker_socket_responds(path: &Path) -> bool {
360+
let _ = path;
361+
false
362+
}
363+
353364
/// Server configuration.
354365
///
355366
/// Built programmatically in [`crate::Config::new`] and the gateway CLI from
@@ -960,9 +971,9 @@ mod tests {
960971
use super::{
961972
ComputeDriverKind, Config, DEFAULT_SERVICE_ROUTING_DOMAIN, GatewayInterceptorBindingPolicy,
962973
GatewayInterceptorConfig, GatewayInterceptorFailurePolicy, GatewayJwtConfig,
963-
GatewayProviderProfileSourceConfig, detect_driver, docker_host_unix_socket_path,
964-
is_unix_socket, normalize_compute_driver_name, podman_socket_candidates_from_env,
965-
podman_socket_responds,
974+
GatewayProviderProfileSourceConfig, detect_docker_socket_from_candidates, detect_driver,
975+
docker_host_unix_socket_path, docker_socket_responds, is_unix_socket,
976+
normalize_compute_driver_name, podman_socket_candidates_from_env, podman_socket_responds,
966977
};
967978
#[cfg(unix)]
968979
use std::io::{Read as _, Write as _};
@@ -1239,6 +1250,90 @@ mod tests {
12391250
handle.join().expect("probe server exits");
12401251
}
12411252

1253+
#[cfg(unix)]
1254+
#[test]
1255+
fn docker_socket_probe_accepts_successful_ping_response() {
1256+
let temp_dir = tempfile::tempdir().expect("create temp dir");
1257+
let socket_path = temp_dir.path().join("docker.sock");
1258+
let listener = UnixListener::bind(&socket_path).expect("bind docker socket");
1259+
1260+
let handle = std::thread::spawn(move || {
1261+
let (mut stream, _) = listener.accept().expect("accept docker probe");
1262+
let mut request = [0_u8; 128];
1263+
let n = stream.read(&mut request).expect("read docker probe");
1264+
assert!(request[..n].starts_with(b"GET /_ping HTTP/1.1\r\n"));
1265+
stream
1266+
.write_all(
1267+
b"HTTP/1.1 200 OK\r\nApi-Version: 1.51\r\nDocker-Experimental: false\r\nContent-Length: 2\r\n\r\nOK",
1268+
)
1269+
.expect("write docker ping response");
1270+
});
1271+
1272+
assert!(docker_socket_responds(&socket_path));
1273+
handle.join().expect("probe server exits");
1274+
}
1275+
1276+
#[cfg(unix)]
1277+
#[test]
1278+
fn docker_socket_probe_rejects_podman_ping_response() {
1279+
let temp_dir = tempfile::tempdir().expect("create temp dir");
1280+
let socket_path = temp_dir.path().join("podman.sock");
1281+
let listener = UnixListener::bind(&socket_path).expect("bind podman socket");
1282+
1283+
let handle = std::thread::spawn(move || {
1284+
let (mut stream, _) = listener.accept().expect("accept docker probe");
1285+
let mut request = [0_u8; 128];
1286+
let n = stream.read(&mut request).expect("read docker probe");
1287+
assert!(request[..n].starts_with(b"GET /_ping HTTP/1.1\r\n"));
1288+
stream
1289+
.write_all(
1290+
b"HTTP/1.1 200 OK\r\nLibpod-Api-Version: 5.8.2\r\nContent-Length: 2\r\n\r\nOK",
1291+
)
1292+
.expect("write podman ping response");
1293+
});
1294+
1295+
assert!(!docker_socket_responds(&socket_path));
1296+
handle.join().expect("probe server exits");
1297+
}
1298+
1299+
#[cfg(unix)]
1300+
#[test]
1301+
fn docker_socket_probe_rejects_inactive_socket() {
1302+
let temp_dir = tempfile::tempdir().expect("create temp dir");
1303+
let socket_path = temp_dir.path().join("docker.sock");
1304+
let listener = UnixListener::bind(&socket_path).expect("bind docker socket");
1305+
drop(listener);
1306+
1307+
assert!(is_unix_socket(&socket_path));
1308+
assert!(!docker_socket_responds(&socket_path));
1309+
}
1310+
1311+
#[cfg(unix)]
1312+
#[test]
1313+
fn docker_socket_detection_returns_the_responsive_candidate() {
1314+
let temp_dir = tempfile::tempdir().expect("create temp dir");
1315+
let inactive_path = temp_dir.path().join("inactive.sock");
1316+
let inactive_listener = UnixListener::bind(&inactive_path).expect("bind inactive socket");
1317+
drop(inactive_listener);
1318+
1319+
let responsive_path = temp_dir.path().join("responsive.sock");
1320+
let listener = UnixListener::bind(&responsive_path).expect("bind responsive socket");
1321+
let handle = std::thread::spawn(move || {
1322+
let (mut stream, _) = listener.accept().expect("accept docker probe");
1323+
let mut request = [0_u8; 128];
1324+
let _ = stream.read(&mut request).expect("read docker probe");
1325+
stream
1326+
.write_all(b"HTTP/1.1 200 OK\r\nApi-Version: 1.51\r\nContent-Length: 2\r\n\r\nOK")
1327+
.expect("write docker ping response");
1328+
});
1329+
1330+
assert_eq!(
1331+
detect_docker_socket_from_candidates(&[inactive_path, responsive_path.clone(),]),
1332+
Some(responsive_path)
1333+
);
1334+
handle.join().expect("probe server exits");
1335+
}
1336+
12421337
#[test]
12431338
fn podman_socket_candidates_include_env_runtime_and_home_paths() {
12441339
let candidates = podman_socket_candidates_from_env(

crates/openshell-driver-docker/README.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,12 @@ The driver manages sandbox containers through the local Docker daemon with the
66
`bollard` client. It is intended for developer environments where Docker is
77
already available and running Kubernetes would be unnecessary.
88

9+
The driver connects to `[openshell.drivers.docker].socket_path` when configured.
10+
Otherwise, it uses the first standard local Docker socket that responds to an
11+
API ping, which is the same selection mechanism used by gateway auto-detection.
12+
An explicitly selected Docker driver falls back to `/var/run/docker.sock` when
13+
no candidate responds.
14+
915
## Runtime Model
1016

1117
The gateway runs as a host process. The Docker driver creates one container per

crates/openshell-driver-docker/src/lib.rs

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,11 @@ pub trait SupervisorReadiness: Send + Sync + 'static {
9393
#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)]
9494
#[serde(default, deny_unknown_fields)]
9595
pub struct DockerComputeConfig {
96+
/// Docker API Unix socket. When unset, use the socket selected by gateway
97+
/// auto-detection, falling back to `/var/run/docker.sock` for an explicitly
98+
/// configured Docker driver.
99+
pub socket_path: Option<PathBuf>,
100+
96101
/// Default OCI image for sandboxes.
97102
pub default_image: String,
98103

@@ -145,6 +150,7 @@ pub struct DockerComputeConfig {
145150
impl Default for DockerComputeConfig {
146151
fn default() -> Self {
147152
Self {
153+
socket_path: None,
148154
default_image: openshell_core::image::default_sandbox_image(),
149155
image_pull_policy: String::new(),
150156
sandbox_namespace: "default".to_string(),
@@ -307,8 +313,22 @@ impl DockerComputeDriver {
307313
docker_config: &DockerComputeConfig,
308314
supervisor_readiness: Arc<dyn SupervisorReadiness>,
309315
) -> CoreResult<Self> {
310-
let docker = Docker::connect_with_local_defaults()
311-
.map_err(|err| Error::execution(format!("failed to create Docker client: {err}")))?;
316+
let socket_path = docker_config
317+
.socket_path
318+
.clone()
319+
.or_else(openshell_core::config::detect_docker_socket)
320+
.unwrap_or_else(|| PathBuf::from("/var/run/docker.sock"));
321+
let socket_path_str = socket_path.to_str().ok_or_else(|| {
322+
Error::config(format!(
323+
"Docker socket path is not valid UTF-8: {}",
324+
socket_path.display()
325+
))
326+
})?;
327+
let docker =
328+
Docker::connect_with_socket(socket_path_str, 120, bollard::API_DEFAULT_VERSION)
329+
.map_err(|err| {
330+
Error::execution(format!("failed to create Docker client: {err}"))
331+
})?;
312332
let version = docker.version().await.map_err(|err| {
313333
Error::execution(format!("failed to query Docker daemon version: {err}"))
314334
})?;

crates/openshell-server/src/cli.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ struct RunArgs {
100100
/// `kubernetes,podman`. The configuration format is future-proofed for
101101
/// multiple drivers, but the gateway currently requires exactly one.
102102
/// When unset, the gateway auto-detects the driver based on the runtime
103-
/// environment (Kubernetes → Podman → Docker CLI or socket). VM is never
103+
/// environment (Kubernetes → Podman → Docker). VM is never
104104
/// auto-detected and requires explicit configuration.
105105
#[arg(
106106
long,

crates/openshell-server/src/compute/driver_config.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -321,6 +321,21 @@ enable_bind_mounts = true
321321
assert!(cfg.enable_bind_mounts);
322322
}
323323

324+
#[test]
325+
fn docker_config_reads_socket_path_from_driver_table() {
326+
let file: config_file::ConfigFile = toml::from_str(
327+
r#"
328+
[openshell.drivers.docker]
329+
socket_path = "/tmp/docker.sock"
330+
"#,
331+
)
332+
.expect("valid config");
333+
334+
let cfg = docker_config_from_context(test_context(Some(&file))).expect("docker config");
335+
336+
assert_eq!(cfg.socket_path, Some(PathBuf::from("/tmp/docker.sock")));
337+
}
338+
324339
#[test]
325340
fn remote_driver_config_reads_socket_path_from_named_table() {
326341
let file: config_file::ConfigFile = toml::from_str(

docs/reference/gateway-config.mdx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,7 @@ log_level = "info"
287287
compute_drivers = ["docker"]
288288

289289
[openshell.drivers.docker]
290+
socket_path = "/var/run/docker.sock"
290291
default_image = "ghcr.io/nvidia/openshell/sandbox:latest"
291292
# Docker vocabulary: Always | IfNotPresent | Never. Empty behaves like IfNotPresent.
292293
image_pull_policy = "IfNotPresent"

docs/reference/sandbox-compute-drivers.mdx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ Reserved built-in values are `docker`, `podman`, `kubernetes`, and `vm`.
2525
Non-reserved names select an extension driver and require a
2626
`socket_path` in `[openshell.drivers.<name>]`.
2727

28-
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.
28+
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.
2929

3030
Common gateway options:
3131

@@ -115,7 +115,7 @@ The gateway talks to the Docker daemon to create sandbox containers. Docker is a
115115

116116
For maintainer-level implementation details, refer to the [Docker driver README](https://github.com/NVIDIA/OpenShell/blob/main/crates/openshell-driver-docker/README.md).
117117

118-
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]`.
118+
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.
119119

120120
For GPU-backed Docker sandboxes, configure Docker CDI before starting the gateway so OpenShell can detect the daemon capability.
121121

0 commit comments

Comments
 (0)