fix(zone-exec): catch panic errors and show all errors immediately

This commit is contained in:
Alex Zenla 2024-08-24 22:39:59 -07:00
parent 694de5d1fd
commit d9d016700f
No known key found for this signature in database
GPG Key ID: 067B238899B51269
3 changed files with 40 additions and 34 deletions

View File

@ -1,4 +1,4 @@
use anyhow::{anyhow, Result}; use anyhow::Result;
use async_stream::stream; use async_stream::stream;
use crossterm::{ use crossterm::{
terminal::{disable_raw_mode, enable_raw_mode, is_raw_mode_enabled}, terminal::{disable_raw_mode, enable_raw_mode, is_raw_mode_enabled},
@ -115,7 +115,12 @@ impl StdioConsoleStream {
return if reply.error.is_empty() { return if reply.error.is_empty() {
Ok(reply.exit_code) Ok(reply.exit_code)
} else { } else {
Err(anyhow!("exec failed: {}", reply.error)) StdioConsoleStream::restore_terminal_mode();
stderr
.write_all(format!("Error: exec failed: {}\n", reply.error).as_bytes())
.await?;
stderr.flush().await?;
Ok(-1)
}; };
} }
} }

View File

@ -27,7 +27,7 @@ use tokio::{
select, select,
sync::{ sync::{
mpsc::{channel, Receiver, Sender}, mpsc::{channel, Receiver, Sender},
Mutex, RwLock, RwLock,
}, },
task::JoinHandle, task::JoinHandle,
time::sleep, time::sleep,
@ -45,16 +45,9 @@ enum ZoneReconcilerResult {
} }
struct ZoneReconcilerEntry { struct ZoneReconcilerEntry {
task: JoinHandle<()>,
sender: Sender<()>, sender: Sender<()>,
} }
impl Drop for ZoneReconcilerEntry {
fn drop(&mut self) {
self.task.abort();
}
}
#[derive(Clone)] #[derive(Clone)]
pub struct ZoneReconciler { pub struct ZoneReconciler {
devices: DaemonDeviceManager, devices: DaemonDeviceManager,
@ -66,7 +59,7 @@ pub struct ZoneReconciler {
kernel_path: PathBuf, kernel_path: PathBuf,
initrd_path: PathBuf, initrd_path: PathBuf,
addons_path: PathBuf, addons_path: PathBuf,
tasks: Arc<Mutex<HashMap<Uuid, ZoneReconcilerEntry>>>, tasks: Arc<RwLock<HashMap<Uuid, ZoneReconcilerEntry>>>,
zone_reconciler_notify: Sender<Uuid>, zone_reconciler_notify: Sender<Uuid>,
zone_reconcile_lock: Arc<RwLock<()>>, zone_reconcile_lock: Arc<RwLock<()>>,
ip_assignment: IpAssignment, ip_assignment: IpAssignment,
@ -99,7 +92,7 @@ impl ZoneReconciler {
kernel_path, kernel_path,
initrd_path, initrd_path,
addons_path: modules_path, addons_path: modules_path,
tasks: Arc::new(Mutex::new(HashMap::new())), tasks: Arc::new(RwLock::new(HashMap::new())),
zone_reconciler_notify, zone_reconciler_notify,
zone_reconcile_lock: Arc::new(RwLock::with_max_readers((), PARALLEL_LIMIT)), zone_reconcile_lock: Arc::new(RwLock::with_max_readers((), PARALLEL_LIMIT)),
ip_assignment, ip_assignment,
@ -125,7 +118,7 @@ impl ZoneReconciler {
error!("failed to start zone reconciler task {}: {}", uuid, error); error!("failed to start zone reconciler task {}: {}", uuid, error);
} }
let map = self.tasks.lock().await; let map = self.tasks.read().await;
if let Some(entry) = map.get(&uuid) { if let Some(entry) = map.get(&uuid) {
if let Err(error) = entry.sender.send(()).await { if let Err(error) = entry.sender.send(()).await {
error!("failed to notify zone reconciler task {}: {}", uuid, error); error!("failed to notify zone reconciler task {}: {}", uuid, error);
@ -271,7 +264,7 @@ impl ZoneReconciler {
if destroyed { if destroyed {
self.zones.remove(uuid).await?; self.zones.remove(uuid).await?;
let mut map = self.tasks.lock().await; let mut map = self.tasks.write().await;
map.remove(&uuid); map.remove(&uuid);
} else { } else {
self.zones.update(uuid, zone.clone()).await?; self.zones.update(uuid, zone.clone()).await?;
@ -337,7 +330,7 @@ impl ZoneReconciler {
} }
async fn launch_task_if_needed(&self, uuid: Uuid) -> Result<()> { async fn launch_task_if_needed(&self, uuid: Uuid) -> Result<()> {
let mut map = self.tasks.lock().await; let mut map = self.tasks.write().await;
match map.entry(uuid) { match map.entry(uuid) {
Entry::Occupied(_) => {} Entry::Occupied(_) => {}
Entry::Vacant(entry) => { Entry::Vacant(entry) => {
@ -350,7 +343,7 @@ impl ZoneReconciler {
async fn launch_task(&self, uuid: Uuid) -> Result<ZoneReconcilerEntry> { async fn launch_task(&self, uuid: Uuid) -> Result<ZoneReconcilerEntry> {
let this = self.clone(); let this = self.clone();
let (sender, mut receiver) = channel(10); let (sender, mut receiver) = channel(10);
let task = tokio::task::spawn(async move { tokio::task::spawn(async move {
'notify_loop: loop { 'notify_loop: loop {
if receiver.recv().await.is_none() { if receiver.recv().await.is_none() {
break 'notify_loop; break 'notify_loop;
@ -372,7 +365,7 @@ impl ZoneReconciler {
} }
} }
}); });
Ok(ZoneReconcilerEntry { task, sender }) Ok(ZoneReconcilerEntry { sender })
} }
} }

View File

@ -70,16 +70,21 @@ impl ZoneExecTask {
if start.tty { if start.tty {
let pty = Pty::new().map_err(|error| anyhow!("unable to allocate pty: {}", error))?; let pty = Pty::new().map_err(|error| anyhow!("unable to allocate pty: {}", error))?;
pty.resize(Size::new(24, 80))?; pty.resize(Size::new(24, 80))?;
let mut child = ChildDropGuard { let pts = pty
inner: pty_process::Command::new(exe) .pts()
.map_err(|error| anyhow!("unable to allocate pts: {}", error))?;
let child = std::panic::catch_unwind(move || {
let pts = pts;
pty_process::Command::new(exe)
.args(cmd) .args(cmd)
.envs(env) .envs(env)
.current_dir(dir) .current_dir(dir)
.spawn( .spawn(&pts)
&pty.pts() })
.map_err(|error| anyhow!("unable to allocate pts: {}", error))?, .map_err(|_| anyhow!("internal error"))
) .map_err(|error| anyhow!("failed to spawn: {}", error))??;
.map_err(|error| anyhow!("failed to spawn: {}", error))?, let mut child = ChildDropGuard {
inner: child,
kill: true, kill: true,
}; };
let pid = child let pid = child
@ -148,16 +153,19 @@ impl ZoneExecTask {
let _ = join!(pty_read_task); let _ = join!(pty_read_task);
stdin_task.abort(); stdin_task.abort();
} else { } else {
let mut child = Command::new(exe) let mut child = std::panic::catch_unwind(|| {
.args(cmd) Command::new(exe)
.envs(env) .args(cmd)
.current_dir(dir) .envs(env)
.stdin(Stdio::piped()) .current_dir(dir)
.stdout(Stdio::piped()) .stdin(Stdio::piped())
.stderr(Stdio::piped()) .stdout(Stdio::piped())
.kill_on_drop(true) .stderr(Stdio::piped())
.spawn() .kill_on_drop(true)
.map_err(|error| anyhow!("failed to spawn: {}", error))?; .spawn()
})
.map_err(|_| anyhow!("internal error"))
.map_err(|error| anyhow!("failed to spawn: {}", error))??;
let pid = child.id().ok_or_else(|| anyhow!("pid is not provided"))?; let pid = child.id().ok_or_else(|| anyhow!("pid is not provided"))?;
let mut stdin = child let mut stdin = child