From 7440446bbb826e7b4cce181db2d5f15ff49ffbea Mon Sep 17 00:00:00 2001 From: Julian Sommer <115516918+juliansommer@users.noreply.github.com> Date: Tue, 11 Aug 2026 09:50:42 +1000 Subject: [PATCH] feat: install ext4 with role-assigned drives for spinifex and predastore (#759) --- cmd/installer/autoinstall/autoinstall.go | 56 ++- cmd/installer/autoinstall/autoinstall_test.go | 47 ++- cmd/installer/install/disk.go | 44 +- cmd/installer/install/diskconfig.go | 187 ++++++++- cmd/installer/install/diskconfig_test.go | 2 +- cmd/installer/install/install.go | 140 ++++++- cmd/installer/install/roles_test.go | 391 ++++++++++++++++++ cmd/installer/ui/storage_test.go | 139 +++++++ cmd/installer/ui/ui.go | 103 ++++- 9 files changed, 1074 insertions(+), 35 deletions(-) create mode 100644 cmd/installer/install/roles_test.go create mode 100644 cmd/installer/ui/storage_test.go diff --git a/cmd/installer/autoinstall/autoinstall.go b/cmd/installer/autoinstall/autoinstall.go index 65cf5d1de..9654167d8 100644 --- a/cmd/installer/autoinstall/autoinstall.go +++ b/cmd/installer/autoinstall/autoinstall.go @@ -138,9 +138,11 @@ func buildConfig() (*install.Config, error) { // resolveStorage builds the disk configuration from the kernel cmdline. // -// SPINIFEX_FS ext4 (default) or one of the zfs-* topologies -// SPINIFEX_DISKS explicit ordered member list, required for multi-disk pools -// SPINIFEX_DISK single-disk selector, as before +// SPINIFEX_FS ext4 (default) or one of the zfs-* topologies +// SPINIFEX_DISKS explicit ordered member list, required for multi-disk pools +// SPINIFEX_DISK the OS disk, as before +// SPINIFEX_DISK_SPINIFEX ext4 only: dedicated disk for /var/lib/spinifex +// SPINIFEX_DISK_PREDASTORE ext4 only: dedicated disk for /var/lib/spinifex/predastore func resolveStorage() (install.DiskConfig, error) { cfg := install.DiskConfig{FS: install.FSExt4} if v := strings.TrimSpace(os.Getenv("SPINIFEX_FS")); v != "" { @@ -171,6 +173,9 @@ func resolveStorage() (install.DiskConfig, error) { cfg.Disks = []install.Disk{d} } + if err := applyDiskRoles(&cfg); err != nil { + return cfg, err + } if cfg.ZFS, err = parseZFSOpts(); err != nil { return cfg, err } @@ -181,6 +186,51 @@ func resolveStorage() (install.DiskConfig, error) { return cfg, nil } +// roleVars maps each optional data role to the variable that assigns it. The OS +// role is not here: it always comes from SPINIFEX_DISK. +var roleVars = []struct { + role install.DiskRole + env string +}{ + {install.RoleSpinifex, "SPINIFEX_DISK_SPINIFEX"}, + {install.RolePredastore, "SPINIFEX_DISK_PREDASTORE"}, +} + +// applyDiskRoles attaches dedicated data drives to an ext4 install. Roles are +// always set on ext4, even with no extra drives, so the OS assignment is +// explicit rather than implied by position. +func applyDiskRoles(cfg *install.DiskConfig) error { + if cfg.FS.IsZFS() { + for _, rv := range roleVars { + if strings.TrimSpace(os.Getenv(rv.env)) != "" { + return fmt.Errorf("%s applies to ext4 only — %s spans every member already", rv.env, cfg.FS.Label()) + } + } + return nil + } + if len(cfg.Disks) != 1 { + return fmt.Errorf("ext4 takes one OS disk, %d given — set SPINIFEX_DISK, and SPINIFEX_DISK_SPINIFEX or SPINIFEX_DISK_PREDASTORE for dedicated drives", + len(cfg.Disks)) + } + + roles := []install.RoleMount{{Role: install.RoleOS, Disk: cfg.Disks[0]}} + for _, rv := range roleVars { + spec := strings.TrimSpace(os.Getenv(rv.env)) + if spec == "" { + continue + } + // Resolved with the same fail-closed selector as the OS disk, so an + // ambiguous value lists the candidates rather than picking one. + d, err := resolveDisk(spec) + if err != nil { + return fmt.Errorf("%s: %w", rv.env, err) + } + roles = append(roles, install.RoleMount{Role: rv.role, Disk: d}) + } + *cfg = cfg.WithRoles(roles) + return nil +} + // parseZFSOpts reads the advanced pool tunables. Every one is optional; unset // values are computed from the hardware at install time. func parseZFSOpts() (install.ZFSOpts, error) { diff --git a/cmd/installer/autoinstall/autoinstall_test.go b/cmd/installer/autoinstall/autoinstall_test.go index ab02d4de0..9dc387393 100644 --- a/cmd/installer/autoinstall/autoinstall_test.go +++ b/cmd/installer/autoinstall/autoinstall_test.go @@ -436,10 +436,53 @@ func TestResolveStorage(t *testing.T) { wantErr: `"vdz" is not an available disk`, }, { - name: "member list that fails validation", + name: "member list is for pools, not ext4 roles", disks: four, env: map[string]string{"SPINIFEX_DISKS": "vdb,vdc"}, - wantErr: "supports a single disk only", + wantErr: "ext4 takes one OS disk", + }, + { + name: "dedicated role drives, os first", + disks: four, + env: map[string]string{ + "SPINIFEX_DISK": "vdb", + "SPINIFEX_DISK_SPINIFEX": "vdc", + "SPINIFEX_DISK_PREDASTORE": "vdd", + }, + wantFS: install.FSExt4, + wantPaths: []string{"/dev/vdb", "/dev/vdc", "/dev/vdd"}, + }, + { + // The two-drive layout: predastore gets its own disk while + // /var/lib/spinifex stays on the root filesystem. + name: "predastore alone", + disks: four, + env: map[string]string{"SPINIFEX_DISK": "vdb", "SPINIFEX_DISK_PREDASTORE": "vdc"}, + wantFS: install.FSExt4, + wantPaths: []string{"/dev/vdb", "/dev/vdc"}, + }, + { + name: "role drive on a pool", + disks: four, + env: map[string]string{ + "SPINIFEX_FS": "zfs-raid1", "SPINIFEX_DISKS": "vdb,vdc", + "SPINIFEX_DISK_SPINIFEX": "vdd", + }, + wantErr: "SPINIFEX_DISK_SPINIFEX applies to ext4 only", + }, + { + name: "role drive that is not present", + disks: four[:1], + env: map[string]string{"SPINIFEX_DISK": "vdb", "SPINIFEX_DISK_PREDASTORE": "vdz"}, + wantErr: "SPINIFEX_DISK_PREDASTORE", + }, + { + // Two roles cannot share a drive; the disk is erased once and the + // second mount would land on top of the first. + name: "role drive repeats the os disk", + disks: four, + env: map[string]string{"SPINIFEX_DISK": "vdb", "SPINIFEX_DISK_SPINIFEX": "vdb"}, + wantErr: "selected more than once", }, { name: "bad zfs tunable", diff --git a/cmd/installer/install/disk.go b/cmd/installer/install/disk.go index 5f95af107..a7d8f5ac8 100644 --- a/cmd/installer/install/disk.go +++ b/cmd/installer/install/disk.go @@ -14,6 +14,10 @@ const ( biosPartNum = 1 espPartNum = 2 rootPartNum = 3 + + // dataPartNum is the sole partition on a role drive. Data drives carry no + // bootloader, so they start at 1 rather than mirroring the boot layout. + dataPartNum = 1 ) // GPT partition type GUIDs, by their sgdisk short code. @@ -60,14 +64,43 @@ func partitionDisks(cfg DiskConfig) error { if cfg.FS.IsZFS() { ptype = typeZFS } - for _, d := range cfg.Disks { + for _, d := range cfg.bootDisks() { if err := partitionOne(d, ptype); err != nil { return fmt.Errorf("partition %s: %w", d.Path, err) } } + for _, rm := range cfg.DataMounts() { + if err := partitionDataDisk(rm); err != nil { + return fmt.Errorf("partition %s (%s): %w", rm.Disk.Path, rm.Role, err) + } + } return waitForPartitions(cfg) } +// partitionDataDisk writes the single-partition GPT for a role drive. It gets +// no bios_boot and no ESP: nothing installs a bootloader there, and a formatted +// ESP no firmware entry points at only misleads whoever reads lsblk in an +// incident. +func partitionDataDisk(rm RoleMount) error { + if err := clearDisk(rm.Disk); err != nil { + return fmt.Errorf("clear: %w", err) + } + // Same tail reserve as a boot drive, for the same reason: a replacement of + // the "same" size is routinely a few thousand sectors smaller. + if err := run("sgdisk", + "-n", fmt.Sprintf("%d:1M:-%dM", dataPartNum, tailReserveMiB), + "-t", fmt.Sprintf("%d:%s", dataPartNum, typeLinuxFS), + "-c", fmt.Sprintf("%d:%s", dataPartNum, rm.Role.Label()), + rm.Disk.Path, + ); err != nil { + return err + } + if err := run("sgdisk", "-G", rm.Disk.Path); err != nil { + slog.Warn("could not randomise GPT GUIDs", "disk", rm.Disk.Path, "err", err) + } + return nil +} + // partitionOne writes the GPT for a single disk. func partitionOne(d Disk, rootType string) error { // Stale ZFS labels and filesystem signatures survive a new partition table @@ -115,7 +148,7 @@ func waitForPartitions(cfg DiskConfig) error { } deadline := time.Now().Add(15 * time.Second) - for _, d := range cfg.Disks { + for _, d := range cfg.bootDisks() { wanted := []string{d.PartitionPath(espPartNum), d.PartitionPath(rootPartNum)} // The pool is built from by-id paths, so their symlinks have to exist // too — udev publishes them later than the kernel device node. @@ -128,6 +161,11 @@ func waitForPartitions(cfg DiskConfig) error { } } } + for _, rm := range cfg.DataMounts() { + if err := waitForPath(rm.Disk.PartitionPath(dataPartNum), deadline); err != nil { + return err + } + } return nil } @@ -146,7 +184,7 @@ func waitForPath(path string, deadline time.Time) error { // formatESPs makes a FAT32 filesystem on every disk's ESP. Each one is // independent — there is no RAID here, they are kept in step by content. func formatESPs(cfg DiskConfig) error { - for _, d := range cfg.Disks { + for _, d := range cfg.bootDisks() { esp := d.PartitionPath(espPartNum) // A distinct label per member so `lsblk -f` on a degraded machine tells // the operator which disk each ESP belongs to. diff --git a/cmd/installer/install/diskconfig.go b/cmd/installer/install/diskconfig.go index 1990ea7af..83c8edf08 100644 --- a/cmd/installer/install/diskconfig.go +++ b/cmd/installer/install/diskconfig.go @@ -82,6 +82,45 @@ func ParseFSType(s string) (FSType, error) { return f, nil } +// DiskRole is the purpose a selected drive serves on an ext4 install. A ZFS +// pool carries every workload on one set of members, so roles do not apply +// there — the topology already spans all the disks. +type DiskRole string + +const ( + RoleOS DiskRole = "os" + RoleSpinifex DiskRole = "spinifex" + RolePredastore DiskRole = "predastore" +) + +// AllDiskRoles is canonical order, outermost mountpoint first. Mounting, fstab +// emission and rsync protect filters all walk it forwards; teardown walks it +// back, because predastore nests inside spinifex. +var AllDiskRoles = []DiskRole{RoleOS, RoleSpinifex, RolePredastore} + +// roleMountpoints is where each role lands on the installed system. A role left +// unassigned simply stays on the filesystem above it. +var roleMountpoints = map[DiskRole]string{ + RoleOS: "/", + RoleSpinifex: "/var/lib/spinifex", + RolePredastore: "/var/lib/spinifex/predastore", +} + +// Label is the role name as shown in the UI and written to partition labels. +func (r DiskRole) Label() string { return string(r) } + +// Mountpoint is the role's path on the installed system. +func (r DiskRole) Mountpoint() string { return roleMountpoints[r] } + +// RoleMount binds one drive to one role. +type RoleMount struct { + Role DiskRole + Disk Disk +} + +// Mountpoint is where this drive's filesystem is mounted on the target. +func (r RoleMount) Mountpoint() string { return r.Role.Mountpoint() } + // ZFSOpts are the advanced tunables. A zero value means "use the computed // default", which is why Ashift and ARCMaxMiB are plain ints — 0 is not legal // for either, so it is unambiguous as a sentinel. @@ -94,14 +133,81 @@ type ZFSOpts struct { } // DiskConfig is the storage half of the installer's configuration. Disks is -// ordered and the order is significant: RAID10 pairs members two at a time. +// ordered and the order is significant: RAID10 pairs members two at a time, and +// on ext4 the first entry is the OS drive. type DiskConfig struct { FS FSType Disks []Disk ZFS ZFSOpts + + // Roles assigns ext4 drives to mountpoints, in AllDiskRoles order. Empty + // means a single-disk install with everything on the root filesystem. + Roles []RoleMount } -// Primary is the first selected disk — the whole target in ext4 mode, and the +// WithRoles returns a copy with ext4 drive roles assigned and Disks rebuilt +// from them in canonical order, so the list of drives to erase and the list of +// roles cannot drift apart — there is one place a drive can be named, not two. +func (d DiskConfig) WithRoles(roles []RoleMount) DiskConfig { + d.Roles, d.Disks = nil, nil + add := func(rm RoleMount) { + d.Roles = append(d.Roles, rm) + d.Disks = append(d.Disks, rm.Disk) + } + for _, want := range AllDiskRoles { + for _, rm := range roles { + if rm.Role == want { + add(rm) + } + } + } + // Anything with an unknown or duplicate role is kept so Validate can name + // it, rather than being dropped into a silently smaller selection. + for _, rm := range roles { + if _, known := roleMountpoints[rm.Role]; !known { + add(rm) + } + } + return d +} + +// RolesForDisks assigns roles positionally, which is the default the UI offers: +// first disk picked is the OS, second is spinifex, third is predastore. +func RolesForDisks(disks []Disk) []RoleMount { + out := make([]RoleMount, 0, len(disks)) + for i, disk := range disks { + var role DiskRole + if i < len(AllDiskRoles) { + role = AllDiskRoles[i] + } + out = append(out, RoleMount{Role: role, Disk: disk}) + } + return out +} + +// DataMounts is every role except the OS drive, shallowest mountpoint first. +// This is the traversal every stage of the install shares. +func (d DiskConfig) DataMounts() []RoleMount { + var out []RoleMount + for _, rm := range d.Roles { + if rm.Role != RoleOS { + out = append(out, rm) + } + } + return out +} + +// bootDisks are the drives a bootloader is installed to. Every ZFS member gets +// its own ESP; on ext4 only the OS drive does, because a data drive holding a +// bootloader that nothing points at is a trap during recovery. +func (d DiskConfig) bootDisks() []Disk { + if d.FS.IsZFS() || len(d.Disks) == 0 { + return d.Disks + } + return d.Disks[:1] +} + +// Primary is the first selected disk — the OS drive in ext4 mode, and the // disk shown in single-disk summaries. func (d DiskConfig) Primary() Disk { if len(d.Disks) == 0 { @@ -222,9 +328,8 @@ func (d DiskConfig) Validate() error { if len(d.Disks) == 0 { return fmt.Errorf("%s: select at least one disk", d.FS.Label()) } - if !d.FS.IsZFS() && len(d.Disks) > 1 { - return fmt.Errorf("%s: supports a single disk only, %d selected — choose a zfs mode to use them all", - d.FS.Label(), len(d.Disks)) + if err := d.validateRoles(); err != nil { + return err } if n := len(d.Disks); n < d.FS.MinDisks() { return fmt.Errorf("%s: needs at least %d disks, %d selected", d.FS.Label(), d.FS.MinDisks(), n) @@ -243,21 +348,66 @@ func (d DiskConfig) Validate() error { if disk.LiveMedia { return fmt.Errorf("%s is the installer's own boot media and cannot be erased", disk.Path) } - // GRUB's i386-pc target cannot read a 4Kn drive, so on legacy BIOS the - // installed system would have no reachable bootloader. - if !bootedEFI() && disk.LogicalBlockSize == 4096 { - return fmt.Errorf("%s is a 4Kn drive (4096-byte sectors) and this machine booted in legacy BIOS mode — booting from it is not supported; enable UEFI in firmware", - disk.Path) - } if disk.Bytes < minRootBytes { return fmt.Errorf("%s is too small (%s, need at least %dGiB)", disk.Path, disk.SizeHuman(), minRootBytes>>30) } } + // GRUB's i386-pc target cannot read a 4Kn drive, so on legacy BIOS a drive + // holding a bootloader would leave the installed system unreachable. Only + // boot drives are constrained: a data role is never booted from. + for _, disk := range d.bootDisks() { + if !bootedEFI() && disk.LogicalBlockSize == 4096 { + return fmt.Errorf("%s is a 4Kn drive (4096-byte sectors) and this machine booted in legacy BIOS mode — booting from it is not supported; enable UEFI in firmware", + disk.Path) + } + } + return d.validateSizes() } +// validateRoles checks the ext4 role assignment. Roles are the only way to use +// more than one disk without ZFS, so a multi-disk selection without them is +// rejected rather than quietly installed onto the first drive. +func (d DiskConfig) validateRoles() error { + if d.FS.IsZFS() { + if len(d.Roles) > 0 { + return fmt.Errorf("%s: drive roles apply to ext4 only — a pool spans every member already", d.FS.Label()) + } + return nil + } + if len(d.Roles) == 0 { + if len(d.Disks) > 1 { + return fmt.Errorf("%s: %d disks selected but no roles assigned — assign os, spinifex and predastore, or choose a zfs mode", + d.FS.Label(), len(d.Disks)) + } + return nil + } + if len(d.Roles) > len(AllDiskRoles) { + return fmt.Errorf("%s: at most %d disks, %d selected — the roles are os, spinifex and predastore", + d.FS.Label(), len(AllDiskRoles), len(d.Roles)) + } + + seen := map[DiskRole]bool{} + for _, rm := range d.Roles { + if _, ok := roleMountpoints[rm.Role]; !ok { + return fmt.Errorf("%s has no role assigned — every selected disk needs one", rm.Disk.Path) + } + if seen[rm.Role] { + return fmt.Errorf("two disks are assigned the %s role", rm.Role) + } + seen[rm.Role] = true + } + if !seen[RoleOS] { + return fmt.Errorf("%s: no disk assigned the os role — one disk must hold the operating system", d.FS.Label()) + } + if len(d.Roles) != len(d.Disks) { + return fmt.Errorf("internal: %d roles for %d disks", len(d.Roles), len(d.Disks)) + } + return nil +} + // minRootBytes matches Proxmox's hard floor for a root disk. const minRootBytes = 2 << 30 @@ -314,5 +464,20 @@ func (d DiskConfig) Warnings() []string { if d.FS.IsZFS() && d.FS != FSZFSRAID0 && d.Buildable() && d.Tolerated() == 0 { out = append(out, "this topology stores no redundancy") } + // A three-drive role layout looks like an array and is not one: each drive + // is an independent point of failure, so the node loses whatever sat on it. + if len(d.DataMounts()) > 0 { + out = append(out, "role-assigned drives store no redundancy — losing any one drive loses what it held") + } + for _, rm := range d.DataMounts() { + if rm.Role == RolePredastore && rm.Disk.Bytes < smallPredastoreBytes { + out = append(out, fmt.Sprintf("%s is small for object storage (%s) — predastore will fill quickly", + rm.Disk.Path, rm.Disk.SizeHuman())) + } + } return out } + +// smallPredastoreBytes is where a dedicated object-store drive stops being +// worth the drive. Advisory only: the install is legal below it. +const smallPredastoreBytes = 64 << 30 diff --git a/cmd/installer/install/diskconfig_test.go b/cmd/installer/install/diskconfig_test.go index 4a530a37e..5523c67e8 100644 --- a/cmd/installer/install/diskconfig_test.go +++ b/cmd/installer/install/diskconfig_test.go @@ -20,7 +20,7 @@ func TestDiskConfigValidate(t *testing.T) { {"no filesystem chosen", DiskConfig{Disks: disks(1, 40)}, "no filesystem selected"}, {"no disks", DiskConfig{FS: FSZFSRAIDZ1}, "select at least one disk"}, - {"ext4 cannot span disks", DiskConfig{FS: FSExt4, Disks: disks(2, 40)}, "single disk only"}, + {"ext4 spans disks only with roles", DiskConfig{FS: FSExt4, Disks: disks(2, 40)}, "no roles assigned"}, {"raidz1 needs three", DiskConfig{FS: FSZFSRAIDZ1, Disks: disks(2, 40)}, "at least 3 disks"}, {"raidz2 needs four", DiskConfig{FS: FSZFSRAIDZ2, Disks: disks(3, 40)}, "at least 4 disks"}, {"raidz3 needs five", DiskConfig{FS: FSZFSRAIDZ3, Disks: disks(4, 40)}, "at least 5 disks"}, diff --git a/cmd/installer/install/install.go b/cmd/installer/install/install.go index fbc44162e..3dde95b50 100644 --- a/cmd/installer/install/install.go +++ b/cmd/installer/install/install.go @@ -61,6 +61,7 @@ func Run(cfg *Config) error { } steps = append(steps, step{"copy rootfs", func() error { return copyRootfs(cfg.Storage) }}, + step{"verify role layout", func() error { return verifyRoleLayout(cfg.Storage) }}, step{"create swap", func() error { return createSwap(cfg.Storage) }}, step{"write fstab", func() error { return writeFstab(cfg.Storage) }}, step{"install spinifex", func() error { return installSpinifex(cfg) }}, @@ -84,7 +85,7 @@ func Run(cfg *Config) error { return reboot() } -// ext4RootSteps formats and mounts a single-disk ext4 root. +// ext4RootSteps formats and mounts the ext4 root and any role filesystems. func ext4RootSteps(cfg *Config) []step { return []step{ {"format partitions", func() error { return formatPartitions(cfg.Storage) }}, @@ -124,15 +125,36 @@ func cleanupTarget(cfg DiskConfig) { exportPool() return } + // Deepest first: predastore nests inside spinifex, and unmounting the outer + // filesystem fails with EBUSY while the inner one is still attached. + for _, rm := range slices.Backward(cfg.DataMounts()) { + _ = runQuiet("umount", targetPath(rm.Mountpoint())) + } _ = runQuiet("umount", efiPart()) _ = runQuiet("umount", mountRoot) } +// targetPath maps a path on the installed system to its location under the +// install mount root. +func targetPath(mountpoint string) string { + return filepath.Join(mountRoot, strings.TrimPrefix(mountpoint, "/")) +} + func formatPartitions(cfg DiskConfig) error { if err := formatESPs(cfg); err != nil { return err } - return run("mkfs.ext4", "-F", cfg.Primary().PartitionPath(rootPartNum)) + if err := run("mkfs.ext4", "-F", cfg.Primary().PartitionPath(rootPartNum)); err != nil { + return err + } + for _, rm := range cfg.DataMounts() { + // Labelled with the role so `lsblk -f` identifies each drive on a + // console that has no access to the installer log. + if err := run("mkfs.ext4", "-F", "-L", rm.Role.Label(), rm.Disk.PartitionPath(dataPartNum)); err != nil { + return fmt.Errorf("format %s (%s): %w", rm.Disk.Path, rm.Role, err) + } + } + return nil } func mountPartitions(cfg DiskConfig) error { @@ -146,7 +168,36 @@ func mountPartitions(cfg DiskConfig) error { if err := os.MkdirAll(efiPart(), 0o755); err != nil { return err } - return run("mount", d.PartitionPath(espPartNum), efiPart()) + if err := run("mount", d.PartitionPath(espPartNum), efiPart()); err != nil { + return err + } + // Role filesystems are mounted here, before the rootfs copy, so rsync lays + // each tree down on its own drive with the ownership setup.sh gave it at + // ISO build time. Mounting afterwards hides those trees and loses it. + for _, rm := range cfg.DataMounts() { + target := targetPath(rm.Mountpoint()) + if err := os.MkdirAll(target, 0o755); err != nil { + return err + } + if err := run("mount", rm.Disk.PartitionPath(dataPartNum), target); err != nil { + return fmt.Errorf("mount %s at %s: %w", rm.Disk.Path, rm.Mountpoint(), err) + } + } + return nil +} + +// mountProtectFilters stops rsync's --delete from acting across a mount +// boundary. Every mountpoint the installer created before the copy needs one, +// whether it is a ZFS dataset or a role filesystem. +func mountProtectFilters(cfg DiskConfig) []string { + if cfg.FS.IsZFS() { + return datasetProtectFilters(cfg) + } + var out []string + for _, rm := range cfg.DataMounts() { + out = append(out, "--filter=P "+rm.Mountpoint()) + } + return out } // copyRootfs copies the live squashfs environment onto the target disk using @@ -171,7 +222,7 @@ func copyRootfs(cfg DiskConfig) error { "--exclude=/lost+found", "--exclude=/boot/efi", } - args = append(args, datasetProtectFilters(cfg)...) + args = append(args, mountProtectFilters(cfg)...) args = append(args, "/", mountRoot+"/") if err := run("rsync", args...); err != nil { return err @@ -213,6 +264,71 @@ func copyRootfs(cfg DiskConfig) error { return nil } +// verifyRoleLayout asserts the rootfs copy landed on the role drives rather +// than on top of them. A mount that happened in the wrong order produces a node +// that boots, starts every service, and has each one writing to an empty +// directory on the wrong drive — healthy-looking and silently wrong. +func verifyRoleLayout(cfg DiskConfig) error { + for _, rm := range cfg.DataMounts() { + target := targetPath(rm.Mountpoint()) + dev, err := deviceOf(target) + if err != nil { + return fmt.Errorf("verify %s: %w", rm.Mountpoint(), err) + } + parentDev, err := deviceOf(filepath.Dir(target)) + if err != nil { + return fmt.Errorf("verify %s: %w", rm.Mountpoint(), err) + } + if dev == parentDev { + return fmt.Errorf("%s is not a separate filesystem — %s was not mounted before the rootfs copy", + rm.Mountpoint(), rm.Disk.Path) + } + if err := matchesLiveOwnership(target, rm.Mountpoint()); err != nil { + return err + } + // Logged so the layout is recoverable from the install log alone, which + // on a headless node is the only record of where each workload landed. + slog.Info("role mounted", "role", rm.Role, "mountpoint", rm.Mountpoint(), "disk", rm.Disk.Path) + } + return nil +} + +// matchesLiveOwnership compares a copied directory against the live rootfs, +// which is where setup.sh set every service owner at ISO build time. Comparing +// rather than hardcoding uids keeps this correct as services are added. +func matchesLiveOwnership(target, source string) error { + src, err := os.Stat(source) + if err != nil { + // Nothing to compare against; the mount check above still holds. + slog.Warn("skipping ownership check, path absent in live environment", "path", source, "err", err) + return nil + } + dst, err := os.Stat(target) + if err != nil { + return fmt.Errorf("verify %s: %w", source, err) + } + srcSt, dstSt := src.Sys().(*syscall.Stat_t), dst.Sys().(*syscall.Stat_t) + if srcSt.Uid != dstSt.Uid || srcSt.Gid != dstSt.Gid || src.Mode().Perm() != dst.Mode().Perm() { + return fmt.Errorf("%s copied as %d:%d %o, expected %d:%d %o — the drive was mounted after the rootfs copy", + source, dstSt.Uid, dstSt.Gid, dst.Mode().Perm(), srcSt.Uid, srcSt.Gid, src.Mode().Perm()) + } + return nil +} + +// deviceOf returns the filesystem a path lives on, for distinguishing a real +// mount from a directory of the same name. +func deviceOf(path string) (uint64, error) { + fi, err := os.Stat(path) + if err != nil { + return 0, err + } + st, ok := fi.Sys().(*syscall.Stat_t) + if !ok { + return 0, fmt.Errorf("%s: no stat information", path) + } + return st.Dev, nil +} + func installSpinifex(cfg *Config) error { // The rootfs copy already contains spx and spinifex-installer at // /usr/local/bin/ — no binary copy needed. Regenerate machine-specific @@ -808,6 +924,18 @@ func writeFstab(cfg DiskConfig) error { } fstab += fmt.Sprintf("UUID=%s / ext4 errors=remount-ro 0 1\nUUID=%s /boot/efi vfat umask=0077 0 1\n", rootUUID, efiUUID) + + // Shallowest first, so `mount -a` in a rescue shell works top-down. + // Deliberately no nofail: a missing data drive must stop the boot for + // an admin, not let predastore write objects onto the OS drive while + // every service reports healthy. fsck pass 2 keeps root first. + for _, rm := range cfg.DataMounts() { + uuid, err := partUUID(rm.Disk.PartitionPath(dataPartNum)) + if err != nil { + return fmt.Errorf("get %s UUID: %w", rm.Role, err) + } + fstab += fmt.Sprintf("UUID=%s %s ext4 errors=remount-ro 0 2\n", uuid, rm.Mountpoint()) + } } // Only claim the swap the previous step actually produced; systemd fails the @@ -1014,7 +1142,9 @@ func parseSize(s string) (int64, error) { return n * mult, nil } -func partUUID(dev string) (string, error) { +// partUUID reads a partition's filesystem UUID. A variable so fstab generation +// can be tested without real block devices. +var partUUID = func(dev string) (string, error) { out, err := exec.Command("blkid", "-s", "UUID", "-o", "value", dev).Output() if err != nil { return "", fmt.Errorf("blkid %s: %w", dev, err) diff --git a/cmd/installer/install/roles_test.go b/cmd/installer/install/roles_test.go new file mode 100644 index 000000000..1ad5a6417 --- /dev/null +++ b/cmd/installer/install/roles_test.go @@ -0,0 +1,391 @@ +package install + +import ( + "fmt" + "os" + "path/filepath" + "slices" + "strings" + "testing" +) + +// roleCfg builds an ext4 configuration from role/disk-name pairs, in whatever +// order they are given — WithRoles is responsible for normalising them. +func roleCfg(pairs ...any) DiskConfig { + var roles []RoleMount + for i := 0; i+1 < len(pairs); i += 2 { + roles = append(roles, RoleMount{ + Role: pairs[i].(DiskRole), + Disk: disk(pairs[i+1].(string), 100), + }) + } + return DiskConfig{FS: FSExt4}.WithRoles(roles) +} + +// fakeUUIDs makes partUUID return a value derived from the device path, so a +// generated fstab can be checked without touching a real disk. +func fakeUUIDs(t *testing.T) { + t.Helper() + prev := partUUID + partUUID = func(dev string) (string, error) { return "uuid" + strings.ReplaceAll(dev, "/", "-"), nil } + t.Cleanup(func() { partUUID = prev }) +} + +func TestWithRolesNormalisesOrderAndDerivesDisks(t *testing.T) { + // Given deepest-first, which is the order that breaks mounting. + cfg := roleCfg(RolePredastore, "c", RoleSpinifex, "b", RoleOS, "a") + + wantRoles := []DiskRole{RoleOS, RoleSpinifex, RolePredastore} + for i, rm := range cfg.Roles { + if rm.Role != wantRoles[i] { + t.Fatalf("role %d = %s, want %s", i, rm.Role, wantRoles[i]) + } + } + // Disks is derived, so the OS drive is first and every role drive appears + // exactly once in the list the installer erases. + if got := cfg.Paths(); !slices.Equal(got, []string{"/dev/a", "/dev/b", "/dev/c"}) { + t.Fatalf("Paths() = %v, want the roles in canonical order", got) + } + if cfg.Primary().Path != "/dev/a" { + t.Errorf("Primary() = %s, want the os drive", cfg.Primary().Path) + } +} + +func TestDataMountsExcludeTheOSDriveAndNestShallowestFirst(t *testing.T) { + got := roleCfg(RoleOS, "a", RoleSpinifex, "b", RolePredastore, "c").DataMounts() + want := []string{"/var/lib/spinifex", "/var/lib/spinifex/predastore"} + for i, rm := range got { + if rm.Mountpoint() != want[i] { + t.Fatalf("mount %d = %s, want %s", i, rm.Mountpoint(), want[i]) + } + } + if len(got) != len(want) { + t.Fatalf("got %d data mounts, want %d", len(got), len(want)) + } +} + +// A predastore drive without a spinifex drive is the two-drive layout, and it +// nests a mount inside the root filesystem rather than inside another mount. +func TestPredastoreRoleWithoutSpinifexIsValid(t *testing.T) { + cfg := roleCfg(RoleOS, "a", RolePredastore, "b") + if err := cfg.Validate(); err != nil { + t.Fatalf("Validate() = %v, want nil", err) + } + if got := cfg.DataMounts(); len(got) != 1 || got[0].Mountpoint() != "/var/lib/spinifex/predastore" { + t.Fatalf("DataMounts() = %v, want predastore only", got) + } +} + +func TestValidateRoles(t *testing.T) { + twoRoles := func(a, b DiskRole) DiskConfig { return roleCfg(a, "a", b, "b") } + + tests := []struct { + name string + cfg DiskConfig + wantErr string + }{ + {"single disk needs no roles", DiskConfig{FS: FSExt4, Disks: disks(1, 40)}, ""}, + {"os alone", roleCfg(RoleOS, "a"), ""}, + {"all three", roleCfg(RoleOS, "a", RoleSpinifex, "b", RolePredastore, "c"), ""}, + {"no os role", twoRoles(RoleSpinifex, RolePredastore), "no disk assigned the os role"}, + {"duplicate role", twoRoles(RoleOS, RoleOS), "two disks are assigned the os role"}, + {"unknown role", roleCfg(RoleOS, "a", DiskRole("cache"), "b"), "no role assigned"}, + {"more disks than roles", roleCfg( + RoleOS, "a", RoleSpinifex, "b", RolePredastore, "c", DiskRole("cache"), "d"), "at most 3 disks"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.cfg.Validate() + switch { + case tt.wantErr == "" && err != nil: + t.Fatalf("Validate() = %v, want nil", err) + case tt.wantErr != "" && (err == nil || !strings.Contains(err.Error(), tt.wantErr)): + t.Fatalf("Validate() = %v, want error containing %q", err, tt.wantErr) + } + }) + } +} + +// Roles describe where a workload lives on a plain filesystem. A pool already +// spans every member, so accepting them there would silently do nothing. +func TestValidateRejectsRolesOnZFS(t *testing.T) { + cfg := DiskConfig{FS: FSZFSRAID1, Disks: disks(2, 40)} + cfg.Roles = []RoleMount{{Role: RoleOS, Disk: cfg.Disks[0]}} + if err := cfg.Validate(); err == nil || !strings.Contains(err.Error(), "ext4 only") { + t.Fatalf("Validate() = %v, want a roles-are-ext4-only error", err) + } +} + +func TestBootDisksAreOnlyTheOSDriveOnExt4(t *testing.T) { + cfg := roleCfg(RoleOS, "a", RoleSpinifex, "b", RolePredastore, "c") + if got := cfg.bootDisks(); len(got) != 1 || got[0].Path != "/dev/a" { + t.Fatalf("bootDisks() = %v, want the os drive only", got) + } + // Every ZFS member carries its own ESP, so all of them stay boot disks. + zfs := DiskConfig{FS: FSZFSRAID1, Disks: disks(2, 40)} + if got := zfs.bootDisks(); len(got) != 2 { + t.Fatalf("bootDisks() = %v, want every pool member", got) + } +} + +func TestWriteFstabEmitsRoleMountsShallowestFirst(t *testing.T) { + root := withMountRoot(t) + targetSkeleton(t, root) + fakeUUIDs(t) + + cfg := roleCfg(RoleOS, "a", RoleSpinifex, "b", RolePredastore, "c") + if err := writeFstab(cfg); err != nil { + t.Fatalf("writeFstab: %v", err) + } + got := readFile(t, root, "etc/fstab") + + // Order matters: `mount -a` in a rescue shell walks the file top-down, and + // predastore cannot be mounted before the filesystem it sits inside. + wantOrder := []string{" / ", "/boot/efi", " /var/lib/spinifex ", " /var/lib/spinifex/predastore "} + at := -1 + for _, want := range wantOrder { + i := strings.Index(got, want) + if i < 0 { + t.Fatalf("fstab is missing %q:\n%s", want, got) + } + if i < at { + t.Fatalf("%q is out of order:\n%s", want, got) + } + at = i + } + + for _, dev := range []string{"/dev/b1", "/dev/c1"} { + if want := "UUID=uuid-dev-" + strings.TrimPrefix(dev, "/dev/"); !strings.Contains(got, want) { + t.Errorf("fstab does not mount %s by UUID:\n%s", dev, got) + } + } + // Data mounts are fsck'd after root, never alongside it. + for line := range strings.SplitSeq(got, "\n") { + if strings.Contains(line, "/var/lib/spinifex") && !strings.HasSuffix(line, "0 2") { + t.Errorf("data mount should be fsck pass 2: %q", line) + } + } +} + +// Fail-closed by design: a missing data drive must stop the boot for an admin, +// not let predastore write objects onto the OS drive while everything reports +// healthy. nofail would silently produce exactly that. +func TestWriteFstabNeverMarksRoleMountsNofail(t *testing.T) { + root := withMountRoot(t) + targetSkeleton(t, root) + fakeUUIDs(t) + + if err := writeFstab(roleCfg(RoleOS, "a", RoleSpinifex, "b", RolePredastore, "c")); err != nil { + t.Fatalf("writeFstab: %v", err) + } + if got := readFile(t, root, "etc/fstab"); strings.Contains(got, "nofail") { + t.Errorf("role mounts must not be nofail:\n%s", got) + } +} + +func TestWriteFstabSingleDiskIsUnchanged(t *testing.T) { + root := withMountRoot(t) + targetSkeleton(t, root) + fakeUUIDs(t) + + if err := writeFstab(roleCfg(RoleOS, "a")); err != nil { + t.Fatalf("writeFstab: %v", err) + } + if got := readFile(t, root, "etc/fstab"); strings.Contains(got, "/var/lib/spinifex") { + t.Errorf("a single-disk install should mount nothing extra:\n%s", got) + } +} + +// rsync runs with --delete, so every mountpoint that exists before the copy +// needs protecting or the copy can delete across a mount boundary on a retry. +func TestMountProtectFiltersCoverEveryRoleMount(t *testing.T) { + got := mountProtectFilters(roleCfg(RoleOS, "a", RoleSpinifex, "b", RolePredastore, "c")) + want := []string{"--filter=P /var/lib/spinifex", "--filter=P /var/lib/spinifex/predastore"} + if !slices.Equal(got, want) { + t.Fatalf("mountProtectFilters() = %v, want %v", got, want) + } + if got := mountProtectFilters(roleCfg(RoleOS, "a")); len(got) != 0 { + t.Errorf("a single-disk install has no mounts to protect, got %v", got) + } + // ZFS keeps its dataset filters — the union is by mode, not additive. + if got := mountProtectFilters(DiskConfig{FS: FSZFSRAID1, Disks: disks(2, 40)}); len(got) == 0 { + t.Error("zfs mode should still protect its dataset mountpoints") + } +} + +func TestMountPartitionsMountsRolesAfterRootAndBeforeTheCopy(t *testing.T) { + f := fakeCommands(t) + root := withMountRoot(t) + + cfg := roleCfg(RoleOS, "a", RoleSpinifex, "b", RolePredastore, "c") + if err := mountPartitions(cfg); err != nil { + t.Fatalf("mountPartitions: %v", err) + } + + // The nested mount cannot precede the one it sits inside, or it is masked. + want := []string{ + "mount /dev/a3 " + root, + "mount /dev/a2 " + root + "/boot/efi", + "mount /dev/b1 " + root + "/var/lib/spinifex", + "mount /dev/c1 " + root + "/var/lib/spinifex/predastore", + } + var mounts []string + for _, c := range f.calls { + if strings.HasPrefix(c, "mount ") { + mounts = append(mounts, c) + } + } + if !slices.Equal(mounts, want) { + t.Fatalf("mount order:\n got %v\nwant %v", mounts, want) + } +} + +func TestCleanupTargetUnmountsDeepestFirst(t *testing.T) { + f := fakeCommands(t) + root := withMountRoot(t) + + cleanupTarget(roleCfg(RoleOS, "a", RoleSpinifex, "b", RolePredastore, "c")) + + // Unmounting the outer filesystem first fails with EBUSY and leaves a + // half-mounted target behind for the next attempt. + want := []string{ + "umount " + root + "/var/lib/spinifex/predastore", + "umount " + root + "/var/lib/spinifex", + "umount " + root + "/boot/efi", + "umount " + root, + } + // The chroot binds are released first and are not part of the target order. + var got []string + for _, c := range f.calls { + if strings.HasPrefix(c, "umount ") && slices.Contains(want, c) { + got = append(got, c) + } + } + if !slices.Equal(got, want) { + t.Fatalf("unmount order:\n got %v\nwant %v", got, want) + } +} + +func TestPartitionDisksGivesDataDrivesOnePartitionAndNoESP(t *testing.T) { + f := fakeCommands(t) + nodes := tempDisks(t, 2, 200) + cfg := DiskConfig{FS: FSExt4}.WithRoles( + []RoleMount{{Role: RoleOS, Disk: nodes[0]}, {Role: RoleSpinifex, Disk: nodes[1]}}) + osDisk, data := nodes[0].Path, nodes[1].Path + + if err := partitionDisks(cfg); err != nil { + t.Fatalf("partitionDisks: %v", err) + } + + f.mustRun(t, "sgdisk", osDisk, "-n 3:0:-1024M", "-t 3:8300") + f.mustRun(t, "sgdisk", osDisk, "-t 2:EF00") + // A data drive is never booted from, so a bios_boot or an ESP there would + // only mislead whoever reads lsblk during an incident. + for _, c := range f.calls { + if strings.Contains(c, data) && (strings.Contains(c, "EF02") || strings.Contains(c, "EF00")) { + t.Errorf("data drive must get no bios_boot or ESP: %q", c) + } + } + f.mustRun(t, "sgdisk", data, "-n 1:1M:-1024M", "-t 1:8300", "-c 1:spinifex") +} + +func TestFormatPartitionsLabelsEachRoleFilesystem(t *testing.T) { + f := fakeCommands(t) + cfg := roleCfg(RoleOS, "a", RoleSpinifex, "b", RolePredastore, "c") + + if err := formatPartitions(cfg); err != nil { + t.Fatalf("formatPartitions: %v", err) + } + // One ESP only: a data drive holding a bootloader nothing points at would + // mislead whoever reads lsblk during an incident. + esps := 0 + for _, c := range f.calls { + if strings.HasPrefix(c, "mkfs.fat") { + esps++ + } + } + if esps != 1 { + t.Errorf("want one ESP, got %d:\n%v", esps, f.calls) + } + f.mustRun(t, "mkfs.ext4", "-L", "spinifex", "/dev/b1") + f.mustRun(t, "mkfs.ext4", "-L", "predastore", "/dev/c1") +} + +func TestWarningsFlagTheAbsenceOfRedundancy(t *testing.T) { + got := strings.Join(roleCfg(RoleOS, "a", RoleSpinifex, "b").Warnings(), "\n") + if !strings.Contains(got, "no redundancy") { + t.Errorf("a multi-drive role layout must warn it is not an array, got %q", got) + } + if got := roleCfg(RoleOS, "a").Warnings(); len(got) != 0 { + t.Errorf("a single-disk install has nothing to warn about, got %v", got) + } +} + +func TestWarningsFlagASmallPredastoreDrive(t *testing.T) { + cfg := DiskConfig{FS: FSExt4}.WithRoles([]RoleMount{ + {Role: RoleOS, Disk: disk("a", 500)}, + {Role: RolePredastore, Disk: disk("b", 8)}, + }) + got := strings.Join(cfg.Warnings(), "\n") + if !strings.Contains(got, "small for object storage") { + t.Errorf("an 8GiB predastore drive should be flagged, got %q", got) + } + if err := cfg.Validate(); err != nil { + t.Errorf("the warning must not block the install: %v", err) + } +} + +func TestRolesForDisksAssignsPositionally(t *testing.T) { + got := RolesForDisks(disks(3, 40)) + for i, rm := range got { + if want := AllDiskRoles[i]; rm.Role != want { + t.Errorf("disk %d got role %s, want %s", i, rm.Role, want) + } + } + // A fourth disk has no role, which Validate is left to reject by name. + if extra := RolesForDisks(disks(4, 40)); extra[3].Role != "" { + t.Errorf("disk 4 got role %q, want none", extra[3].Role) + } +} + +func TestFourKnDataDriveIsAllowedWhereABootDriveIsNot(t *testing.T) { + if bootedEFI() { + t.Skip("machine booted in EFI mode; the 4Kn rule does not apply") + } + fourKn := func(name string) Disk { + d := disk(name, 100) + d.LogicalBlockSize = 4096 + return d + } + // GRUB's i386-pc target cannot read a 4Kn drive, but nothing boots from a + // data drive, so only the OS role is constrained. + ok := DiskConfig{FS: FSExt4}.WithRoles( + []RoleMount{{Role: RoleOS, Disk: disk("a", 100)}, {Role: RoleSpinifex, Disk: fourKn("b")}}) + if err := ok.Validate(); err != nil { + t.Errorf("a 4Kn data drive should be accepted: %v", err) + } + + bad := DiskConfig{FS: FSExt4}.WithRoles( + []RoleMount{{Role: RoleOS, Disk: fourKn("a")}, {Role: RoleSpinifex, Disk: disk("b", 100)}}) + if err := bad.Validate(); err == nil || !strings.Contains(err.Error(), "4Kn") { + t.Errorf("Validate() = %v, want a 4Kn rejection for the os drive", err) + } +} + +func TestVerifyRoleLayoutCatchesAMountThatNeverHappened(t *testing.T) { + root := withMountRoot(t) + // Plain directories on one filesystem: exactly what a mount performed after + // the rootfs copy, or not at all, leaves behind. + if err := os.MkdirAll(filepath.Join(root, "var/lib/spinifex/predastore"), 0o755); err != nil { + t.Fatal(err) + } + + err := verifyRoleLayout(roleCfg(RoleOS, "a", RoleSpinifex, "b")) + if err == nil || !strings.Contains(err.Error(), "not a separate filesystem") { + t.Fatalf("verifyRoleLayout() = %v, want a missing-mount error", err) + } + if !strings.Contains(fmt.Sprint(err), "/dev/b") { + t.Errorf("the error must name the drive that was not mounted: %v", err) + } +} diff --git a/cmd/installer/ui/storage_test.go b/cmd/installer/ui/storage_test.go new file mode 100644 index 000000000..3ae0b9f16 --- /dev/null +++ b/cmd/installer/ui/storage_test.go @@ -0,0 +1,139 @@ +package ui + +import ( + "slices" + "strings" + "testing" + + "github.com/mulgadc/spinifex/cmd/installer/install" +) + +// diskModel is a storage screen with n unselected disks and ext4 chosen, which +// is where the wizard starts. +func diskModel(n int) model { + m := model{fsCursor: slices.Index(install.AllFSTypes, install.FSExt4)} + for i := range n { + m.disks = append(m.disks, install.Disk{ + Path: string(rune('a'+i)) + "-disk", + Bytes: 200 << 30, + LogicalBlockSize: 512, + PhysicalBlockSize: 512, + }) + } + return m +} + +func rolePaths(cfg install.DiskConfig) map[install.DiskRole]string { + out := map[install.DiskRole]string{} + for _, rm := range cfg.Roles { + out[rm.Role] = rm.Disk.Path + } + return out +} + +// A single-disk machine should need no storage input at all, so the disk that +// newModel preselects has to arrive already holding the os role. +func TestPreselectedDiskIsAValidConfiguration(t *testing.T) { + m := newModel([]install.Disk{{Path: "/dev/sda", Bytes: 200 << 30, LogicalBlockSize: 512}}, nil) + cfg := m.storage() + if err := cfg.Validate(); err != nil { + t.Fatalf("the preselected disk must be installable as-is: %v", err) + } + if cfg.Primary().Path != "/dev/sda" { + t.Errorf("Primary() = %q, want the preselected disk", cfg.Primary().Path) + } +} + +// Picking disks in order gives the layout an operator almost always wants, so +// the role key is only needed for the exceptions. +func TestSelectingDisksAssignsRolesInOrder(t *testing.T) { + m := diskModel(3) + for i := range 3 { + m = m.toggleDisk(i) + } + got := rolePaths(m.storage()) + for role, want := range map[install.DiskRole]string{ + install.RoleOS: "a-disk", install.RoleSpinifex: "b-disk", install.RolePredastore: "c-disk", + } { + if got[role] != want { + t.Errorf("%s = %q, want %q", role, got[role], want) + } + } + if err := m.storage().Validate(); err != nil { + t.Errorf("the default assignment must be valid: %v", err) + } +} + +// Deselecting has to drop the role with the disk, or every later role shifts +// onto the wrong drive. +func TestDeselectingADiskReleasesItsRole(t *testing.T) { + m := diskModel(3) + for i := range 3 { + m = m.toggleDisk(i) + } + m = m.toggleDisk(1) // drop the spinifex drive + + got := rolePaths(m.storage()) + if got[install.RoleOS] != "a-disk" || got[install.RolePredastore] != "c-disk" { + t.Fatalf("remaining roles moved: %v", got) + } + if _, ok := got[install.RoleSpinifex]; ok { + t.Errorf("the spinifex role should be free again: %v", got) + } +} + +// The two-drive predastore layout is only reachable by reassigning, so cycling +// swaps rather than refusing a role another disk already holds. +func TestCycleRoleSwapsWithTheDiskThatHoldsIt(t *testing.T) { + m := diskModel(2) + m = m.toggleDisk(0).toggleDisk(1) + // b-disk: spinifex → predastore, which no disk holds, so no swap. + m = m.cycleRole(1) + + got := rolePaths(m.storage()) + if got[install.RoleOS] != "a-disk" || got[install.RolePredastore] != "b-disk" { + t.Fatalf("roles = %v, want os on a-disk and predastore on b-disk", got) + } + + // Cycling past the end wraps onto os, which a-disk holds, so they trade. + m = m.cycleRole(1) + got = rolePaths(m.storage()) + if got[install.RoleOS] != "b-disk" || got[install.RolePredastore] != "a-disk" { + t.Fatalf("roles = %v, want the two disks to have swapped", got) + } + if err := m.storage().Validate(); err != nil { + t.Errorf("a swap must always leave a valid assignment: %v", err) + } +} + +// ZFS pairs mirrors by selection order and has no roles to assign. +func TestStorageLeavesRolesUnsetOnZFS(t *testing.T) { + m := diskModel(2) + m.fsCursor = slices.Index(install.AllFSTypes, install.FSZFSRAID1) + m = m.toggleDisk(0).toggleDisk(1) + + cfg := m.storage() + if len(cfg.Roles) != 0 { + t.Errorf("Roles = %v, want none on a pool", cfg.Roles) + } + if !slices.Equal(cfg.Paths(), []string{"a-disk", "b-disk"}) { + t.Errorf("Paths() = %v, want the selection order preserved", cfg.Paths()) + } +} + +// A three-drive selection looks like an array and is not one, so the mapping +// and the absence of redundancy both have to be on screen before the erase. +func TestConfirmScreenStatesTheLayoutAndTheRisk(t *testing.T) { + m := diskModel(3) + m.height = 40 + for i := range 3 { + m = m.toggleDisk(i) + } + got := m.viewDiskConfirm(100) + + for _, want := range []string{"/var/lib/spinifex/predastore", "c-disk", "losing one loses"} { + if !strings.Contains(got, want) { + t.Errorf("confirm screen is missing %q:\n%s", want, got) + } + } +} diff --git a/cmd/installer/ui/ui.go b/cmd/installer/ui/ui.go index 944b75b21..4e4074b96 100644 --- a/cmd/installer/ui/ui.go +++ b/cmd/installer/ui/ui.go @@ -47,9 +47,13 @@ type model struct { // // selected holds indexes into disks in the order they were picked, which is // what pairs members for RAID10 — so it is a slice, not a set. + // + // roleOf runs parallel to selected and holds the ext4 drive role, so the + // two are edited together and cannot drift. disks []install.Disk storageCursor int selected []int + roleOf []install.DiskRole fsCursor int zfsCursor int zfs install.ZFSOpts @@ -173,11 +177,14 @@ func newModel(disks []install.Disk, nics []netprobe.NIC) model { } // The first non-live, non-removable disk is preselected so a single-disk - // machine needs no storage input at all. + // machine needs no storage input at all. It takes the os role, since on ext4 + // a selected disk with no role is not a valid configuration. var preselected []int + var preselectedRoles []install.DiskRole for i, d := range disks { if !d.LiveMedia && !d.Removable { preselected = []int{i} + preselectedRoles = []install.DiskRole{install.RoleOS} break } } @@ -186,6 +193,7 @@ func newModel(disks []install.Disk, nics []netprobe.NIC) model { screen: screenWelcome, disks: disks, selected: preselected, + roleOf: preselectedRoles, nics: nics, eraseInput: eraseIn, roles: roles, @@ -439,11 +447,22 @@ func (m model) fsType() install.FSType { return install.AllFSTypes[m.fsCursor] } // storage assembles the disk configuration from the current selection. func (m model) storage() install.DiskConfig { cfg := install.DiskConfig{FS: m.fsType(), ZFS: m.zfs} - for _, i := range m.selected { - if i < len(m.disks) { - cfg.Disks = append(cfg.Disks, m.disks[i]) + if cfg.FS.IsZFS() { + for _, i := range m.selected { + if i < len(m.disks) { + cfg.Disks = append(cfg.Disks, m.disks[i]) + } } + return cfg } + roles := make([]install.RoleMount, 0, len(m.selected)) + for si, i := range m.selected { + if i >= len(m.disks) || si >= len(m.roleOf) { + continue + } + roles = append(roles, install.RoleMount{Role: m.roleOf[si], Disk: m.disks[i]}) + } + cfg = cfg.WithRoles(roles) return cfg } @@ -479,6 +498,10 @@ func (m model) handleDiskKey(key string) (tea.Model, tea.Cmd) { m.zfsCursor = 0 m.screen = screenZFSOptions } + case "r": + if !m.fsType().IsZFS() && m.storageCursor != fsRow { + m = m.cycleRole(slices.Index(m.selected, m.storageCursor-1)) + } case "esc": m.validationErr = "" m.screen = screenWelcome @@ -502,12 +525,45 @@ func (m model) handleDiskKey(key string) (tea.Model, tea.Cmd) { // array with the version bubbletea still holds. func (m model) toggleDisk(i int) model { sel := slices.Clone(m.selected) + roles := slices.Clone(m.roleOf) if at := slices.Index(sel, i); at >= 0 { sel = slices.Delete(sel, at, at+1) + roles = slices.Delete(roles, at, at+1) } else { sel = append(sel, i) + roles = append(roles, freeRole(roles)) } m.selected = sel + m.roleOf = roles + m.validationErr = "" + return m +} + +// freeRole is the first role no selected disk holds yet, so picking disks in +// order produces the usual layout without touching the role key at all. +func freeRole(taken []install.DiskRole) install.DiskRole { + for _, r := range install.AllDiskRoles { + if !slices.Contains(taken, r) { + return r + } + } + return "" +} + +// cycleRole moves the disk at selection index si to the next role, swapping +// with whichever disk already holds it. Swapping keeps every press legal, so +// the operator never has to unassign one drive before reassigning another. +func (m model) cycleRole(si int) model { + if si < 0 || si >= len(m.roleOf) { + return m + } + roles := slices.Clone(m.roleOf) + next := install.AllDiskRoles[(slices.Index(install.AllDiskRoles, roles[si])+1)%len(install.AllDiskRoles)] + if other := slices.Index(roles, next); other >= 0 { + roles[other] = roles[si] + } + roles[si] = next + m.roleOf = roles m.validationErr = "" return m } @@ -685,7 +741,7 @@ func (m model) viewDisk(w int) string { subtitle := styleMuted.Render("All data on the selected disks will be permanently erased.") fs := m.fsType() - req := "single disk" + req := "1-3 disks, one per role" if n := fs.MinDisks(); n > 1 { req = fmt.Sprintf("%d+ disks, same size", n) } else if fs.IsZFS() { @@ -702,8 +758,12 @@ func (m model) viewDisk(w int) string { for i, d := range m.disks { marker := "[ ]" if at := slices.Index(m.selected, i); at >= 0 { - // Numbered, not ticked: the order is what pairs mirrors. + // Numbered, not ticked: the order is what pairs mirrors. On ext4 the + // role is what matters instead, so it takes the same column. marker = fmt.Sprintf("[%d]", at+1) + if !fs.IsZFS() && at < len(m.roleOf) { + marker = fmt.Sprintf("[%s]", m.roleOf[at]) + } } note := d.Content switch { @@ -712,7 +772,7 @@ func (m model) viewDisk(w int) string { case d.Removable: note += ", removable" } - line := fmt.Sprintf(" %s %-16s %-8s %-20s %s", marker, d.Path, d.SizeHuman(), truncate(d.Model, 20), note) + line := fmt.Sprintf(" %-12s %-16s %-8s %-20s %s", marker, d.Path, d.SizeHuman(), truncate(d.Model, 20), note) if m.storageCursor == i+1 { line = styleSelected.Render("> " + strings.TrimPrefix(line, " ")) } else { @@ -736,7 +796,7 @@ func (m model) viewDisk(w int) string { rows = append(rows, styleError.Render(" "+m.validationErr)) } - keys := "↑/↓ to move • ←/→ filesystem • Space to select disk • Enter to continue" + keys := "↑/↓ to move • ←/→ filesystem • Space to select disk • R to change its role • Enter to continue" if fs.IsZFS() { keys = "↑/↓ to move • ←/→ filesystem • Space to select disk • A for ZFS options • Enter to continue" } @@ -761,6 +821,11 @@ func (m model) geometryPreview() string { return styleMuted.Render(fmt.Sprintf(" %s %s — %d selected.", cfg.FS.Label(), cfg.Requirement(), len(cfg.Disks))) } + // A role layout has no single capacity worth quoting — what matters is + // which drive each workload lands on. + if mounts := cfg.DataMounts(); len(mounts) > 0 { + return styleLabel.Render(" " + roleLayoutLine(cfg)) + } line := fmt.Sprintf(" %s across %d disk(s) — %s usable", cfg.FS.Label(), len(cfg.Disks), humanBytes(cfg.UsableBytes())) if n := cfg.Tolerated(); n > 0 { @@ -769,6 +834,16 @@ func (m model) geometryPreview() string { return styleLabel.Render(line) } +// roleLayoutLine renders the mountpoint each drive will carry, which is the +// only summary of a role install an operator can check at a glance. +func roleLayoutLine(cfg install.DiskConfig) string { + parts := make([]string, 0, len(cfg.Roles)) + for _, rm := range cfg.Roles { + parts = append(parts, fmt.Sprintf("%s → %s (%s)", rm.Mountpoint(), rm.Disk.Path, rm.Disk.SizeHuman())) + } + return strings.Join(parts, " ") +} + func (m model) viewZFSOptions(w int) string { title := styleTitle.Render("ZFS Options") subtitle := styleMuted.Render("Defaults are computed from the selected disks and this machine's memory.") @@ -793,12 +868,20 @@ func (m model) viewZFSOptions(w int) string { } func (m model) viewDiskConfirm(w int) string { + cfg := m.storage() title := styleTitle.Render("Confirm Disk Erasure") - disk := styleLabel.Render(strings.Join(m.storage().Paths(), ", ")) + disk := styleLabel.Render(strings.Join(cfg.Paths(), ", ")) msg := fmt.Sprintf("All data on %s will be permanently erased.\nType 'yes' to confirm:", disk) var lines []string - lines = append(lines, title, msg, "", m.eraseInput.View()) + lines = append(lines, title) + // A multi-drive ext4 layout resembles an array and is not one, so the + // mapping and the absence of redundancy are both stated before the erase. + if len(cfg.DataMounts()) > 0 { + lines = append(lines, styleLabel.Render(roleLayoutLine(cfg)), + styleWarning.Render("Each drive is independent — losing one loses everything it held."), "") + } + lines = append(lines, msg, "", m.eraseInput.View()) if m.validationErr != "" { lines = append(lines, "", styleError.Render(m.validationErr)) }