From b48d31084b26d39e0026c8637caf79df86ec69b0 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sun, 29 Jun 2025 15:37:07 -0400 Subject: [PATCH 1/6] ostree-ext: Add sysroot listing + creation helpers Signed-off-by: Colin Walters --- crates/ostree-ext/src/container/deploy.rs | 4 +- crates/ostree-ext/src/sysroot.rs | 237 +++++++++++++++++++++- 2 files changed, 237 insertions(+), 4 deletions(-) diff --git a/crates/ostree-ext/src/container/deploy.rs b/crates/ostree-ext/src/container/deploy.rs index b93df17aa..3b6f9c2a0 100644 --- a/crates/ostree-ext/src/container/deploy.rs +++ b/crates/ostree-ext/src/container/deploy.rs @@ -1,8 +1,6 @@ //! Perform initial setup for a container image based system root use std::collections::HashSet; -#[cfg(feature = "bootc")] -use std::os::fd::BorrowedFd; use anyhow::Result; use fn_error_context::context; @@ -161,7 +159,7 @@ pub async fn deploy( use cap_std_ext::cmdext::CapStdExtCommandExt; use ocidir::cap_std::fs::Dir; - let sysroot_dir = &Dir::reopen_dir(&sysroot_fd(sysroot))?; + let sysroot_dir = &Dir::reopen_dir(&crate::sysroot::sysroot_fd(sysroot))?; // Note that the sysroot is provided as `.` but we use cwd_dir to // make the process current working directory the sysroot. diff --git a/crates/ostree-ext/src/sysroot.rs b/crates/ostree-ext/src/sysroot.rs index 7ba0db7a8..27fdb4f5a 100644 --- a/crates/ostree-ext/src/sysroot.rs +++ b/crates/ostree-ext/src/sysroot.rs @@ -1,8 +1,14 @@ //! Helpers for interacting with sysroots. -use std::ops::Deref; +use std::{ops::Deref, os::fd::BorrowedFd, time::SystemTime}; use anyhow::Result; +use chrono::Datelike as _; +use ocidir::cap_std::fs_utf8::Dir; +use ostree::gio; + +/// We may automatically allocate stateroots, this string is the prefix. +const AUTO_STATEROOT_PREFIX: &str = "state-"; use crate::utils::async_task_with_spinner; @@ -32,6 +38,117 @@ impl Deref for SysrootLock { } } + +/// Access the file descriptor for a sysroot +#[allow(unsafe_code)] +pub fn sysroot_fd(sysroot: &ostree::Sysroot) -> BorrowedFd { + unsafe { BorrowedFd::borrow_raw(sysroot.fd()) } +} + +/// A stateroot can match our auto "state-" prefix, or be manual. +#[derive(Debug, PartialEq, Eq)] +pub enum StaterootKind { + /// This stateroot has an automatic name + Auto((u64, u64)), + /// This stateroot is manually named + Manual, +} + +/// Metadata about a stateroot. +#[derive(Debug, PartialEq, Eq)] +pub struct Stateroot { + /// The name + pub name: String, + /// Kind + pub kind: StaterootKind, + /// Creation timestamp (from the filesystem) + pub creation: SystemTime, +} + +impl StaterootKind { + fn new(name: &str) -> Self { + if let Some(v) = parse_auto_stateroot_name(name) { + return Self::Auto(v); + } + Self::Manual + } +} + +/// Load metadata for a stateroot +fn read_stateroot(sysroot_dir: &Dir, name: &str) -> Result { + let path = format!("ostree/deploy/{name}"); + let kind = StaterootKind::new(&name); + let creation = sysroot_dir.symlink_metadata(&path)?.created()?.into_std(); + let r = Stateroot { + name: name.to_owned(), + kind, + creation, + }; + Ok(r) +} + +/// Enumerate stateroots, which are basically the default place for `/var`. +pub fn list_stateroots(sysroot: &ostree::Sysroot) -> Result> { + let sysroot_dir = &Dir::reopen_dir(&sysroot_fd(sysroot))?; + let r = sysroot_dir + .read_dir("ostree/deploy")? + .try_fold(Vec::new(), |mut acc, v| { + let v = v?; + let name = v.file_name()?; + if sysroot_dir.try_exists(format!("ostree/deploy/{name}/deploy"))? { + acc.push(read_stateroot(sysroot_dir, &name)?); + } + anyhow::Ok(acc) + })?; + Ok(r) +} + +/// Given a string, if it matches the form of an automatic state root, parse it into its . pair. +fn parse_auto_stateroot_name(name: &str) -> Option<(u64, u64)> { + let Some(statename) = name.strip_prefix(AUTO_STATEROOT_PREFIX) else { + return None; + }; + let Some((year, serial)) = statename.split_once("-") else { + return None; + }; + let Ok(year) = year.parse::() else { + return None; + }; + let Ok(serial) = serial.parse::() else { + return None; + }; + Some((year, serial)) +} + +/// Given a set of stateroots, allocate a new one +pub fn allocate_new_stateroot( + sysroot: &ostree::Sysroot, + stateroots: &[Stateroot], + now: chrono::DateTime, +) -> Result { + let sysroot_dir = &Dir::reopen_dir(&sysroot_fd(sysroot))?; + + let current_year = now.year().try_into().unwrap_or_default(); + let (year, serial) = stateroots + .iter() + .filter_map(|v| { + if let StaterootKind::Auto(v) = v.kind { + Some(v) + } else { + None + } + }) + .max() + .map(|(year, serial)| (year, serial + 1)) + .unwrap_or((current_year, 0)); + + let name = format!("state-{year}-{serial}"); + + sysroot.init_osname(&name, gio::Cancellable::NONE)?; + + read_stateroot(sysroot_dir, &name) +} + impl SysrootLock { /// Asynchronously acquire a sysroot lock. If the lock cannot be acquired /// immediately, a status message will be printed to standard output. @@ -55,3 +172,121 @@ impl SysrootLock { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_parse_auto_stateroot_name_valid() { + let test_cases = [ + // Basic valid cases + ("state-2024-0", Some((2024, 0))), + ("state-2024-1", Some((2024, 1))), + ("state-2023-123", Some((2023, 123))), + // Large numbers + ( + "state-18446744073709551615-18446744073709551615", + Some((18446744073709551615, 18446744073709551615)), + ), + // Zero values + ("state-0-0", Some((0, 0))), + ("state-0-123", Some((0, 123))), + // Leading zeros (should work - u64::parse handles them) + ("state-0002024-001", Some((2024, 1))), + ("state-000-000", Some((0, 0))), + ]; + + for (input, expected) in test_cases { + assert_eq!( + parse_auto_stateroot_name(input), + expected, + "Failed for input: {}", + input + ); + } + } + + #[test] + fn test_parse_auto_stateroot_name_invalid() { + let test_cases = [ + // Missing prefix + "2024-1", + // Wrong prefix + "stat-2024-1", + "states-2024-1", + "prefix-2024-1", + // Empty string + "", + // Only prefix + "state-", + // Missing separator + "state-20241", + // Wrong separator + "state-2024.1", + "state-2024_1", + "state-2024:1", + // Multiple separators + "state-2024-1-2", + // Missing year or serial + "state--1", + "state-2024-", + // Non-numeric year + "state-abc-1", + "state-2024a-1", + // Non-numeric serial + "state-2024-abc", + "state-2024-1a", + // Both non-numeric + "state-abc-def", + // Negative numbers (handled by parse::() failure) + "state--2024-1", + "state-2024--1", + // Floating point numbers + "state-2024.5-1", + "state-2024-1.5", + // Numbers with whitespace + "state- 2024-1", + "state-2024- 1", + "state-2024 -1", + "state-2024- 1 ", + // Case sensitivity (should fail - prefix is lowercase) + "State-2024-1", + "STATE-2024-1", + // Unicode characters + "state-2024-1🦀", + "state-2024🦀-1", + // Hex-like strings (should fail - not decimal) + "state-0x2024-1", + "state-2024-0x1", + ]; + + for input in test_cases { + assert_eq!( + parse_auto_stateroot_name(input), + None, + "Expected None for input: {}", + input + ); + } + } + + #[test] + fn test_stateroot_kind_new() { + let test_cases = [ + ("state-2024-1", StaterootKind::Auto((2024, 1))), + ("manual-name", StaterootKind::Manual), + ("state-invalid", StaterootKind::Manual), + ("", StaterootKind::Manual), + ]; + + for (input, expected) in test_cases { + assert_eq!( + StaterootKind::new(input), + expected, + "Failed for input: {}", + input + ); + } + } +} From 0bd553cf97defda30c25eb4e5104902dd21bfaf9 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sun, 29 Jun 2025 10:48:12 -0400 Subject: [PATCH 2/6] install: Add `reset` This is a nondestructive variant of `to-existing-root`. Signed-off-by: Colin Walters --- crates/lib/src/cli.rs | 20 +++- crates/lib/src/deploy.rs | 68 ++++++++----- crates/lib/src/install.rs | 157 ++++++++++++++++++++++++++++++- crates/ostree-ext/src/sysroot.rs | 1 - lib/src/kernel.rs | 66 +++++++++++++ 5 files changed, 281 insertions(+), 31 deletions(-) create mode 100644 lib/src/kernel.rs diff --git a/crates/lib/src/cli.rs b/crates/lib/src/cli.rs index 6677342cc..ebcce62fc 100644 --- a/crates/lib/src/cli.rs +++ b/crates/lib/src/cli.rs @@ -42,7 +42,7 @@ use crate::bootc_composefs::{ switch::switch_composefs, update::upgrade_composefs, }; -use crate::deploy::RequiredHostSpec; +use crate::deploy::{MergeState, RequiredHostSpec, RequiredHostSpec}; use crate::lints; use crate::podstorage::set_additional_image_store; use crate::progress_jsonl::{ProgressWriter, RawProgressFd}; @@ -262,6 +262,12 @@ pub(crate) enum InstallOpts { /// will be wiped, but the content of the existing root will otherwise be retained, and will /// need to be cleaned up if desired when rebooted into the new root. ToExistingRoot(crate::install::InstallToExistingRootOpts), + /// Nondestructively create a fresh installation state inside an existing bootc system. + /// + /// This is a nondestructive variant of `install to-existing-root` that works only inside + /// an existing bootc system. + #[clap(hide = true)] + Reset(crate::install::InstallResetOpts), /// Execute this as the penultimate step of an installation using `install to-filesystem`. /// Finalize { @@ -962,8 +968,9 @@ async fn upgrade(opts: UpgradeOpts) -> Result<()> { } else if booted_unchanged { println!("No update available.") } else { - let osname = booted_deployment.osname(); - crate::deploy::stage(sysroot, &osname, &fetched, &spec, prog.clone()).await?; + let stateroot = booted_deployment.osname(); + let from = MergeState::from_stateroot(sysroot, &stateroot)?; + crate::deploy::stage(sysroot, from, &fetched, &spec, prog.clone()).await?; changed = true; if let Some(prev) = booted_image.as_ref() { if let Some(fetched_manifest) = fetched.get_manifest(repo)? { @@ -1082,7 +1089,8 @@ async fn switch(opts: SwitchOpts) -> Result<()> { } let stateroot = booted_deployment.osname(); - crate::deploy::stage(sysroot, &stateroot, &fetched, &new_spec, prog.clone()).await?; + let from = MergeState::from_stateroot(sysroot, &stateroot)?; + crate::deploy::stage(sysroot, from, &fetched, &new_spec, prog.clone()).await?; sysroot.update_mtime()?; @@ -1161,7 +1169,8 @@ async fn edit(opts: EditOpts) -> Result<()> { // TODO gc old layers here let stateroot = booted_deployment.osname(); - crate::deploy::stage(sysroot, &stateroot, &fetched, &new_spec, prog.clone()).await?; + let from = MergeState::from_stateroot(sysroot, &stateroot)?; + crate::deploy::stage(sysroot, from, &fetched, &new_spec, prog.clone()).await?; sysroot.update_mtime()?; @@ -1441,6 +1450,7 @@ async fn run_from_opt(opt: Opt) -> Result<()> { InstallOpts::ToExistingRoot(opts) => { crate::install::install_to_existing_root(opts).await } + InstallOpts::Reset(opts) => crate::install::install_reset(opts).await, InstallOpts::PrintConfiguration => crate::install::print_configuration(), InstallOpts::EnsureCompletion {} => { let rootfs = &Dir::open_ambient_dir("/", cap_std::ambient_authority())?; diff --git a/crates/lib/src/deploy.rs b/crates/lib/src/deploy.rs index 7b7ff59e1..b13aff8f0 100644 --- a/crates/lib/src/deploy.rs +++ b/crates/lib/src/deploy.rs @@ -577,8 +577,7 @@ pub(crate) fn get_base_commit(repo: &ostree::Repo, commit: &str) -> Result, - stateroot: &str, + from: MergeState, image: &ImageState, origin: &glib::KeyFile, ) -> Result { @@ -586,16 +585,18 @@ async fn deploy( // a merge deployment. The kargs code also always looks at the booted root (which // is a distinct minor issue, but not super important as right now the install path // doesn't use this API). - let override_kargs = if let Some(deployment) = merge_deployment { - Some(crate::bootc_kargs::get_kargs(sysroot, &deployment, image)?) - } else { - None + let (stateroot, override_kargs) = match &from { + MergeState::MergeDeployment(deployment) => { + let kargs = crate::kargs::get_kargs(sysroot, &deployment, image)?; + (deployment.stateroot().into(), kargs) + } + MergeState::Reset { stateroot, kargs } => (stateroot.clone(), kargs.clone()), }; // Clone all the things to move to worker thread let ostree = sysroot.get_ostree_cloned()?; // ostree::Deployment is incorrectly !Send 😢 so convert it to an integer + let merge_deployment = from.as_merge_deployment(); let merge_deployment = merge_deployment.map(|d| d.index() as usize); - let stateroot = stateroot.to_string(); let ostree_commit = image.ostree_commit.to_string(); // GKeyFile also isn't Send! So we serialize that as a string... let origin_data = origin.to_data(); @@ -609,12 +610,11 @@ async fn deploy( // Because the C API expects a Vec<&str>, we need to generate a new Vec<> // that borrows. let override_kargs = override_kargs - .as_deref() - .map(|v| v.iter().map(|s| s.as_str()).collect::>()); - if let Some(kargs) = override_kargs.as_deref() { - opts.override_kernel_argv = Some(&kargs); - } - let deployments = ostree.deployments(); + .iter() + .map(|s| s.as_str()) + .collect::>(); + opts.override_kernel_argv = Some(&override_kargs); + let deployments = sysroot.deployments(); let merge_deployment = merge_deployment.map(|m| &deployments[m]); let origin = glib::KeyFile::new(); origin.load_from_data(&origin_data, glib::KeyFileFlags::NONE)?; @@ -649,11 +649,41 @@ fn origin_from_imageref(imgref: &ImageReference) -> Result { Ok(origin) } +/// The source of data for staging a new deployment +#[derive(Debug)] +pub(crate) enum MergeState { + /// Use the provided merge deployment + MergeDeployment(Deployment), + /// Don't use a merge deployment, but only this + /// provided initial state. + Reset { + stateroot: String, + kargs: Vec, + }, +} +impl MergeState { + /// Initialize using the default merge deployment for the given stateroot. + pub(crate) fn from_stateroot(sysroot: &Storage, stateroot: &str) -> Result { + let merge_deployment = sysroot.merge_deployment(Some(stateroot)).ok_or_else(|| { + anyhow::anyhow!("No merge deployment found for stateroot {stateroot}") + })?; + Ok(Self::MergeDeployment(merge_deployment)) + } + + /// Cast this to a merge deployment case. + pub(crate) fn as_merge_deployment(&self) -> Option<&Deployment> { + match self { + Self::MergeDeployment(d) => Some(d), + Self::Reset { .. } => None, + } + } +} + /// Stage (queue deployment of) a fetched container image. #[context("Staging")] pub(crate) async fn stage( sysroot: &Storage, - stateroot: &str, + from: MergeState, image: &ImageState, spec: &RequiredHostSpec<'_>, prog: ProgressWriter, @@ -694,7 +724,6 @@ pub(crate) async fn stage( .collect(), }) .await; - let merge_deployment = ostree.merge_deployment(Some(stateroot)); subtask.completed = true; subtasks.push(subtask.clone()); @@ -717,14 +746,7 @@ pub(crate) async fn stage( }) .await; let origin = origin_from_imageref(spec.image)?; - let deployment = crate::deploy::deploy( - sysroot, - merge_deployment.as_ref(), - stateroot, - image, - &origin, - ) - .await?; + let deployment = crate::deploy::deploy(sysroot, from, image, &origin).await?; subtask.completed = true; subtasks.push(subtask.clone()); diff --git a/crates/lib/src/install.rs b/crates/lib/src/install.rs index fe11d8e34..465593590 100644 --- a/crates/lib/src/install.rs +++ b/crates/lib/src/install.rs @@ -44,7 +44,7 @@ use ostree::gio; use ostree_ext::ostree; use ostree_ext::ostree_prepareroot::{ComposefsState, Tristate}; use ostree_ext::prelude::Cast; -use ostree_ext::sysroot::SysrootLock; +use ostree_ext::sysroot::{allocate_new_stateroot, list_stateroots, SysrootLock}; use ostree_ext::{container as ostree_container, ostree_prepareroot}; #[cfg(feature = "install-to-disk")] use rustix::fs::FileTypeExt; @@ -57,7 +57,10 @@ use self::baseline::InstallBlockDeviceOpts; use crate::bootc_composefs::{boot::setup_composefs_boot, repo::initialize_composefs_repository}; use crate::boundimage::{BoundImage, ResolvedBoundImage}; use crate::containerenv::ContainerExecutionInfo; -use crate::deploy::{prepare_for_pull, pull_from_prepared, PreparedImportMeta, PreparedPullResult}; +use crate::deploy::{ + prepare_for_pull, pull_from_prepared, MergeState, PreparedImportMeta, PreparedPullResult, +}; +use crate::kernel_cmdline::Cmdline; use crate::lsm; use crate::progress_jsonl::ProgressWriter; use crate::spec::{Bootloader, ImageReference}; @@ -393,6 +396,50 @@ pub(crate) struct InstallToExistingRootOpts { pub(crate) composefs_opts: InstallComposefsOpts, } +#[derive(Debug, clap::Parser, PartialEq, Eq)] +pub(crate) struct InstallResetOpts { + /// Acknowledge that this command is experimental. + #[clap(long)] + pub(crate) experimental: bool, + + #[clap(flatten)] + pub(crate) source_opts: InstallSourceOpts, + + #[clap(flatten)] + pub(crate) target_opts: InstallTargetOpts, + + /// Name of the target stateroot. If not provided, one will be automatically + /// generated of the form s- where starts at zero and + /// increments automatically. + #[clap(long)] + pub(crate) stateroot: Option, + + /// Don't display progress + #[clap(long)] + pub(crate) quiet: bool, + + #[clap(flatten)] + pub(crate) progress: crate::cli::ProgressOptions, + + /// Restart or reboot into the new target image. + /// + /// Currently, this option always reboots. In the future this command + /// will detect the case where no kernel changes are queued, and perform + /// a userspace-only restart. + #[clap(long)] + pub(crate) apply: bool, + + /// Skip inheriting any automatically discovered root file system kernel arguments. + #[clap(long)] + no_root_kargs: bool, + + /// Add a kernel argument. This option can be provided multiple times. + /// + /// Example: --karg=nosmt --karg=console=ttyS0,114800n8 + #[clap(long)] + karg: Option>, +} + /// Global state captured from the container. #[derive(Debug, Clone)] pub(crate) struct SourceInfo { @@ -438,6 +485,24 @@ pub(crate) struct State { pub(crate) detected_bootloader: crate::spec::Bootloader, } +impl InstallTargetOpts { + pub(crate) fn imageref(&self) -> Result> { + let Some(target_imgname) = self.target_imgref.as_deref() else { + return Ok(None); + }; + let target_transport = + ostree_container::Transport::try_from(self.target_transport.as_str())?; + let target_imgref = ostree_container::OstreeImageReference { + sigverify: ostree_container::SignatureSource::ContainerPolicyAllowInsecure, + imgref: ostree_container::ImageReference { + transport: target_transport, + name: target_imgname.to_string(), + }, + }; + Ok(Some(target_imgref)) + } +} + impl State { #[context("Loading SELinux policy")] pub(crate) fn load_policy(&self) -> Result> { @@ -2181,6 +2246,94 @@ pub(crate) async fn install_to_existing_root(opts: InstallToExistingRootOpts) -> install_to_filesystem(opts, true, cleanup).await } +pub(crate) async fn install_reset(opts: InstallResetOpts) -> Result<()> { + let rootfs = &Dir::open_ambient_dir("/", cap_std::ambient_authority())?; + if !opts.experimental { + anyhow::bail!("This command requires --experimental"); + } + + let prog: ProgressWriter = opts.progress.try_into()?; + + let sysroot = &crate::cli::get_storage().await?; + let repo = &sysroot.repo(); + let (booted_deployment, _deployments, host) = + crate::status::get_status_require_booted(sysroot)?; + + let stateroots = list_stateroots(sysroot)?; + dbg!(&stateroots); + let target_stateroot = if let Some(s) = opts.stateroot { + s + } else { + let now = chrono::Utc::now(); + let r = allocate_new_stateroot(&sysroot, &stateroots, now)?; + r.name + }; + + let booted_stateroot = booted_deployment.osname(); + assert!(booted_stateroot.as_str() != target_stateroot); + let (fetched, spec) = if let Some(target) = opts.target_opts.imageref()? { + let mut new_spec = host.spec; + new_spec.image = Some(target.into()); + let fetched = crate::deploy::pull( + repo, + &new_spec.image.as_ref().unwrap(), + None, + opts.quiet, + prog.clone(), + ) + .await?; + (fetched, new_spec) + } else { + let imgstate = host + .status + .booted + .map(|b| b.query_image(repo)) + .transpose()? + .flatten() + .ok_or_else(|| anyhow::anyhow!("No image source specified"))?; + (Box::new((*imgstate).into()), host.spec) + }; + let spec = crate::deploy::RequiredHostSpec::from_spec(&spec)?; + + // Compute the kernel arguments to inherit. By default, that's only those involved + // in the root filesystem. + let root_kargs = if opts.no_root_kargs { + Vec::new() + } else { + let bootcfg = booted_deployment + .bootconfig() + .ok_or_else(|| anyhow!("Missing bootcfg for booted deployment"))?; + if let Some(options) = bootcfg.get("options") { + let options = options.split_ascii_whitespace().collect::>(); + crate::kernel::root_args_from_cmdline(&options) + .into_iter() + .map(ToOwned::to_owned) + .collect::>() + } else { + Vec::new() + } + }; + + let kargs = crate::kargs::get_kargs_in_root(rootfs, std::env::consts::ARCH)? + .into_iter() + .chain(root_kargs.into_iter()) + .chain(opts.karg.unwrap_or_default()) + .collect::>(); + + let from = MergeState::Reset { + stateroot: target_stateroot, + kargs, + }; + crate::deploy::stage(sysroot, from, &fetched, &spec, prog.clone()).await?; + + sysroot.update_mtime()?; + + if opts.apply { + crate::reboot::reboot()?; + } + Ok(()) +} + /// Implementation of `bootc install finalize`. pub(crate) async fn install_finalize(target: &Utf8Path) -> Result<()> { // Log the installation finalization operation to systemd journal diff --git a/crates/ostree-ext/src/sysroot.rs b/crates/ostree-ext/src/sysroot.rs index 27fdb4f5a..a4bef6266 100644 --- a/crates/ostree-ext/src/sysroot.rs +++ b/crates/ostree-ext/src/sysroot.rs @@ -38,7 +38,6 @@ impl Deref for SysrootLock { } } - /// Access the file descriptor for a sysroot #[allow(unsafe_code)] pub fn sysroot_fd(sysroot: &ostree::Sysroot) -> BorrowedFd { diff --git a/lib/src/kernel.rs b/lib/src/kernel.rs new file mode 100644 index 000000000..ebd0970d0 --- /dev/null +++ b/lib/src/kernel.rs @@ -0,0 +1,66 @@ +use anyhow::Result; +use fn_error_context::context; + +/// The default root filesystem mount specification. +pub(crate) const ROOT: &str = "root="; +/// This is used by dracut. +pub(crate) const INITRD_ARG_PREFIX: &str = "rd."; +/// The kernel argument for configuring the rootfs flags. +pub(crate) const ROOTFLAGS: &str = "rootflags="; + +/// Parse the kernel command line. This is strictly +/// speaking not a correct parser, as the Linux kernel +/// supports quotes. However, we don't yet need that here. +/// +/// See systemd's code for one userspace parser. +#[context("Reading /proc/cmdline")] +pub(crate) fn parse_cmdline() -> Result> { + let cmdline = std::fs::read_to_string("/proc/cmdline")?; + let r = cmdline + .split_ascii_whitespace() + .map(ToOwned::to_owned) + .collect(); + Ok(r) +} + +/// Return the value for the string in the vector which has the form target_key=value +pub(crate) fn find_first_cmdline_arg<'a>( + args: impl Iterator, + target_key: &str, +) -> Option<&'a str> { + args.filter_map(|arg| { + if let Some((k, v)) = arg.split_once('=') { + if target_key == k { + return Some(v); + } + } + None + }) + .next() +} + +/// Find the subset of kernel argumetns which describe how to mount the root filesystem. +pub(crate) fn root_args_from_cmdline<'a>(cmdline: &'a [&str]) -> Vec<&'a str> { + cmdline + .iter() + .filter(|arg| { + arg.starts_with(ROOT) + || arg.starts_with(ROOTFLAGS) + || arg.starts_with(INITRD_ARG_PREFIX) + }) + .copied() + .collect() +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_find_first() { + let kargs = &["foo=bar", "root=/dev/vda", "blah", "root=/dev/other"]; + let kargs = || kargs.iter().copied(); + assert_eq!(find_first_cmdline_arg(kargs(), "root"), Some("/dev/vda")); + assert_eq!(find_first_cmdline_arg(kargs(), "nonexistent"), None); + } +} From dffc4560a43dc1f11f734bc944488f8e8c74003e Mon Sep 17 00:00:00 2001 From: ckyrouac Date: Wed, 20 Aug 2025 11:53:46 -0400 Subject: [PATCH 3/6] reset: Move kernel code into bootc_kargs Also cleans up some bugs from rebasing previous commits. Signed-off-by: ckyrouac --- crates/lib/src/bootc_kargs.rs | 19 +++++++ crates/lib/src/deploy.rs | 9 ++-- crates/lib/src/install.rs | 16 +++--- crates/ostree-ext/src/container/deploy.rs | 10 +--- crates/ostree-ext/src/sysroot.rs | 2 +- lib/src/kernel.rs | 66 ----------------------- 6 files changed, 32 insertions(+), 90 deletions(-) delete mode 100644 lib/src/kernel.rs diff --git a/crates/lib/src/bootc_kargs.rs b/crates/lib/src/bootc_kargs.rs index ec1be4e6c..85fcac10b 100644 --- a/crates/lib/src/bootc_kargs.rs +++ b/crates/lib/src/bootc_kargs.rs @@ -19,6 +19,13 @@ use crate::store::Storage; /// The relative path to the kernel arguments which may be embedded in an image. const KARGS_PATH: &str = "usr/lib/bootc/kargs.d"; +/// The default root filesystem mount specification. +pub(crate) const ROOT: &str = "root="; +/// This is used by dracut. +pub(crate) const INITRD_ARG_PREFIX: &str = "rd."; +/// The kernel argument for configuring the rootfs flags. +pub(crate) const ROOTFLAGS: &str = "rootflags="; + /// The kargs.d configuration file. #[derive(Deserialize)] #[serde(rename_all = "kebab-case", deny_unknown_fields)] @@ -54,6 +61,18 @@ pub(crate) fn get_kargs_in_root(d: &Dir, sys_arch: &str) -> Result> Ok(ret) } +pub(crate) fn root_args_from_cmdline<'a>(cmdline: &'a [&str]) -> Vec<&'a str> { + cmdline + .iter() + .filter(|arg| { + arg.starts_with(ROOT) + || arg.starts_with(ROOTFLAGS) + || arg.starts_with(INITRD_ARG_PREFIX) + }) + .copied() + .collect() +} + /// Load kargs.d files from the target ostree commit root pub(crate) fn get_kargs_from_ostree_root( repo: &ostree::Repo, diff --git a/crates/lib/src/deploy.rs b/crates/lib/src/deploy.rs index b13aff8f0..d8cd9a999 100644 --- a/crates/lib/src/deploy.rs +++ b/crates/lib/src/deploy.rs @@ -587,7 +587,7 @@ async fn deploy( // doesn't use this API). let (stateroot, override_kargs) = match &from { MergeState::MergeDeployment(deployment) => { - let kargs = crate::kargs::get_kargs(sysroot, &deployment, image)?; + let kargs = crate::bootc_kargs::get_kargs(sysroot, &deployment, image)?; (deployment.stateroot().into(), kargs) } MergeState::Reset { stateroot, kargs } => (stateroot.clone(), kargs.clone()), @@ -614,7 +614,7 @@ async fn deploy( .map(|s| s.as_str()) .collect::>(); opts.override_kernel_argv = Some(&override_kargs); - let deployments = sysroot.deployments(); + let deployments = ostree.deployments(); let merge_deployment = merge_deployment.map(|m| &deployments[m]); let origin = glib::KeyFile::new(); origin.load_from_data(&origin_data, glib::KeyFileFlags::NONE)?; @@ -664,7 +664,8 @@ pub(crate) enum MergeState { impl MergeState { /// Initialize using the default merge deployment for the given stateroot. pub(crate) fn from_stateroot(sysroot: &Storage, stateroot: &str) -> Result { - let merge_deployment = sysroot.merge_deployment(Some(stateroot)).ok_or_else(|| { + let ostree = sysroot.get_ostree()?; + let merge_deployment = ostree.merge_deployment(Some(stateroot)).ok_or_else(|| { anyhow::anyhow!("No merge deployment found for stateroot {stateroot}") })?; Ok(Self::MergeDeployment(merge_deployment)) @@ -696,13 +697,11 @@ pub(crate) async fn stage( bootc.image.reference = &spec.image.image, bootc.image.transport = &spec.image.transport, bootc.manifest_digest = image.manifest_digest.as_ref(), - bootc.stateroot = stateroot, "Staging image for deployment: {} (digest: {})", spec.image, image.manifest_digest ); - let ostree = sysroot.get_ostree()?; let mut subtask = SubTaskStep { subtask: "merging".into(), description: "Merging Image".into(), diff --git a/crates/lib/src/install.rs b/crates/lib/src/install.rs index 465593590..a132a1964 100644 --- a/crates/lib/src/install.rs +++ b/crates/lib/src/install.rs @@ -60,7 +60,6 @@ use crate::containerenv::ContainerExecutionInfo; use crate::deploy::{ prepare_for_pull, pull_from_prepared, MergeState, PreparedImportMeta, PreparedPullResult, }; -use crate::kernel_cmdline::Cmdline; use crate::lsm; use crate::progress_jsonl::ProgressWriter; use crate::spec::{Bootloader, ImageReference}; @@ -2255,17 +2254,16 @@ pub(crate) async fn install_reset(opts: InstallResetOpts) -> Result<()> { let prog: ProgressWriter = opts.progress.try_into()?; let sysroot = &crate::cli::get_storage().await?; - let repo = &sysroot.repo(); - let (booted_deployment, _deployments, host) = - crate::status::get_status_require_booted(sysroot)?; + let ostree = sysroot.get_ostree()?; + let repo = &ostree.repo(); + let (booted_deployment, _deployments, host) = crate::status::get_status_require_booted(ostree)?; - let stateroots = list_stateroots(sysroot)?; - dbg!(&stateroots); + let stateroots = list_stateroots(ostree)?; let target_stateroot = if let Some(s) = opts.stateroot { s } else { let now = chrono::Utc::now(); - let r = allocate_new_stateroot(&sysroot, &stateroots, now)?; + let r = allocate_new_stateroot(&ostree, &stateroots, now)?; r.name }; @@ -2305,7 +2303,7 @@ pub(crate) async fn install_reset(opts: InstallResetOpts) -> Result<()> { .ok_or_else(|| anyhow!("Missing bootcfg for booted deployment"))?; if let Some(options) = bootcfg.get("options") { let options = options.split_ascii_whitespace().collect::>(); - crate::kernel::root_args_from_cmdline(&options) + crate::bootc_kargs::root_args_from_cmdline(&options) .into_iter() .map(ToOwned::to_owned) .collect::>() @@ -2314,7 +2312,7 @@ pub(crate) async fn install_reset(opts: InstallResetOpts) -> Result<()> { } }; - let kargs = crate::kargs::get_kargs_in_root(rootfs, std::env::consts::ARCH)? + let kargs = crate::bootc_kargs::get_kargs_in_root(rootfs, std::env::consts::ARCH)? .into_iter() .chain(root_kargs.into_iter()) .chain(opts.karg.unwrap_or_default()) diff --git a/crates/ostree-ext/src/container/deploy.rs b/crates/ostree-ext/src/container/deploy.rs index 3b6f9c2a0..32345f8e8 100644 --- a/crates/ostree-ext/src/container/deploy.rs +++ b/crates/ostree-ext/src/container/deploy.rs @@ -1,10 +1,9 @@ //! Perform initial setup for a container image based system root -use std::collections::HashSet; - use anyhow::Result; use fn_error_context::context; use ostree::glib; +use std::collections::HashSet; use super::store::{gc_image_layers, LayeredImageState}; use super::{ImageReference, OstreeImageReference}; @@ -51,13 +50,6 @@ pub struct DeployOpts<'a> { pub no_clean: bool, } -// Access the file descriptor for a sysroot -#[allow(unsafe_code)] -#[cfg(feature = "bootc")] -pub(crate) fn sysroot_fd(sysroot: &ostree::Sysroot) -> BorrowedFd<'_> { - unsafe { BorrowedFd::borrow_raw(sysroot.fd()) } -} - /// Write a container image to an OSTree deployment. /// /// This API is currently intended for only an initial deployment. diff --git a/crates/ostree-ext/src/sysroot.rs b/crates/ostree-ext/src/sysroot.rs index a4bef6266..ec1e0be9f 100644 --- a/crates/ostree-ext/src/sysroot.rs +++ b/crates/ostree-ext/src/sysroot.rs @@ -40,7 +40,7 @@ impl Deref for SysrootLock { /// Access the file descriptor for a sysroot #[allow(unsafe_code)] -pub fn sysroot_fd(sysroot: &ostree::Sysroot) -> BorrowedFd { +pub fn sysroot_fd(sysroot: &ostree::Sysroot) -> BorrowedFd<'_> { unsafe { BorrowedFd::borrow_raw(sysroot.fd()) } } diff --git a/lib/src/kernel.rs b/lib/src/kernel.rs deleted file mode 100644 index ebd0970d0..000000000 --- a/lib/src/kernel.rs +++ /dev/null @@ -1,66 +0,0 @@ -use anyhow::Result; -use fn_error_context::context; - -/// The default root filesystem mount specification. -pub(crate) const ROOT: &str = "root="; -/// This is used by dracut. -pub(crate) const INITRD_ARG_PREFIX: &str = "rd."; -/// The kernel argument for configuring the rootfs flags. -pub(crate) const ROOTFLAGS: &str = "rootflags="; - -/// Parse the kernel command line. This is strictly -/// speaking not a correct parser, as the Linux kernel -/// supports quotes. However, we don't yet need that here. -/// -/// See systemd's code for one userspace parser. -#[context("Reading /proc/cmdline")] -pub(crate) fn parse_cmdline() -> Result> { - let cmdline = std::fs::read_to_string("/proc/cmdline")?; - let r = cmdline - .split_ascii_whitespace() - .map(ToOwned::to_owned) - .collect(); - Ok(r) -} - -/// Return the value for the string in the vector which has the form target_key=value -pub(crate) fn find_first_cmdline_arg<'a>( - args: impl Iterator, - target_key: &str, -) -> Option<&'a str> { - args.filter_map(|arg| { - if let Some((k, v)) = arg.split_once('=') { - if target_key == k { - return Some(v); - } - } - None - }) - .next() -} - -/// Find the subset of kernel argumetns which describe how to mount the root filesystem. -pub(crate) fn root_args_from_cmdline<'a>(cmdline: &'a [&str]) -> Vec<&'a str> { - cmdline - .iter() - .filter(|arg| { - arg.starts_with(ROOT) - || arg.starts_with(ROOTFLAGS) - || arg.starts_with(INITRD_ARG_PREFIX) - }) - .copied() - .collect() -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_find_first() { - let kargs = &["foo=bar", "root=/dev/vda", "blah", "root=/dev/other"]; - let kargs = || kargs.iter().copied(); - assert_eq!(find_first_cmdline_arg(kargs(), "root"), Some("/dev/vda")); - assert_eq!(find_first_cmdline_arg(kargs(), "nonexistent"), None); - } -} From 24203417d18cb3ef5f4607185181291e673c6a3f Mon Sep 17 00:00:00 2001 From: ckyrouac Date: Tue, 23 Sep 2025 12:54:48 -0400 Subject: [PATCH 4/6] reset: Initial tmt test for factory reset Signed-off-by: ckyrouac --- crates/lib/src/cli.rs | 2 +- tmt/plans/integration.fmf | 7 +++ tmt/tests/booted/test-factory-reset.nu | 68 ++++++++++++++++++++++++++ tmt/tests/test-28-factory-reset.fmf | 3 ++ 4 files changed, 79 insertions(+), 1 deletion(-) create mode 100644 tmt/tests/booted/test-factory-reset.nu create mode 100644 tmt/tests/test-28-factory-reset.fmf diff --git a/crates/lib/src/cli.rs b/crates/lib/src/cli.rs index ebcce62fc..6974f1cb8 100644 --- a/crates/lib/src/cli.rs +++ b/crates/lib/src/cli.rs @@ -42,7 +42,7 @@ use crate::bootc_composefs::{ switch::switch_composefs, update::upgrade_composefs, }; -use crate::deploy::{MergeState, RequiredHostSpec, RequiredHostSpec}; +use crate::deploy::{MergeState, RequiredHostSpec}; use crate::lints; use crate::podstorage::set_additional_image_store; use crate::progress_jsonl::{ProgressWriter, RawProgressFd}; diff --git a/tmt/plans/integration.fmf b/tmt/plans/integration.fmf index 173a31e58..bca4bbe41 100644 --- a/tmt/plans/integration.fmf +++ b/tmt/plans/integration.fmf @@ -105,3 +105,10 @@ execute: - when: running_env != image_mode enabled: false because: tmt-reboot does not work with systemd reboot in testing farm environment + +/test-28-factory-reset: + summary: Factory reset + discover: + how: fmf + test: + - /tmt/tests/test-28-factory-reset diff --git a/tmt/tests/booted/test-factory-reset.nu b/tmt/tests/booted/test-factory-reset.nu new file mode 100644 index 000000000..d0a8debf2 --- /dev/null +++ b/tmt/tests/booted/test-factory-reset.nu @@ -0,0 +1,68 @@ +use std assert +use tap.nu + +def initial_build [] { + tap begin "factory reset test" + + # Create test files that should be removed after factory reset + print "Creating test files in /var and /etc" + echo "test file in var" | save /var/test-file-factory-reset + echo "test file in etc" | save /etc/test-file-factory-reset + + # Verify files were created + assert ("/var/test-file-factory-reset" | path exists) + assert ("/etc/test-file-factory-reset" | path exists) + + bootc install reset --experimental + + # sanity check that bootc status shows a new deployment with a non default stateroot + let reset_status = bootc status --json | from json + assert not equal $reset_status.status.otherDeployments.0.ostree.stateroot "default" + + # we need tmt in the new stateroot for second_boot + print "Copying tmt into new stateroot" + + # Get the new stateroot name from the staged deployment + let new_stateroot = $reset_status.status.otherDeployments.0.ostree.stateroot + print $"New stateroot: ($new_stateroot)" + + # Mount /sysroot as read-write and copy tmt directory to the new deployment + mount -o remount,rw /sysroot + + let new_stateroot_path = $"/sysroot/ostree/deploy/($new_stateroot)" + + # locate the workdir_root by looking backwards from a known static dir (TMT_PLAN_DATA) + # e.g. TMT_PLAN_DATA=/var/tmp/tmt/run-035/tmt/plans/integration/test-28-factory-reset/data + let workdir_root = ($env.TMT_PLAN_DATA | path dirname | path dirname | path dirname | path dirname | path dirname | path dirname ) + + # make sure workdir_root's full path exists in new stateroot + mkdir $"($new_stateroot_path)/($workdir_root)" + + # nu's cp doesn't have -T + /usr/bin/cp -r -T $workdir_root $"($new_stateroot_path)/($workdir_root)" + + tmt-reboot +} + +# The second boot; verify we're in the factory reset deployment +def second_boot [] { + print "Verifying factory reset completed successfully" + RUST_LOG=trace bootc status + let status = bootc status --json | from json + assert not equal $status.status.booted.ostree.stateroot "default" + + print "Checking that test files do not exist in the reset deployment" + assert (not ("/var/test-file-factory-reset" | path exists)) "Test file in /var should not exist after factory reset" + assert (not ("/etc/test-file-factory-reset" | path exists)) "Test file in /etc should not exist after factory reset" + print "Factory reset verification completed successfully" + tap ok +} + +def main [] { + # See https://tmt.readthedocs.io/en/stable/stories/features.html#reboot-during-test + match $env.TMT_REBOOT_COUNT? { + null | "0" => initial_build, + "1" => second_boot, + $o => { error make { msg: $"Invalid TMT_REBOOT_COUNT ($o)" } }, + } +} diff --git a/tmt/tests/test-28-factory-reset.fmf b/tmt/tests/test-28-factory-reset.fmf new file mode 100644 index 000000000..4772798e0 --- /dev/null +++ b/tmt/tests/test-28-factory-reset.fmf @@ -0,0 +1,3 @@ +summary: Execute factory reset tests +test: nu booted/test-factory-reset.nu +duration: 30m From f6b1249a8a7bda9587deb8b2478654dd27a0bcab Mon Sep 17 00:00:00 2001 From: ckyrouac Date: Tue, 28 Oct 2025 10:19:52 -0400 Subject: [PATCH 5/6] status: Check ostree version before enabling soft reboot Add ostree version check to has_soft_reboot_capability() to ensure soft reboot is disabled when ostree < 2025.7 and ostree= karg is missing. This prevents attempting soft reboots on older ostree versions that have a bug when validating kargs during a factory reset. Signed-off-by: ckyrouac --- crates/lib/src/status.rs | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/crates/lib/src/status.rs b/crates/lib/src/status.rs index 22741e2b2..bbce2537f 100644 --- a/crates/lib/src/status.rs +++ b/crates/lib/src/status.rs @@ -95,7 +95,25 @@ impl From for OstreeImageReference { /// Check if a deployment has soft reboot capability fn has_soft_reboot_capability(sysroot: &SysrootLock, deployment: &ostree::Deployment) -> bool { - ostree_ext::systemd_has_soft_reboot() && sysroot.deployment_can_soft_reboot(deployment) + if !ostree_ext::systemd_has_soft_reboot() { + return false; + } + + // When the ostree version is < 2025.7 and the deployment is + // missing the ostree= karg (happens during a factory reset), + // there is a bug that causes deployment_can_soft_reboot to crash. + // So in this case default to disabling soft reboot. + let has_ostree_karg = deployment + .bootconfig() + .and_then(|bootcfg| bootcfg.get("options")) + .map(|options| options.contains("ostree=")) + .unwrap_or(false); + + if !ostree::check_version(2025, 7) && !has_ostree_karg { + return false; + } + + sysroot.deployment_can_soft_reboot(deployment) } /// Parse an ostree origin file (a keyfile) and extract the targeted From c5eeb09582614c95460327d923b6562ea5db37a9 Mon Sep 17 00:00:00 2001 From: ckyrouac Date: Wed, 29 Oct 2025 10:28:38 -0400 Subject: [PATCH 6/6] reset: Preserve /boot fstab entry in new deployment Add logic to copy the /boot mount specification from the existing /etc/fstab to the newly created stateroot during factory reset. This ensures that the /boot partition configuration is preserved across the reset operation. Includes helper function read_boot_fstab_entry() to parse /etc/fstab and locate the /boot mount entry, along with comprehensive unit tests. Assisted-by: Claude Code Signed-off-by: ckyrouac --- crates/lib/src/install.rs | 98 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 97 insertions(+), 1 deletion(-) diff --git a/crates/lib/src/install.rs b/crates/lib/src/install.rs index a132a1964..e6a20d0f9 100644 --- a/crates/lib/src/install.rs +++ b/crates/lib/src/install.rs @@ -2245,6 +2245,36 @@ pub(crate) async fn install_to_existing_root(opts: InstallToExistingRootOpts) -> install_to_filesystem(opts, true, cleanup).await } +/// Read the /boot entry from /etc/fstab, if it exists +fn read_boot_fstab_entry(root: &Dir) -> Result> { + let fstab_path = "etc/fstab"; + let fstab = match root.open_optional(fstab_path)? { + Some(f) => f, + None => return Ok(None), + }; + + let reader = std::io::BufReader::new(fstab); + for line in std::io::BufRead::lines(reader) { + let line = line?; + let line = line.trim(); + + // Skip empty lines and comments + if line.is_empty() || line.starts_with('#') { + continue; + } + + // Parse the mount spec + let spec = MountSpec::from_str(line)?; + + // Check if this is a /boot entry + if spec.target == "/boot" { + return Ok(Some(spec)); + } + } + + Ok(None) +} + pub(crate) async fn install_reset(opts: InstallResetOpts) -> Result<()> { let rootfs = &Dir::open_ambient_dir("/", cap_std::ambient_authority())?; if !opts.experimental { @@ -2319,11 +2349,35 @@ pub(crate) async fn install_reset(opts: InstallResetOpts) -> Result<()> { .collect::>(); let from = MergeState::Reset { - stateroot: target_stateroot, + stateroot: target_stateroot.clone(), kargs, }; crate::deploy::stage(sysroot, from, &fetched, &spec, prog.clone()).await?; + // Copy /boot entry from /etc/fstab to the new stateroot if it exists + if let Some(boot_spec) = read_boot_fstab_entry(rootfs)? { + let staged_deployment = ostree + .staged_deployment() + .ok_or_else(|| anyhow!("No staged deployment found"))?; + let deployment_path = ostree.deployment_dirpath(&staged_deployment); + let sysroot_dir = crate::utils::sysroot_dir(ostree)?; + let deployment_root = sysroot_dir.open_dir(&deployment_path)?; + + // Write the /boot entry to /etc/fstab in the new deployment + crate::lsm::atomic_replace_labeled( + &deployment_root, + "etc/fstab", + 0o644.into(), + None, + |w| writeln!(w, "{}", boot_spec.to_fstab()).map_err(Into::into), + )?; + + tracing::debug!( + "Copied /boot entry to new stateroot: {}", + boot_spec.to_fstab() + ); + } + sysroot.update_mtime()?; if opts.apply { @@ -2457,4 +2511,46 @@ mod tests { Ok(()) } + + #[test] + fn test_read_boot_fstab_entry() -> Result<()> { + let td = cap_std_ext::cap_tempfile::TempDir::new(cap_std::ambient_authority())?; + + // Test with no /etc/fstab + assert!(read_boot_fstab_entry(&td)?.is_none()); + + // Test with /etc/fstab but no /boot entry + td.create_dir("etc")?; + td.write("etc/fstab", "UUID=test-uuid / ext4 defaults 0 0\n")?; + assert!(read_boot_fstab_entry(&td)?.is_none()); + + // Test with /boot entry + let fstab_content = "\ +# /etc/fstab +UUID=root-uuid / ext4 defaults 0 0 +UUID=boot-uuid /boot ext4 ro 0 0 +UUID=home-uuid /home ext4 defaults 0 0 +"; + td.write("etc/fstab", fstab_content)?; + let boot_spec = read_boot_fstab_entry(&td)?.unwrap(); + assert_eq!(boot_spec.source, "UUID=boot-uuid"); + assert_eq!(boot_spec.target, "/boot"); + assert_eq!(boot_spec.fstype, "ext4"); + assert_eq!(boot_spec.options, Some("ro".to_string())); + + // Test with /boot entry with comments + let fstab_content = "\ +# /etc/fstab +# Created by anaconda +UUID=root-uuid / ext4 defaults 0 0 +# Boot partition +UUID=boot-uuid /boot ext4 defaults 0 0 +"; + td.write("etc/fstab", fstab_content)?; + let boot_spec = read_boot_fstab_entry(&td)?.unwrap(); + assert_eq!(boot_spec.source, "UUID=boot-uuid"); + assert_eq!(boot_spec.target, "/boot"); + + Ok(()) + } }