diff --git a/rust/Cargo.lock b/rust/Cargo.lock index 2dc5c9f2c0..8020865382 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -76,6 +76,7 @@ dependencies = [ "agama-utils", "async-trait", "serde_json", + "strum", "tempfile", "test-context", "thiserror 2.0.18", diff --git a/rust/agama-files/Cargo.toml b/rust/agama-files/Cargo.toml index ce812bd900..78574b5441 100644 --- a/rust/agama-files/Cargo.toml +++ b/rust/agama-files/Cargo.toml @@ -8,6 +8,7 @@ edition.workspace = true agama-software = { version = "0.1.0", path = "../agama-software" } agama-utils = { path = "../agama-utils" } async-trait = "0.1.89" +strum = "0.27.2" tempfile = "3.23.0" thiserror = "2.0.17" tokio = { version = "1.48.0", features = ["sync"] } diff --git a/rust/agama-files/src/runner.rs b/rust/agama-files/src/runner.rs index e2ade83408..7dff2cbe48 100644 --- a/rust/agama-files/src/runner.rs +++ b/rust/agama-files/src/runner.rs @@ -52,7 +52,7 @@ const NM_RESOLV_CONF_PATH: &str = "run/NetworkManager/resolv.conf"; /// Implements the logic to run a script. /// -/// It takes care of running the script, reporting errors (and asking whether to retry) and write +/// It takes care of running the script, reporting errors (and asking whether to retry) and writing /// the logs. pub struct ScriptsRunner { progress: Handler, @@ -94,7 +94,8 @@ impl ScriptsRunner { /// /// * `scripts`: scripts to run. pub async fn run(&self, scripts: &[&Script]) -> Result<(), Error> { - self.start_progress(scripts); + let scripts: Vec<_> = self.find_scripts_to_run(&scripts); + self.start_progress(&scripts); let mut resolv_linked = false; if scripts.iter().any(|s| s.chroot()) { @@ -122,12 +123,9 @@ impl ScriptsRunner { /// /// If the script fails, it asks the user whether it should try again. async fn run_script(&self, script: &Script) -> Result<(), Error> { - loop { - let path = self - .workdir - .join(script.group().to_string()) - .join(script.name()); + let path = self.workdir.join(script.relative_script_path()); + loop { let Err(error) = self.run_command(&path, script.chroot()).await else { return Ok(()); }; @@ -215,6 +213,23 @@ impl ScriptsRunner { _ = self.progress.cast(progress_action); } + /// Returns the scripts to run from the given collection + /// + /// It exclues any script that already ran. + fn find_scripts_to_run<'a>(&self, scripts: &[&'a Script]) -> Vec<&'a Script> { + scripts + .into_iter() + .filter(|s| { + let stdout_file = self + .workdir + .join(s.relative_script_path()) + .with_extension("stdout"); + !std::fs::exists(stdout_file).unwrap_or(false) + }) + .cloned() + .collect() + } + /// Reads the last n bytes of the file and returns them as a string. fn read_n_last_bytes(path: &Path, n_bytes: u64) -> io::Result { let mut file = File::open(path)?; diff --git a/rust/agama-files/src/service.rs b/rust/agama-files/src/service.rs index d0b001a4c4..573f048baa 100644 --- a/rust/agama-files/src/service.rs +++ b/rust/agama-files/src/service.rs @@ -27,12 +27,13 @@ use agama_software::{self as software, Resolvable, ResolvableType}; use agama_utils::{ actor::{self, Actor, Handler, MessageHandler}, api::files::{ - scripts::{self, ScriptsRepository}, + scripts::{self, ScriptsGroup, ScriptsRepository}, user_file, ScriptsConfig, UserFile, }, progress, question, }; use async_trait::async_trait; +use strum::IntoEnumIterator; use tokio::sync::Mutex; use crate::{message, ScriptsRunner}; @@ -128,9 +129,15 @@ impl Service { Starter::new(progress, questions, software) } + /// Clear the scripts. + /// + /// Keep the pre-scripts because they are expected to run as soon as they are imported. pub async fn clear_scripts(&mut self) -> Result<(), Error> { let mut repo = self.scripts.lock().await; - repo.clear()?; + let groups: Vec<_> = ScriptsGroup::iter() + .filter(|g| g != &ScriptsGroup::Pre) + .collect(); + repo.clear(groups.as_slice())?; Ok(()) } diff --git a/rust/agama-manager/src/lib.rs b/rust/agama-manager/src/lib.rs index ab95100a04..cd591af4bb 100644 --- a/rust/agama-manager/src/lib.rs +++ b/rust/agama-manager/src/lib.rs @@ -135,25 +135,6 @@ mod test { Ok(()) } - #[test_context(Context)] - #[tokio::test] - async fn test_update_config_without_product(ctx: &mut Context) { - let input_config = Config { - l10n: Some(l10n::Config { - locale: Some("es_ES.UTF-8".to_string()), - keymap: Some("es".to_string()), - timezone: Some("Atlantic/Canary".to_string()), - }), - ..Default::default() - }; - - let error = ctx - .handler - .call(message::SetConfig::new(input_config.clone())) - .await; - assert!(matches!(error, Err(crate::service::Error::MissingProduct))); - } - #[test_context(Context)] #[tokio::test] async fn test_patch_config(ctx: &mut Context) -> Result<(), Error> { @@ -184,28 +165,4 @@ mod test { Ok(()) } - - #[test_context(Context)] - #[tokio::test] - async fn test_patch_config_without_product(ctx: &mut Context) -> Result<(), Error> { - let input_config = Config { - l10n: Some(l10n::Config { - keymap: Some("es".to_string()), - ..Default::default() - }), - ..Default::default() - }; - - let result = ctx - .handler - .call(message::UpdateConfig::new(input_config.clone())) - .await; - assert!(matches!(result, Err(crate::service::Error::MissingProduct))); - - let extended_config = ctx.handler.call(message::GetExtendedConfig).await?; - let l10n_config = extended_config.l10n.unwrap(); - assert_eq!(l10n_config.keymap, Some("us".to_string())); - - Ok(()) - } } diff --git a/rust/agama-manager/src/service.rs b/rust/agama-manager/src/service.rs index 79de3ca20d..e67d68e5ef 100644 --- a/rust/agama-manager/src/service.rs +++ b/rust/agama-manager/src/service.rs @@ -45,8 +45,6 @@ use tokio::sync::{broadcast, RwLock}; #[derive(Debug, thiserror::Error)] pub enum Error { - #[error("Missing product")] - MissingProduct, #[error(transparent)] Event(#[from] broadcast::error::SendError), #[error(transparent)] @@ -434,71 +432,27 @@ impl Service { } async fn set_config(&mut self, config: Config) -> Result<(), Error> { + tracing::debug!("SetConfig: {config:?}"); self.set_product(&config)?; + self.config = config; - let Some(product) = &self.product else { - return Err(Error::MissingProduct); + let action = SetConfigAction { + bootloader: self.bootloader.clone(), + files: self.files.clone(), + hostname: self.hostname.clone(), + iscsi: self.iscsi.clone(), + l10n: self.l10n.clone(), + network: self.network.clone(), + proxy: self.proxy.clone(), + questions: self.questions.clone(), + security: self.security.clone(), + software: self.software.clone(), + storage: self.storage.clone(), + users: self.users.clone(), }; - self.security - .call(security::message::SetConfig::new(config.security.clone())) - .await?; - - self.hostname - .call(hostname::message::SetConfig::new(config.hostname.clone())) - .await?; - - self.proxy - .call(proxy::message::SetConfig::new(config.proxy.clone())) - .await?; - - self.files - .call(files::message::SetConfig::new(config.files.clone())) - .await?; - - self.run_pre_scripts(); - - self.questions - .call(question::message::SetConfig::new(config.questions.clone())) - .await?; - - self.software - .call(software::message::SetConfig::new( - Arc::clone(product), - config.software.clone(), - )) - .await?; - - self.l10n - .call(l10n::message::SetConfig::new(config.l10n.clone())) - .await?; + action.run(self.product.clone(), &self.config).await; - self.users - .call(users::message::SetConfig::new(config.users.clone())) - .await?; - - self.iscsi - .call(iscsi::message::SetConfig::new(config.iscsi.clone())) - .await?; - - self.storage.cast(storage::message::SetConfig::new( - Arc::clone(product), - config.storage.clone(), - ))?; - - // call bootloader always after storage to ensure that bootloader reflect new storage settings - self.bootloader - .call(bootloader::message::SetConfig::new( - config.bootloader.clone(), - )) - .await?; - - if let Some(network) = config.network.clone() { - self.network.update_config(network).await?; - self.network.apply().await?; - } - - self.config = config; Ok(()) } @@ -621,30 +575,6 @@ impl Service { Ok(Some(software_config)) } - - /// It runs pre-scripts and asks storage for probing. - fn run_pre_scripts(&self) { - let files = self.files.clone(); - let storage = self.storage.clone(); - tokio::spawn(async move { - let pre_scripts_ran = match files - .call(files::message::RunScripts::new(ScriptsGroup::Pre)) - .await - { - Ok(result) => result, - Err(error) => { - tracing::error!("Failed to run pre-scripts: {error}"); - return; - } - }; - - if pre_scripts_ran { - if let Err(error) = storage.cast(storage::message::Probe) { - tracing::error!("Failed to ask for storage probing: {error}"); - } - } - }); - } } impl Actor for Service { @@ -1059,3 +989,113 @@ impl FinishAction { } } } + +/// Implements the set config logic. +/// +/// This action runs on a separate Tokio task to prevent the manager from blocking. +struct SetConfigAction { + bootloader: Handler, + files: Handler, + hostname: Handler, + iscsi: Handler, + l10n: Handler, + network: NetworkSystemClient, + proxy: Handler, + questions: Handler, + security: Handler, + software: Handler, + storage: Handler, + users: Handler, +} + +impl SetConfigAction { + pub async fn run(self, product: Option>>, config: &Config) { + let config = config.clone(); + tokio::spawn(async move { + tracing::info!("Updating the configuration"); + match self.set_config(product, config).await { + Ok(_) => tracing::info!("Configuration updated successfully"), + Err(error) => tracing::error!("Failed to update the configuration: {error}"), + } + }); + } + + async fn set_config( + self, + product: Option>>, + config: Config, + ) -> Result<(), Error> { + self.security + .call(security::message::SetConfig::new(config.security.clone())) + .await?; + + self.hostname + .call(hostname::message::SetConfig::new(config.hostname.clone())) + .await?; + + self.proxy + .call(proxy::message::SetConfig::new(config.proxy.clone())) + .await?; + + self.files + .call(files::message::SetConfig::new(config.files.clone())) + .await?; + + self.files + .call(files::message::RunScripts::new(ScriptsGroup::Pre)) + .await?; + + self.questions + .call(question::message::SetConfig::new(config.questions.clone())) + .await?; + + self.l10n + .call(l10n::message::SetConfig::new(config.l10n.clone())) + .await?; + + self.users + .call(users::message::SetConfig::new(config.users.clone())) + .await?; + + self.iscsi + .call(iscsi::message::SetConfig::new(config.iscsi.clone())) + .await?; + + if let Some(network) = config.network.clone() { + self.network.update_config(network).await?; + self.network.apply().await?; + } + + match &product { + Some(product) => { + self.software + .call(software::message::SetConfig::new( + Arc::clone(product), + config.software.clone(), + )) + .await?; + + self.storage + .call(storage::message::SetConfig::new( + Arc::clone(product), + config.storage.clone(), + )) + .await?; + + // call bootloader always after storage to ensure that bootloader reflect new storage settings + self.bootloader + .call(bootloader::message::SetConfig::new( + config.bootloader.clone(), + )) + .await?; + } + + None => { + // TODO: reset software and storage proposals. + tracing::info!("No product is selected."); + } + } + + Ok(()) + } +} diff --git a/rust/agama-server/tests/server_service.rs b/rust/agama-server/tests/server_service.rs index 324f3c6b7e..6a728aa475 100644 --- a/rust/agama-server/tests/server_service.rs +++ b/rust/agama-server/tests/server_service.rs @@ -150,30 +150,6 @@ async fn test_put_config_success(ctx: &mut Context) -> Result<(), Box Ok(()) } -#[test_context(Context)] -#[test] -async fn test_put_config_without_product(ctx: &mut Context) -> Result<(), Box> { - let json = r#" - { - "l10n": { - "locale": "es_ES.UTF-8", "keymap": "es", "timezone": "Atlantic/Canary" - } - } - "#; - - let request = Request::builder() - .uri("/config") - .header("Content-Type", "application/json") - .method(Method::PUT) - .body(json.to_string()) - .unwrap(); - - let response = ctx.client.send_request(request).await; - assert_eq!(response.status(), StatusCode::BAD_REQUEST); - - Ok(()) -} - #[test_context(Context)] #[test] async fn test_put_config_without_mode(ctx: &mut Context) -> Result<(), Box> { @@ -275,28 +251,6 @@ async fn test_patch_config_success(ctx: &mut Context) -> Result<(), Box Result<(), Box> { - let json = r#"{ "update": { "l10n": { "keymap": "en" } } }"#; - let request = Request::builder() - .uri("/config") - .header("Content-Type", "application/json") - .method(Method::PATCH) - .body(json.to_string()) - .unwrap(); - - let response = ctx.client.send_request(request).await; - assert_eq!(response.status(), StatusCode::BAD_REQUEST); - - let body = body_to_string(response.into_body()).await; - assert_eq!(body, r#"{"error":"Missing product"}"#); - - Ok(()) -} - #[test_context(Context)] #[test] async fn test_patch_config_invalid_json(ctx: &mut Context) -> Result<(), Box> { diff --git a/rust/agama-utils/src/api/files/scripts.rs b/rust/agama-utils/src/api/files/scripts.rs index 45d95bc146..14576d60e1 100644 --- a/rust/agama-utils/src/api/files/scripts.rs +++ b/rust/agama-utils/src/api/files/scripts.rs @@ -132,6 +132,14 @@ impl Script { self.base().write(&path) } + /// Returns the relative script path. + /// + /// The full script path depends on the workdir. This method returns + /// the relative path (e.g., "pre/my-script.sh"). + pub fn relative_script_path(&self) -> PathBuf { + PathBuf::from(self.group().to_string()).join(&self.base().name) + } + /// Script's group. /// /// It determines whether the script runs. @@ -289,8 +297,10 @@ impl ScriptsRepository { } /// Removes all the scripts from the repository. - pub fn clear(&mut self) -> Result<(), Error> { - for group in ScriptsGroup::iter() { + /// + /// * `groups`: groups of scripts to clear. + pub fn clear(&mut self, groups: &[ScriptsGroup]) -> Result<(), Error> { + for group in ScriptsGroup::iter().filter(|g| groups.contains(&g)) { let path = self.workdir.join(group.to_string()); if path.exists() { std::fs::remove_dir_all(path)?; @@ -319,14 +329,18 @@ impl Default for ScriptsRepository { #[cfg(test)] mod test { + use std::path::PathBuf; + use tempfile::TempDir; - use tokio::test; - use crate::api::files::{BaseScript, FileSource, PreScript, Script}; + use crate::api::files::{ + scripts::ScriptsGroup, BaseScript, FileSource, InitScript, PostPartitioningScript, + PostScript, PreScript, Script, + }; use super::ScriptsRepository; - #[test] + #[tokio::test] async fn test_add_script() { let tmp_dir = TempDir::with_prefix("scripts-").expect("a temporary directory"); let mut repo = ScriptsRepository::new(&tmp_dir); @@ -347,7 +361,7 @@ mod test { assert!(script_path.exists()); } - #[test] + #[tokio::test] async fn test_clear_scripts() { let tmp_dir = TempDir::with_prefix("scripts-").expect("a temporary directory"); let mut repo = ScriptsRepository::new(&tmp_dir); @@ -365,10 +379,34 @@ mod test { let script_path = tmp_dir.path().join("pre").join("test"); assert!(script_path.exists()); - _ = repo.clear(); + _ = repo.clear(&[ScriptsGroup::Pre]); assert!(!script_path.exists()); // the directory for AutoYaST scripts is not removed assert!(autoyast_path.exists()) } + + #[test] + fn test_relative_script_path() { + let base = BaseScript { + name: "test".to_string(), + source: FileSource::Text { + content: "".to_string(), + }, + }; + let script = Script::Pre(PreScript { base: base.clone() }); + assert_eq!(script.relative_script_path(), PathBuf::from("pre/test")); + let script = Script::PostPartitioning(PostPartitioningScript { base: base.clone() }); + assert_eq!( + script.relative_script_path(), + PathBuf::from("postPartitioning/test") + ); + let script = Script::Post(PostScript { + base: base.clone(), + chroot: Some(false), + }); + assert_eq!(script.relative_script_path(), PathBuf::from("post/test")); + let script = Script::Init(InitScript { base: base.clone() }); + assert_eq!(script.relative_script_path(), PathBuf::from("init/test")); + } } diff --git a/rust/package/agama.changes b/rust/package/agama.changes index 1d840616e5..0e6bc11c70 100644 --- a/rust/package/agama.changes +++ b/rust/package/agama.changes @@ -1,3 +1,10 @@ +------------------------------------------------------------------- +Fri Feb 6 07:38:38 UTC 2026 - Imobach Gonzalez Sosa + +- Allow loading a profile with no product (bsc#1257067 and bsc#1257082). +- Make the bootloader proposal after the storage one is ready (bsc#1257530). +- Run pre-scripts only once to prevent potential infinite loops. + ------------------------------------------------------------------- Thu Feb 5 06:46:46 UTC 2026 - Imobach Gonzalez Sosa