From 30b3e31b6934659b97527ce61fd4e4f4c039511a Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sat, 30 Sep 2023 15:39:08 -0400 Subject: [PATCH] Add support for `--replace-mode=alongside` for ostree target Ironically our support for `--replace-mode=alongside` breaks when we're targeting an already extant ostree host, because when we first blow away the `/boot` directory, this means the ostree stack loses its knowledge that we're in a booted deployment, and will attempt to GC it... https://github.com/ostreedev/ostree-rs-ext/pull/550/commits/8fa019bfa821303cfb7a7f069ae2320f4c3800fa is a key part of the fix for that. However, a notable improvement we can do here is to grow this whole thing into a real "factory reset" mode, and this will be a compelling answer to https://github.com/coreos/fedora-coreos-tracker/issues/399 To implement this though we need to support configuring the stateroot and not just hardcode `default`. Signed-off-by: Omer Tuchfeld --- Cargo.toml | 2 +- lib/src/install.rs | 84 ++++++++++++++++++++++++++------ lib/src/utils.rs | 21 +++++++- tests-integration/src/install.rs | 20 ++++++++ 4 files changed, 110 insertions(+), 17 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 0f775d4a7..7d5381cda 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,7 +20,7 @@ lto = "yes" [workspace.dependencies] anyhow = "1.0.82" camino = "1.1.6" -cap-std-ext = "4.0.2" +cap-std-ext = "4.0.3" chrono = { version = "0.4.38", default-features = false } clap = "4.5.4" indoc = "2.0.5" diff --git a/lib/src/install.rs b/lib/src/install.rs index 29e747081..2fb988dc0 100644 --- a/lib/src/install.rs +++ b/lib/src/install.rs @@ -20,8 +20,8 @@ use std::str::FromStr; use std::sync::Arc; use std::time::Duration; -use anyhow::Ok; use anyhow::{anyhow, Context, Result}; +use anyhow::{ensure, Ok}; use bootc_utils::CommandRunExt; use camino::Utf8Path; use camino::Utf8PathBuf; @@ -564,7 +564,7 @@ pub(crate) fn print_configuration() -> Result<()> { } #[context("Creating ostree deployment")] -async fn initialize_ostree_root(state: &State, root_setup: &RootSetup) -> Result { +async fn initialize_ostree_root(state: &State, root_setup: &RootSetup) -> Result<(Storage, bool)> { let sepolicy = state.load_policy()?; let sepolicy = sepolicy.as_ref(); // Load a fd for the mounted target physical root @@ -572,18 +572,26 @@ async fn initialize_ostree_root(state: &State, root_setup: &RootSetup) -> Result let rootfs = root_setup.rootfs.as_path(); let cancellable = gio::Cancellable::NONE; + let stateroot = state.stateroot(); + + let has_ostree = rootfs_dir.try_exists("ostree/repo")?; + if !has_ostree { + Task::new_and_run( + "Initializing ostree layout", + "ostree", + ["admin", "init-fs", "--modern", rootfs.as_str()], + )?; + } else { + println!("Reusing extant ostree layout"); + let path = ".".into(); + let _ = crate::utils::open_dir_remount_rw(rootfs_dir, path) + .context("remounting sysroot as read-write")?; + } + // Ensure that the physical root is labeled. // Another implementation: https://github.com/coreos/coreos-assembler/blob/3cd3307904593b3a131b81567b13a4d0b6fe7c90/src/create_disk.sh#L295 crate::lsm::ensure_dir_labeled(rootfs_dir, "", Some("/".into()), 0o755.into(), sepolicy)?; - let stateroot = state.stateroot(); - - Task::new_and_run( - "Initializing ostree layout", - "ostree", - ["admin", "init-fs", "--modern", rootfs.as_str()], - )?; - // And also label /boot AKA xbootldr, if it exists let bootdir = rootfs.join("boot"); if bootdir.try_exists()? { @@ -607,6 +615,11 @@ async fn initialize_ostree_root(state: &State, root_setup: &RootSetup) -> Result let sysroot = ostree::Sysroot::new(Some(&gio::File::for_path(rootfs))); sysroot.load(cancellable)?; + let stateroot_exists = rootfs_dir.try_exists(format!("ostree/deploy/{stateroot}"))?; + ensure!( + !stateroot_exists, + "Cannot redeploy over extant stateroot {stateroot}" + ); sysroot .init_osname(stateroot, cancellable) .context("initializing stateroot")?; @@ -637,7 +650,7 @@ async fn initialize_ostree_root(state: &State, root_setup: &RootSetup) -> Result let sysroot = ostree::Sysroot::new(Some(&gio::File::for_path(rootfs))); sysroot.load(cancellable)?; let sysroot = SysrootLock::new_from_sysroot(&sysroot).await?; - Storage::new(sysroot, &temp_run) + Ok((Storage::new(sysroot, &temp_run)?, has_ostree)) } #[context("Creating ostree deployment")] @@ -645,6 +658,7 @@ async fn install_container( state: &State, root_setup: &RootSetup, sysroot: &ostree::Sysroot, + has_ostree: bool, ) -> Result<(ostree::Deployment, InstallAleph)> { let sepolicy = state.load_policy()?; let sepolicy = sepolicy.as_ref(); @@ -730,6 +744,7 @@ async fn install_container( options.kargs = Some(kargs.as_slice()); options.target_imgref = Some(&state.target_imgref); options.proxy_cfg = proxy_cfg; + options.no_clean = has_ostree; let imgstate = crate::utils::async_task_with_spinner( "Deploying container image", ostree_container::deploy::deploy(&sysroot, stateroot, &src_imageref, Some(options)), @@ -1272,10 +1287,11 @@ async fn install_with_sysroot( sysroot: &Storage, boot_uuid: &str, bound_images: &[crate::boundimage::ResolvedBoundImage], + has_ostree: bool, ) -> Result<()> { // And actually set up the container in that root, returning a deployment and // the aleph state (see below). - let (_deployment, aleph) = install_container(state, rootfs, &sysroot).await?; + let (_deployment, aleph) = install_container(state, rootfs, &sysroot, has_ostree).await?; // Write the aleph data that captures the system state at the time of provisioning for aid in future debugging. rootfs .rootfs_fd @@ -1336,6 +1352,12 @@ async fn install_to_filesystem_impl(state: &State, rootfs: &mut RootSetup) -> Re .ok_or_else(|| anyhow!("No uuid for boot/root"))?; tracing::debug!("boot uuid={boot_uuid}"); + // If we're doing an alongside install, then the /dev bootupd sees needs to be the host's. + ensure!( + crate::mount::is_same_as_host(Utf8Path::new("/dev"))?, + "Missing /dev mount to host /dev" + ); + let bound_images = if state.config_opts.skip_bound_images { Vec::new() } else { @@ -1356,8 +1378,16 @@ async fn install_to_filesystem_impl(state: &State, rootfs: &mut RootSetup) -> Re // Initialize the ostree sysroot (repo, stateroot, etc.) { - let sysroot = initialize_ostree_root(state, rootfs).await?; - install_with_sysroot(state, rootfs, &sysroot, &boot_uuid, &bound_images).await?; + let (sysroot, has_ostree) = initialize_ostree_root(state, rootfs).await?; + install_with_sysroot( + state, + rootfs, + &sysroot, + &boot_uuid, + &bound_images, + has_ostree, + ) + .await?; // We must drop the sysroot here in order to close any open file // descriptors. } @@ -1498,7 +1528,8 @@ fn remove_all_in_dir_no_xdev(d: &Dir) -> Result<()> { #[context("Removing boot directory content")] fn clean_boot_directories(rootfs: &Dir) -> Result<()> { - let bootdir = rootfs.open_dir(BOOT).context("Opening /boot")?; + let bootdir = + crate::utils::open_dir_remount_rw(rootfs, BOOT.into()).context("Opening /boot")?; // This should not remove /boot/efi note. remove_all_in_dir_no_xdev(&bootdir)?; if ARCH_USES_EFI { @@ -1589,12 +1620,35 @@ pub(crate) async fn install_to_filesystem( if !st.is_dir() { anyhow::bail!("Not a directory: {root_path}"); } + + let inspect = crate::mount::inspect_filesystem(&fsopts.root_path)?; + + let alternative_root = fsopts.root_path.join("sysroot"); + let root_path = match inspect.fstype.as_str() { + // Our target filesystem is an overlay, the true root is in `/sysroot` + "overlay" => { + tracing::debug!( + "Overlay filesystem detected, using {alternative_root} instead of {root_path} as target root" + ); + &alternative_root + } + _ => root_path, + }; let rootfs_fd = Dir::open_ambient_dir(root_path, cap_std::ambient_authority()) .with_context(|| format!("Opening target root directory {root_path}"))?; + + tracing::debug!("Root filesystem: {root_path}"); + if let Some(false) = ostree_ext::mountutil::is_mountpoint(&rootfs_fd, ".")? { anyhow::bail!("Not a mountpoint: {root_path}"); } + let fsopts = { + let mut fsopts = fsopts.clone(); + fsopts.root_path = root_path.clone(); + fsopts + }; + // Gather global state, destructuring the provided options. // IMPORTANT: We might re-execute the current process in this function (for SELinux among other things) // IMPORTANT: and hence anything that is done before MUST BE IDEMPOTENT. diff --git a/lib/src/utils.rs b/lib/src/utils.rs index 81b253fe2..d4240c7c7 100644 --- a/lib/src/utils.rs +++ b/lib/src/utils.rs @@ -5,7 +5,10 @@ use std::process::Command; use std::time::Duration; use anyhow::{Context, Result}; -use cap_std_ext::cap_std::fs::Dir; +use camino::Utf8Path; +use cap_std_ext::dirext::CapStdExtDirExt; +use cap_std_ext::{cap_std::fs::Dir, prelude::CapStdExtCommandExt}; +use fn_error_context::context; use ostree::glib; use ostree_ext::container::SignatureSource; use ostree_ext::ostree; @@ -55,6 +58,22 @@ pub(crate) fn find_mount_option<'a>( .next() } +/// Given a target directory, if it's a read-only mount, then remount it writable +#[context("Opening {target} with writable mount")] +#[cfg(feature = "install")] +pub(crate) fn open_dir_remount_rw(root: &Dir, target: &Utf8Path) -> Result { + if matches!(root.is_mountpoint(target), Ok(Some(true))) { + tracing::debug!("Target {target} is a mountpoint, remounting rw"); + let st = Command::new("mount") + .args(["-o", "remount,rw", target.as_str()]) + .cwd_dir(root.try_clone()?) + .status()?; + + anyhow::ensure!(st.success(), "Failed to remount: {st:?}"); + } + root.open_dir(target).map_err(anyhow::Error::new) +} + pub(crate) fn spawn_editor(tmpf: &tempfile::NamedTempFile) -> Result<()> { let editor_variables = ["EDITOR"]; // These roughly match https://github.com/systemd/systemd/blob/769ca9ab557b19ee9fb5c5106995506cace4c68f/src/shared/edit-util.c#L275 diff --git a/tests-integration/src/install.rs b/tests-integration/src/install.rs index d7c046937..8485f63ba 100644 --- a/tests-integration/src/install.rs +++ b/tests-integration/src/install.rs @@ -27,6 +27,7 @@ pub(crate) const BASE_ARGS: &[&str] = &[ // Arbitrary const NON_DEFAULT_STATEROOT: &str = "foo"; +const SOME_OTHER_STATEROOT: &str = "bar"; /// Clear out and delete any ostree roots, leverage bootc hidden wipe-ostree command to get rid of /// otherwise hard to delete deployment files @@ -157,6 +158,25 @@ pub(crate) fn run_alongside(image: &str, mut testargs: libtest_mimic::Arguments) ); Ok(()) }), + Trial::test("Install on already ostree target", move || { + // Do an initial install just to get ostree on our system + let sh = &xshell::Shell::new()?; + reset_root(sh, image)?; + cmd!(sh, "sudo {BASE_ARGS...} {target_args...} {image} bootc install to-existing-root --stateroot {NON_DEFAULT_STATEROOT} --acknowledge-destructive {generic_inst_args...}").run()?; + generic_post_install_verification()?; + assert!( + Utf8Path::new(&format!("/ostree/deploy/{NON_DEFAULT_STATEROOT}")).try_exists()? + ); + + // Now try again to a different stateroot + let sh = &xshell::Shell::new()?; + cmd!(sh, "sudo {BASE_ARGS...} {target_args...} {image} bootc install to-existing-root --replace alongside --stateroot {SOME_OTHER_STATEROOT} --acknowledge-destructive {generic_inst_args...}").run()?; + generic_post_install_verification()?; + assert!( + Utf8Path::new(&format!("/ostree/deploy/{SOME_OTHER_STATEROOT}")).try_exists()? + ); + Ok(()) + }), Trial::test("without an install config", move || { let sh = &xshell::Shell::new()?; reset_root(sh, image)?;