diff options
| author | Craig Jennings <c@cjennings.net> | 2026-04-26 18:31:35 -0500 |
|---|---|---|
| committer | Craig Jennings <c@cjennings.net> | 2026-04-26 18:31:35 -0500 |
| commit | f02ba03c12599bdd61b5208c9a65f458ae05aa15 (patch) | |
| tree | 249a2e5eed52aaaa5cc2c9ca57d549a21560c8c6 | |
| parent | 6de9f378cf52b9e9b0e89b396a12b978700241ff (diff) | |
| download | archangel-f02ba03c12599bdd61b5208c9a65f458ae05aa15.tar.gz archangel-f02ba03c12599bdd61b5208c9a65f458ae05aa15.zip | |
refactor: unify partition_disks across ZFS and Btrfs install paths
The monolith's partition_disks() at installer/archangel was ZFS-only and silently shadowed lib/disk.sh:partition_disks(), which had been dead code since the Btrfs install path was added. install_btrfs was assembling partitioning manually via partition_disk (singular) plus a separate format_efi_partitions call. Two parallel implementations meant fixes had to land in two places and the lib version drifted with no visible warning.
The unified partition_disks now lives in lib/disk.sh. It reads SELECTED_DISKS, dispatches the per-disk layout on FILESYSTEM (BF00 for ZFS, 8300 for Btrfs), populates EFI_PARTS + ROOT_PARTS, and formats each EFI partition with EFI0, EFI1, ... labels.
Folded in two pre-existing divergences while consolidating. wipefs -af now runs on every disk, not just the ZFS path. The Btrfs path was missing this defense against non-GPT signatures (LVM, mdadm, ext) that sgdisk --zap-all alone won't touch. EFI labels standardized on EFI0, EFI1, ... across both paths. The lib version was producing asymmetric EFI / EFI2 labels. No consumer in the repo references the labels after format, so that change is cosmetic.
ZFS_PARTS renamed to ROOT_PARTS for symmetry with get_root_partition. Deleted four orphaned helpers: format_efi, format_efi_partitions (only caller was the collapsed install_btrfs section), get_efi_partitions, get_root_partitions (test-only callers after install_btrfs simplified).
Verified end to end: bats 134/134, make test-install passing 12/12 configs across both install paths.
| -rwxr-xr-x | installer/archangel | 85 | ||||
| -rw-r--r-- | installer/lib/disk.sh | 108 | ||||
| -rw-r--r-- | testing-strategy.org | 6 | ||||
| -rw-r--r-- | tests/unit/test_disk.bats | 190 |
4 files changed, 202 insertions, 187 deletions
diff --git a/installer/archangel b/installer/archangel index 55eac9c..10ec68a 100755 --- a/installer/archangel +++ b/installer/archangel @@ -55,8 +55,8 @@ ASHIFT="12" # 4K sectors (use 13 for 8K) # Multi-disk RAID support SELECTED_DISKS=() # Array of selected disk paths (/dev/sda, /dev/sdb, ...) -ZFS_PARTS=() # Array of ZFS partition paths -EFI_PARTS=() # Array of EFI partition paths +ROOT_PARTS=() # Array of root partition paths (populated by partition_disks) +EFI_PARTS=() # Array of EFI partition paths (populated by partition_disks) RAID_LEVEL="" # "", "mirror", "raidz1", "raidz2", "raidz3" ENABLE_SSH="yes" # Enable SSH with root login (default yes for headless) NO_ENCRYPT="no" # Skip ZFS encryption (for testing only) @@ -611,52 +611,6 @@ show_summary() { # Phase 2: Installation ############################# -partition_disks() { - step "Partitioning ${#SELECTED_DISKS[@]} disk(s)" - - EFI_PARTS=() - ZFS_PARTS=() - - for disk in "${SELECTED_DISKS[@]}"; do - info "Partitioning $disk..." - - # Wipe existing signatures - wipefs -af "$disk" - sgdisk --zap-all "$disk" - - # Create partitions: 512M EFI + rest for ZFS - # EFI only needs to hold ZFSBootMenu binary (~64MB) - 512MB is plenty - sgdisk -n 1:0:+512M -t 1:ef00 -c 1:"EFI" "$disk" - sgdisk -n 2:0:0 -t 2:bf00 -c 2:"ZFS" "$disk" - - # Determine partition names (handle nvme/mmcblk naming) - local efi_part zfs_part - if [[ "$disk" == *"nvme"* ]] || [[ "$disk" == *"mmcblk"* ]]; then - efi_part="${disk}p1" - zfs_part="${disk}p2" - else - efi_part="${disk}1" - zfs_part="${disk}2" - fi - - EFI_PARTS+=("$efi_part") - ZFS_PARTS+=("$zfs_part") - - sleep 1 - partprobe "$disk" - done - - sleep 2 - - # Format all EFI partitions - for i in "${!EFI_PARTS[@]}"; do - info "Formatting EFI partition ${EFI_PARTS[$i]}..." - mkfs.fat -F32 -n "EFI$i" "${EFI_PARTS[$i]}" - done - - info "Partitioning complete. Created ${#EFI_PARTS[@]} EFI and ${#ZFS_PARTS[@]} ZFS partitions." -} - create_zfs_pool() { step "Creating ZFS Pool with Native Encryption" @@ -669,14 +623,14 @@ create_zfs_pool() { local pool_config if [[ "$RAID_LEVEL" == "stripe" ]]; then # Stripe: just list devices without a vdev type (RAID0 equivalent) - pool_config="${ZFS_PARTS[*]}" - info "Creating striped pool with ${#ZFS_PARTS[@]} disks (NO redundancy)..." + pool_config="${ROOT_PARTS[*]}" + info "Creating striped pool with ${#ROOT_PARTS[@]} disks (NO redundancy)..." warn "Data loss will occur if ANY disk fails!" elif [[ -n "$RAID_LEVEL" ]]; then - pool_config="$RAID_LEVEL ${ZFS_PARTS[*]}" - info "Creating $RAID_LEVEL pool with ${#ZFS_PARTS[@]} disks..." + pool_config="$RAID_LEVEL ${ROOT_PARTS[*]}" + info "Creating $RAID_LEVEL pool with ${#ROOT_PARTS[@]} disks..." else - pool_config="${ZFS_PARTS[0]}" + pool_config="${ROOT_PARTS[0]}" info "Creating single-disk pool..." fi @@ -1496,30 +1450,19 @@ install_btrfs() { trap 'install_failure_cleanup' ERR INT TERM local btrfs_devices=() - local efi_parts=() - local root_parts=() - # Collect partition references for all disks - for disk in "${SELECTED_DISKS[@]}"; do - root_parts+=("$(get_root_partition "$disk")") - efi_parts+=("$(get_efi_partition "$disk")") - done - - # Partition and format EFI - for disk in "${SELECTED_DISKS[@]}"; do - partition_disk "$disk" - done - format_efi_partitions "${SELECTED_DISKS[@]}" + # Partition + format EFI; populates EFI_PARTS and ROOT_PARTS globals. + partition_disks # LUKS (optional) fills btrfs_devices with either /dev/mapper/* or raw parts - btrfs_open_encryption root_parts btrfs_devices + btrfs_open_encryption ROOT_PARTS btrfs_devices # Btrfs filesystem + subvolumes + EFI mount btrfs_make_filesystem btrfs_devices create_btrfs_subvolumes "${btrfs_devices[0]}" mount_btrfs_subvolumes "${btrfs_devices[0]}" mkdir -p /mnt/efi - mount "${efi_parts[0]}" /mnt/efi + mount "${EFI_PARTS[0]}" /mnt/efi # Base install + system config install_base @@ -1528,13 +1471,13 @@ install_btrfs() { configure_ssh # LUKS config inside chroot (no-op if encryption disabled) - btrfs_configure_luks_target root_parts + btrfs_configure_luks_target ROOT_PARTS - generate_btrfs_fstab "${btrfs_devices[0]}" "${efi_parts[0]}" + generate_btrfs_fstab "${btrfs_devices[0]}" "${EFI_PARTS[0]}" configure_btrfs_initramfs # Bootloader - btrfs_install_grub efi_parts + btrfs_install_grub EFI_PARTS # Snapshots + services configure_snapper diff --git a/installer/lib/disk.sh b/installer/lib/disk.sh index f40d828..b548b4f 100644 --- a/installer/lib/disk.sh +++ b/installer/lib/disk.sh @@ -6,46 +6,66 @@ # Partition Disks ############################# -# Partition a single disk for ZFS/Btrfs installation -# Creates: EFI partition (512M) + root partition (rest) -# Uses global FILESYSTEM variable to determine partition type +# Partition a single disk for ZFS or Btrfs installation. Wipes +# non-GPT signatures (LVM, mdadm, ext) with wipefs, zaps the GPT +# with sgdisk, then lays down a 512M EFI partition plus a root +# partition that fills the rest. Root partition type code is +# selected from FILESYSTEM (BF00 for ZFS, 8300 for Btrfs). partition_disk() { local disk="$1" local efi_size="${2:-512M}" - # Determine root partition type based on filesystem - local root_type="BF00" # ZFS (Solaris root) + local root_type="BF00" if [[ "$FILESYSTEM" == "btrfs" ]]; then - root_type="8300" # Linux filesystem + root_type="8300" fi info "Partitioning $disk..." - # Wipe existing partition table - sgdisk --zap-all "$disk" || error "Failed to wipe $disk" - - # Create EFI partition (512M, type EF00) + wipefs -af "$disk" || error "Failed to wipe signatures on $disk" + sgdisk --zap-all "$disk" || error "Failed to zap GPT on $disk" sgdisk -n 1:0:+${efi_size} -t 1:EF00 -c 1:"EFI" "$disk" || error "Failed to create EFI partition on $disk" - - # Create root partition (rest of disk) sgdisk -n 2:0:0 -t 2:$root_type -c 2:"ROOT" "$disk" || error "Failed to create root partition on $disk" - # Notify kernel of partition changes partprobe "$disk" 2>/dev/null || true sleep 1 info "Partitioned $disk: EFI=${efi_size}, ROOT=remainder" } -# Partition multiple disks (for RAID configurations) +# Partition every disk in SELECTED_DISKS, format each EFI partition, +# and populate the EFI_PARTS + ROOT_PARTS arrays for downstream +# callers (create_zfs_pool, btrfs_open_encryption, +# sync_efi_partitions, fstab generation). +# +# EFI labels are EFI0, EFI1, ... in selection order so multi-disk +# layouts get a stable, distinguishable scheme that lsblk -f can +# show. Errors out if SELECTED_DISKS is empty so a misconfigured +# install can't silently skip partitioning. partition_disks() { - local efi_size="${1:-512M}" - shift - local disks=("$@") + if [[ ${#SELECTED_DISKS[@]} -eq 0 ]]; then + error "partition_disks: SELECTED_DISKS is empty" + fi + + step "Partitioning ${#SELECTED_DISKS[@]} disk(s)" + + EFI_PARTS=() + ROOT_PARTS=() + + for disk in "${SELECTED_DISKS[@]}"; do + partition_disk "$disk" + EFI_PARTS+=("$(get_efi_partition "$disk")") + ROOT_PARTS+=("$(get_root_partition "$disk")") + done + + sleep 2 - for disk in "${disks[@]}"; do - partition_disk "$disk" "$efi_size" + for i in "${!EFI_PARTS[@]}"; do + info "Formatting EFI partition ${EFI_PARTS[$i]}..." + mkfs.fat -F32 -n "EFI$i" "${EFI_PARTS[$i]}" || error "Failed to format ${EFI_PARTS[$i]}" done + + info "Partitioning complete. Created ${#EFI_PARTS[@]} EFI and ${#ROOT_PARTS[@]} ROOT partitions." } ############################# @@ -72,56 +92,6 @@ get_root_partition() { fi } -# Get all root partitions from disk array -get_root_partitions() { - local disks=("$@") - local parts=() - for disk in "${disks[@]}"; do - parts+=("$(get_root_partition "$disk")") - done - printf '%s\n' "${parts[@]}" -} - -# Get all EFI partitions from disk array -get_efi_partitions() { - local disks=("$@") - local parts=() - for disk in "${disks[@]}"; do - parts+=("$(get_efi_partition "$disk")") - done - printf '%s\n' "${parts[@]}" -} - -############################# -# EFI Partition Management -############################# - -# Format EFI partition -format_efi() { - local partition="$1" - local label="${2:-EFI}" - - info "Formatting EFI partition: $partition" - mkfs.fat -F32 -n "$label" "$partition" || error "Failed to format EFI: $partition" -} - -# Format all EFI partitions -format_efi_partitions() { - local disks=("$@") - local first=true - - for disk in "${disks[@]}"; do - local efi - efi=$(get_efi_partition "$disk") - if $first; then - format_efi "$efi" "EFI" - first=false - else - format_efi "$efi" "EFI2" - fi - done -} - ############################# # Disk Selection (Interactive) ############################# diff --git a/testing-strategy.org b/testing-strategy.org index 0f81a3a..cfc3953 100644 --- a/testing-strategy.org +++ b/testing-strategy.org @@ -64,9 +64,9 @@ Current coverage lives in =tests/unit/=: | =test_common.bats= | =command_exists=, =require_command=, =info=/=warn=/=error=, =enable_color=, =log=, =prompt_password=, =pacstrap_packages=, =install_dropin= | | =test_config.bats= | =parse_args=, =load_config=, =validate_config=, =check_config= | | =test_raid.bats= | =raid_valid_levels_for_count=, =raid_is_valid=, =raid_usable_bytes=, =raid_fault_tolerance= | -| =test_disk.bats= | =get_efi_partition=, =get_root_partition=, =get_efi_partitions=, =get_root_partitions= | +| =test_disk.bats= | =get_efi_partition=, =get_root_partition=, =partition_disks= (orchestration shape) | | =test_btrfs.bats= | =get_luks_devices= | -| =test_archangel.bats= | =gather_input= (unattended branch only) | +| =test_archangel.bats= | =gather_input= (unattended branch only), =install_failure_cleanup= (dispatch shape) | ** What bats does NOT cover (deliberately) @@ -79,7 +79,7 @@ The full list of deliberately-skipped tools and conditions: - *Filesystem creation*: =mkfs.fat=, =mkfs.btrfs=, =mkfs.ext4=, generic =mkfs= - *Encryption*: =cryptsetup= (LUKS), ZFS native encryption via =zfs create -O encryption= - *ZFS pool / dataset operations*: =zpool create=, =zpool import=, =zfs create=, =zfs snapshot=, =zfs rollback= -- *Partitioning*: =sgdisk=, =partprobe=, =blkid= +- *Partitioning*: =sgdisk=, =partprobe=, =blkid=, =wipefs= - *Bootstrap and chroot*: =pacstrap=, =arch-chroot=, =genfstab= - *Bootloader*: =grub-install=, =grub-mkconfig=, =efibootmgr= - *Snapshot management*: =snapper=, =zfs-pre-snapshot= diff --git a/tests/unit/test_disk.bats b/tests/unit/test_disk.bats index f4e6929..1b3cfba 100644 --- a/tests/unit/test_disk.bats +++ b/tests/unit/test_disk.bats @@ -1,11 +1,13 @@ #!/usr/bin/env bats # Unit tests for installer/lib/disk.sh # -# Coverage scope: pure partition-path helpers only. Side-effecting -# functions (partition_disk, partition_disks, format_efi, -# format_efi_partitions, select_disks) wrap sgdisk / mkfs.fat / -# partprobe / fzf and are validated by VM integration per the -# project's testing-strategy.org policy. +# Coverage scope: pure partition-path helpers (get_efi_partition, +# get_root_partition) plus the partition_disks orchestration shape +# (which globals get populated, which destructive tools get invoked, +# how often). The destructive tools themselves (sgdisk, wipefs, +# partprobe, mkfs.fat) and the interactive select_disks (fzf) are +# validated by VM integration per the project's testing-strategy.org +# policy. setup() { # shellcheck disable=SC1091 @@ -75,58 +77,158 @@ setup() { } ############################# -# get_efi_partitions +# partition_disks (orchestration) ############################# +# partition_disks reads SELECTED_DISKS, dispatches the per-disk +# layout (sgdisk + wipefs + mkfs.fat) on FILESYSTEM, and populates +# EFI_PARTS + ROOT_PARTS for downstream callers (create_zfs_pool, +# btrfs_open_encryption, sync_efi_partitions, fstab generation). +# +# The destructive system tools (sgdisk, wipefs, partprobe, mkfs.fat) +# are mocked to capture invocation shape only — actual partition +# behavior is validated by VM integration per testing-strategy.org. -@test "get_efi_partitions: two SATA disks emit two suffixed partitions" { - run get_efi_partitions /dev/sda /dev/sdb - [ "$status" -eq 0 ] - [ "$output" = "/dev/sda1 -/dev/sdb1" ] +partition_disks_setup() { + SELECTED_DISKS=() + EFI_PARTS=() + ROOT_PARTS=() + CALLS=() + sgdisk() { CALLS+=("sgdisk $*"); return 0; } + wipefs() { CALLS+=("wipefs $*"); return 0; } + partprobe() { CALLS+=("partprobe $*"); return 0; } + mkfs.fat() { CALLS+=("mkfs.fat $*"); return 0; } + sleep() { :; } + info() { :; } + step() { :; } + error() { CALLS+=("error $*"); return 1; } } -@test "get_efi_partitions: mixed SATA + NVMe gets correct per-disk suffix" { - run get_efi_partitions /dev/sda /dev/nvme0n1 - [ "$status" -eq 0 ] - [ "$output" = "/dev/sda1 -/dev/nvme0n1p1" ] +@test "partition_disks: populates EFI_PARTS and ROOT_PARTS for single SATA disk" { + partition_disks_setup + SELECTED_DISKS=(/dev/sda) + FILESYSTEM=zfs + + partition_disks + + [ "${#EFI_PARTS[@]}" -eq 1 ] + [ "${#ROOT_PARTS[@]}" -eq 1 ] + [ "${EFI_PARTS[0]}" = "/dev/sda1" ] + [ "${ROOT_PARTS[0]}" = "/dev/sda2" ] } -@test "get_efi_partitions: single NVMe disk emits one p1 partition" { - run get_efi_partitions /dev/nvme0n1 - [ "$status" -eq 0 ] - [ "$output" = "/dev/nvme0n1p1" ] +@test "partition_disks: NVMe disk gets p1/p2 suffixes in globals" { + partition_disks_setup + SELECTED_DISKS=(/dev/nvme0n1) + FILESYSTEM=zfs + + partition_disks + + [ "${EFI_PARTS[0]}" = "/dev/nvme0n1p1" ] + [ "${ROOT_PARTS[0]}" = "/dev/nvme0n1p2" ] } -@test "get_efi_partitions: three disks emit three lines in order" { - run get_efi_partitions /dev/sda /dev/sdb /dev/sdc - [ "$status" -eq 0 ] - local lines - lines=$(echo "$output" | wc -l) - [ "$lines" -eq 3 ] +@test "partition_disks: multi-disk SATA populates both arrays in order" { + partition_disks_setup + SELECTED_DISKS=(/dev/sda /dev/sdb /dev/sdc) + FILESYSTEM=zfs + + partition_disks + + [ "${#EFI_PARTS[@]}" -eq 3 ] + [ "${#ROOT_PARTS[@]}" -eq 3 ] + [ "${EFI_PARTS[0]}" = "/dev/sda1" ] + [ "${EFI_PARTS[1]}" = "/dev/sdb1" ] + [ "${EFI_PARTS[2]}" = "/dev/sdc1" ] + [ "${ROOT_PARTS[0]}" = "/dev/sda2" ] + [ "${ROOT_PARTS[1]}" = "/dev/sdb2" ] + [ "${ROOT_PARTS[2]}" = "/dev/sdc2" ] } -############################# -# get_root_partitions -############################# +@test "partition_disks: mixed SATA + NVMe applies correct suffix per disk" { + partition_disks_setup + SELECTED_DISKS=(/dev/sda /dev/nvme0n1) + FILESYSTEM=zfs -@test "get_root_partitions: two SATA disks emit two suffix-2 partitions" { - run get_root_partitions /dev/sda /dev/sdb - [ "$status" -eq 0 ] - [ "$output" = "/dev/sda2 -/dev/sdb2" ] + partition_disks + + [ "${EFI_PARTS[0]}" = "/dev/sda1" ] + [ "${EFI_PARTS[1]}" = "/dev/nvme0n1p1" ] + [ "${ROOT_PARTS[0]}" = "/dev/sda2" ] + [ "${ROOT_PARTS[1]}" = "/dev/nvme0n1p2" ] } -@test "get_root_partitions: mixed SATA + NVMe applies correct suffix per disk" { - run get_root_partitions /dev/sda /dev/nvme0n1 - [ "$status" -eq 0 ] - [ "$output" = "/dev/sda2 -/dev/nvme0n1p2" ] +@test "partition_disks: ZFS dispatch passes BF00 root type to sgdisk" { + partition_disks_setup + SELECTED_DISKS=(/dev/sda) + FILESYSTEM=zfs + + partition_disks + + [[ " ${CALLS[*]} " == *"sgdisk -n 2:0:0 -t 2:BF00"* ]] } -@test "get_root_partitions: NVMe array gets p2 suffix on each entry" { - run get_root_partitions /dev/nvme0n1 /dev/nvme1n1 - [ "$status" -eq 0 ] - [ "$output" = "/dev/nvme0n1p2 -/dev/nvme1n1p2" ] +@test "partition_disks: Btrfs dispatch passes 8300 root type to sgdisk" { + partition_disks_setup + SELECTED_DISKS=(/dev/sda) + FILESYSTEM=btrfs + + partition_disks + + [[ " ${CALLS[*]} " == *"sgdisk -n 2:0:0 -t 2:8300"* ]] +} + +@test "partition_disks: invokes wipefs -af on each disk" { + partition_disks_setup + SELECTED_DISKS=(/dev/sda /dev/sdb) + FILESYSTEM=zfs + + partition_disks + + local count + count=$(printf '%s\n' "${CALLS[@]}" | grep -c '^wipefs -af ') + [ "$count" -eq 2 ] + [[ " ${CALLS[*]} " == *"wipefs -af /dev/sda"* ]] + [[ " ${CALLS[*]} " == *"wipefs -af /dev/sdb"* ]] +} + +@test "partition_disks: invokes mkfs.fat once per EFI partition with EFI<i> label" { + partition_disks_setup + SELECTED_DISKS=(/dev/sda /dev/sdb) + FILESYSTEM=zfs + + partition_disks + + local count + count=$(printf '%s\n' "${CALLS[@]}" | grep -c '^mkfs.fat ') + [ "$count" -eq 2 ] + [[ " ${CALLS[*]} " == *"mkfs.fat -F32 -n EFI0 /dev/sda1"* ]] + [[ " ${CALLS[*]} " == *"mkfs.fat -F32 -n EFI1 /dev/sdb1"* ]] +} + +@test "partition_disks: re-initializes globals on second call" { + partition_disks_setup + SELECTED_DISKS=(/dev/sda /dev/sdb) + FILESYSTEM=zfs + + partition_disks + [ "${#EFI_PARTS[@]}" -eq 2 ] + + SELECTED_DISKS=(/dev/sdc) + partition_disks + + [ "${#EFI_PARTS[@]}" -eq 1 ] + [ "${#ROOT_PARTS[@]}" -eq 1 ] + [ "${EFI_PARTS[0]}" = "/dev/sdc1" ] +} + +@test "partition_disks: empty SELECTED_DISKS calls error and skips work" { + partition_disks_setup + SELECTED_DISKS=() + FILESYSTEM=zfs + + partition_disks || true + + [[ " ${CALLS[*]} " == *"error partition_disks: SELECTED_DISKS is empty"* ]] + [[ " ${CALLS[*]} " != *"sgdisk"* ]] + [[ " ${CALLS[*]} " != *"wipefs"* ]] } |
