fix: storage node provisioning — wipe all disks, stable disk paths, verify volume injection

Three fixes for storage node provisioning:

1. WipeAllDisks for storage nodes: Old Talos installs on non-OS NVMe
   disks cause boot loops — the server boots from the stale install
   instead of the new one. Storage nodes (ephemeralSize set) now wipe
   ALL NVMe disks during fresh provision. Compute nodes still use
   WipeOSDisk to preserve Ceph data on other disks.

2. Stable /dev/disk/by-id/ paths: NVMe device names (/dev/nvme0n1)
   swap between rescue mode and Talos boot due to different PCI probe
   order. ResolveStableDiskPath resolves the bare device to its by-id
   symlink (based on serial number). This stable path is used for both
   the Talos installer in rescue AND injected into the machineconfig
   (machine.install.disk) so Talos references the correct physical
   disk at boot. The resolved path is persisted in status.resolvedInstallDisk
   for use during stateApplyConfig.

3. Verified injectStorageVolumes: Already called correctly in
   stateApplyConfig when ephemeralSize is set, with proper log message.

Also fixes pre-existing test failures where injectVLANConfig and
injectIPv6Config tests were missing the primaryMAC argument added
when those functions switched to deviceSelector-based NIC matching.

Author: shtse8

api/v1alpha1/hetznerrobotmachine_types.go

@@ -93,6 +93,13 @@ type HetznerRobotMachineStatus struct {
 	// +optional
 	PrimaryMAC string `json:"primaryMAC,omitempty"`
 
+	// ResolvedInstallDisk is the stable /dev/disk/by-id/ path resolved during rescue.
+	// NVMe device names (/dev/nvme0n1) swap between rescue and Talos boot due to
+	// different PCI probe order. This stable path ensures both the installer and
+	// Talos machineconfig reference the same physical disk.
+	// +optional
+	ResolvedInstallDisk string `json:"resolvedInstallDisk,omitempty"`
+
 	// FailureReason is a brief string indicating why this machine failed.
 	// +optional
 	FailureReason *string `json:"failureReason,omitempty"`

controllers/hetznerrobotmachine_controller.go

@@ -562,25 +562,65 @@ func (r *HetznerRobotMachineReconciler) stateInstallTalos(
 			"configured", configuredDisk, "resolved", installDisk, "ip", serverIP)
 	}
 
