cleanup: Mount the previous system's /var before removing its packages

contrib/scripts/fedora-bootc-destructive-cleanup

@@ -3,12 +3,65 @@
 
 set -xeuo pipefail
 
+# The previous OS may have had /var on a separate filesystem, e.g. the
+# "var" btrfs subvolume of Fedora Cloud images. Only its root filesystem
+# is at /sysroot after the reinstall, so mount its /var there too: package
+# scriptlets run chrooted in /sysroot and need state like /var/lib/alternatives.
+# This unit uses PrivateMounts=true, so the mount is not visible outside it.
+# Every way out that leaves the previous /var unmounted says why, since the
+# package removal and the container prune below then work without it.
+mount_old_var() {
+    local fstab=/sysroot/etc/fstab
+    if [ ! -f "${fstab}" ]; then
+        echo "previous system has no /etc/fstab, not mounting a separate /var" 1>&2
+        return 0
+    fi
+    if mountpoint -q /sysroot/var; then
+        echo "/sysroot/var is already a mount point, leaving it as is" 1>&2
+        return 0
+    fi
+    local source
+    if ! source=$(findmnt --fstab --tab-file "${fstab}" --mountpoint /var --evaluate --noheadings --output SOURCE); then
+        echo "no /var entry found in the previous fstab, not mounting a separate /var" 1>&2
+        return 0
+    fi
+    # e.g. a UUID= or LABEL= that matches no device in the new system
+    if [ -z "${source}" ]; then
+        echo "warning: source of /var in the previous fstab could not be resolved, not mounting it" 1>&2
+        return 0
+    fi
+    # Only handle block devices; e.g. a bind mount source would be resolved
+    # against the new root, not the previous one.
+    if [ ! -b "${source}" ]; then
+        echo "warning: not mounting previous /var from '${source}': not a block device" 1>&2
+        return 0
+    fi
+    if ! mount --fstab "${fstab}" --target-prefix /sysroot /var; then
+        echo "warning: failed to mount previous /var from '${source}'" 1>&2
+        return 0
+    fi
+    # The previous /var must be a filesystem of its own: neither the previous
+    # root (a mistaken fstab entry) nor the one the new system uses as /var.
+    local dev
+    dev=$(stat -c %d /sysroot/var)
+    if [ "${dev}" = "$(stat -c %d /sysroot)" ] || [ "${dev}" = "$(stat -c %d /var)" ]; then
+        echo "warning: previous /var from '${source}' is not a separate filesystem, not using it" 1>&2
+        umount /sysroot/var
+    fi
+}
+
 # Remove all RPMs installed in the physical root (i.e. the previous OS)
 mount -o remount,rw /sysroot
+mount_old_var
 rpm -qa --root=/sysroot --dbpath=/usr/lib/sysimage/rpm | xargs -r rpm -e --root=/sysroot --dbpath=/usr/lib/sysimage/rpm
 
 # Remove all container images (including the one that was used to install)
 # Note that this does not remove stopped containers, and so some storage
 # may leak. This may change in the future.
-mount --bind -o rw /sysroot/var/lib/containers /var/lib/containers
-podman system prune --all -f
+# The previous /var may not be available (see above).
+if [ -d /sysroot/var/lib/containers ]; then
+    mount --bind -o rw /sysroot/var/lib/containers /var/lib/containers
+    podman system prune --all -f
+else
+    echo "warning: not pruning container images: /sysroot/var/lib/containers does not exist (the previous system had no container storage, or its /var could not be mounted)" 1>&2
+fi

docs/src/man/bootc-destructive-cleanup.service.5.md

@@ -25,9 +25,19 @@ derivatives).
 On Fedora derivatives, the cleanup script performs the following actions:
 
 - Remounts `/sysroot` as read-write
+- If the previous OS's `/etc/fstab` mounts `/var` from a separate block
+  device or btrfs subvolume, mounts it at `/sysroot/var`, visible only to
+  the service, so that package scriptlets find the previous OS's state
 - Removes all RPM packages installed in the physical root (the previous OS)
 - Removes all container images from `/sysroot/var/lib/containers` using
