diff --git a/Cargo.lock b/Cargo.lock index 4adb99e6ae..0fddde485e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2849,7 +2849,6 @@ dependencies = [ "rust-apt", "serde", "serde_json", - "stdio-override", "tempfile", "uuid", ] @@ -3396,15 +3395,6 @@ dependencies = [ "strum", ] -[[package]] -name = "stdio-override" -version = "0.1.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a9c22512d2e95aa9a717da4c955c0085078a1f28b988e0736fc7066fdcc030c8" -dependencies = [ - "libc", -] - [[package]] name = "string_cache" version = "0.8.7" diff --git a/policies/module-types/system-updates/Cargo.toml b/policies/module-types/system-updates/Cargo.toml index fadb645d7d..bdb1615e38 100644 --- a/policies/module-types/system-updates/Cargo.toml +++ b/policies/module-types/system-updates/Cargo.toml @@ -20,7 +20,6 @@ rudder_module_type = { path = "../../rudder-module-type" } #rust-apt = { version = "0.8.0", optional = true } # while we don't have a fixed release rust-apt = { git = "https://gitlab.com/amousset/rust-apt.git", branch = "rudder", optional = true } -stdio-override = { version = "0.1.3", optional = true } memfile = { version = "0.3.2", optional = true } uuid = { version = "1", features = ["v4"] } #librpm = "0.1.1" @@ -30,7 +29,7 @@ log = "0.4.21" [features] # Enable APT package management -apt = ["dep:rust-apt", "dep:stdio-override", "dep:memfile"] +apt = ["dep:rust-apt", "dep:memfile"] [dev-dependencies] rudder_commons_test = { path = "../../rudder-commons-test" } diff --git a/policies/module-types/system-updates/src/db.rs b/policies/module-types/system-updates/src/db.rs index 1bf526bc4b..66b967a183 100644 --- a/policies/module-types/system-updates/src/db.rs +++ b/policies/module-types/system-updates/src/db.rs @@ -10,11 +10,11 @@ use crate::{output::Report, state::UpdateStatus}; use chrono::{DateTime, Duration, SecondsFormat, Utc}; use rudder_module_type::rudder_debug; use rusqlite::{self, Connection, Row}; -use std::fs::Permissions; -use std::os::unix::prelude::PermissionsExt; use std::{ fmt::{Display, Formatter}, fs, + fs::Permissions, + os::unix::prelude::PermissionsExt, path::{Path, PathBuf}, }; @@ -355,8 +355,7 @@ mod tests { use chrono::{Duration, Utc}; use pretty_assertions::assert_eq; use rusqlite::Connection; - use std::ops::Add; - use std::os::unix::prelude::PermissionsExt; + use std::{ops::Add, os::unix::prelude::PermissionsExt}; #[test] fn new_creates_new_database() { @@ -417,6 +416,24 @@ mod tests { assert_eq!(db.lock(0, event_id).unwrap(), None); } + #[test] + fn it_gets_status_regardless_of_event_id_case() { + let mut db = PackageDatabase::new(None).unwrap(); + let event_id = "TEST"; + let campaign_id = "CAMPAIGN"; + let now = Utc::now(); + assert_eq!(db.get_status(event_id).unwrap(), None); + db.schedule_event(event_id, campaign_id, now).unwrap(); + assert_eq!( + db.get_status(event_id).unwrap().unwrap(), + UpdateStatus::ScheduledUpdate + ); + assert_eq!( + db.get_status(&event_id.to_lowercase()).unwrap().unwrap(), + UpdateStatus::ScheduledUpdate + ); + } + #[test] fn start_event_inserts_and_sets_running_update() { let mut db = PackageDatabase::new(None).unwrap(); diff --git a/policies/module-types/system-updates/src/package_manager/apt.rs b/policies/module-types/system-updates/src/package_manager/apt.rs index 2981d73f0a..f36a146e56 100644 --- a/policies/module-types/system-updates/src/package_manager/apt.rs +++ b/policies/module-types/system-updates/src/package_manager/apt.rs @@ -4,12 +4,14 @@ mod filter; mod progress; -use crate::package_manager::apt::progress::AptAcquireProgress; use crate::{ campaign::FullCampaignType, output::ResultOutput, package_manager::{ - apt::filter::{Distribution, PackageFileFilter}, + apt::{ + filter::{Distribution, PackageFileFilter}, + progress::RudderAptAcquireProgress, + }, LinuxPackageManager, PackageId, PackageInfo, PackageList, PackageManager, PackageSpec, }, }; @@ -27,15 +29,7 @@ use rust_apt::{ progress::{AcquireProgress, InstallProgress}, Cache, PackageSort, }; -use std::{ - collections::HashMap, - env, - fs::File, - io::{Read, Seek}, - path::Path, - process::Command, -}; -use stdio_override::{StdoutOverride, StdoutOverrideGuard}; +use std::{collections::HashMap, env, os::fd::AsRawFd, path::Path, process::Command}; /// References: /// * https://www.debian.org/doc/manuals/debian-faq/uptodate.en.html @@ -211,41 +205,19 @@ impl AptPackageManager { } } -// Catch stdout/stderr from the library -pub struct OutputCatcher { - out_file: File, - out_guard: StdoutOverrideGuard, -} - -impl OutputCatcher { - pub fn new() -> Self { - let out_file = MemFile::create_default("stdout").unwrap().into_file(); - let out_guard = StdoutOverride::override_raw(out_file.try_clone().unwrap()).unwrap(); - Self { - out_file, - out_guard, - } - } - - pub fn read(mut self) -> String { - drop(self.out_guard); - let mut out = String::new(); - self.out_file.rewind().unwrap(); - self.out_file.read_to_string(&mut out).unwrap(); - out - } -} - impl LinuxPackageManager for AptPackageManager { fn update_cache(&mut self) -> ResultOutput<()> { let cache = self.cache(); if let Ok(o) = cache.inner { - let mut progress = AcquireProgress::new(AptAcquireProgress::new()); - let catch = OutputCatcher::new(); + let mem_file_acquire = MemFile::create_default("update-acquire").unwrap(); + let mut progress = AcquireProgress::new(RudderAptAcquireProgress::new( + mem_file_acquire.try_clone().unwrap(), + )); let mut r = Self::apt_errors_to_output(o.update(&mut progress)); - let out = catch.read(); - r.stdout(out); + + let acquire_out = RudderAptAcquireProgress::read_mem_file(mem_file_acquire); + r.stdout(acquire_out); r } else { cache.clear_ok() @@ -312,13 +284,23 @@ impl LinuxPackageManager for AptPackageManager { } // Do the changes - let mut acquire_progress = AcquireProgress::new(AptAcquireProgress::new()); - let mut install_progress = InstallProgress::default(); - let catch = OutputCatcher::new(); + let mem_file_acquire = MemFile::create_default("upgrade-acquire").unwrap(); + let mut acquire_progress = AcquireProgress::new(RudderAptAcquireProgress::new( + mem_file_acquire.try_clone().unwrap(), + )); + + let mem_file_install = MemFile::create_default("upgrade-install").unwrap(); + let mut install_progress = InstallProgress::fd(mem_file_install.as_raw_fd()); + let mut res_commit = Self::apt_errors_to_output(c.commit(&mut acquire_progress, &mut install_progress)); - let out = catch.read(); - res_commit.stdout(out); + + let acquire_out = RudderAptAcquireProgress::read_mem_file(mem_file_acquire); + res_commit.stdout(acquire_out); + + let install_out = RudderAptAcquireProgress::read_mem_file(mem_file_install); + res_commit.stdout(install_out); + res_resolve.step(res_commit) } else { cache.clear_ok() @@ -389,14 +371,4 @@ NEEDRESTART-SESS: amousset @ user manager service"; expected2 ); } - - #[test] - // Needs "-- --nocapture --ignored" to run in tests as cargo test also messes with stdout/stderr - #[ignore] - fn it_captures_stdout() { - let catch = OutputCatcher::new(); - println!("plouf"); - let out = catch.read(); - assert_eq!(out, "plouf\n".to_string()); - } } diff --git a/policies/module-types/system-updates/src/package_manager/apt/progress.rs b/policies/module-types/system-updates/src/package_manager/apt/progress.rs index 48bc14d602..f0483847f6 100644 --- a/policies/module-types/system-updates/src/package_manager/apt/progress.rs +++ b/policies/module-types/system-updates/src/package_manager/apt/progress.rs @@ -1,27 +1,42 @@ // SPDX-License-Identifier: GPL-3.0-or-later // SPDX-FileCopyrightText: 2024 Normation SAS -use rust_apt::error::pending_error; -use rust_apt::progress::DynAcquireProgress; -use rust_apt::raw::{AcqTextStatus, ItemDesc, ItemState, PkgAcquire}; -use rust_apt::util::{time_str, unit_str, NumSys}; +use memfile::MemFile; +use rust_apt::{ + error::pending_error, + progress::DynAcquireProgress, + raw::{AcqTextStatus, ItemDesc, ItemState, PkgAcquire}, + util::{time_str, unit_str, NumSys}, +}; +use std::io::{BufWriter, Read, Seek, SeekFrom, Write}; /// Reimplement `rust_apt`'s progress without interactivity. /// /// Try to stay consistent with APT's output, but allow some divergence. /// This struct mimics the output of `apt update`. -#[derive(Default, Debug)] -pub struct AptAcquireProgress {} +#[derive(Debug)] +pub struct RudderAptAcquireProgress { + /// Enforce MemFile as we won't properly handle IO errors + writer: BufWriter, +} -impl AptAcquireProgress { +impl RudderAptAcquireProgress { /// Returns a new default progress instance. - pub fn new() -> Self { - Self::default() + pub fn new(out_file: MemFile) -> Self { + let writer = BufWriter::new(out_file); + Self { writer } + } + + pub fn read_mem_file(mut file: MemFile) -> String { + let mut acquire_out = String::new(); + file.seek(SeekFrom::Start(0)).unwrap(); + file.read_to_string(&mut acquire_out).unwrap(); + acquire_out } } -impl DynAcquireProgress for AptAcquireProgress { +impl DynAcquireProgress for RudderAptAcquireProgress { /// Used to send the pulse interval to the apt progress class. /// /// Pulse Interval is in microseconds. @@ -41,7 +56,13 @@ impl DynAcquireProgress for AptAcquireProgress { /// /// Prints out the short description and the expected size. fn hit(&mut self, item: &ItemDesc) { - println!("\rHit:{} {}", item.owner().id(), item.description()); + write!( + self.writer, + "\rHit:{} {}", + item.owner().id(), + item.description() + ) + .unwrap(); } /// Called when an Item has started to download @@ -55,7 +76,7 @@ impl DynAcquireProgress for AptAcquireProgress { string.push_str(&format!(" [{}]", unit_str(file_size, NumSys::Decimal))); } - println!("{string}"); + write!(self.writer, "{string}").unwrap(); } /// Called when an Item fails to download. @@ -68,18 +89,18 @@ impl DynAcquireProgress for AptAcquireProgress { match item.owner().status() { ItemState::StatIdle | ItemState::StatDone => { - println!("\rIgn: {desc}"); + write!(self.writer, "\rIgn: {desc}").unwrap(); if error_text.is_empty() { show_error = false; } } _ => { - println!("\rErr: {desc}"); + write!(self.writer, "\rErr: {desc}").unwrap(); } } if show_error { - println!("\r{error_text}"); + write!(self.writer, "\r{error_text}").unwrap(); } } @@ -117,14 +138,17 @@ impl DynAcquireProgress for AptAcquireProgress { } if owner.fetched_bytes() != 0 { - println!( + write!( + self.writer, "Fetched {} in {} ({}/s)", unit_str(owner.fetched_bytes(), NumSys::Decimal), time_str(owner.elapsed_time()), unit_str(owner.current_cps(), NumSys::Decimal) - ); + ) + .unwrap(); } else { - println!("Nothing to fetch."); + write!(self.writer, "Nothing to fetch.").unwrap(); } + self.writer.flush().unwrap() } }