From f5b4e383a62ad2bc9d1c82a8689ddc04293b0b86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Thu, 27 Nov 2025 15:42:22 +0000 Subject: [PATCH 1/9] Report progress and errors when running scripts * Run the scripts in background, so it does not block Agama when it takes time or it requires user intervention. --- rust/agama-files/src/lib.rs | 14 +- rust/agama-files/src/runner.rs | 179 ++++++++++++++++++++++ rust/agama-files/src/service.rs | 75 +++++---- rust/agama-manager/src/service.rs | 2 +- rust/agama-utils/src/api/files/scripts.rs | 21 ++- rust/agama-utils/src/api/scope.rs | 1 + 6 files changed, 256 insertions(+), 36 deletions(-) create mode 100644 rust/agama-files/src/runner.rs diff --git a/rust/agama-files/src/lib.rs b/rust/agama-files/src/lib.rs index 5ed26f8c6d..ba6101e00d 100644 --- a/rust/agama-files/src/lib.rs +++ b/rust/agama-files/src/lib.rs @@ -24,6 +24,8 @@ pub mod service; pub use service::{Service, Starter}; pub mod message; +mod runner; +pub use runner::ScriptsRunner; #[cfg(test)] mod tests { @@ -66,10 +68,14 @@ mod tests { let issues = issue::Service::starter(events_tx.clone()).start(); let progress = progress::Service::starter(events_tx.clone()).start(); let questions = question::start(events_tx.clone()).await.unwrap(); - let software = - start_software_service(events_tx.clone(), issues, progress.clone(), questions) - .await; - let handler = Service::starter(events_tx.clone(), progress, software) + let software = start_software_service( + events_tx.clone(), + issues, + progress.clone(), + questions.clone(), + ) + .await; + let handler = Service::starter(progress, questions, software) .with_scripts_workdir(tmp_dir.path()) .with_install_dir(tmp_dir.path()) .start() diff --git a/rust/agama-files/src/runner.rs b/rust/agama-files/src/runner.rs new file mode 100644 index 0000000000..0a7cc5bbed --- /dev/null +++ b/rust/agama-files/src/runner.rs @@ -0,0 +1,179 @@ +// Copyright (c) [2024-2025] SUSE LLC +// +// All Rights Reserved. +// +// This program is free software; you can redistribute it and/or modify it +// under the terms of the GNU General Public License as published by the Free +// Software Foundation; either version 2 of the License, or (at your option) +// any later version. +// +// This program is distributed in the hope that it will be useful, but WITHOUT +// ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +// FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +// more details. +// +// You should have received a copy of the GNU General Public License along +// with this program; if not, contact SUSE LLC. +// +// To contact SUSE LLC about this file by physical or electronic mail, you may +// find current contact information at www.suse.com. + +use std::{ + fs, + path::{Path, PathBuf}, + process::Output, +}; + +use agama_utils::{ + actor::Handler, + api::{files::Script, question::QuestionSpec, Scope}, + command::run_with_retry, + progress, + question::{self, ask_question}, +}; +use tokio::process; + +#[derive(thiserror::Error, Debug)] +pub enum Error { + #[error(transparent)] + Io(#[from] std::io::Error), + #[error("The script failed")] + Script(Output), +} + +/// Implements the logic to run a script. +/// +/// It takes care of running the script, reporting errors (and asking whether to retry) and write +/// the logs. +pub struct ScriptsRunner { + progress: Handler, + questions: Handler, + install_dir: PathBuf, + workdir: PathBuf, +} + +impl ScriptsRunner { + /// Creates a new runner. + /// + /// * `install_dir`: directory where the system is being installed. It is relevant for + /// chrooted scripts. + /// * `workdir`: scripts work directory. + /// * `progress`: handler to report the progress. + /// * `questions`: handler to interact with the user. + pub fn new>( + install_dir: P, + workdir: P, + progress: Handler, + questions: Handler, + ) -> Self { + Self { + progress, + questions, + install_dir: install_dir.as_ref().to_path_buf(), + workdir: workdir.as_ref().to_path_buf(), + } + } + + /// Runs the given scripts. + /// + /// It runs each script. If something goes wrong, it reports the problem to the user through + /// the questions mechanism. + /// + /// * `scripts`: scripts to run. + pub async fn run(&self, scripts: &[&Script]) -> Result<(), Error> { + self.start_progress(scripts); + + for script in scripts { + _ = self + .progress + .cast(progress::message::Next::new(Scope::Files)); + self.run_script(script).await; + } + + _ = self + .progress + .cast(progress::message::Finish::new(Scope::Files)); + Ok(()) + } + + /// Runs the script. + /// + /// If the script fails, it asks the user whether it should try again. + async fn run_script(&self, script: &Script) { + loop { + let path = self + .workdir + .join(script.group().to_string()) + .join(script.name()); + + let Err(error) = self.run_command(&path, script.chroot()).await else { + return; + }; + + if !self.should_retry(&script, error).await { + return; + } + } + } + + /// Asks the user whether it should try to run the script again. + async fn should_retry(&self, script: &Script, error: Error) -> bool { + let text = format!( + "Running the script '{}' failed. Do you want to try again?", + script.name() + ); + let mut question = QuestionSpec::new(&text, "scripts.retry").with_yes_no_actions(); + + if let Error::Script(output) = error { + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); + let code = output.status.to_string(); + question = + question.with_data(&[("stdout", &stdout), ("stderr", &stderr), ("code", &code)]); + } + + let answer = ask_question(&self.questions, question).await.unwrap(); + + return answer.action == "Yes"; + } + + /// Runs the script at the given path. + /// + /// * `path`: script's path. + /// * `chroot`: whether to run the script in a chroot. + async fn run_command>(&self, path: P, chroot: bool) -> Result<(), Error> { + let path = path.as_ref(); + let command = if chroot { + let mut command = process::Command::new("chroot"); + command.args([&self.install_dir, path]); + command + } else { + process::Command::new(path) + }; + + let output = run_with_retry(command) + .await + .inspect_err(|e| tracing::error!("Error executing the script: {e}"))?; + + fs::write(path.with_extension("log"), output.stdout.clone())?; + fs::write(path.with_extension("err"), output.stderr.clone())?; + fs::write(path.with_extension("out"), output.status.to_string())?; + + if !output.status.success() { + return Err(Error::Script(output)); + } + + Ok(()) + } + + /// Ancillary function to start the progress. + fn start_progress(&self, scripts: &[&Script]) { + let messages: Vec<_> = scripts + .iter() + .map(|s| format!("Running user script '{}'", s.name())) + .collect(); + let steps: Vec<_> = messages.iter().map(|s| s.as_ref()).collect(); + let progress_action = progress::message::StartWithSteps::new(Scope::Files, &steps); + _ = self.progress.cast(progress_action); + } +} diff --git a/rust/agama-files/src/service.rs b/rust/agama-files/src/service.rs index 3be665e2b1..a88294925c 100644 --- a/rust/agama-files/src/service.rs +++ b/rust/agama-files/src/service.rs @@ -18,23 +18,24 @@ // To contact SUSE LLC about this file by physical or electronic mail, you may // find current contact information at www.suse.com. -use std::path::{Path, PathBuf}; +use std::{ + path::{Path, PathBuf}, + sync::Arc, +}; use agama_software::{self as software, Resolvable, ResolvableType}; use agama_utils::{ actor::{self, Actor, Handler, MessageHandler}, - api::{ - event, - files::{ - scripts::{self, ScriptsRepository}, - user_file, ScriptsConfig, UserFile, - }, + api::files::{ + scripts::{self, ScriptsRepository}, + user_file, ScriptsConfig, UserFile, }, - progress, + progress, question, }; use async_trait::async_trait; +use tokio::sync::Mutex; -use crate::message; +use crate::{message, ScriptsRunner}; #[derive(thiserror::Error, Debug)] pub enum Error { @@ -55,11 +56,11 @@ const DEFAULT_INSTALL_DIR: &str = "/mnt"; /// /// This structs allows to build a files service. pub struct Starter { - progress: Handler, - events: event::Sender, - software: Handler, scripts_workdir: PathBuf, install_dir: PathBuf, + software: Handler, + progress: Handler, + questions: Handler, } impl Starter { @@ -67,14 +68,14 @@ impl Starter { /// /// * `events`: channel to emit the [localization-specific events](crate::Event). pub fn new( - events: event::Sender, progress: Handler, + questions: Handler, software: Handler, ) -> Self { Self { - events, - progress, software, + progress, + questions, scripts_workdir: PathBuf::from(DEFAULT_SCRIPTS_DIR), install_dir: PathBuf::from(DEFAULT_INSTALL_DIR), } @@ -85,9 +86,9 @@ impl Starter { let scripts = ScriptsRepository::new(self.scripts_workdir); let service = Service { progress: self.progress, - events: self.events, + questions: self.questions, software: self.software, - scripts, + scripts: Arc::new(Mutex::new(scripts)), files: vec![], install_dir: self.install_dir, }; @@ -107,46 +108,52 @@ impl Starter { } pub struct Service { - progress: Handler, - events: event::Sender, software: Handler, - scripts: ScriptsRepository, + progress: Handler, + questions: Handler, + scripts: Arc>, files: Vec, install_dir: PathBuf, } impl Service { pub fn starter( - events: event::Sender, progress: Handler, + questions: Handler, software: Handler, ) -> Starter { - Starter::new(events, progress, software) + Starter::new(progress, questions, software) + } + + pub async fn clear_scripts(&mut self) { + let mut repo = self.scripts.lock().await; + repo.clear(); } pub async fn add_scripts(&mut self, config: ScriptsConfig) -> Result<(), Error> { + let mut repo = self.scripts.lock().await; if let Some(scripts) = config.pre { for pre in scripts { - self.scripts.add(pre.into())?; + repo.add(pre.into())?; } } if let Some(scripts) = config.post_partitioning { for post in scripts { - self.scripts.add(post.into())?; + repo.add(post.into())?; } } if let Some(scripts) = config.post { for post in scripts { - self.scripts.add(post.into())?; + repo.add(post.into())?; } } let mut packages = vec![]; if let Some(scripts) = config.init { for init in scripts { - self.scripts.add(init.into())?; + repo.add(init.into())?; } packages.push(Resolvable::new("agama-scripts", ResolvableType::Package)); } @@ -168,10 +175,9 @@ impl Actor for Service { #[async_trait] impl MessageHandler for Service { async fn handle(&mut self, message: message::SetConfig) -> Result<(), Error> { - self.scripts.clear()?; - let config = message.config.unwrap_or_default(); + self.clear_scripts().await; if let Some(scripts) = config.scripts { self.add_scripts(scripts.clone()).await?; } @@ -185,7 +191,18 @@ impl MessageHandler for Service { #[async_trait] impl MessageHandler for Service { async fn handle(&mut self, message: message::RunScripts) -> Result<(), Error> { - self.scripts.run(message.group).await; + let scripts = self.scripts.clone(); + let install_dir = self.install_dir.clone(); + let progress = self.progress.clone(); + let questions = self.questions.clone(); + + tokio::task::spawn(async move { + let scripts = scripts.lock().await; + let workdir = scripts.workdir.clone(); + let to_run = scripts.by_group(message.group).clone(); + let runner = ScriptsRunner::new(install_dir, workdir, progress, questions); + runner.run(&to_run).await.unwrap(); + }); Ok(()) } } diff --git a/rust/agama-manager/src/service.rs b/rust/agama-manager/src/service.rs index bd272137ec..9efc67df58 100644 --- a/rust/agama-manager/src/service.rs +++ b/rust/agama-manager/src/service.rs @@ -195,7 +195,7 @@ impl Starter { let files = match self.files { Some(files) => files, None => { - files::Service::starter(self.events.clone(), progress.clone(), software.clone()) + files::Service::starter(progress.clone(), self.questions.clone(), software.clone()) .start() .await? } diff --git a/rust/agama-utils/src/api/files/scripts.rs b/rust/agama-utils/src/api/files/scripts.rs index b38129f17d..eb47c6f0b7 100644 --- a/rust/agama-utils/src/api/files/scripts.rs +++ b/rust/agama-utils/src/api/files/scripts.rs @@ -24,8 +24,14 @@ use std::{ }; use crate::{ - api::files::{FileSource, FileSourceError, WithFileSource}, + actor::Handler, + api::{ + files::{FileSource, FileSourceError, WithFileSource}, + question::{Question, QuestionSpec}, + Scope, + }, command::run_with_retry, + progress, question, }; use agama_transfer::Error as TransferError; use serde::{Deserialize, Serialize}; @@ -148,6 +154,13 @@ impl Script { } } + pub fn chroot(&self) -> bool { + match self { + Script::Post(script) => script.chroot.unwrap_or_default(), + _ => false, + } + } + /// Runs the script in the given work directory. /// /// It saves the logs and the exit status of the execution. @@ -311,7 +324,7 @@ impl_with_file_source!(InitScript); /// /// It offers an API to add and execute installation scripts. pub struct ScriptsRepository { - workdir: PathBuf, + pub workdir: PathBuf, pub scripts: Vec