-  `podman system prune --all -f`
+  `podman system prune --all -f`, if that directory exists
+
+Whenever the script leaves the previous `/var` unmounted (for example
+because the previous `/etc/fstab` has no `/var` entry, its source cannot be
+resolved or is not a block device, or the mount fails) or skips the container
+image prune, it prints one line saying why. Look for these in the journal of
+the unit (`journalctl -u bootc-destructive-cleanup.service`) if storage of the
+previous OS was not reclaimed.
 
 **Note:** The cleanup script does not remove stopped containers, so some storage
 may remain. This behavior may change in the future.
@@ -49,6 +59,7 @@ at `/sysroot` (the "physical root"). Previous mount points or subvolumes will
 not be automatically mounted in the new system; for example, a btrfs subvolume
 for /home will not be automatically mounted to /sysroot/home. These filesystems
 persist and can be handled manually or defined as mount points in the bootc image.
+The cleanup service mounts the previous `/var` only for its own duration.
 
 # SEE ALSO
 

tmt/tests/booted/readonly/018-test-destructive-cleanup.nu

@@ -0,0 +1,60 @@
+# Verify that bootc-destructive-cleanup.service did its job after an
+# in-place reinstall (`system-reinstall-bootc`, which passes `--cleanup`).
+#
+# A unit that never ran, or whose steps were all skipped, is not "failed",
+# so 014-test-no-failed-units.nu alone does not notice a cleanup that
+# didn't happen. See https://github.com/bootc-dev/bootc/issues/2559
+use std assert
+use tap.nu
+
+tap begin "verify destructive cleanup after an in-place reinstall"
+
+const unit = "bootc-destructive-cleanup.service"
+# Where the script looks for the previous system's packages
+const old_root = "/sysroot"
+const rpmdb = "/usr/lib/sysimage/rpm"
+# The unit is not ordered before the tests, so allow it to finish
+const wait_limit = 5min
+const wait_step = 5sec
+
+# Only `cargo xtask run-tmt --provision=reinstall` sets this; the other
+# provisioning paths install to a fresh disk, where there is nothing to clean up.
+let reinstalled = ($env.BOOTC_REINSTALL_IMAGE? | default "" | is-not-empty)
+
+if not $reinstalled {
+    print "# skip: not provisioned via in-place reinstall (BOOTC_REINSTALL_IMAGE not set)"
+} else {
+    # A oneshot unit without RemainAfterExit goes back to "inactive" when
+    # done, which is also its state before it starts; its main process
+    # having an exit timestamp tells the two apart.
+    mut waited = 0sec
+    loop {
+        let state = (systemctl show -P ActiveState $unit | str trim)
+        let exited = (systemctl show -P ExecMainExitTimestampMonotonic $unit | str trim)
+        if $state == "failed" or ($state == "inactive" and $exited != "0") {
+            break
+        }
+        assert ($waited < $wait_limit) $"($unit) did not run to completion within ($wait_limit): ActiveState=($state)"
+        sleep $wait_step
+        $waited = $waited + $wait_step
+    }
+
+    let result = (systemctl show -P Result $unit | str trim)
+    let status = (systemctl show -P ExecMainStatus $unit | str trim)
+    print $"($unit): Result=($result) ExecMainStatus=($status)"
+    assert equal $result "success" $"($unit) did not succeed"
+    assert equal $status "0" $"($unit) exited with a non-zero status"
+
+    let old_rpmdb = $"($old_root)($rpmdb)"
+    if not ($old_rpmdb | path exists) {
+        print $"No rpmdb at ($old_rpmdb): no packages of the previous system remain"
+    } else {
+        let query = (rpm -qa $"--root=($old_root)" $"--dbpath=($rpmdb)" | complete)
+        assert equal $query.exit_code 0 $"rpm -qa on ($old_root) failed: ($query.stderr)"
+        let packages = ($query.stdout | lines | where $it != "")
+        print $"Packages of the previous system left in ($old_root): ($packages | length)"
+        assert equal ($packages | length) 0 $"Previous system's packages were not removed: ($packages | first 10 | str join ' ')"
+    }
+}
+
+tap ok