-	// Wipe only the OS install disk — Ceph OSD data on other disks must survive
-	// reprovision. Wiping all disks would destroy storage cluster data.
-	logger.Info("Wiping OS disk on server", "ip", serverIP, "disk", installDisk)
-	if out, err := sshClient.WipeOSDisk(installDisk); err != nil {
-		return ctrl.Result{}, fmt.Errorf("wipe OS disk %s on %s: %w\nOutput: %s", installDisk, serverIP, err, out)
+	// Resolve the install disk to a stable /dev/disk/by-id/ path.
+	// NVMe device names (/dev/nvme0n1) swap between rescue and Talos boot due to
+	// different PCI probe order. The by-id path references the physical disk by
+	// serial number, so both the installer in rescue AND Talos at boot will
+	// reference the same physical disk regardless of enumeration order.
+	stableDisk, err := sshClient.ResolveStableDiskPath(installDisk)
+	if err != nil {
+		// Non-fatal: fall back to unstable path
+		logger.Info("Could not resolve stable disk path, using bare device name",
+			"disk", installDisk, "error", err)
+		stableDisk = installDisk
+	}
+	if stableDisk != installDisk {
+		logger.Info("Resolved install disk to stable by-id path",
+			"bare", installDisk, "stable", stableDisk, "ip", serverIP)
+	}
+
+	// Persist the stable disk path in status so stateApplyConfig can inject it
+	// into the Talos machineconfig. This ensures Talos uses the by-id path for
+	// upgrades, surviving NVMe enumeration changes across reboots.
+	hrm.Status.ResolvedInstallDisk = stableDisk
+
+	// Storage nodes (ephemeralSize set): wipe ALL NVMe disks for fresh provision.
+	// Old Talos installs on OTHER disks cause boot loops — the server boots from
+	// the old install instead of the new one. This is safe because fresh storage
+	// nodes have no existing Ceph data.
+	//
+	// Compute nodes (no ephemeralSize): wipe ONLY the OS install disk.
+	// Ceph OSD data on other disks must survive reprovision.
+	isStorageNode := hrm.Spec.EphemeralSize != ""
+	if isStorageNode {
+		logger.Info("Storage node: wiping ALL NVMe disks for fresh provision",
+			"ip", serverIP, "installDisk", stableDisk, "ephemeralSize", hrm.Spec.EphemeralSize)
+		if out, err := sshClient.WipeAllDisks(stableDisk); err != nil {
+			return ctrl.Result{}, fmt.Errorf("wipe all disks on %s: %w\nOutput: %s", serverIP, err, out)
+		} else {
+			logger.Info("All NVMe disks wiped", "ip", serverIP, "output", out)
+		}
 	} else {
-		logger.Info("OS disk wiped", "ip", serverIP, "disk", installDisk, "output", out)
+		logger.Info("Compute node: wiping OS disk only", "ip", serverIP, "disk", stableDisk)
+		if out, err := sshClient.WipeOSDisk(installDisk); err != nil {
+			return ctrl.Result{}, fmt.Errorf("wipe OS disk %s on %s: %w\nOutput: %s", installDisk, serverIP, err, out)
+		} else {
+			logger.Info("OS disk wiped", "ip", serverIP, "disk", installDisk, "output", out)
+		}
 	}
 
 	factoryURL := hrc.Spec.TalosFactoryBaseURL
 	if factoryURL == "" {
 		factoryURL = talosFactoryDefaultBaseURL
 	}
 
+	// Use stable by-id path for the Talos installer so the correct physical disk
+	// is targeted regardless of NVMe enumeration order at boot.
 	if err := sshClient.InstallTalos(
 		factoryURL,
 		hrm.Spec.TalosSchematic,
 		hrm.Spec.TalosVersion,
-		installDisk,
+		stableDisk,
 	); err != nil {
 		return ctrl.Result{}, fmt.Errorf("install Talos on %s: %w", serverIP, err)
 	}
@@ -1171,9 +1211,16 @@ func (r *HetznerRobotMachineReconciler) stateApplyConfig(
 
 	// Inject install disk into machineconfig — CAPT doesn't include it,
 	// but Talos requires machine.install.disk to be set.
-	installDisk := hrm.Spec.InstallDisk
+	// Prefer the stable /dev/disk/by-id/ path resolved during rescue install.
+	// This ensures Talos references the correct physical disk regardless of
+	// NVMe enumeration order (which can differ between rescue and Talos boot).
+	installDisk := hrm.Status.ResolvedInstallDisk
 	if installDisk == "" {
-		installDisk = "/dev/nvme0n1"
+		// Fallback for machines provisioned before this fix was deployed
+		installDisk = hrm.Spec.InstallDisk
+		if installDisk == "" {
+			installDisk = "/dev/nvme0n1"
+		}
 	}
 	bootstrapData, err = injectInstallDisk(bootstrapData, installDisk)
 	if err != nil {

controllers/inject_test.go

@@ -64,7 +64,7 @@ cluster:
 		PrefixLength: 24,
 	}
 
-	result, err := injectVLANConfig(input, vlanCfg, "10.10.0.1")
+	result, err := injectVLANConfig(input, vlanCfg, "10.10.0.1", "aa:bb:cc:dd:ee:ff")
 	if err != nil {
 		t.Fatalf("injectVLANConfig failed: %v", err)
 	}
@@ -83,8 +83,10 @@ cluster:
 	}
 
 	iface := interfaces[0].(map[string]interface{})
-	if iface["interface"] != "enp193s0f0np0" {
-		t.Errorf("expected interface enp193s0f0np0, got %v", iface["interface"])
+	// injectVLANConfig uses deviceSelector by MAC, not interface name
+	selector := iface["deviceSelector"].(map[string]interface{})
+	if selector["hardwareAddr"] != "aa:bb:cc:dd:ee:ff" {
+		t.Errorf("expected deviceSelector.hardwareAddr aa:bb:cc:dd:ee:ff, got %v", selector["hardwareAddr"])
 	}
 	if iface["dhcp"] != true {
 		t.Errorf("expected dhcp true, got %v", iface["dhcp"])
@@ -110,7 +112,7 @@ func TestInjectVLANConfig_NilConfig(t *testing.T) {
 	input := []byte(`machine:
   type: controlplane
 `)
-	result, err := injectVLANConfig(input, nil, "10.10.0.1")
+	result, err := injectVLANConfig(input, nil, "10.10.0.1", "aa:bb:cc:dd:ee:ff")
 	if err != nil {
 		t.Fatalf("injectVLANConfig failed: %v", err)
 	}
@@ -124,7 +126,7 @@ func TestInjectVLANConfig_EmptyInternalIP(t *testing.T) {
   type: controlplane
 `)
 	vlanCfg := &infrav1.VLANConfig{ID: 4000, Interface: "eth0"}
-	result, err := injectVLANConfig(input, vlanCfg, "")
+	result, err := injectVLANConfig(input, vlanCfg, "", "aa:bb:cc:dd:ee:ff")
 	if err != nil {
 		t.Fatalf("injectVLANConfig failed: %v", err)
 	}
@@ -134,11 +136,12 @@ func TestInjectVLANConfig_EmptyInternalIP(t *testing.T) {
 }
 
 func TestInjectVLANConfig_MergeExistingInterface(t *testing.T) {
-	// Existing config already has the interface with some settings
+	// Existing config already has the interface matched by deviceSelector MAC
 	input := []byte(`machine:
   network:
     interfaces:
-      - interface: enp193s0f0np0
+      - deviceSelector:
+          hardwareAddr: "aa:bb:cc:dd:ee:ff"
         mtu: 9000
 `)
 	vlanCfg := &infrav1.VLANConfig{
@@ -147,7 +150,7 @@ func TestInjectVLANConfig_MergeExistingInterface(t *testing.T) {
 		PrefixLength: 24,
 	}
 
-	result, err := injectVLANConfig(input, vlanCfg, "10.10.0.2")
+	result, err := injectVLANConfig(input, vlanCfg, "10.10.0.2", "aa:bb:cc:dd:ee:ff")
 	if err != nil {
 		t.Fatalf("injectVLANConfig failed: %v", err)
 	}
@@ -338,7 +341,7 @@ func TestInjectVLANConfig_DefaultPrefixLength(t *testing.T) {
 		// PrefixLength not set — should default to 24
 	}
 
-	result, err := injectVLANConfig(input, vlanCfg, "10.10.0.3")
+	result, err := injectVLANConfig(input, vlanCfg, "10.10.0.3", "aa:bb:cc:dd:ee:ff")
 	if err != nil {
 		t.Fatalf("injectVLANConfig failed: %v", err)
 	}
@@ -366,7 +369,7 @@ func TestInjectIPv6Config(t *testing.T) {
 	input := []byte(`machine:
   type: controlplane
 `)
-	result, err := injectIPv6Config(input, "2a01:4f8:271:3b49::", "enp193s0f0np0")
+	result, err := injectIPv6Config(input, "2a01:4f8:271:3b49::", "aa:bb:cc:dd:ee:ff", "10.10.0.1")
 	if err != nil {
 		t.Fatalf("injectIPv6Config failed: %v", err)
 	}
@@ -385,8 +388,10 @@ func TestInjectIPv6Config(t *testing.T) {
 	}
 
 	iface := interfaces[0].(map[string]interface{})
-	if iface["interface"] != "enp193s0f0np0" {
-		t.Errorf("expected interface enp193s0f0np0, got %v", iface["interface"])
+	// Uses deviceSelector by MAC, not interface name
+	deviceSelector := iface["deviceSelector"].(map[string]interface{})
+	if deviceSelector == nil {
+		t.Errorf("expected deviceSelector, got nil")
 	}
 
 	addrs := iface["addresses"].([]interface{})
@@ -414,7 +419,7 @@ func TestInjectIPv6Config_Empty(t *testing.T) {
 	input := []byte(`machine:
   type: controlplane
 `)
-	result, err := injectIPv6Config(input, "", "enp193s0f0np0")
+	result, err := injectIPv6Config(input, "", "aa:bb:cc:dd:ee:ff", "10.10.0.1")
 	if err != nil {
 		t.Fatalf("injectIPv6Config failed: %v", err)
 	}
@@ -424,15 +429,17 @@ func TestInjectIPv6Config_Empty(t *testing.T) {
 }
 
 func TestInjectIPv6Config_MergeExistingInterface(t *testing.T) {
+	// Existing config uses deviceSelector with MAC — injectIPv6Config matches by hardwareAddr
 	input := []byte(`machine:
   network:
     interfaces:
-      - interface: enp193s0f0np0
+      - deviceSelector:
+          hardwareAddr: aa:bb:cc:dd:ee:ff
         dhcp: true
         addresses:
           - 10.0.0.1/24
 `)
-	result, err := injectIPv6Config(input, "2a01:4f8::", "enp193s0f0np0")
+	result, err := injectIPv6Config(input, "2a01:4f8::", "aa:bb:cc:dd:ee:ff", "10.10.0.1")
 	if err != nil {
 		t.Fatalf("injectIPv6Config failed: %v", err)
 	}

pkg/sshrescue/client.go

@@ -196,6 +196,90 @@ func (c *Client) WipeOSDisk(disk string) (string, error) {
 	return c.Run(cmd)
 }
 
+// WipeAllDisks wipes ALL NVMe disks on the server for a fresh provision.
+// Used for storage nodes where old Talos installs on ANY disk cause boot loops —
+// the server boots from the old install instead of the new one. Unlike WipeOSDisk,
+// this does NOT check for ceph_bluestore because it is only called during fresh
+// provisioning when no Ceph data exists yet.
+//
+// The installDisk parameter is logged but all NVMe disks are wiped regardless.
+func (c *Client) WipeAllDisks(installDisk string) (string, error) {
+	// List all NVMe block devices
+	listCmd := `lsblk --noheadings --nodeps --paths --output NAME,TYPE | grep disk | grep nvme | awk '{print $1}'`
+	out, err := c.Run(listCmd)
+	if err != nil {
+		return out, fmt.Errorf("list NVMe disks: %w", err)
+	}
+
+	disks := strings.Fields(strings.TrimSpace(out))
+	if len(disks) == 0 {
+		return "", fmt.Errorf("no NVMe disks found")
+	}
+
+	var results []string
+	for _, disk := range disks {
+		cmd := fmt.Sprintf(
+			`set -e; `+
+				`echo "Wiping disk %[1]s..."; `+
+				`wipefs -af %[1]q 2>/dev/null || true; `+
+				`sgdisk --zap-all %[1]q 2>/dev/null || true; `+
+				`dd if=/dev/zero of=%[1]q bs=1M count=100 conv=notrunc 2>/dev/null || true; `+
+				`blkdiscard %[1]q 2>/dev/null || true; `+
+				`sync; `+
+				`echo "Disk %[1]s wiped"`,
+			disk,
+		)
+		wipeOut, wipeErr := c.Run(cmd)
+		if wipeErr != nil {
+			return wipeOut, fmt.Errorf("wipe disk %s: %w", disk, wipeErr)
+		}
+		results = append(results, strings.TrimSpace(wipeOut))
+	}
+
+	summary := fmt.Sprintf("Wiped %d NVMe disks (install=%s): %s", len(disks), installDisk, strings.Join(disks, ", "))
+	results = append(results, summary)
+	return strings.Join(results, "\n"), nil
+}
+
+// ResolveStableDiskPath resolves a bare NVMe device path (e.g. /dev/nvme0n1)
+// to its stable /dev/disk/by-id/ path using the disk's serial number.
+// This is critical because NVMe device names swap between rescue mode and Talos
+// boot due to different PCI probe order. The by-id path references the physical
+// disk deterministically regardless of enumeration order.
+//
+// If the disk is already a /dev/disk/by-id/ path, it is returned as-is.
+// Returns the original disk path if resolution fails (best-effort).
+func (c *Client) ResolveStableDiskPath(disk string) (string, error) {
+	// Already a stable path — nothing to resolve
+	if strings.HasPrefix(disk, "/dev/disk/by-id/") {
+		return disk, nil
+	}
+
+	// Get the basename (e.g. "nvme0n1" from "/dev/nvme0n1")
+	basename := disk
+	if idx := strings.LastIndex(disk, "/"); idx >= 0 {
+		basename = disk[idx+1:]
+	}
+
+	// Find the by-id symlink that points to this device (excluding partition entries)
+	cmd := fmt.Sprintf(
+		`ls -la /dev/disk/by-id/ 2>/dev/null | grep -E '%s$' | grep nvme | grep -v part | awk '{print $9}' | head -1`,
+		basename,
+	)
+	out, err := c.Run(cmd)
+	if err != nil {
+		return disk, nil // best-effort: return original on failure
+	}
+
+	byIDName := strings.TrimSpace(out)
+	if byIDName == "" {
+		return disk, nil // no by-id link found, return original
+	}
+
+	stablePath := "/dev/disk/by-id/" + byIDName
+	return stablePath, nil
+}
+
 // InstallTalos installs Talos Linux on the server using the official OCI
 // installer binary inside a minimal namespace created by unshare(1).
 //