From d44269b12643b5f9905a284a898c07ece9971c23 Mon Sep 17 00:00:00 2001 From: politerealism Date: Mon, 28 Sep 2026 13:50:13 -0400 Subject: [PATCH 1/3] test(podman): move podman_preflight into driver-podman integration tests podman_preflight verifies that openshell-driver-podman fails fast when its Podman socket is unreachable. It only needs the standalone driver binary, not a gateway, so it never fit the gateway-backed e2e-podman harness it lived under and never ran anywhere in CI. Move it into crates/openshell-driver-podman/tests/ as a plain Cargo integration test. It now runs via the existing required workspace test job with no special mise task, workflow step, or coverage exception. Signed-off-by: politerealism --- Cargo.lock | 1 + crates/openshell-driver-podman/Cargo.toml | 1 + .../tests/podman_preflight.rs | 70 +++++++++---------- e2e/rust/Cargo.toml | 5 -- tests/artifacts.nix | 4 -- 5 files changed, 37 insertions(+), 44 deletions(-) rename {e2e/rust => crates/openshell-driver-podman}/tests/podman_preflight.rs (64%) 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/e2e/rust/tests/podman_preflight.rs b/crates/openshell-driver-podman/tests/podman_preflight.rs similarity index 64% rename from e2e/rust/tests/podman_preflight.rs rename to crates/openshell-driver-podman/tests/podman_preflight.rs index afb3bc1c38..37ea37dfdc 100644 --- a/e2e/rust/tests/podman_preflight.rs +++ b/crates/openshell-driver-podman/tests/podman_preflight.rs @@ -1,49 +1,47 @@ // 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. +//! Podman driver daemon-unavailable integration 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 +//! 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. +//! 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. -use std::path::{Path, PathBuf}; +use std::path::PathBuf; use std::time::{Duration, Instant}; -use openshell_e2e::harness::output::strip_ansi; +/// Strip ANSI escape codes (e.g. colors) from a string, for readable failure +/// messages. Not required for the assertions below to pass — the driver's +/// own tracing output carries ANSI codes even when captured non-interactively, +/// but they never fragment the substrings these tests check for — this is +/// purely so a failed assertion's `{clean}` output is readable. +fn strip_ansi(s: &str) -> String { + let mut out = String::with_capacity(s.len()); + let mut chars = s.chars().peekable(); -/// 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() -} + while let Some(c) = chars.next() { + if c == '\x1b' { + if chars.peek() == Some(&'[') { + chars.next(); + for c in chars.by_ref() { + if c.is_ascii_alphabetic() { + break; + } + } + } + } else { + out.push(c); + } + } -/// 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 + out } /// Run `openshell-driver-podman` pointed at a Podman socket that does not @@ -53,17 +51,19 @@ fn driver_podman_bin() -> PathBuf { /// 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 missing_socket = tmpdir + .path() + .join("openshell-driver-podman-nonexistent.sock"); let start = Instant::now(); - let mut cmd = tokio::process::Command::new(driver_podman_bin()); + let mut cmd = tokio::process::Command::new(env!("CARGO_BIN_EXE_openshell-driver-podman")); 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()) + let output = tokio::time::timeout(Duration::from_mins(1), cmd.output()) .await .expect("openshell-driver-podman should exit instead of hanging") .expect("spawn openshell-driver-podman"); 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/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 From b9c4f2cf742f74a9f940be7562e8ff9d053e52ac Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Mon, 28 Sep 2026 21:15:30 +0200 Subject: [PATCH 2/3] test(podman): make preflight diagnostics portable Signed-off-by: Evan Lezar --- .../tests/podman_preflight.rs | 43 ++++--------------- 1 file changed, 8 insertions(+), 35 deletions(-) diff --git a/crates/openshell-driver-podman/tests/podman_preflight.rs b/crates/openshell-driver-podman/tests/podman_preflight.rs index 37ea37dfdc..439cb95d98 100644 --- a/crates/openshell-driver-podman/tests/podman_preflight.rs +++ b/crates/openshell-driver-podman/tests/podman_preflight.rs @@ -17,33 +17,6 @@ use std::path::PathBuf; use std::time::{Duration, Instant}; -/// Strip ANSI escape codes (e.g. colors) from a string, for readable failure -/// messages. Not required for the assertions below to pass — the driver's -/// own tracing output carries ANSI codes even when captured non-interactively, -/// but they never fragment the substrings these tests check for — this is -/// purely so a failed assertion's `{clean}` output is readable. -fn strip_ansi(s: &str) -> String { - let mut out = String::with_capacity(s.len()); - let mut chars = s.chars().peekable(); - - while let Some(c) = chars.next() { - if c == '\x1b' { - if chars.peek() == Some(&'[') { - chars.next(); - for c in chars.by_ref() { - if c.is_ascii_alphabetic() { - break; - } - } - } - } else { - out.push(c); - } - } - - out -} - /// Run `openshell-driver-podman` pointed at a Podman socket that does not /// exist, and wait for it to exit. /// @@ -51,14 +24,15 @@ fn strip_ansi(s: &str) -> String { /// 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-driver-podman-nonexistent.sock"); + // 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()); @@ -102,15 +76,14 @@ 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}" + output.contains("connection error"), + "driver error should describe a connection failure:\n{output}" ); assert!( - clean.contains(missing_socket.to_str().expect("socket path is utf-8")), - "driver error should name the unreachable socket path {}:\n{clean}", + output.contains(missing_socket.to_str().expect("socket path is utf-8")), + "driver error should name the unreachable socket path {}:\n{output}", missing_socket.display() ); } From 43d57e023f2f351c6eff335f0bee999bf3fdb292 Mon Sep 17 00:00:00 2001 From: Evan Lezar Date: Mon, 28 Sep 2026 21:18:09 +0200 Subject: [PATCH 3/3] test(podman): cover retry policy deterministically Signed-off-by: Evan Lezar --- crates/openshell-driver-podman/src/driver.rs | 107 ++++++++++++++---- .../tests/podman_preflight.rs | 80 ++++--------- 2 files changed, 107 insertions(+), 80 deletions(-) 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 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() ); }