From 835d036f2500fbea4a2d2ae73af93c9897ac92b4 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 24 Sep 2025 20:12:58 -0400 Subject: [PATCH 1/2] build-sys: Remove previous bootc units As soon as we went from a single binary to shipping systemd units, we can't just blindly `COPY /new /` as it will leak old files. In the end as the comment says we probably need to go to building packages eventually. Signed-off-by: Colin Walters --- Dockerfile | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index 5878b51e7..3fe6785b8 100644 --- a/Dockerfile +++ b/Dockerfile @@ -87,8 +87,15 @@ RUN --mount=type=cache,target=/build/target --mount=type=cache,target=/var/rooth # The final image that derives from the original base and adds the release binaries FROM base -# First, create a layer that is our new binaries. +RUN < Date: Fri, 12 Sep 2025 13:13:53 -0400 Subject: [PATCH 2/2] generator: Conditinally enable bootc-status units Right now this service fails in `bcvk run-ephemeral`, but also likely fails in any non-bootc system that has `subscription-manager` installed. A problem is that dependencies of units are started even if the dependee has a condition that disables it. This basically the target and path depend on `/run/ostree-booted` being present (which yes, won't work for composefs...) Tests: Covered by extant `012-test-unit-status.nu` Signed-off-by: Colin Walters --- Makefile | 10 -- crates/lib/src/generator.rs | 104 +++++++++++++++++- crates/lib/src/install.rs | 4 +- .../tests-integration/src/system_reinstall.rs | 1 - systemd/bootc-destructive-cleanup.service | 4 +- systemd/bootc-status-updated-onboot.target | 7 +- systemd/bootc-status-updated.path | 4 +- 7 files changed, 109 insertions(+), 25 deletions(-) diff --git a/Makefile b/Makefile index d8ce2002f..483a6b12d 100644 --- a/Makefile +++ b/Makefile @@ -48,9 +48,6 @@ install: install -D -m 0644 -t $(DESTDIR)$(prefix)/share/man/man5 target/man/*.5; \ install -D -m 0644 -t $(DESTDIR)$(prefix)/share/man/man8 target/man/*.8; \ install -D -m 0644 -t $(DESTDIR)/$(prefix)/lib/systemd/system systemd/*.service systemd/*.timer systemd/*.path systemd/*.target - install -d -m 0755 $(DESTDIR)/$(prefix)/lib/systemd/system/multi-user.target.wants - ln -s ../bootc-status-updated.path $(DESTDIR)/$(prefix)/lib/systemd/system/multi-user.target.wants/bootc-status-updated.path - ln -s ../bootc-status-updated-onboot.target $(DESTDIR)/$(prefix)/lib/systemd/system/multi-user.target.wants/bootc-status-updated-onboot.target install -D -m 0644 -t $(DESTDIR)/$(prefix)/share/doc/bootc/baseimage/base/usr/lib/ostree/ baseimage/base/usr/lib/ostree/prepare-root.conf install -d -m 755 $(DESTDIR)/$(prefix)/share/doc/bootc/baseimage/base/sysroot cp -PfT baseimage/base/ostree $(DESTDIR)/$(prefix)/share/doc/bootc/baseimage/base/ostree @@ -60,13 +57,6 @@ install: # Copy dracut and systemd config files cp -Prf baseimage/dracut $(DESTDIR)$(prefix)/share/doc/bootc/baseimage/dracut cp -Prf baseimage/systemd $(DESTDIR)$(prefix)/share/doc/bootc/baseimage/systemd - # Install fedora-bootc-destructive-cleanup in fedora derivatives - ID=$$(. /usr/lib/os-release && echo $$ID); \ - ID_LIKE=$$(. /usr/lib/os-release && echo $$ID_LIKE); \ - if [ "$$ID" = "fedora" ] || [[ "$$ID_LIKE" == *"fedora"* ]]; then \ - ln -s ../bootc-destructive-cleanup.service $(DESTDIR)/$(prefix)/lib/systemd/system/multi-user.target.wants/bootc-destructive-cleanup.service; \ - install -D -m 0755 -t $(DESTDIR)/$(prefix)/lib/bootc contrib/scripts/fedora-bootc-destructive-cleanup; \ - fi # Run this to also take over the functionality of `ostree container` for example. # Only needed for OS/distros that have callers invoking `ostree container` and not bootc. diff --git a/crates/lib/src/generator.rs b/crates/lib/src/generator.rs index d1fd4bf31..9f28e3a3b 100644 --- a/crates/lib/src/generator.rs +++ b/crates/lib/src/generator.rs @@ -1,12 +1,19 @@ use std::io::BufRead; use anyhow::{Context, Result}; +use camino::Utf8PathBuf; 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 ostree_ext::container_utils::{is_ostree_booted_in, OSTREE_BOOTED}; use rustix::{fd::AsFd, fs::StatVfsMountFlags}; +use crate::install::DESTRUCTIVE_CLEANUP; + +const STATUS_ONBOOT_UNIT: &str = "bootc-status-updated-onboot.target"; +const STATUS_PATH_UNIT: &str = "bootc-status-updated.path"; +const CLEANUP_UNIT: &str = "bootc-destructive-cleanup.service"; +const MULTI_USER_TARGET: &str = "multi-user.target"; const EDIT_UNIT: &str = "bootc-fstab-edit.service"; const FSTAB_ANACONDA_STAMP: &str = "Created by anaconda"; pub(crate) const BOOTC_EDITED_STAMP: &str = "Updated by bootc-fstab-edit.service"; @@ -47,9 +54,50 @@ pub(crate) fn fstab_generator_impl(root: &Dir, unit_dir: &Dir) -> Result { Ok(false) } +pub(crate) fn enable_unit(unitdir: &Dir, name: &str, target: &str) -> Result<()> { + let wants = Utf8PathBuf::from(format!("{target}.wants")); + unitdir + .create_dir_all(&wants) + .with_context(|| format!("Creating {wants}"))?; + let source = format!("/usr/lib/systemd/system/{name}"); + let target = wants.join(name); + unitdir.remove_file_optional(&target)?; + unitdir + .symlink_contents(&source, &target) + .with_context(|| format!("Writing {name}"))?; + Ok(()) +} + +/// Enable our units +pub(crate) fn unit_enablement_impl(sysroot: &Dir, unit_dir: &Dir) -> Result<()> { + for unit in [STATUS_ONBOOT_UNIT, STATUS_PATH_UNIT] { + enable_unit(unit_dir, unit, MULTI_USER_TARGET)?; + } + + if sysroot.try_exists(DESTRUCTIVE_CLEANUP)? { + tracing::debug!("Found {DESTRUCTIVE_CLEANUP}"); + enable_unit(unit_dir, CLEANUP_UNIT, MULTI_USER_TARGET)?; + } else { + tracing::debug!("Didn't find {DESTRUCTIVE_CLEANUP}"); + } + + Ok(()) +} + /// Main entrypoint for the generator pub(crate) fn generator(root: &Dir, unit_dir: &Dir) -> Result<()> { - // Right now we only do something if the root is a read-only overlayfs (a composefs really) + // Only run on ostree systems + if !root.try_exists(OSTREE_BOOTED)? { + return Ok(()); + } + + let Some(ref sysroot) = root.open_dir_optional("sysroot")? else { + return Ok(()); + }; + + unit_enablement_impl(sysroot, unit_dir)?; + + // Also only run if the root is a read-only overlayfs (a composefs really) let st = rustix::fs::fstatfs(root.as_fd())?; if st.f_type != libc::OVERLAYFS_SUPER_MAGIC { tracing::trace!("Root is not overlayfs"); @@ -62,6 +110,7 @@ pub(crate) fn generator(root: &Dir, unit_dir: &Dir) -> Result<()> { } let updated = fstab_generator_impl(root, unit_dir)?; tracing::trace!("Generated fstab: {updated}"); + Ok(()) } @@ -89,12 +138,16 @@ ExecStart=bootc internals fixup-etc-fstab\n\ #[cfg(test)] mod tests { + use camino::Utf8Path; + use cap_std_ext::cmdext::CapStdExtCommandExt as _; + use super::*; fn fixture() -> Result { let tempdir = cap_std_ext::cap_tempfile::tempdir(cap_std::ambient_authority())?; tempdir.create_dir("etc")?; tempdir.create_dir("run")?; + tempdir.create_dir("sysroot")?; tempdir.create_dir_all("run/systemd/system")?; Ok(tempdir) } @@ -109,6 +162,53 @@ mod tests { Ok(()) } + #[test] + fn test_units() -> Result<()> { + let tempdir = &fixture()?; + let sysroot = &tempdir.open_dir("sysroot").unwrap(); + let unit_dir = &tempdir.open_dir("run/systemd/system")?; + + let verify = |wantsdir: &Dir, n: u32| -> Result<()> { + assert_eq!(unit_dir.entries()?.count(), 1); + let r = wantsdir.read_link_contents(STATUS_ONBOOT_UNIT)?; + let r: Utf8PathBuf = r.try_into().unwrap(); + assert_eq!(r, format!("/usr/lib/systemd/system/{STATUS_ONBOOT_UNIT}")); + assert_eq!(wantsdir.entries()?.count(), n as usize); + anyhow::Ok(()) + }; + + // Explicitly run this twice to test idempotency + + unit_enablement_impl(sysroot, &unit_dir).unwrap(); + unit_enablement_impl(sysroot, &unit_dir).unwrap(); + let wantsdir = &unit_dir.open_dir("multi-user.target.wants")?; + verify(wantsdir, 2)?; + assert!(wantsdir + .symlink_metadata_optional(CLEANUP_UNIT) + .unwrap() + .is_none()); + + // Now create sysroot and rerun the generator + unit_enablement_impl(sysroot, &unit_dir).unwrap(); + verify(wantsdir, 2)?; + + // Create the destructive stamp + sysroot + .create_dir_all(Utf8Path::new(DESTRUCTIVE_CLEANUP).parent().unwrap()) + .unwrap(); + sysroot.atomic_write(DESTRUCTIVE_CLEANUP, b"").unwrap(); + unit_enablement_impl(sysroot, unit_dir).unwrap(); + verify(wantsdir, 3)?; + + // And now the unit should be enabled + assert!(wantsdir + .symlink_metadata(CLEANUP_UNIT) + .unwrap() + .is_symlink()); + + Ok(()) + } + #[cfg(test)] mod test { use super::*; diff --git a/crates/lib/src/install.rs b/crates/lib/src/install.rs index d5f917781..f40d51fed 100644 --- a/crates/lib/src/install.rs +++ b/crates/lib/src/install.rs @@ -77,7 +77,7 @@ const RUN_BOOTC: &str = "/run/bootc"; /// The default path for the host rootfs const ALONGSIDE_ROOT_MOUNT: &str = "/target"; /// Global flag to signal the booted system was provisioned via an alongside bootc install -const DESTRUCTIVE_CLEANUP: &str = "bootc-destructive-cleanup"; +pub(crate) const DESTRUCTIVE_CLEANUP: &str = "etc/bootc-destructive-cleanup"; /// This is an ext4 special directory we need to ignore. const LOST_AND_FOUND: &str = "lost+found"; /// The filename of the composefs EROFS superblock; TODO move this into ostree @@ -1494,7 +1494,7 @@ async fn ostree_install(state: &State, rootfs: &RootSetup, cleanup: Cleanup) -> if matches!(cleanup, Cleanup::TriggerOnNextBoot) { let sysroot_dir = crate::utils::sysroot_dir(ostree)?; tracing::debug!("Writing {DESTRUCTIVE_CLEANUP}"); - sysroot_dir.atomic_write(format!("etc/{}", DESTRUCTIVE_CLEANUP), b"")?; + sysroot_dir.atomic_write(DESTRUCTIVE_CLEANUP, b"")?; } // We must drop the sysroot here in order to close any open file diff --git a/crates/tests-integration/src/system_reinstall.rs b/crates/tests-integration/src/system_reinstall.rs index f6ed44a5d..91e12fc2c 100644 --- a/crates/tests-integration/src/system_reinstall.rs +++ b/crates/tests-integration/src/system_reinstall.rs @@ -96,7 +96,6 @@ pub(crate) fn run(image: &str, testargs: libtest_mimic::Arguments) -> Result<()> let files = [ "usr/lib/bootc/fedora-bootc-destructive-cleanup", "usr/lib/systemd/system/bootc-destructive-cleanup.service", - "usr/lib/systemd/system/multi-user.target.wants/bootc-destructive-cleanup.service", "etc/tmpfiles.d/bootc-root-ssh.conf", ]; diff --git a/systemd/bootc-destructive-cleanup.service b/systemd/bootc-destructive-cleanup.service index 97cc93bc3..ad66f9331 100644 --- a/systemd/bootc-destructive-cleanup.service +++ b/systemd/bootc-destructive-cleanup.service @@ -1,12 +1,10 @@ [Unit] Description=Cleanup previous the installation after an alongside installation Documentation=man:bootc(8) -ConditionPathExists=/sysroot/etc/bootc-destructive-cleanup [Service] Type=oneshot ExecStart=/usr/lib/bootc/fedora-bootc-destructive-cleanup PrivateMounts=true -[Install] -WantedBy=multi-user.target +# No [Install] section, this is enabled via generator diff --git a/systemd/bootc-status-updated-onboot.target b/systemd/bootc-status-updated-onboot.target index 7b898fadb..2e7678ccd 100644 --- a/systemd/bootc-status-updated-onboot.target +++ b/systemd/bootc-status-updated-onboot.target @@ -1,7 +1,6 @@ [Unit] -Description=Target for bootc status changes on boot +Description=Bootc status trigger state sync Documentation=man:bootc-status-updated.target(8) -ConditionPathExists=/run/ostree-booted +After=sysinit.target -[Install] -WantedBy=multi-user.target +# No [Install] section, this is enabled via generator diff --git a/systemd/bootc-status-updated.path b/systemd/bootc-status-updated.path index fbb22eacb..d9d071915 100644 --- a/systemd/bootc-status-updated.path +++ b/systemd/bootc-status-updated.path @@ -1,11 +1,9 @@ [Unit] Description=Monitor bootc for status changes Documentation=man:bootc-status-updated.path(8) -ConditionPathExists=/run/ostree-booted [Path] PathChanged=/ostree/bootc Unit=bootc-status-updated.target -[Install] -WantedBy=multi-user.target +# No [Install] section, this is enabled via generator