diff --git a/Cargo.lock b/Cargo.lock index c6330b8897..0854d0bf41 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4264,6 +4264,7 @@ dependencies = [ "serde_json", "tar", "temp-env", + "tempfile", "thiserror 2.0.20", "tokio", "tokio-stream", diff --git a/crates/openshell-driver-podman/Cargo.toml b/crates/openshell-driver-podman/Cargo.toml index c49c309f65..18b1928c33 100644 --- a/crates/openshell-driver-podman/Cargo.toml +++ b/crates/openshell-driver-podman/Cargo.toml @@ -51,6 +51,7 @@ openshell-otel-test-support = { path = "../openshell-otel-test-support" } opentelemetry_sdk = { workspace = true, features = ["testing"] } prost-types = { workspace = true } temp-env = "0.3" +tempfile = "3" tokio = { workspace = true, features = ["test-util"] } [lints] diff --git a/crates/openshell-driver-podman/src/driver.rs b/crates/openshell-driver-podman/src/driver.rs index 3788c756f4..baa5a34b34 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)] @@ -413,12 +416,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()); @@ -449,23 +473,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 { @@ -2048,7 +2056,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}; @@ -2109,6 +2117,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 new file mode 100644 index 0000000000..255038e767 --- /dev/null +++ b/crates/openshell-driver-podman/tests/podman_preflight.rs @@ -0,0 +1,51 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +//! Standalone driver smoke test for the Podman-unavailable diagnostic path. +//! +//! 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::process::Stdio; +use std::time::Duration; + +#[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 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(Stdio::piped()) + .stderr(Stdio::piped()); + + let output = tokio::time::timeout(Duration::from_secs(30), cmd.output()) + .await + .expect("driver should stop after its bounded retry window") + .expect("spawn openshell-driver-podman"); + + assert!( + !output.status.success(), + "driver should exit non-zero when Podman is unreachable" + ); + + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); + let combined = format!("{stdout}{stderr}"); + assert!( + combined.contains("connection error"), + "driver error should describe a connection failure:\n{combined}" + ); + assert!( + 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() + ); +} diff --git a/e2e/rust/Cargo.toml b/e2e/rust/Cargo.toml index b492c8ac86..6868193431 100644 --- a/e2e/rust/Cargo.toml +++ b/e2e/rust/Cargo.toml @@ -93,11 +93,6 @@ name = "podman_host_gateway" path = "tests/podman_host_gateway.rs" required-features = ["e2e-podman"] -[[test]] -name = "podman_preflight" -path = "tests/podman_preflight.rs" -required-features = ["e2e-podman"] - [[test]] name = "podman_corporate_proxy" path = "tests/podman_corporate_proxy.rs" diff --git a/e2e/rust/tests/podman_preflight.rs b/e2e/rust/tests/podman_preflight.rs deleted file mode 100644 index afb3bc1c38..0000000000 --- a/e2e/rust/tests/podman_preflight.rs +++ /dev/null @@ -1,116 +0,0 @@ -// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -// SPDX-License-Identifier: Apache-2.0 - -#![cfg(feature = "e2e-podman")] - -//! Podman driver daemon-unavailable e2e tests. -//! -//! 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. -//! -//! The tests 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. - -use std::path::{Path, PathBuf}; -use std::time::{Duration, Instant}; - -use openshell_e2e::harness::output::strip_ansi; - -/// Locate the workspace root by walking up from this crate's manifest directory. -fn workspace_root() -> PathBuf { - Path::new(env!("CARGO_MANIFEST_DIR")) - .ancestors() - .nth(2) - .expect("failed to resolve workspace root from CARGO_MANIFEST_DIR") - .to_path_buf() -} - -/// Return the path to the `openshell-driver-podman` binary. -/// -/// Uses `OPENSHELL_EXTERNAL_DRIVER_BIN` when set (the same env var the shell -/// e2e harness uses for prebuilt standalone driver artifacts), otherwise -/// expects the binary at `/target/debug/openshell-driver-podman`. -fn driver_podman_bin() -> PathBuf { - let bin = std::env::var_os("OPENSHELL_EXTERNAL_DRIVER_BIN").map_or_else( - || workspace_root().join("target/debug/openshell-driver-podman"), - PathBuf::from, - ); - assert!( - bin.is_file(), - "openshell-driver-podman binary not found at {} — set OPENSHELL_EXTERNAL_DRIVER_BIN \ - or run `cargo build -p openshell-driver-podman` first", - bin.display() - ); - bin -} - -/// 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) { - let tmpdir = tempfile::tempdir().expect("create isolated socket dir"); - let missing_socket = tmpdir.path().join("openshell-e2e-nonexistent-podman.sock"); - - let start = Instant::now(); - let mut cmd = tokio::process::Command::new(driver_podman_bin()); - cmd.arg("--podman-socket") - .arg(&missing_socket) - .kill_on_drop(true) - .stdout(std::process::Stdio::piped()) - .stderr(std::process::Stdio::piped()); - - let output = tokio::time::timeout(Duration::from_secs(60), cmd.output()) - .await - .expect("openshell-driver-podman should exit instead of hanging") - .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() - ); -} - -/// 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 clean = strip_ansi(&output); - - assert!( - clean.contains("connection error"), - "driver error should describe a connection failure:\n{clean}" - ); - assert!( - clean.contains(missing_socket.to_str().expect("socket path is utf-8")), - "driver error should name the unreachable socket path {}:\n{clean}", - missing_socket.display() - ); -} diff --git a/tests/artifacts.nix b/tests/artifacts.nix index c3017a5da5..c86d8f6d30 100644 --- a/tests/artifacts.nix +++ b/tests/artifacts.nix @@ -116,10 +116,6 @@ let "podman_corporate_proxy" "podman_gateway_start" "podman_oci_identity" - # This validates the standalone driver binary's daemon-unavailable path and - # belongs in the Podman driver crate's integration tests. The E2E archive - # does not contain `openshell-driver-podman`. - "podman_preflight" "provider_auto_create" # The provider-refresh feature suite covers revoked Keycloak grants. This # binary instead covers stable workload handles across repeated rotations