From 9a58dd112ada0c34863dbe969f1270f06af5b701 Mon Sep 17 00:00:00 2001 From: Sfikas Date: Tue, 28 Jan 2025 13:10:13 +0100 Subject: [PATCH] feat(update-agent): publishing update-agent progress to dbus (#349) * feat(update-agent): publishing update-agent progress to dbus * adding detailed reporting * ryan review 1 * review vol 2 * missing signal * Working Signals * review vol 3 --------- Co-authored-by: Ryan Butler --- Cargo.lock | 9 + Cargo.toml | 1 + update-agent/Cargo.toml | 2 +- update-agent/dbus/Cargo.toml | 15 ++ update-agent/dbus/src/lib.rs | 66 ++++++++ update-agent/src/component.rs | 36 +++- update-agent/src/dbus/interfaces.rs | 53 ++++++ update-agent/src/dbus/mod.rs | 2 + update-agent/src/{dbus.rs => dbus/proxies.rs} | 0 update-agent/src/main.rs | 154 ++++++++++++++---- 10 files changed, 297 insertions(+), 41 deletions(-) create mode 100644 update-agent/dbus/Cargo.toml create mode 100644 update-agent/dbus/src/lib.rs create mode 100644 update-agent/src/dbus/interfaces.rs create mode 100644 update-agent/src/dbus/mod.rs rename update-agent/src/{dbus.rs => dbus/proxies.rs} (100%) diff --git a/Cargo.lock b/Cargo.lock index 60cf37ca..f644cd85 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5264,6 +5264,7 @@ dependencies = [ "orb-slot-ctrl 0.3.0", "orb-telemetry", "orb-update-agent-core", + "orb-update-agent-dbus", "orb-zbus-proxies", "polling 2.5.2", "reqwest 0.11.23", @@ -5303,6 +5304,14 @@ dependencies = [ "url", ] +[[package]] +name = "orb-update-agent-dbus" +version = "0.0.0" +dependencies = [ + "serde", + "zbus", +] + [[package]] name = "orb-update-verifier" version = "0.2.6" diff --git a/Cargo.toml b/Cargo.toml index a0918b05..2642e28c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -102,6 +102,7 @@ orb-security-utils.path = "security-utils" orb-slot-ctrl.path = "slot-ctrl" orb-telemetry.path = "telemetry" orb-update-agent-core.path = "update-agent/core" +orb-update-agent-dbus.path = "update-agent/dbus" orb-zbus-proxies.path = "zbus-proxies" seek-camera.path = "seek-camera/wrapper" diff --git a/update-agent/Cargo.toml b/update-agent/Cargo.toml index 1b942a67..39b12e2e 100644 --- a/update-agent/Cargo.toml +++ b/update-agent/Cargo.toml @@ -35,6 +35,7 @@ once_cell = "1.17.0" orb-build-info.workspace = true orb-telemetry.workspace = true orb-update-agent-core.workspace = true +orb-update-agent-dbus.workspace = true orb-zbus-proxies = { workspace = true, features = ["login1"] } polling = "2.2.0" serde = { workspace = true, features = ["derive"] } @@ -50,7 +51,6 @@ url = "2.2.2" xz2 = "0.1.6" zbus.workspace = true - [dependencies.update-agent-can] git = "https://github.com/worldcoin/orb-software" rev = "f13df5b723272efc55abf22cacce3625bbd1af04" diff --git a/update-agent/dbus/Cargo.toml b/update-agent/dbus/Cargo.toml new file mode 100644 index 00000000..9a151585 --- /dev/null +++ b/update-agent/dbus/Cargo.toml @@ -0,0 +1,15 @@ +[package] +name = "orb-update-agent-dbus" +version = "0.0.0" +authors = ["Theodore Sfikas "] +description = "Dbus proxy and interface for orb-update-agent" +publish = false + +edition.workspace = true +license.workspace = true +repository.workspace = true +rust-version.workspace = true + +[dependencies] +zbus.workspace = true +serde.workspace = true diff --git a/update-agent/dbus/src/lib.rs b/update-agent/dbus/src/lib.rs new file mode 100644 index 00000000..b5fa3dec --- /dev/null +++ b/update-agent/dbus/src/lib.rs @@ -0,0 +1,66 @@ +//! Query the current update status: +//! ```bash +//! gdbus call --session -d org.worldcoin.UpdateAgentManager1 -o \ +//! '/org/worldcoin/UpdateAgentManager1' -m \ +//! org.freedesktop.DBus.Properties.Get org.worldcoin.UpdateAgentManager1 Progress +//! ``` +//! +//! Monitor for signals: +//! ```bash +//! export DBUS_SESSION_BUS_ADDRESS=unix:path=/tmp/worldcoin_bus_socket +//! dbus-monitor --session type='signal',sender='org.worldcoin.UpdateAgentManager1' +//! ``` + +use serde::{Deserialize, Serialize}; +use zbus::interface; +use zbus::zvariant::{OwnedValue, Type, Value}; + +/// A trait representing update progress behavior. +/// +/// This trait is implemented by types that can provide information about the current update status. +/// It abstracts the behavior to allow multiple implementations, enabling dependency injection, +/// mocking for tests, and sharing the same interface across both client and server code. +pub trait UpdateAgentManagerT: Send + Sync + 'static { + fn progress(&self) -> Vec; +} + +/// A wrapper struct for types implementing [`UpdateAgentManagerT`]. +pub struct UpdateAgentManager(pub T); + +#[derive( + Debug, Serialize, Deserialize, Type, Clone, Copy, Eq, PartialEq, Value, OwnedValue, +)] +pub enum ComponentState { + None = 1, + Downloading = 2, + Fetched = 3, + Processed = 4, + Installed = 5, +} + +#[derive( + Debug, Serialize, Deserialize, Type, Eq, PartialEq, Clone, Value, OwnedValue, +)] +pub struct ComponentStatus { + /// Component Name + pub name: String, + /// Current state of acomponent + pub state: ComponentState, + /// Progress through the current state (0-100) + pub progress: u8, +} + +/// DBus interface implementation for [`UpdateProgress`]. +#[interface( + name = "org.worldcoin.UpdateAgentManager1", + proxy( + default_service = "org.worldcoin.UpdateAgentManager1", + default_path = "/org/worldcoin/UpdateAgentManager1", + ) +)] +impl UpdateAgentManagerT for UpdateAgentManager { + #[zbus(property)] + fn progress(&self) -> Vec { + self.0.progress() + } +} diff --git a/update-agent/src/component.rs b/update-agent/src/component.rs index fdc94555..488146e8 100644 --- a/update-agent/src/component.rs +++ b/update-agent/src/component.rs @@ -15,13 +15,22 @@ use orb_update_agent_core::{ components, manifest::InstallationPhase, Claim, LocalOrRemote, ManifestComponent, MimeType, Slot, Source, }; +use orb_update_agent_dbus::{ComponentState, UpdateAgentManager}; use reqwest::{ header::{ToStrError, CONTENT_LENGTH, RANGE}, Url, }; use tracing::{info, warn}; - -use crate::{dbus, update::Update as _, util}; +use zbus::blocking::object_server::InterfaceRef; + +use crate::{ + dbus::{ + interfaces::{self, UpdateProgress}, + proxies, + }, + update::Update as _, + util, +}; const CHUNK_SIZE: u32 = 4 * 1024 * 1024; @@ -292,13 +301,15 @@ fn extract>(path: P, uncompressed_download_path: P) -> eyre::Resu } #[expect(clippy::result_large_err)] +#[expect(clippy::too_many_arguments)] pub fn download>( url: &Url, name: &str, unique_name: &str, size: u64, dst_dir: &P, - supervisor_proxy: Option<&dbus::SupervisorProxyBlocking<'static>>, + supervisor_proxy: Option<&proxies::SupervisorProxyBlocking<'static>>, + update_iface: Option<&InterfaceRef>>, download_delay: Duration, ) -> Result { let component_path = util::make_component_path(dst_dir, unique_name); @@ -427,10 +438,21 @@ pub fn download>( { if progress_percent != current_progress_percent { progress_percent = current_progress_percent; - info!("downloading component `{name}`: {progress_percent}%"); } } else { - info!("downloading component `{name}`: 100%"); + progress_percent = 100; + } + + info!("downloading component `{name}`: {progress_percent}%"); + if let Some(iface) = update_iface { + if let Err(e) = interfaces::update_dbus_properties( + name, + ComponentState::Downloading, + progress_percent as u8, + iface, + ) { + warn!("{e:?}"); + } } // We are using `downloads_allowed` as a proxy to set/unset the sleep duration for now. @@ -492,7 +514,8 @@ pub fn fetch>( system_component: &components::Component, source: &Source, dst_dir: P, - supervisor: Option<&dbus::SupervisorProxyBlocking<'static>>, + supervisor: Option<&proxies::SupervisorProxyBlocking<'static>>, + update_iface: Option<&InterfaceRef>>, download_delay: Duration, ) -> Result { let path = match &source.url { @@ -504,6 +527,7 @@ pub fn fetch>( source.size, &dst_dir, supervisor, + update_iface, download_delay, )?, }; diff --git a/update-agent/src/dbus/interfaces.rs b/update-agent/src/dbus/interfaces.rs new file mode 100644 index 00000000..fb4ff3fc --- /dev/null +++ b/update-agent/src/dbus/interfaces.rs @@ -0,0 +1,53 @@ +use eyre::WrapErr; +use orb_update_agent_core::ManifestComponent; +use orb_update_agent_dbus::{ + ComponentState, ComponentStatus, UpdateAgentManager, UpdateAgentManagerT, +}; +use zbus::blocking::object_server::InterfaceRef; + +#[derive(Debug, Clone, Default)] +pub struct UpdateProgress { + pub components: Vec, +} + +impl UpdateAgentManagerT for UpdateProgress { + fn progress(&self) -> Vec { + self.components.clone() + } +} + +pub fn init_dbus_properties( + components: &[ManifestComponent], + iface: &InterfaceRef>, +) { + iface.get_mut().0.components = components + .iter() + .map(|c| ComponentStatus { + name: c.name.clone(), + state: ComponentState::None, + progress: 0, + }) + .collect(); +} + +pub fn update_dbus_properties( + name: &str, + state: ComponentState, + progress: u8, + iface: &InterfaceRef>, +) -> eyre::Result<()> { + if let Some(component) = iface + .get_mut() + .0 + .components + .iter_mut() + .find(|c| c.name == name) + { + component.state = state; + component.progress = progress; + } + zbus::block_on(iface.get_mut().progress_changed(iface.signal_context())) + .wrap_err("Failed to emit progress_changed signal")?; + + Ok(()) +} diff --git a/update-agent/src/dbus/mod.rs b/update-agent/src/dbus/mod.rs new file mode 100644 index 00000000..314307d3 --- /dev/null +++ b/update-agent/src/dbus/mod.rs @@ -0,0 +1,2 @@ +pub mod interfaces; +pub mod proxies; diff --git a/update-agent/src/dbus.rs b/update-agent/src/dbus/proxies.rs similarity index 100% rename from update-agent/src/dbus.rs rename to update-agent/src/dbus/proxies.rs diff --git a/update-agent/src/main.rs b/update-agent/src/main.rs index 92f2805f..b6753162 100644 --- a/update-agent/src/main.rs +++ b/update-agent/src/main.rs @@ -30,15 +30,22 @@ use clap::Parser as _; use eyre::{bail, ensure, WrapErr}; use nix::sys::statvfs; use orb_update_agent::{ - component, component::Component, dbus, update, update_component_version_on_disk, - Args, Settings, + component, + component::Component, + dbus::{ + interfaces::{self, UpdateProgress}, + proxies, + }, + update, update_component_version_on_disk, Args, Settings, }; use orb_update_agent_core::{ version_map::SlotVersion, Claim, Slot, VersionMap, Versions, }; +use orb_update_agent_dbus::{ComponentState, UpdateAgentManager}; use orb_zbus_proxies::login1; use slot_ctrl::EfiVar; use tracing::{debug, error, info, warn}; +use zbus::blocking::{connection, InterfaceRef}; mod update_agent_result; use update_agent_result::UpdateAgentResult; @@ -77,6 +84,58 @@ fn get_config_source(args: &Args) -> Cow<'_, Path> { } } +fn setup_dbus() -> ( + Option>, + Option>>, +) { + fn setup_conn() -> eyre::Result { + connection::Builder::session() + .wrap_err("failed creating a new session dbus connection")? + .name("org.worldcoin.UpdateAgentManager1") + .wrap_err("failed to register dbus connection name: `org.worldcoin.UpdateAgentManager1``")? + .serve_at( + "/org/worldcoin/UpdateAgentManager1", + UpdateAgentManager(UpdateProgress::default()), + ) + .wrap_err("failed to serve dbus interface at `/org/worldcoin/UpdateAgentManager1`")? + .build() + .wrap_err("failed to build dbus connection") + } + + let dbus_conn = match setup_conn() { + Ok(conn) => conn, + Err(e) => { + warn!("failed to setup dbus connection: {e:?}"); + return (None, None); + } + }; + + // Build a dbus proxy for the supervisor service + // Used to request permission to update to proceed with installation + // Controlls the throttle of the update downloads + let supervisor_proxy = proxies::SupervisorProxyBlocking::builder(&dbus_conn) + .cache_properties(zbus::CacheProperties::No) + .build() + .map_err(|e| { + warn!("failed creating supervisor proxy: {e:?}"); + }) + .ok(); + + // Build a dbus interface for the update agent manager + // Used to report the progress of the update through dbus + let update_iface = dbus_conn + .object_server() + .interface::<_, UpdateAgentManager>( + "/org/worldcoin/UpdateAgentManager1", + ) + .map_err(|e| { + warn!("failed to setup UpdateAgentManager1 dbus interface: {e:?}"); + }) + .ok(); + + (supervisor_proxy, update_iface) +} + fn run(args: &Args) -> eyre::Result<()> { // TODO: In the event of a corrupt EFIVAR slot, we would be put into an unrecoverable state let active_slot = @@ -99,27 +158,11 @@ fn run(args: &Args) -> eyre::Result<()> { prepare_environment(&settings).wrap_err("failed preparing environment to run")?; - let supervisor_proxy = if settings.nodbus || settings.recovery { + let (supervisor_proxy, update_iface) = if settings.nodbus || settings.recovery { debug!("nodbus flag set or in recovery; not connecting to dbus"); - None + (None, None) } else { - match zbus::blocking::Connection::session() - .wrap_err("failed establishing a `session` dbus connection") - .and_then(|conn| { - dbus::SupervisorProxyBlocking::builder(&conn) - .cache_properties(zbus::CacheProperties::No) - .build() - .wrap_err("failed creating a supervisor dbus proxy") - }) { - Ok(proxy) => Some(proxy), - Err(e) => { - warn!( - "failed connecting to DBus; updates will be downloaded but not installed: \ - {e:?}" - ); - None - } - } + setup_dbus() }; info!( @@ -162,9 +205,9 @@ fn run(args: &Args) -> eyre::Result<()> { .map(|version_map| { if version_map != version_map_from_legacy { warn!( - "version map on disk does not match version map constructed from legacy \ - versions.json; preferring legacy. this will be an error in the future" - ); + "version map on disk does not match version map constructed from legacy \ + versions.json; preferring legacy. this will be an error in the future" + ); version_map_from_legacy.clone() } else { version_map @@ -186,6 +229,10 @@ fn run(args: &Args) -> eyre::Result<()> { let claim = orb_update_agent::claim::get(&settings, &version_map) .wrap_err("unable to get update claim")?; + if let Some(iface) = &update_iface { + interfaces::init_dbus_properties(claim.manifest_components(), iface); + } + match serde_json::to_string(&claim) { Ok(s) => info!("update claim received: {s}"), Err(e) => { @@ -215,6 +262,7 @@ fn run(args: &Args) -> eyre::Result<()> { &settings.workspace, &settings.downloads, supervisor_proxy.as_ref(), + update_iface.as_ref(), settings.download_delay, ) .wrap_err("failed fetching update components")?; @@ -228,9 +276,9 @@ fn run(args: &Args) -> eyre::Result<()> { if settings.nodbus || settings.recovery { debug!( - "nodbus option set or in recovery mode; not requesting update permission and \ - performing update immediately" - ); + "nodbus option set or in recovery mode; not requesting update permission and \ + performing update immediately" + ); } else if let Some(supervisor_proxy) = supervisor_proxy.as_ref() { supervisor_proxy.request_update_permission().wrap_err( "failed querying supervisor service for update permission; bailing", @@ -252,6 +300,18 @@ fn run(args: &Args) -> eyre::Result<()> { info!("running update for component `{}`", component.name()); component .run_update(target_slot, &claim, settings.recovery) + .inspect(|_| { + if let Some(iface) = &update_iface { + if let Err(e) = interfaces::update_dbus_properties( + component.name(), + ComponentState::Installed, + 0, + iface, + ) { + warn!("{e:?}"); + } + } + }) .wrap_err_with(|| { format!( "failed executing update for component `{}`", @@ -357,7 +417,8 @@ fn fetch_update_components( claim: &Claim, manifest_dst: &Path, dst: &Path, - supervisor_proxy: Option<&dbus::SupervisorProxyBlocking<'static>>, + supervisor_proxy: Option<&proxies::SupervisorProxyBlocking<'static>>, + update_iface: Option<&InterfaceRef>>, download_delay: Duration, ) -> eyre::Result> { orb_update_agent::manifest::compare_to_disk(claim.manifest(), manifest_dst)?; @@ -369,22 +430,47 @@ fn fetch_update_components( source, dst, supervisor_proxy, + update_iface, download_delay, ) .wrap_err_with(|| { format!("failed fetching source for component `{}`", source.name) })?; + + if let Some(iface) = update_iface { + if let Err(e) = interfaces::update_dbus_properties( + component.name(), + ComponentState::Fetched, + 0, + iface, + ) { + warn!("{e:?}"); + } + } components.push(component); } components .iter_mut() .try_for_each(|comp| { - comp.process(dst).wrap_err_with(|| { - format!( - "failed to process update file for component `{}`", - comp.name(), - ) - }) + comp.process(dst) + .inspect(|_| { + if let Some(iface) = update_iface { + if let Err(e) = interfaces::update_dbus_properties( + comp.name(), + ComponentState::Processed, + 0, + iface, + ) { + warn!("{e:?}"); + } + } + }) + .wrap_err_with(|| { + format!( + "failed to process update file for component `{}`", + comp.name(), + ) + }) }) .wrap_err("failed post processing downloaded components")?; Ok(components)