diff options
| author | Craig Jennings <c@cjennings.net> | 2026-09-13 10:59:31 -0500 |
|---|---|---|
| committer | Craig Jennings <c@cjennings.net> | 2026-09-13 10:59:31 -0500 |
| commit | 6e99a7fbae089009e7948cb3853b68364f2f527b (patch) | |
| tree | 4c4165815ba363824a98485e4a45f16a97e10ffc /installer | |
| parent | 3bab9f1ff287c61e85504ad5ab0c5a0d69bf0a61 (diff) | |
| download | archangel-6e99a7fbae089009e7948cb3853b68364f2f527b.tar.gz archangel-6e99a7fbae089009e7948cb3853b68364f2f527b.zip | |
fix(install): run the failure cleanup when an install step fails
Every install step runs inside a function, and bash never delivers an ERR trap to a command failing inside a called function. With `set -e` the installer exited without `install_failure_cleanup` ever running. That left /mnt mounted and the pool imported, so a re-run or the test harness's retry died at "Disk in use". The `|| error` shape had the same problem, because `error()` calls a plain `exit 1` that no ERR trap sees.
The failure trap now also fires on EXIT, and `arm_failure_trap` and `disarm_failure_trap` set and clear it in one place. I left errtrace off on purpose. It hands the trap to command substitutions, so a masked failing `$(...)` would run the cleanup in a subshell and unmount /mnt while the install carried on.
Once the cleanup runs, three more things could stop it from finishing. A SIGTERM to the whole process group also kills the logging tee, and the cleanup then died on SIGPIPE at its first message. It now ignores SIGPIPE and turns errexit off for itself with `local -`. It also ignores INT and TERM, so a second Ctrl-C can't cut it short. The pool export could race pacman's still-exiting chroot processes and fail as busy, so the export and the Btrfs LUKS close now retry for up to 15 seconds. The Btrfs branch also gets the recursive unmount with a lazy fallback that the ZFS branch already had.
Diffstat (limited to 'installer')
| -rwxr-xr-x | installer/archangel | 103 | ||||
| -rw-r--r-- | installer/lib/btrfs.sh | 11 |
2 files changed, 101 insertions, 13 deletions
diff --git a/installer/archangel b/installer/archangel index 065772c..56290d9 100755 --- a/installer/archangel +++ b/installer/archangel @@ -1374,16 +1374,87 @@ cleanup() { info "Cleanup complete." } -# Trap target for ERR / INT / TERM during install_zfs and +# The trap set that routes a mid-install failure to install_failure_cleanup. +# EXIT is the one that does the work. Every install step is a function, and +# bash never delivers an ERR trap to a command failing inside a called +# function, so errexit used to end the script with the cleanup unrun. The +# `|| error "..."` shape leaves through a plain `exit 1`, which no ERR trap +# sees either. Both left /mnt mounted and the pool imported for the next +# attempt to trip over (2026-08-06, 2026-09-12). ERR still fires for a +# failure directly in install_zfs/install_btrfs, and INT and TERM cover +# Ctrl-C and a kill. The success path disarms all four before its own +# cleanup, whose non-zero exits are expected. +# +# I deliberately don't turn on errtrace (set -E) to widen ERR. It hands the +# trap to command substitutions too, so a masked failing `$(...)` ran the +# whole cleanup inside that subshell and unmounted /mnt while the install +# carried on in the parent. +arm_failure_trap() { + trap 'install_failure_cleanup' ERR INT TERM EXIT +} + +# How many one-second tries the failure cleanup gives a busy pool export or +# LUKS close. When a signal interrupts pacstrap, pacman's children in the +# chroot can still be exiting as the cleanup runs, so the first export fails +# busy and a manual one seconds later succeeds (VM, 2026-09-13). +CLEANUP_BUSY_ATTEMPTS="${CLEANUP_BUSY_ATTEMPTS:-15}" + +# Run a command until it succeeds, up to <attempts> times, sleeping one +# second between tries. Returns 1 if the last try still fails. +# +# Usage: retry_busy <attempts> <command> [args...] +retry_busy() { + local attempts="$1" i + shift + for ((i = 1; i <= attempts; i++)); do + "$@" && return 0 + ((i < attempts)) && sleep 1 + done + return 1 +} + +# Close this install's LUKS mappings and succeed only once none is left. +# close_luks_container swallows cryptsetup's errors, so a busy close has to +# be detected by looking for the mapping afterwards. +close_luks_and_confirm() { + btrfs_close_encryption 2>/dev/null + ! luks_mappings_open +} + +disarm_failure_trap() { + trap - ERR INT TERM EXIT +} + +# Trap target for ERR / INT / TERM / EXIT during install_zfs and # install_btrfs. Captures the failing exit code first, disarms the -# trap to prevent recursion, clears sensitive variables, and -# dispatches to the right per-filesystem cleanup before exiting via -# error(). All cleanup steps swallow their own errors — partial -# state is expected when this fires mid-install, so individual tool -# failures are not fatal. +# trap to prevent recursion (its own error() exit would re-fire EXIT), +# clears sensitive variables, and dispatches to the right +# per-filesystem cleanup before exiting via error(). All cleanup +# steps swallow their own errors — partial state is expected when +# this fires mid-install, so individual tool failures are not fatal. install_failure_cleanup() { local exit_code=$? - trap - ERR INT TERM + disarm_failure_trap + + # A SIGTERM sent to the whole process group (kill -- -PGID, timeout, a + # service stop) also kills the tee init_logging put on stdout. With the + # tee gone, the first warn below raised SIGPIPE and killed the shell + # before anything was unmounted. Ctrl-C doesn't hit this, because bash + # starts a process substitution with SIGINT ignored. Ignore SIGPIPE, and + # turn errexit off so a failed write to the dead pipe can't end the + # cleanup either. `local -` scopes that to this function: the cleanup + # normally leaves through error(), but a caller that gets control back + # (the unit tests) would otherwise keep running with errexit off. + local - + trap '' PIPE + set +e + + # disarm_failure_trap put INT and TERM back to their defaults, so a + # second Ctrl-C or SIGTERM during the busy retries below (several silent + # seconds, easy to read as a hang) killed the cleanup halfway and left + # the pool imported. Ignore both until the cleanup exits. It's bounded + # by CLEANUP_BUSY_ATTEMPTS, and SIGKILL still stops it. + trap '' INT TERM ROOT_PASSWORD="" ZFS_PASSPHRASE="" @@ -1402,7 +1473,7 @@ install_failure_cleanup() { umount "$EFI_DIR" 2>/dev/null || umount -l "$EFI_DIR" 2>/dev/null || true umount -R "$MNTPOINT" 2>/dev/null || umount -R -l "$MNTPOINT" 2>/dev/null || true if zpool list "$POOL_NAME" >/dev/null 2>&1; then - zpool export "$POOL_NAME" 2>/dev/null \ + retry_busy "$CLEANUP_BUSY_ATTEMPTS" zpool export "$POOL_NAME" 2>/dev/null \ || zpool export -f "$POOL_NAME" 2>/dev/null \ || true fi @@ -1410,7 +1481,13 @@ install_failure_cleanup() { btrfs) umount "$EFI_DIR" 2>/dev/null || umount -l "$EFI_DIR" 2>/dev/null || true btrfs_cleanup 2>/dev/null || true - btrfs_close_encryption 2>/dev/null || true + # btrfs_cleanup only unmounts the subvolumes it mounted. A + # pacstrap interrupted mid-transaction can leave its /proc, /sys + # and /dev bind mounts under the root, which keep it busy, so the + # LUKS mapping can't close. Same recursive-then-lazy fallback as + # the ZFS branch, before closing encryption. + umount -R "$MNTPOINT" 2>/dev/null || umount -R -l "$MNTPOINT" 2>/dev/null || true + retry_busy "$CLEANUP_BUSY_ATTEMPTS" close_luks_and_confirm || true ;; esac @@ -1531,7 +1608,7 @@ install_zfs() { # cleanup (unmount /mnt + export pool) so the live ISO is left in a # state where the user can re-run the installer without manual # intervention. - trap 'install_failure_cleanup' ERR INT TERM + arm_failure_trap partition_disks create_zfs_pool @@ -1553,7 +1630,7 @@ install_zfs() { # Disarm the failure trap before the success-path cleanup. The # success cleanup may emit non-zero exit codes that we don't want # to interpret as "installation failed". - trap - ERR INT TERM + disarm_failure_trap cleanup print_summary } @@ -1565,7 +1642,7 @@ install_zfs() { install_btrfs() { # Arm the failure trap before any destructive operation. See the # matching block in install_zfs() for the rationale. - trap 'install_failure_cleanup' ERR INT TERM + arm_failure_trap local btrfs_devices=() @@ -1605,7 +1682,7 @@ install_btrfs() { # Disarm the failure trap before the success-path cleanup. See # the matching block in install_zfs() for the rationale. - trap - ERR INT TERM + disarm_failure_trap # Cleanup btrfs_cleanup diff --git a/installer/lib/btrfs.sh b/installer/lib/btrfs.sh index 67c96a0..96ad29e 100644 --- a/installer/lib/btrfs.sh +++ b/installer/lib/btrfs.sh @@ -153,6 +153,17 @@ close_luks_containers() { done } +# Succeed when any of this install's LUKS mappings is still open. Names +# follow get_luks_devices: the bare LUKS_MAPPER_NAME for the first disk, +# then LUKS_MAPPER_NAME1, 2, ... MAPPER_DIR is overridable for tests. +luks_mappings_open() { + local dir="${MAPPER_DIR:-/dev/mapper}" m + for m in "$dir/$LUKS_MAPPER_NAME" "$dir/$LUKS_MAPPER_NAME"[0-9]*; do + [[ -e "$m" ]] && return 0 + done + return 1 +} + # Get list of opened LUKS mapper devices get_luks_devices() { local count="$1" |
