generator: Conditinally enable bootc-status units

cgwalters · jeckersb · commit cd8fa591a950 · 2025-09-25T16:38:55.000-04:00
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 &lt;walters@verbum.org&gt;
diff --git 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
@@ -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<bool> {
     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<cap_std_ext::cap_tempfile::TempDir> {
         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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
