From dfda6265d5c7bc1dd6722e9e6a475965e4d90c0f Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 5 Dec 2024 16:56:49 -0500 Subject: [PATCH] Use a shared const and helpers for run/ostree-booted Just a code cleanup. Closes: https://github.com/containers/bootc/issues/934 Signed-off-by: Colin Walters --- lib/src/cli.rs | 3 +- lib/src/generator.rs | 74 +++++++++++++++++-------------- lib/src/status.rs | 4 +- ostree-ext/src/container_utils.rs | 27 ++++++++--- ostree-ext/src/integrationtest.rs | 4 +- 5 files changed, 69 insertions(+), 43 deletions(-) diff --git a/lib/src/cli.rs b/lib/src/cli.rs index 29564fc75..19b168fc2 100644 --- a/lib/src/cli.rs +++ b/lib/src/cli.rs @@ -17,6 +17,7 @@ use fn_error_context::context; use ostree::gio; use ostree_container::store::PrepareResult; use ostree_ext::container as ostree_container; +use ostree_ext::container_utils::is_ostree_booted; use ostree_ext::keyfileext::KeyFileExt; use ostree_ext::ostree; use schemars::schema_for; @@ -611,7 +612,7 @@ fn prepare_for_write() -> Result<()> { if ostree_ext::container_utils::running_in_container() { anyhow::bail!("Detected container; this command requires a booted host system."); } - if !std::path::Path::new("/run/ostree-booted").exists() { + if !is_ostree_booted()? { anyhow::bail!("This command requires an ostree-booted host system"); } crate::cli::require_root()?; diff --git a/lib/src/generator.rs b/lib/src/generator.rs index 086723ea9..b0ff1566c 100644 --- a/lib/src/generator.rs +++ b/lib/src/generator.rs @@ -4,6 +4,7 @@ use anyhow::{Context, Result}; use cap_std::fs::Dir; use cap_std_ext::{cap_std, dirext::CapStdExtDirExt}; use fn_error_context::context; +use ostree_ext::container_utils::is_ostree_booted_in; use rustix::{fd::AsFd, fs::StatVfsMountFlags}; const EDIT_UNIT: &str = "bootc-fstab-edit.service"; @@ -14,7 +15,7 @@ pub(crate) const BOOTC_EDITED_STAMP: &str = "Updated by bootc-fstab-edit.service #[context("bootc generator")] pub(crate) fn fstab_generator_impl(root: &Dir, unit_dir: &Dir) -> Result { // Do nothing if not ostree-booted - if !root.try_exists("run/ostree-booted")? { + if !is_ostree_booted_in(root)? { return Ok(false); } @@ -105,31 +106,37 @@ fn test_generator_no_fstab() -> Result<()> { Ok(()) } -#[test] -fn test_generator_fstab() -> Result<()> { - let tempdir = fixture()?; - let unit_dir = &tempdir.open_dir("run/systemd/system")?; - // Should still be a no-op - tempdir.atomic_write("etc/fstab", "# Some dummy fstab")?; - fstab_generator_impl(&tempdir, &unit_dir).unwrap(); - assert_eq!(unit_dir.entries()?.count(), 0); - - // Also a no-op, not booted via ostree - tempdir.atomic_write("etc/fstab", &format!("# {FSTAB_ANACONDA_STAMP}"))?; - fstab_generator_impl(&tempdir, &unit_dir).unwrap(); - assert_eq!(unit_dir.entries()?.count(), 0); - - // Now it should generate - tempdir.atomic_write("run/ostree-booted", "ostree booted")?; - fstab_generator_impl(&tempdir, &unit_dir).unwrap(); - assert_eq!(unit_dir.entries()?.count(), 2); - - Ok(()) -} +#[cfg(test)] +mod test { + use super::*; + + use ostree_ext::container_utils::OSTREE_BOOTED; + + #[test] + fn test_generator_fstab() -> Result<()> { + let tempdir = fixture()?; + let unit_dir = &tempdir.open_dir("run/systemd/system")?; + // Should still be a no-op + tempdir.atomic_write("etc/fstab", "# Some dummy fstab")?; + fstab_generator_impl(&tempdir, &unit_dir).unwrap(); + assert_eq!(unit_dir.entries()?.count(), 0); + + // Also a no-op, not booted via ostree + tempdir.atomic_write("etc/fstab", &format!("# {FSTAB_ANACONDA_STAMP}"))?; + fstab_generator_impl(&tempdir, &unit_dir).unwrap(); + assert_eq!(unit_dir.entries()?.count(), 0); + + // Now it should generate + tempdir.atomic_write(OSTREE_BOOTED, "ostree booted")?; + fstab_generator_impl(&tempdir, &unit_dir).unwrap(); + assert_eq!(unit_dir.entries()?.count(), 2); + + Ok(()) + } -#[test] -fn test_generator_fstab_idempotent() -> Result<()> { - let anaconda_fstab = indoc::indoc! { " + #[test] + fn test_generator_fstab_idempotent() -> Result<()> { + let anaconda_fstab = indoc::indoc! { " # # /etc/fstab # Created by anaconda on Tue Mar 19 12:24:29 2024 @@ -144,14 +151,15 @@ fn test_generator_fstab_idempotent() -> Result<()> { UUID=715be2b7-c458-49f2-acec-b2fdb53d9089 / xfs ro 0 0 UUID=341c4712-54e8-4839-8020-d94073b1dc8b /boot xfs defaults 0 0 " }; - let tempdir = fixture()?; - let unit_dir = &tempdir.open_dir("run/systemd/system")?; + let tempdir = fixture()?; + let unit_dir = &tempdir.open_dir("run/systemd/system")?; - tempdir.atomic_write("etc/fstab", anaconda_fstab)?; - tempdir.atomic_write("run/ostree-booted", "ostree booted")?; - let updated = fstab_generator_impl(&tempdir, &unit_dir).unwrap(); - assert!(!updated); - assert_eq!(unit_dir.entries()?.count(), 0); + tempdir.atomic_write("etc/fstab", anaconda_fstab)?; + tempdir.atomic_write(OSTREE_BOOTED, "ostree booted")?; + let updated = fstab_generator_impl(&tempdir, &unit_dir).unwrap(); + assert!(!updated); + assert_eq!(unit_dir.entries()?.count(), 0); - Ok(()) + Ok(()) + } } diff --git a/lib/src/status.rs b/lib/src/status.rs index 0dc800708..e2b613a7b 100644 --- a/lib/src/status.rs +++ b/lib/src/status.rs @@ -5,11 +5,11 @@ use std::io::Read; use std::io::Write; use anyhow::{Context, Result}; -use camino::Utf8Path; use fn_error_context::context; use ostree::glib; use ostree_container::OstreeImageReference; use ostree_ext::container as ostree_container; +use ostree_ext::container_utils::is_ostree_booted; use ostree_ext::keyfileext::KeyFileExt; use ostree_ext::oci_spec; use ostree_ext::ostree; @@ -294,7 +294,7 @@ pub(crate) async fn status(opts: super::cli::StatusOpts) -> Result<()> { 0 | 1 => {} o => anyhow::bail!("Unsupported format version: {o}"), }; - let host = if !Utf8Path::new("/run/ostree-booted").try_exists()? { + let host = if !is_ostree_booted()? { Default::default() } else { let sysroot = super::cli::get_storage().await?; diff --git a/ostree-ext/src/container_utils.rs b/ostree-ext/src/container_utils.rs index f4c7ed934..9ceb193d1 100644 --- a/ostree-ext/src/container_utils.rs +++ b/ostree-ext/src/container_utils.rs @@ -1,11 +1,18 @@ //! Helpers for interacting with containers at runtime. -use crate::keyfileext::KeyFileExt; -use anyhow::Result; -use ostree::glib; +use std::io; use std::io::Read; use std::path::Path; +use anyhow::Result; +use ocidir::cap_std::fs::Dir; +use ostree::glib; + +use crate::keyfileext::KeyFileExt; + +/// The relative path to the stamp file which signals this is an ostree-booted system. +pub const OSTREE_BOOTED: &str = "run/ostree-booted"; + // See https://github.com/coreos/rpm-ostree/pull/3285#issuecomment-999101477 // For compatibility with older ostree, we stick this in /sysroot where // it will be ignored. @@ -64,6 +71,17 @@ pub fn is_bare_split_xattrs() -> Result { } } +/// Returns true if the system appears to have been booted via ostree. +/// This accesses global state in /run. +pub fn is_ostree_booted() -> io::Result { + Path::new(&format!("/{OSTREE_BOOTED}")).try_exists() +} + +/// Returns true if the target root appears to have been booted via ostree. +pub fn is_ostree_booted_in(rootfs: &Dir) -> io::Result { + rootfs.try_exists(OSTREE_BOOTED) +} + /// Returns `true` if the current booted filesystem appears to be an ostree-native container. /// /// This just invokes [`is_bare_split_xattrs`] and [`running_in_container`]. @@ -73,8 +91,7 @@ pub fn is_ostree_container() -> Result { // If we have a container-ostree repo format, then we'll assume we're // running in a container unless there's strong evidence not (we detect // we're part of a systemd unit or are in a booted ostree system). - let maybe_container = running_in_container() - || (!running_in_systemd && !Path::new("/run/ostree-booted").exists()); + let maybe_container = running_in_container() || (!running_in_systemd && !is_ostree_booted()?); Ok(is_container_ostree && maybe_container) } diff --git a/ostree-ext/src/integrationtest.rs b/ostree-ext/src/integrationtest.rs index d8166ab2d..b7e81583b 100644 --- a/ostree-ext/src/integrationtest.rs +++ b/ostree-ext/src/integrationtest.rs @@ -2,7 +2,7 @@ use std::path::Path; -use crate::container_utils::is_ostree_container; +use crate::container_utils::{is_ostree_booted, is_ostree_container}; use anyhow::{anyhow, Context, Result}; use camino::Utf8Path; use cap_std::fs::Dir; @@ -21,7 +21,7 @@ use xshell::cmd; pub(crate) fn detectenv() -> Result<&'static str> { let r = if is_ostree_container()? { "ostree-container" - } else if Path::new("/run/ostree-booted").exists() { + } else if is_ostree_booted()? { "ostree" } else if crate::container_utils::running_in_container() { "container"