From 875cee455f2469d1e32e332fbfe48057d9b07196 Mon Sep 17 00:00:00 2001 From: stevenhorsman Date: Wed, 1 Jul 2026 07:59:48 -0700 Subject: [PATCH] =?UTF-8?q?deps:=20bump=20subprocess=200.2.8=20=E2=86=92?= =?UTF-8?q?=201.2.0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Migrate hooks.rs from the removed Popen/PopenConfig/CommunicateError API to the subprocess 1.x Exec/Job builder API: - Replace Popen::create() + PopenConfig with Exec builder + Job - communicate_start().read_string() → job.communicate()?.limit_time().read_string() - read_string() now returns io::Result<(String,String)> (no Option wrapper) - ExitStatus enum variants replaced with ExitStatus::code() / signal() methods - Timeout errors propagated with io::ErrorKind::TimedOut as before Generated-by: IBM Bob Signed-off-by: stevenhorsman --- Cargo.lock | 4 +- src/libs/kata-sys-util/Cargo.toml | 2 +- src/libs/kata-sys-util/src/hooks.rs | 251 +++++++++++++--------------- 3 files changed, 117 insertions(+), 140 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 03d9cf4187..ee71c64b9a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7650,9 +7650,9 @@ dependencies = [ [[package]] name = "subprocess" -version = "0.2.15" +version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2c56e8662b206b9892d7a5a3f2ecdbcb455d3d6b259111373b7e08b8055158a8" +checksum = "feb179a3921e9debb9b1479c5a0abf0df75d05a412d8af9c81409c088d449413" dependencies = [ "libc", "winapi", diff --git a/src/libs/kata-sys-util/Cargo.toml b/src/libs/kata-sys-util/Cargo.toml index 3538cb3e6a..43c912ad15 100644 --- a/src/libs/kata-sys-util/Cargo.toml +++ b/src/libs/kata-sys-util/Cargo.toml @@ -21,7 +21,7 @@ serde = { version = "1.0.138", features = ["derive"] } serde_json = "1.0.73" slog = "2.5.2" slog-scope = "4.4.0" -subprocess = "0.2.8" +subprocess = "1.2.0" rand = { version = "0.10.1", features = ["std", "std_rng", "thread_rng"] } thiserror = { workspace = true } hex = "0.4.3" diff --git a/src/libs/kata-sys-util/src/hooks.rs b/src/libs/kata-sys-util/src/hooks.rs index cc0e3b6aa7..fd1a65e301 100644 --- a/src/libs/kata-sys-util/src/hooks.rs +++ b/src/libs/kata-sys-util/src/hooks.rs @@ -7,11 +7,11 @@ use std::collections::HashMap; use std::ffi::OsString; use std::hash::{Hash, Hasher}; -use std::io::{self, Read, Result}; +use std::io::{self, Result}; use std::time::Duration; use oci_spec::runtime as oci; -use subprocess::{ExitStatus, Popen, PopenConfig, PopenError, Redirection}; +use subprocess::{Exec, ExitStatus, Job, Redirection}; use crate::sl; use crate::validate::valid_env; @@ -130,34 +130,37 @@ impl HookStates { self.states.insert(hook.into(), HookState::Pending); - let mut executor = HookExecutor::new(hook)?; - let stdin = if state.is_some() { - Redirection::Pipe - } else { - Redirection::None - }; - let mut popen = Popen::create( - &executor.args, - PopenConfig { - stdin, - stdout: Redirection::Pipe, - stderr: Redirection::Pipe, - executable: executor.executable.to_owned(), - detached: true, - env: Some(executor.envs.clone()), - ..Default::default() - }, - ) - .map_err(|e| { - std::io::Error::other(format!( - "failed to create subprocess for hook {hook:?}: {e}" - )) - })?; + let executor = HookExecutor::new(hook)?; + let mut job = executor.spawn(state.as_ref())?; - if let Some(state) = state { - executor.execute_with_input(&mut popen, state)?; + let communicate_result = job + .communicate()? + .limit_time(Duration::from_secs(executor.timeout)) + .read_string(); + + match communicate_result { + Err(ref e) if e.kind() == io::ErrorKind::TimedOut => { + error!(sl!(), "hook poll failed, kill it"); + let _ = job.kill(); + let _ = job.wait(); + return Err(io::Error::from(io::ErrorKind::TimedOut)); + } + Err(e) => { + return Err(std::io::Error::other(format!( + "communicate hook {hook:?}: {e}" + ))); + } + Ok((ref stdout, ref stderr)) => { + if !stderr.is_empty() { + error!(sl!(), "hook {} stderr: {}", hook.path().display(), stderr); + } + if !stdout.is_empty() { + info!(sl!(), "hook {} stdout: {}", hook.path().display(), stdout); + } + } } - executor.execute_and_wait(&mut popen)?; + + executor.wait_and_check(&mut job)?; info!(sl!(), "hook {} finished", hook.path().display()); self.states.insert(hook.into(), HookState::Done); @@ -191,8 +194,8 @@ impl HookStates { struct HookExecutor<'a> { hook: &'a oci::Hook, - executable: Option, - args: Vec, + executable: OsString, + args: Vec, envs: Vec<(OsString, OsString)>, timeout: u64, } @@ -200,24 +203,23 @@ struct HookExecutor<'a> { impl<'a> HookExecutor<'a> { fn new(hook: &'a oci::Hook) -> Result { // Ensure Hook.path is present and is an absolute path. - let executable = if !hook.path().exists() { + let path = hook.path(); + if !path.exists() { return Err(std::io::Error::other(format!( "path of hook {hook:?} is empty" ))); - } else { - let path = hook.path(); - if !path.is_absolute() { - return Err(std::io::Error::other(format!( - "path of hook {hook:?} is not absolute" - ))); - } - Some(path.as_os_str().to_os_string()) - }; + } + if !path.is_absolute() { + return Err(std::io::Error::other(format!( + "path of hook {hook:?} is not absolute" + ))); + } + let executable = path.as_os_str().to_os_string(); // Hook.args is optional, use Hook.path as arg0 if Hook.args is empty. - let args = match hook.args() { - Some(args) => args.clone(), - None => vec![hook.path().display().to_string()], + let args: Vec = match hook.args() { + Some(args) => args.iter().map(OsString::from).collect(), + None => vec![OsString::from(hook.path())], }; let mut envs: Vec<(OsString, OsString)> = Vec::new(); @@ -246,110 +248,85 @@ impl<'a> HookExecutor<'a> { }) } - fn execute_with_input(&mut self, popen: &mut Popen, state: runtime_spec::State) -> Result<()> { - let st = serde_json::to_string(&state)?; - let (stdout, stderr) = popen - .communicate_start(Some(st.as_bytes().to_vec())) - .limit_time(Duration::from_secs(self.timeout)) - .read_string() - .map_err(|e| e.error)?; - if let Some(err) = stderr { - if !err.is_empty() { - error!( - sl!(), - "hook {} exec failed: {}", - self.hook.path().display(), - err - ); - } + /// Spawn the subprocess, optionally feeding `state` to its stdin. + fn spawn(&self, state: Option<&runtime_spec::State>) -> Result { + let mut exec = Exec::cmd(&self.args[0]) + .args(&self.args[1..]) + .arg0(&self.executable) + .env_clear() + .env_extend( + self.envs + .iter() + .map(|(k, v)| (k.as_os_str(), v.as_os_str())), + ) + .detached() + .stdout(Redirection::Pipe) + .stderr(Redirection::Pipe); + + if let Some(st) = state { + let json = serde_json::to_string(st)?; + exec = exec.stdin(json.into_bytes()); + } else { + exec = exec.stdin(Redirection::None); } - if let Some(out) = stdout { - if !out.is_empty() { + + exec.start().map_err(|e| { + std::io::Error::other(format!( + "failed to create subprocess for hook {:?}: {e}", + self.hook + )) + }) + } + + /// Wait for the process to finish and check its exit status. + fn wait_and_check(&self, job: &mut Job) -> Result<()> { + match job.wait_timeout(Duration::from_secs(1)) { + Ok(Some(exit_status)) => { info!( sl!(), - "hook {} exec stdout: {}", - self.hook.path().display(), - out + "exit status of hook {:?} : {:?}", self.hook, exit_status ); + self.check_exit_status(exit_status) + } + Ok(None) => { + // Timeout — kill the process. + error!(sl!(), "hook poll failed, kill it"); + let _ = job.kill(); + let _ = job.wait(); + Err(io::Error::from(io::ErrorKind::TimedOut)) + } + Err(e) => { + error!(sl!(), "wait_timeout for hook {:?} failed: {}", self.hook, e); + Err(std::io::Error::other(format!( + "wait_timeout for hook {:?} failed: {}", + self.hook, e, + ))) } } - // Give a grace period for `execute_and_wait()`. - self.timeout = 1; - Ok(()) } - fn execute_and_wait(&mut self, popen: &mut Popen) -> Result<()> { - match popen.wait_timeout(Duration::from_secs(self.timeout)) { - Ok(v) => self.handle_exit_status(v, popen), - Err(e) => self.handle_popen_wait_error(e, popen), - } - } - - fn handle_exit_status(&mut self, result: Option, popen: &mut Popen) -> Result<()> { - if let Some(exit_status) = result { - // the process has finished - info!( - sl!(), - "exit status of hook {:?} : {:?}", self.hook, exit_status - ); - self.print_result(popen); - match exit_status { - subprocess::ExitStatus::Exited(code) => { - if code == 0 { - info!(sl!(), "hook {:?} succeeds", self.hook); - Ok(()) - } else { - warn!(sl!(), "hook {:?} exit status with {}", self.hook, code,); - Err(std::io::Error::other(format!( - "hook {:?} exit status with {}", - self.hook, code, - ))) - } - } - _ => { - error!( - sl!(), - "no exit code for hook {:?}: {:?}", self.hook, exit_status - ); - Err(std::io::Error::other(format!( - "no exit code for hook {:?}: {:?}", - self.hook, exit_status, - ))) - } + fn check_exit_status(&self, exit_status: ExitStatus) -> Result<()> { + match exit_status.code() { + Some(0) => { + info!(sl!(), "hook {:?} succeeds", self.hook); + Ok(()) } - } else { - // may be timeout - error!(sl!(), "hook poll failed, kill it"); - // it is still running, kill it - popen.kill()?; - let _ = popen.wait(); - self.print_result(popen); - Err(io::Error::from(io::ErrorKind::TimedOut)) - } - } - - fn handle_popen_wait_error(&mut self, e: PopenError, popen: &mut Popen) -> Result<()> { - self.print_result(popen); - error!(sl!(), "wait_timeout for hook {:?} failed: {}", self.hook, e); - Err(std::io::Error::other(format!( - "wait_timeout for hook {:?} failed: {}", - self.hook, e, - ))) - } - - fn print_result(&mut self, popen: &mut Popen) { - if let Some(file) = popen.stdout.as_mut() { - let mut buffer = String::new(); - file.read_to_string(&mut buffer).ok(); - if !buffer.is_empty() { - info!(sl!(), "hook stdout: {}", buffer); + Some(code) => { + warn!(sl!(), "hook {:?} exit status with {}", self.hook, code); + Err(std::io::Error::other(format!( + "hook {:?} exit status with {}", + self.hook, code, + ))) } - } - if let Some(file) = popen.stderr.as_mut() { - let mut buffer = String::new(); - file.read_to_string(&mut buffer).ok(); - if !buffer.is_empty() { - info!(sl!(), "hook stderr: {}", buffer); + None => { + error!( + sl!(), + "no exit code for hook {:?}: {:?}", self.hook, exit_status + ); + Err(std::io::Error::other(format!( + "no exit code for hook {:?}: {:?}", + self.hook, exit_status, + ))) } } }