From ffc8f7dd111c0f90efd585d793caf265269e8239 Mon Sep 17 00:00:00 2001 From: Vineeth Sai Narajala Date: Mon, 2 Nov 2026 17:15:39 +0000 Subject: [PATCH 1/2] fix(gateway): keep VM driver alive during Ctrl+C shutdown A terminal sends SIGINT to its foreground process group. The VM driver and its launchers shared that group, so they exited before the gateway's shutdown sweep could stop running sandboxes. Give the driver its own group and relay its stdout and stderr through the foreground gateway. The pipes keep driver logs visible without suspending the background group in TOSTOP terminals. Add unit coverage for process-group separation and piped output. Signed-off-by: Vineeth Sai Narajala --- crates/openshell-gateway/src/vm.rs | 101 ++++++++++++++++++++++++++--- 1 file changed, 92 insertions(+), 9 deletions(-) diff --git a/crates/openshell-gateway/src/vm.rs b/crates/openshell-gateway/src/vm.rs index 2f9ea5cc8e..5b76a55c9c 100644 --- a/crates/openshell-gateway/src/vm.rs +++ b/crates/openshell-gateway/src/vm.rs @@ -539,11 +539,7 @@ pub async fn spawn( let guest_tls_paths = compute_driver_guest_tls_paths(vm_config)?; prepare_compute_driver_socket_path(vm_config, &socket_path)?; - let mut command = Command::new(&driver_bin); - command.kill_on_drop(true); - command.stdin(Stdio::null()); - command.stdout(Stdio::inherit()); - command.stderr(Stdio::inherit()); + let mut command = compute_driver_command(&driver_bin); command.arg("--bind-socket").arg(&socket_path); command.arg("--admission-config-json").arg( serde_json::to_string(&openshell_core::resource_admission::DriverAdmissionConfig { @@ -592,6 +588,7 @@ pub async fn spawn( driver_bin.display() )) })?; + forward_compute_driver_output(&mut child); let channel = wait_for_compute_driver(&socket_path, &mut child).await?; let process = Arc::new(ManagedDriverProcess::new(child, socket_path)); Ok(AcquiredRemoteDriverEndpoint::managed( @@ -599,6 +596,47 @@ pub async fn spawn( )) } +/// Base command for the VM compute-driver subprocess. +/// +/// The driver gets its own process group. Ctrl+C on a foreground gateway +/// sends SIGINT to the terminal's foreground process group; if the driver +/// shared the gateway's group it would exit on that signal before the +/// gateway's shutdown sweep could stop its sandboxes. In its own group the +/// driver keeps serving until the gateway stops it through +/// `ManagedDriverProcess::shutdown` after the sweep. A gateway that dies +/// without that shutdown still takes the driver with it: the driver arms a +/// parent-death signal at startup, which does not depend on the group. Pipe +/// stdout and stderr so the gateway can write the driver's logs to its own +/// terminal; the driver's background group must not write there when TOSTOP +/// is enabled. +#[cfg(unix)] +fn compute_driver_command(driver_bin: &Path) -> Command { + let mut command = Command::new(driver_bin); + command.kill_on_drop(true); + command.process_group(0); + command.stdin(Stdio::null()); + command.stdout(Stdio::piped()); + command.stderr(Stdio::piped()); + command +} + +#[cfg(unix)] +fn forward_compute_driver_output(child: &mut tokio::process::Child) { + let mut child_stdout = child.stdout.take().expect("vm driver stdout is piped"); + tokio::spawn(async move { + if let Err(error) = tokio::io::copy(&mut child_stdout, &mut tokio::io::stdout()).await { + tracing::warn!(%error, "failed to forward vm compute driver stdout"); + } + }); + + let mut child_stderr = child.stderr.take().expect("vm driver stderr is piped"); + tokio::spawn(async move { + if let Err(error) = tokio::io::copy(&mut child_stderr, &mut tokio::io::stderr()).await { + tracing::warn!(%error, "failed to forward vm compute driver stderr"); + } + }); +} + fn validate_vm_sandbox_identity(config: &VmComputeConfig) -> Result<()> { let range = openshell_policy::MIN_SANDBOX_UID..=openshell_policy::MAX_SANDBOX_UID; for (field, value) in [ @@ -797,10 +835,10 @@ async fn connect_compute_driver(socket_path: &Path) -> Result { mod tests { use super::{ VmComputeConfig, append_otlp_args, append_vm_identity_args, append_vm_network_trust_args, - append_vm_proxy_and_spiffe_args, append_vm_rootfs_tar_args, compute_driver_guest_tls_paths, - compute_driver_socket_path, current_euid, prepare_compute_driver_socket_path, - prepare_vm_state_dir, resolve_compute_driver_bin, resolve_driver_search_dirs, - validate_vm_sandbox_identity, + append_vm_proxy_and_spiffe_args, append_vm_rootfs_tar_args, compute_driver_command, + compute_driver_guest_tls_paths, compute_driver_socket_path, current_euid, + prepare_compute_driver_socket_path, prepare_vm_state_dir, resolve_compute_driver_bin, + resolve_driver_search_dirs, validate_vm_sandbox_identity, }; use openshell_core::{NetworkSupervisorTrustBundle, UpstreamProxyConfig}; use openshell_server::config_file::{OcsfOtlpConfig, OtlpConfig, OtlpTracesConfig}; @@ -845,6 +883,51 @@ mod tests { .expect("an explicit bootstrap image is valid"); } + #[tokio::test] + async fn vm_driver_command_runs_driver_in_its_own_process_group() { + use rustix::process::{Pid, getpgid}; + + let mut child = compute_driver_command(std::path::Path::new("sleep")) + .arg("30") + .spawn() + .expect("spawn stand-in driver"); + let pid = Pid::from_raw(i32::try_from(child.id().expect("child pid")).unwrap()) + .expect("child pid is non-zero"); + let child_group = getpgid(Some(pid)).expect("read child process group"); + let gateway_group = getpgid(None).expect("read own process group"); + child.kill().await.expect("stop stand-in driver"); + + assert_eq!( + child_group, pid, + "the driver must lead its own process group" + ); + assert_ne!( + child_group, gateway_group, + "terminal SIGINT to the gateway's group must not reach the driver" + ); + } + + #[tokio::test] + async fn vm_driver_command_pipes_both_output_streams() { + let child = compute_driver_command(std::path::Path::new("sh")) + .arg("-c") + .arg("printf 'driver stdout'; printf 'driver stderr' >&2") + .spawn() + .expect("run stand-in driver"); + + assert!(child.stdout.is_some(), "driver stdout must be piped"); + assert!(child.stderr.is_some(), "driver stderr must be piped"); + + let output = child + .wait_with_output() + .await + .expect("collect driver output"); + + assert!(output.status.success()); + assert_eq!(output.stdout, b"driver stdout"); + assert_eq!(output.stderr, b"driver stderr"); + } + #[test] fn vm_driver_command_includes_gateway_otlp_configuration() { let mut command = tokio::process::Command::new("openshell-driver-vm"); From afb89df65b24dfaca8074132d292869d99155cd0 Mon Sep 17 00:00:00 2001 From: Vineeth Sai Narajala Date: Mon, 2 Nov 2026 18:12:33 +0000 Subject: [PATCH 2/2] fix(vm): reject managed driver after gateway exit Signed-off-by: Vineeth Sai Narajala --- crates/openshell-driver-vm/src/main.rs | 59 +++++++++++++++---- .../openshell-driver-vm/tests/parent_death.rs | 56 ++++++++++++++++++ 2 files changed, 104 insertions(+), 11 deletions(-) create mode 100644 crates/openshell-driver-vm/tests/parent_death.rs diff --git a/crates/openshell-driver-vm/src/main.rs b/crates/openshell-driver-vm/src/main.rs index 0ab4743771..aac009bc59 100644 --- a/crates/openshell-driver-vm/src/main.rs +++ b/crates/openshell-driver-vm/src/main.rs @@ -322,17 +322,17 @@ async fn main() -> Result<()> { let listen_mode = compute_driver_listen_mode(&args).map_err(|err| miette::miette!("{err}"))?; - // Arm procguard so that if the gateway is killed (SIGKILL or crash) - // we also die. Without this the driver is reparented to init and - // keeps its per-sandbox VM launchers alive forever. Launchers have - // their own procguards (armed in `run_vm`) which cascade cleanup of - // the libkrun worker the moment this driver exits. - if let Err(err) = procguard::die_with_parent() { - tracing::warn!( - error = %err, - "procguard arm failed; gateway crashes may orphan this driver" - ); - } + // The managed driver must not serve after its gateway exits. Its own + // process group keeps terminal SIGINT away during the gateway shutdown + // sweep, so a failed parent-death guard must stop startup instead of + // leaving the driver and its VM launchers behind. + let expected_gateway_pid = match &listen_mode { + ComputeDriverListenMode::Unix { + expected_peer_pid, .. + } => *expected_peer_pid, + ComputeDriverListenMode::Tcp(_) => None, + }; + arm_driver_parent_guard(expected_gateway_pid)?; let driver = VmDriver::new(VmDriverConfig { allow_driver_config: args.admission_config_json.allow_driver_config, @@ -424,6 +424,43 @@ async fn main() -> Result<()> { result } +fn arm_driver_parent_guard(expected_gateway_pid: Option) -> Result<()> { + if let Some(expected) = expected_gateway_pid { + ensure_gateway_parent(expected)?; + } + + // Launchers arm their own procguards in `run_vm`, which cascade cleanup + // to libkrun workers when this driver exits. + match procguard::die_with_parent() { + Ok(()) => {} + Err(err) if expected_gateway_pid.is_some() => { + return Err(miette::miette!( + "failed to arm managed VM driver parent-death guard: {err}" + )); + } + Err(err) => { + tracing::warn!(error = %err, "procguard arm failed; driver may outlive its parent"); + } + } + + // A subreaper can adopt an orphan before procguard reads getppid(). In + // that case it could arm against the subreaper instead of the gateway. + if let Some(expected) = expected_gateway_pid { + ensure_gateway_parent(expected)?; + } + Ok(()) +} + +fn ensure_gateway_parent(expected: u32) -> Result<()> { + let actual = nix::unistd::getppid().as_raw(); + if i64::from(actual) != i64::from(expected) { + return Err(miette::miette!( + "expected gateway parent PID {expected}, found {actual}" + )); + } + Ok(()) +} + async fn shutdown_signal() { let mut terminate = tokio::signal::unix::signal(tokio::signal::unix::SignalKind::terminate()) .expect("install SIGTERM handler"); diff --git a/crates/openshell-driver-vm/tests/parent_death.rs b/crates/openshell-driver-vm/tests/parent_death.rs new file mode 100644 index 0000000000..62ea6d1e81 --- /dev/null +++ b/crates/openshell-driver-vm/tests/parent_death.rs @@ -0,0 +1,56 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +use std::io::Read; +use std::process::{Command, Stdio}; +use std::time::{Duration, Instant}; + +#[test] +fn managed_driver_rejects_a_missing_gateway_parent() { + let state = tempfile::tempdir().expect("create driver state directory"); + let socket = state.path().join("driver.sock"); + let stale_gateway_pid = std::process::id().wrapping_add(1); + + let mut child = Command::new(env!("CARGO_BIN_EXE_openshell-driver-vm")) + .arg("--bind-socket") + .arg(&socket) + .arg("--expected-peer-pid") + .arg(stale_gateway_pid.to_string()) + .arg("--grpc-endpoint") + .arg("http://127.0.0.1:9") + .arg("--bootstrap-image") + .arg("example.invalid/bootstrap:latest") + .arg("--state-dir") + .arg(state.path()) + .stdout(Stdio::null()) + .stderr(Stdio::piped()) + .spawn() + .expect("start managed driver"); + + let deadline = Instant::now() + Duration::from_secs(10); + let status = loop { + if let Some(status) = child.try_wait().expect("poll driver") { + break status; + } + if Instant::now() >= deadline { + child.kill().expect("stop driver after timeout"); + child.wait().expect("reap driver after timeout"); + panic!("driver kept running after its expected gateway parent was absent"); + } + std::thread::sleep(Duration::from_millis(20)); + }; + + let mut stderr = String::new(); + child + .stderr + .take() + .expect("driver stderr") + .read_to_string(&mut stderr) + .expect("read driver stderr"); + assert!(!status.success(), "driver unexpectedly succeeded: {stderr}"); + assert!( + stderr.contains("expected gateway parent PID"), + "driver did not reject the stale gateway parent: {stderr}" + ); + assert!(!socket.exists(), "driver created a socket after rejection"); +}