diff --git a/crates/openshell-driver-podman/src/driver.rs b/crates/openshell-driver-podman/src/driver.rs index d3023cddd8..cc5d489894 100644 --- a/crates/openshell-driver-podman/src/driver.rs +++ b/crates/openshell-driver-podman/src/driver.rs @@ -25,6 +25,7 @@ use openshell_core::proto::compute::v1::{ GpuResourceRequirements, MemoryResourceCapabilities, ResourceCapabilities, }; use std::collections::HashMap; +use std::future::Future; use std::path::{Path, PathBuf}; use std::sync::Arc; use std::time::Duration; @@ -32,6 +33,8 @@ use tracing::{Instrument as _, debug, info, warn}; const STOP_COMPLETION_POLL_INTERVAL: Duration = Duration::from_millis(50); const STOP_COMPLETION_TIMEOUT_HEADROOM: Duration = Duration::from_secs(5); +const MAX_PING_RETRIES: u32 = 5; +const PING_RETRY_DELAY: Duration = Duration::from_secs(2); const POLICY_DNS_RESOLV_CONF: &[u8] = b"nameserver 127.0.0.53\n"; #[derive(Clone, Copy, Debug, Eq, PartialEq)] @@ -409,12 +412,33 @@ fn resolve_socket_path( }) } +async fn ping_with_retry(mut ping: F) -> Result<(), PodmanApiError> +where + F: FnMut() -> Fut, + Fut: Future>, +{ + let mut retries = 0; + loop { + match ping().await { + Ok(()) => return Ok(()), + Err(error) if retries < MAX_PING_RETRIES => { + retries += 1; + warn!( + attempt = retries, + max_retries = MAX_PING_RETRIES, + error = %error, + "Podman socket not ready, retrying" + ); + tokio::time::sleep(PING_RETRY_DELAY).await; + } + Err(error) => return Err(error), + } + } +} + impl PodmanComputeDriver { /// Create a new driver, verifying the Podman socket is reachable. pub async fn new(mut config: PodmanComputeConfig) -> Result { - const MAX_PING_RETRIES: u32 = 5; - const PING_RETRY_DELAY: Duration = Duration::from_secs(2); - let socket_path = resolve_socket_path(config.socket_path.clone(), detect_socket)?; config.socket_path = Some(socket_path.clone()); @@ -445,23 +469,7 @@ impl PodmanComputeDriver { // unavailability (e.g. podman.socket restarting after a package // upgrade). The systemd unit uses Wants=podman.socket (not Requires), // so the gateway may start while the socket is briefly re-activating. - let mut attempts = 0; - loop { - match client.ping().await { - Ok(()) => break, - Err(e) if attempts < MAX_PING_RETRIES => { - attempts += 1; - warn!( - attempt = attempts, - max_retries = MAX_PING_RETRIES, - error = %e, - "Podman socket not ready, retrying" - ); - tokio::time::sleep(PING_RETRY_DELAY).await; - } - Err(e) => return Err(e), - } - } + ping_with_retry(|| client.ping()).await?; // Verify cgroups v2, detect rootless mode, and log system info. let rootless = match client.system_info().await { @@ -2044,7 +2052,7 @@ mod tests { use openshell_core::proto::compute::v1::{ DriverSandboxSpec, DriverSandboxTemplate, ResourceRequirements, }; - use std::collections::HashMap; + use std::collections::{HashMap, VecDeque}; use std::fs; use std::path::{Path, PathBuf}; @@ -2105,6 +2113,63 @@ mod tests { assert!(err.to_string().contains("no responsive Podman API socket")); } + #[tokio::test(start_paused = true)] + async fn ping_retries_transient_failures() { + let mut outcomes = VecDeque::from([ + Err(PodmanApiError::Connection("first".to_string())), + Err(PodmanApiError::Connection("second".to_string())), + Ok(()), + ]); + let started = tokio::time::Instant::now(); + + ping_with_retry(|| std::future::ready(outcomes.pop_front().expect("ping outcome"))) + .await + .expect("a later successful ping should stop retries"); + + assert!(outcomes.is_empty()); + assert_eq!(started.elapsed(), PING_RETRY_DELAY * 2); + } + + #[tokio::test(start_paused = true)] + async fn ping_failure_is_bounded_by_retry_policy() { + let mut attempts = 0; + let started = tokio::time::Instant::now(); + + let error = ping_with_retry(|| { + attempts += 1; + std::future::ready(Err(PodmanApiError::Connection(format!( + "attempt {attempts}" + )))) + }) + .await + .expect_err("persistent connection failures should be returned"); + + assert_eq!(attempts, MAX_PING_RETRIES + 1); + assert_eq!(started.elapsed(), PING_RETRY_DELAY * MAX_PING_RETRIES); + assert_eq!( + error.to_string(), + format!("connection error: attempt {}", MAX_PING_RETRIES + 1) + ); + } + + #[tokio::test] + async fn missing_socket_connection_error_names_configured_path() { + let tempdir = tempfile::tempdir().expect("create isolated socket directory"); + let missing_socket = tempdir.path().join("missing-podman.sock"); + + let error = PodmanClient::new(missing_socket.clone()) + .ping() + .await + .expect_err("a missing socket should fail to connect"); + + assert!(matches!(error, PodmanApiError::Connection(_))); + assert!( + error + .to_string() + .contains(&missing_socket.display().to_string()) + ); + } + fn cdi_devices_config(device_ids: &[&str]) -> prost_types::Struct { prost_types::Struct { fields: std::iter::once(( diff --git a/crates/openshell-driver-podman/tests/podman_preflight.rs b/crates/openshell-driver-podman/tests/podman_preflight.rs index 439cb95d98..255038e767 100644 --- a/crates/openshell-driver-podman/tests/podman_preflight.rs +++ b/crates/openshell-driver-podman/tests/podman_preflight.rs @@ -1,89 +1,51 @@ // SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -//! Podman driver daemon-unavailable integration tests. +//! Standalone driver smoke test for the Podman-unavailable diagnostic path. //! -//! These tests verify that `openshell-driver-podman` fails fast with an -//! actionable error when it cannot reach a Podman API socket, instead of -//! hanging or silently serving gRPC against a dead connection. -//! -//! They do NOT require a running Podman daemon or gateway — they point -//! `--podman-socket` at a path that is guaranteed not to exist to simulate -//! the daemon being unavailable. As a plain Cargo integration test in this -//! crate, this runs via the normal `cargo test -p openshell-driver-podman` -//! lane with no special CI wiring: Cargo provides `CARGO_BIN_EXE_` for -//! this crate's own `[[bin]]` target automatically. +//! Retry timing and connection failures are covered deterministically by unit +//! tests. This test retains only the executable boundary: argument wiring, +//! process exit status, and the rendered error shown to operators. use std::path::PathBuf; -use std::time::{Duration, Instant}; +use std::process::Stdio; +use std::time::Duration; -/// Run `openshell-driver-podman` pointed at a Podman socket that does not -/// exist, and wait for it to exit. -/// -/// The driver retries a handful of times before giving up (to tolerate the -/// socket briefly re-activating), so this can take several seconds. -async fn run_with_unreachable_podman_socket() -> (String, i32, Duration, PathBuf) { +#[tokio::test] +async fn missing_podman_socket_exits_with_actionable_diagnostic() { let tmpdir = tempfile::tempdir().expect("create isolated socket dir"); // Use a short relative path so miette cannot insert a line-wrap gutter // inside it on platforms with long temporary-directory paths. let missing_socket = PathBuf::from("missing-podman.sock"); - let start = Instant::now(); let mut cmd = tokio::process::Command::new(env!("CARGO_BIN_EXE_openshell-driver-podman")); cmd.arg("--podman-socket") .arg(&missing_socket) .current_dir(tmpdir.path()) .kill_on_drop(true) - .stdout(std::process::Stdio::piped()) - .stderr(std::process::Stdio::piped()); + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); - let output = tokio::time::timeout(Duration::from_mins(1), cmd.output()) + let output = tokio::time::timeout(Duration::from_secs(30), cmd.output()) .await - .expect("openshell-driver-podman should exit instead of hanging") + .expect("driver should stop after its bounded retry window") .expect("spawn openshell-driver-podman"); - let elapsed = start.elapsed(); - let stdout = String::from_utf8_lossy(&output.stdout).to_string(); - let stderr = String::from_utf8_lossy(&output.stderr).to_string(); - let combined = format!("{stdout}{stderr}"); - let code = output.status.code().unwrap_or(-1); - (combined, code, elapsed, missing_socket) -} - -/// `openshell-driver-podman` should exit non-zero, not hang, when its -/// configured Podman socket does not exist. -#[tokio::test] -async fn driver_exits_when_podman_socket_unreachable() { - let (output, code, elapsed, _) = run_with_unreachable_podman_socket().await; - - assert_ne!( - code, 0, - "driver should exit non-zero when Podman is unreachable, output:\n{output}" - ); assert!( - elapsed < Duration::from_secs(30), - "driver should give up retrying and exit within its bounded retry \ - window (took {}s), output:\n{output}", - elapsed.as_secs() + !output.status.success(), + "driver should exit non-zero when Podman is unreachable" ); -} - -/// The error surfaced when the Podman socket is unreachable should name the -/// configured socket path and describe a connection failure, not a generic -/// panic or timeout with no actionable detail. -#[tokio::test] -async fn driver_error_names_unreachable_socket() { - let (output, code, _, missing_socket) = run_with_unreachable_podman_socket().await; - - assert_ne!(code, 0); + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); + let combined = format!("{stdout}{stderr}"); assert!( - output.contains("connection error"), - "driver error should describe a connection failure:\n{output}" + combined.contains("connection error"), + "driver error should describe a connection failure:\n{combined}" ); assert!( - output.contains(missing_socket.to_str().expect("socket path is utf-8")), - "driver error should name the unreachable socket path {}:\n{output}", + combined.contains(missing_socket.to_str().expect("socket path is utf-8")), + "driver error should name the unreachable socket path {}:\n{combined}", missing_socket.display() ); }