OCPBUGS-81880: Mark raw FC/iSCSI multipath members as ineligible

cmd/agentbasedinstaller/host_config.go

@@ -138,7 +138,7 @@ func applyRootDeviceHints(log *log.Logger, host *models.Host, inventory *models.
 
 	diskID := "/dev/not-found-by-hints"
 	if len(acceptableDisks) > 0 {
-		diskID = hostutil.GetPreferredDiskID(acceptableDisks)
+		diskID = acceptableDisks[0].ID
 		log.Infof("Selecting disk %s for installation", diskID)
 	} else {
 		log.Info("No disk found matching root device hints")

internal/controller/controllers/bmh_agent_controller.go

@@ -1169,7 +1169,7 @@ func (r *BMACReconciler) findInstallationDiskID(devices []aiv1beta1.HostDisk, hi
 
 	acceptable := hostutil.GetAcceptableDisksWithHints(disks, hints)
 	if len(acceptable) > 0 {
-		return hostutil.GetPreferredDiskID(acceptable)
+		return getDiskID(acceptable)
 	}
 
 	// If hints are provided but we did not find an eligible disk, we need to raise an error.
@@ -1182,6 +1182,18 @@ func (r *BMACReconciler) findInstallationDiskID(devices []aiv1beta1.HostDisk, hi
 	return "/dev/not-found-by-hints"
 }
 
+// Given a list of acceptable disks,
+// find the multipath disk ID, if it exists,
+// otherwise return the ID of the first disk in the list.
+func getDiskID(disks []*models.Disk) string {
+	for _, disk := range disks {
+		if strings.EqualFold(string(disk.DriveType), string(models.DriveTypeMultipath)) {
+			return disk.ID
+		}
+	}
+	return disks[0].ID
+}
+
 // Finds the agents related to this ClusterDeployment
 //
 // The ClusterDeployment <-> agent relation is one-to-many.

internal/hardware/validator.go

@@ -33,6 +33,7 @@ const (
 	wrongDriveTypeTemplate                  = "Drive type is %s, it must be one of %s."
 	wrongMultipathTypeTemplate              = "Multipath device has path of unsupported type. It must be %s"
 	mixedTypesInMultipath                   = "Multipath device has paths of different types, but they must all be the same type"
+	diskIsMultipathMember                   = "Disk is a member of multipath device %s and is not eligible for installation. Use the multipath device instead."
 	wrongISCSINetworkTemplate               = "iSCSI host IP %s is the same as host IP, they must be different"
 	ErrsInIscsiDisableMultipathInstallation = "Installation on multipath device is not possible due to errors on at least one iSCSI disk"
 	iscsiHostIPNotAvailable                 = "Host IP address is not available"
@@ -62,6 +63,7 @@ func NewValidator(log logrus.FieldLogger, cfg ValidatorCfg, operatorsAPI operato
 		compileDiskReasonTemplate(wrongDriveTypeTemplate, ".*", ".*"),
 		compileDiskReasonTemplate(wrongMultipathTypeTemplate, ".*"),
 		compileDiskReasonTemplate(mixedTypesInMultipath),
+		compileDiskReasonTemplate(diskIsMultipathMember, ".*"),
 		compileDiskReasonTemplate(wrongISCSINetworkTemplate, ".*"),
 		compileDiskReasonTemplate(ErrsInIscsiDisableMultipathInstallation),
 		compileDiskReasonTemplate(iscsiHostIPNotAvailable),
@@ -160,6 +162,14 @@ func (v *validator) DiskIsEligible(ctx context.Context, disk *models.Disk, infra
 			fmt.Sprintf(wrongDriveTypeTemplate, disk.DriveType, strings.Join(v.getValidDeviceStorageTypes(hostArchitecture, clusterVersion), ", ")))
 	}
 
+	// We do not allow FC or iSCSI disks that are members of a multipath device
+	if disk.DriveType == models.DriveTypeFC || disk.DriveType == models.DriveTypeISCSI {
+		err = hasMultipathHolder(disk, inventory)
+		if err != nil {
+			notEligibleReasons = append(notEligibleReasons, err.Error())
+		}
+	}
+
 	if disk.DriveType == models.DriveTypeMultipath {
 		fcDisks := hostutil.GetDisksOfHolderByType(inventory.Disks, disk, models.DriveTypeFC)
 		iSCSIDisks := hostutil.GetDisksOfHolderByType(inventory.Disks, disk, models.DriveTypeISCSI)
@@ -203,6 +213,29 @@ func (v *validator) DiskIsEligible(ctx context.Context, disk *models.Disk, infra
 	return notEligibleReasons, nil
 }
 
+// hasMultipathHolder checks if a disk is a member of a multipath volume by
+// looking for multipath devices in its holders list.
+func hasMultipathHolder(disk *models.Disk, inventory *models.Inventory) error {
+	if disk.Holders == "" {
+		return nil
+	}
+
+	multipathDiskNamesMap := make(map[string]struct{})
+	for _, inventoryDisk := range inventory.Disks {
+		if inventoryDisk.DriveType == models.DriveTypeMultipath {
+			multipathDiskNamesMap[inventoryDisk.Name] = struct{}{}
+		}
+	}
+
+	holders := strings.Split(disk.Holders, ",")
+	for _, holder := range holders {
+		if _, exists := multipathDiskNamesMap[holder]; exists {
+			return fmt.Errorf(diskIsMultipathMember, holder)
+		}
+	}
+	return nil
+}
+
 // isISCSINetworkingValid checks if the iSCSI disk is not connected through the
 // default network interface. The default network interface is the interface
 // which is used by the default gateway.

internal/hardware/validator_test.go

@@ -255,15 +255,15 @@ var _ = Describe("Disk eligibility", func() {
 		Expect(err).ToNot(HaveOccurred())
 		Expect(notEligibleReasons).To(BeEmpty())
 
-		By("Check that iSCSI is eligible when its holder is Multipath.", func() {
+		By("Check that iSCSI is not eligible when its holder is Multipath.", func() {
 			testDisk.Holders = "dm-0"
 			allDisks := []*models.Disk{&testDisk, {Name: "iscsi0", DriveType: models.DriveTypeISCSI, Holders: "dm-0"}, {Name: "dm-0", DriveType: models.DriveTypeMultipath}}
 			inventory.Disks = allDisks
 
 			notEligibleReasons, err := hwvalidator.DiskIsEligible(ctx, &testDisk, infraEnv, &cluster, &host, inventory)
 
 			Expect(err).ToNot(HaveOccurred())
-			Expect(notEligibleReasons).To(BeEmpty())
+			Expect(notEligibleReasons).To(ContainElement(fmt.Sprintf(diskIsMultipathMember, "dm-0")))
 		})
 	})
 
@@ -308,6 +308,18 @@ var _ = Describe("Disk eligibility", func() {
 		Expect(notEligibleReasons).To(BeEmpty())
 	})
 
+	It("Check that FC disk with multipath holder is not eligible", func() {
+		testDisk.DriveType = models.DriveTypeFC
+		testDisk.Holders = "dm-0"
+		allDisks := []*models.Disk{&testDisk, {Name: "sdb", DriveType: models.DriveTypeFC, Holders: "dm-0"}, {Name: "dm-0", DriveType: models.DriveTypeMultipath}}
+		inventory.Disks = allDisks
+
+		notEligibleReasons, err := hwvalidator.DiskIsEligible(ctx, &testDisk, infraEnv, &cluster, &host, inventory)
+
+		Expect(err).ToNot(HaveOccurred())
+		Expect(notEligibleReasons).To(ContainElement(fmt.Sprintf(diskIsMultipathMember, "dm-0")))
+	})
+
 	It("Check that mixed types under multipath isn't eligible", func() {
 		testDisk.Name = "dm-0"
 		testDisk.DriveType = models.DriveTypeMultipath

internal/host/hostutil/host_utils.go

@@ -399,18 +399,6 @@ func GetIgnitionEndpointAndCert(cluster *common.Cluster, host *models.Host, logg
 	return ignitionEndpointUrl, cert, nil
 }
 
-// GetPreferredDiskID returns the ID of the preferred installation disk from a list of acceptable disks.
-// If any disk is a multipath device, it is preferred over raw paths (e.g. FC or iSCSI).
-// Otherwise, the first disk in the list is returned.
-func GetPreferredDiskID(disks []*models.Disk) string {
-	for _, disk := range disks {
-		if strings.EqualFold(string(disk.DriveType), string(models.DriveTypeMultipath)) {
-			return disk.ID
-		}
-	}
-	return disks[0].ID
-}
-
 func GetDisksOfHolderByType(allDisks []*models.Disk, holderDisk *models.Disk, driveTypeFilter models.DriveType) []*models.Disk {
 	disksOfHolder := GetAllDisksOfHolder(allDisks, holderDisk)
 	return lo.Filter(disksOfHolder, func(d *models.Disk, _ int) bool { return d.DriveType == driveTypeFilter })