From 9a9c5f9356ac7c020fa01e6494990f070a846c3f Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 4 Mar 2021 21:25:29 +0000 Subject: [PATCH] rpm-ostree: Use upstream Rust bindings Depends: https://github.com/coreos/rpm-ostree/pull/2636 Use the upstream client bindings for the status data. An interesting note: In the upstream API, the commit metadata is exposed as a generic `HashMap<>` because that's how it's used by both ostree and rpm-ostree. It'd be a layering violation for us to hardcode `coreos-assembler.basearch` in the rpm-ostree git for example, not to mention `fedora-coreos.stream`. So those constants stay here in zincati. I dropped the `_json` terminology from various functions because that's weird - we parsed the data from JSON, but that's not really very relevant except as an implementation detail. It's just a `Deployment`, not a `DeploymentJSON`. Also I dropped for now the optimization of using `--booted`; it's not a huge amount of data. If we care we can re-add that later. Note it makes caching more complex because then we need to carefully check the booted state too. --- Cargo.lock | 26 +++++- Cargo.toml | 2 + src/identity/mod.rs | 24 ++++-- src/rpm_ostree/actor.rs | 3 +- src/rpm_ostree/cli_status.rs | 153 +++++++---------------------------- src/rpm_ostree/mod.rs | 13 ++- 6 files changed, 84 insertions(+), 137 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 84d98d68..82c33d03 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -109,6 +109,12 @@ dependencies = [ "winapi 0.3.9", ] +[[package]] +name = "anyhow" +version = "1.0.38" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "afddf7f520a80dbf76e6f50a35bca42a2331ef227a28b3b6dc5c2e2338d114b1" + [[package]] name = "assert-json-diff" version = "1.1.0" @@ -1602,6 +1608,17 @@ dependencies = [ "quick-error 1.2.3", ] +[[package]] +name = "rpmostree-client" +version = "0.1.0" +source = "git+https://github.com/coreos/rpm-ostree?rev=3041d648bbce3beaef2d6b69c8461532e508329d#3041d648bbce3beaef2d6b69c8461532e508329d" +dependencies = [ + "anyhow", + "serde", + "serde_derive", + "serde_json", +] + [[package]] name = "rustc-demangle" version = "0.1.18" @@ -1667,18 +1684,18 @@ dependencies = [ [[package]] name = "serde" -version = "1.0.123" +version = "1.0.124" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "92d5161132722baa40d802cc70b15262b98258453e85e5d1d365c757c73869ae" +checksum = "bd761ff957cb2a45fbb9ab3da6512de9de55872866160b23c25f1a841e99d29f" dependencies = [ "serde_derive", ] [[package]] name = "serde_derive" -version = "1.0.123" +version = "1.0.124" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9391c295d64fc0abb2c556bad848f33cb8296276b1ad2677d1ae1ace4f258f31" +checksum = "1800f7693e94e186f5e25a28291ae1570da908aff7d97a095dec1e56ff99069b" dependencies = [ "proc-macro2", "quote", @@ -2357,6 +2374,7 @@ dependencies = [ "rand 0.8.3", "regex", "reqwest", + "rpmostree-client", "serde", "serde_json", "structopt", diff --git a/Cargo.toml b/Cargo.toml index 2d43f2e0..16aea076 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -33,6 +33,8 @@ prometheus = { version = "0.11", default-features = false } rand = "0.8" regex = "1.4" reqwest = { version = "0.10", features = ["json"] } +# Not published to crates.io right now +rpmostree-client = { git = "https://github.com/coreos/rpm-ostree", rev = "3041d648bbce3beaef2d6b69c8461532e508329d" } serde = { version = "1.0", features = ["derive"] } serde_json = "1.0" tempfile = "^3.2" diff --git a/src/identity/mod.rs b/src/identity/mod.rs index 142526dc..006c1ac8 100644 --- a/src/identity/mod.rs +++ b/src/identity/mod.rs @@ -8,6 +8,7 @@ use libsystemd::id128; use ordered_float::NotNan; use prometheus::{Gauge, IntGaugeVec}; use regex::Regex; +use rpm_ostree::Release; use serde::Serialize; use std::collections::HashMap; @@ -90,19 +91,28 @@ impl Identity { /// Try to build default agent identity. pub fn try_default() -> Fallible { // Invoke rpm-ostree to get the status of the currently booted deployment. - let status = rpm_ostree::invoke_cli_status(true)?; - let basearch = rpm_ostree::parse_basearch(&status) - .context("failed to introspect OS base architecture")?; - let current_os = - rpm_ostree::parse_booted(&status).context("failed to introspect booted OS image")?; + let status = rpmostree_client::query_status(&*rpm_ostree::CLI_CLIENT) + .map_err(failure::Error::from_boxed_compat)?; + let booted = status + .require_booted() + .map_err(failure::Error::from_boxed_compat)?; + let basearch = booted + .find_base_commitmeta_string(rpm_ostree::COSA_BASEARCH) + .map_err(failure::Error::from_boxed_compat) + .context("failed to introspect OS base architecture")? + .to_string(); let node_uuid = { let app_id = id128::Id128::try_from_slice(APP_ID) .map_err(|e| format_err!("failed to parse application ID: {}", e))?; compute_node_uuid(&app_id)? }; let platform = platform::read_id("/proc/cmdline")?; - let stream = rpm_ostree::parse_updates_stream(&status) - .context("failed to introspect OS updates stream")?; + let stream = booted + .find_base_commitmeta_string(rpm_ostree::FCOS_STREAM) + .map_err(failure::Error::from_boxed_compat) + .context("failed to introspect OS updates stream")? + .to_string(); + let current_os: Release = booted.into(); let id = Self { basearch, diff --git a/src/rpm_ostree/actor.rs b/src/rpm_ostree/actor.rs index 7b599284..8b77a0da 100644 --- a/src/rpm_ostree/actor.rs +++ b/src/rpm_ostree/actor.rs @@ -1,6 +1,5 @@ //! rpm-ostree client actor. -use super::cli_status::StatusJSON; use super::Release; use actix::prelude::*; use failure::Fallible; @@ -12,7 +11,7 @@ use std::rc::Rc; /// Cache of local deployments. #[derive(Clone, Debug)] pub struct StatusCache { - pub status: Rc, + pub status: Rc, pub mtime: FileTime, } diff --git a/src/rpm_ostree/cli_status.rs b/src/rpm_ostree/cli_status.rs index 72125813..828051ec 100644 --- a/src/rpm_ostree/cli_status.rs +++ b/src/rpm_ostree/cli_status.rs @@ -2,15 +2,16 @@ use super::actor::{RpmOstreeClient, StatusCache}; use super::Release; -use failure::{bail, ensure, format_err, Fallible, ResultExt}; +use failure::{format_err, Fallible, ResultExt}; use filetime::FileTime; use log::trace; use prometheus::IntCounter; -use serde::Deserialize; use std::collections::BTreeSet; use std::fs; use std::rc::Rc; +use super::CLI_CLIENT; + /// Path to local OSTree deployments. We use its mtime to check for modifications (e.g. new deployments) /// to local deployments that might warrant querying `rpm-ostree status` again to update our knowledge /// of the current state of deployments. @@ -37,83 +38,31 @@ lazy_static::lazy_static! { )).unwrap(); } -/// JSON output from `rpm-ostree status --json` -#[derive(Clone, Debug, Deserialize)] -pub struct StatusJSON { - deployments: Vec, -} - -/// Partial deployment object (only fields relevant to zincati). -#[derive(Clone, Debug, Deserialize)] -#[serde(rename_all = "kebab-case")] -pub struct DeploymentJSON { - booted: bool, - base_checksum: Option, - #[serde(rename = "base-commit-meta")] - base_metadata: BaseCommitMetaJSON, - checksum: String, - // NOTE(lucab): missing field means "not staged". - #[serde(default)] - staged: bool, - version: String, -} - -/// Metadata from base commit (only fields relevant to zincati). -#[derive(Clone, Debug, Deserialize)] -struct BaseCommitMetaJSON { - #[serde(rename = "coreos-assembler.basearch")] - basearch: String, - #[serde(rename = "fedora-coreos.stream")] - stream: String, -} - -impl DeploymentJSON { - /// Convert into `Release`. - pub fn into_release(self) -> Release { +impl From<&rpmostree_client::Deployment> for Release { + fn from(d: &rpmostree_client::Deployment) -> Self { Release { - checksum: self.base_revision(), - version: self.version, + checksum: d + .base_checksum + .clone() + .unwrap_or_else(|| d.checksum.clone()), + version: d.version.clone().unwrap_or_default(), age_index: None, } } - - /// Return the deployment base revision. - pub fn base_revision(&self) -> String { - self.base_checksum - .clone() - .unwrap_or_else(|| self.checksum.clone()) - } -} - -/// Parse base architecture for booted deployment from status object. -pub fn parse_basearch(status: &StatusJSON) -> Fallible { - let json = booted_json(status)?; - Ok(json.base_metadata.basearch) -} - -/// Parse the booted deployment from status object. -pub fn parse_booted(status: &StatusJSON) -> Fallible { - let json = booted_json(status)?; - Ok(json.into_release()) -} - -/// Parse updates stream for booted deployment from status object. -pub fn parse_updates_stream(status: &StatusJSON) -> Fallible { - let json = booted_json(status)?; - ensure!(!json.base_metadata.stream.is_empty(), "empty stream value"); - Ok(json.base_metadata.stream) } /// Parse local deployments from a status object. -fn parse_local_deployments(status: &StatusJSON, omit_staged: bool) -> Fallible> { +fn parse_local_deployments( + status: &rpmostree_client::Status, + omit_staged: bool, +) -> Fallible> { let mut deployments = BTreeSet::::new(); for entry in &status.deployments { - if omit_staged && entry.staged { + if omit_staged && entry.staged.unwrap_or_default() { continue; } - let release = entry.clone().into_release(); - deployments.insert(release); + deployments.insert(entry.into()); } Ok(deployments) } @@ -123,29 +72,14 @@ pub fn local_deployments( client: &mut RpmOstreeClient, omit_staged: bool, ) -> Fallible> { - let status = status_json(client)?; + let status = query_status(client)?; let local_depls = parse_local_deployments(&status, omit_staged)?; Ok(local_depls) } -/// Return JSON object for booted deployment. -fn booted_json(status: &StatusJSON) -> Fallible { - let booted = status - .clone() - .deployments - .into_iter() - .find(|d| d.booted) - .ok_or_else(|| format_err!("no booted deployment found"))?; - - ensure!(!booted.base_revision().is_empty(), "empty base revision"); - ensure!(!booted.version.is_empty(), "empty version"); - ensure!(!booted.base_metadata.basearch.is_empty(), "empty basearch"); - Ok(booted) -} - /// Ensure our status cache is up to date; if empty or out of date, run `rpm-ostree status` to populate it. -fn status_json(client: &mut RpmOstreeClient) -> Fallible> { +fn query_status_inner(client: &mut RpmOstreeClient) -> Fallible> { STATUS_CACHE_ATTEMPTS.inc(); let ostree_depls_data = fs::metadata(OSTREE_DEPLS_PATH) .with_context(|e| format_err!("failed to query directory {}: {}", OSTREE_DEPLS_PATH, e))?; @@ -160,7 +94,9 @@ fn status_json(client: &mut RpmOstreeClient) -> Fallible> { STATUS_CACHE_MISSES.inc(); trace!("cache stale, invoking rpm-ostree to retrieve local deployments"); - let status = Rc::new(invoke_cli_status(false)?); + let status = Rc::new( + rpmostree_client::query_status(&*CLI_CLIENT).map_err(failure::Error::from_boxed_compat)?, + ); client.status_cache = Some(StatusCache { status: Rc::clone(&status), mtime: ostree_depls_data_mtime, @@ -170,41 +106,26 @@ fn status_json(client: &mut RpmOstreeClient) -> Fallible> { } /// CLI executor for `rpm-ostree status --json`. -pub fn invoke_cli_status(booted_only: bool) -> Fallible { +pub fn query_status(client: &mut RpmOstreeClient) -> Fallible> { RPM_OSTREE_STATUS_ATTEMPTS.inc(); - let mut cmd = std::process::Command::new("rpm-ostree"); - cmd.arg("status").env("RPMOSTREE_CLIENT_ID", "zincati"); - - // Try to request the minimum scope we need. - if booted_only { - cmd.arg("--booted"); - } - - let cmdrun = cmd - .arg("--json") - .output() - .with_context(|_| "failed to run 'rpm-ostree' binary")?; - - if !cmdrun.status.success() { - RPM_OSTREE_STATUS_FAILURES.inc(); - bail!( - "rpm-ostree status failed:\n{}", - String::from_utf8_lossy(&cmdrun.stderr) - ); + match query_status_inner(client) { + Ok(s) => Ok(s), + Err(e) => { + RPM_OSTREE_STATUS_FAILURES.inc(); + Err(e) + } } - let status: StatusJSON = serde_json::from_slice(&cmdrun.stdout)?; - Ok(status) } #[cfg(test)] mod tests { use super::*; - fn mock_status(path: &str) -> Fallible { + fn mock_status(path: &str) -> Fallible { let fp = std::fs::File::open(path).unwrap(); let bufrd = std::io::BufReader::new(fp); - let status: StatusJSON = serde_json::from_reader(bufrd)?; + let status = serde_json::from_reader(bufrd)?; Ok(status) } @@ -226,18 +147,4 @@ mod tests { assert_eq!(deployments.len(), 1); } } - - #[test] - fn mock_booted_basearch() { - let status = mock_status("tests/fixtures/rpm-ostree-status.json").unwrap(); - let booted = booted_json(&status).unwrap(); - assert_eq!(booted.base_metadata.basearch, "x86_64"); - } - - #[test] - fn mock_booted_updates_stream() { - let status = mock_status("tests/fixtures/rpm-ostree-status.json").unwrap(); - let booted = booted_json(&status).unwrap(); - assert_eq!(booted.base_metadata.stream, "testing-devel"); - } } diff --git a/src/rpm_ostree/mod.rs b/src/rpm_ostree/mod.rs index 6c0b0c2b..0fb5ac68 100644 --- a/src/rpm_ostree/mod.rs +++ b/src/rpm_ostree/mod.rs @@ -1,7 +1,8 @@ mod cli_deploy; mod cli_finalize; mod cli_status; -pub use cli_status::{invoke_cli_status, parse_basearch, parse_booted, parse_updates_stream}; +pub use cli_status::query_status; +use lazy_static::lazy_static; mod actor; pub use actor::{ @@ -16,6 +17,16 @@ use failure::{ensure, format_err, Fallible, ResultExt}; use serde::Serialize; use std::cmp::Ordering; +/// Metadata key the base (RPM) architecture; injected by coreos-assembler +pub(crate) const COSA_BASEARCH: &str = "coreos-assembler.basearch"; +/// Metadata key for the Fedora CoreOS stream, injected by FCOS tooling via cosa. +pub(crate) const FCOS_STREAM: &str = "fedora-coreos.stream"; + +lazy_static! { + pub(crate) static ref CLI_CLIENT: rpmostree_client::CliClient = + rpmostree_client::CliClient::new("zincati"); +} + /// An OS release. #[derive(Clone, Debug, PartialEq, Eq, Serialize)] pub struct Release {