-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix(gateway): probe Docker socket during driver auto-detection #2303
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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,22 +162,14 @@ pub fn detect_driver() -> Option<ComputeDriverKind> { | |
| 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); | ||
| } | ||
|
|
||
| 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<PathBuf> { | ||
| detect_docker_socket_from_candidates(&docker_socket_candidates()) | ||
| } | ||
|
|
||
| fn detect_docker_socket_from_candidates(candidates: &[PathBuf]) -> Option<PathBuf> { | ||
| candidates | ||
| .iter() | ||
| .any(|path| is_unix_socket(path)) | ||
| .find(|path| docker_socket_responds(path)) | ||
| .cloned() | ||
| } | ||
|
|
||
| fn docker_socket_candidates() -> Vec<PathBuf> { | ||
|
|
@@ -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:") | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This negative check ensures that were not detecting a symlinked podman socket, correct? |
||
| }) | ||
| } | ||
|
|
||
| #[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( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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<PathBuf>, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. naming nit: We already have a |
||
|
|
||
| /// 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<dyn SupervisorReadiness>, | ||
| ) -> CoreResult<Self> { | ||
| 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}")) | ||
| })?; | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.