From 02717d84eaf8d174331e8beab98cd4a184c471af Mon Sep 17 00:00:00 2001
From: CrystalChun <cchun@redhat.com>
Date: Tue, 13 Aug 2024 10:45:21 -0700
Subject: [PATCH] MGMT-18545: Prefer multipath disks when selecting install
 disk

Multipath disks now include the WWN (MGMT-17867) hint and are
included in the list of acceptable install disks. The operator-based
assisted-service will now prefer multipath disk selection if there
is an acceptable multipath disk.
---
 .../controllers/bmh_agent_controller.go       | 14 ++++-
 .../controllers/bmh_agent_controller_test.go  | 61 +++++++++++++++++++
 2 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/internal/controller/controllers/bmh_agent_controller.go b/internal/controller/controllers/bmh_agent_controller.go
index 95b59a42c249..6f7a5c0787ee 100644
--- a/internal/controller/controllers/bmh_agent_controller.go
+++ b/internal/controller/controllers/bmh_agent_controller.go
@@ -1117,7 +1117,7 @@ func (r *BMACReconciler) findInstallationDiskID(devices []aiv1beta1.HostDisk, hi
 
 	acceptable := hostutil.GetAcceptableDisksWithHints(disks, hints)
 	if len(acceptable) > 0 {
-		return acceptable[0].ID
+		return getDiskID(acceptable)
 	}
 
 	// If hints are provided but we did not find an eligible disk, we need to raise an error.
@@ -1130,6 +1130,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.
diff --git a/internal/controller/controllers/bmh_agent_controller_test.go b/internal/controller/controllers/bmh_agent_controller_test.go
index 64e57c68a977..079775ca42c5 100644
--- a/internal/controller/controllers/bmh_agent_controller_test.go
+++ b/internal/controller/controllers/bmh_agent_controller_test.go
@@ -932,6 +932,67 @@ var _ = Describe("bmac reconcile", func() {
 				Expect(err).To(BeNil())
 				Expect(updatedAgent.Spec.InstallationDiskID).To(Equal("1"))
 			})
+
+			It("should set the InstallationDiskID to multipath if the wwn RootDeviceHints were provided and match", func() {
+				priorAgent := &v1beta1.Agent{}
+				Expect(c.Get(ctx, types.NamespacedName{Name: agent.Name, Namespace: agent.Namespace}, priorAgent)).To(BeNil())
+				priorAgent.Status.Inventory.Disks = append(priorAgent.Status.Inventory.Disks, v1beta1.HostDisk{
+					ID:                      "3",
+					InstallationEligibility: v1beta1.HostInstallationEligibility{Eligible: true},
+					ByID:                    "/dev/disk/by-id/wwn-0x1111111111111111111111",
+					DriveType:               string(models.DriveTypeMultipath),
+					Name:                    "sda",
+					SizeBytes:               int64(120) * (int64(1) << 30),
+					Bootable:                true,
+				})
+
+				for i := range priorAgent.Status.Inventory.Disks {
+					priorAgent.Status.Inventory.Disks[i].Wwn = "0x1111111111111111111111"
+				}
+				Expect(c.Update(ctx, priorAgent)).To(BeNil())
+
+				updatedHost := &bmh_v1alpha1.BareMetalHost{}
+				Expect(c.Get(ctx, types.NamespacedName{Name: host.Name, Namespace: testNamespace}, updatedHost)).To(BeNil())
+
+				updatedHost.Spec.RootDeviceHints = &bmh_v1alpha1.RootDeviceHints{
+					WWN: "0x1111111111111111111111",
+				}
+				Expect(c.Update(ctx, updatedHost)).To(BeNil())
+
+				result, err := bmhr.Reconcile(ctx, newBMHRequest(host))
+				Expect(err).To(BeNil())
+				Expect(result).To(Equal(ctrl.Result{}))
+
+				updatedAgent := &v1beta1.Agent{}
+				Expect(c.Get(ctx, types.NamespacedName{Name: agent.Name, Namespace: agent.Namespace}, updatedAgent)).To(BeNil())
+				Expect(updatedAgent.Spec.InstallationDiskID).To(Equal("3"))
+			})
+
+			It("should set the InstallationDiskID to the first disk if the wwn RootDeviceHints were provided and match and there's no multipath disks", func() {
+				priorAgent := &v1beta1.Agent{}
+				Expect(c.Get(ctx, types.NamespacedName{Name: agent.Name, Namespace: agent.Namespace}, priorAgent)).To(BeNil())
+				for i := range priorAgent.Status.Inventory.Disks {
+					priorAgent.Status.Inventory.Disks[i].Wwn = "0x1111111111111111111111"
+				}
+				Expect(c.Update(ctx, priorAgent)).To(BeNil())
+
+				updatedHost := &bmh_v1alpha1.BareMetalHost{}
+				Expect(c.Get(ctx, types.NamespacedName{Name: host.Name, Namespace: testNamespace}, updatedHost)).To(BeNil())
+
+				updatedHost.Spec.RootDeviceHints = &bmh_v1alpha1.RootDeviceHints{
+					WWN: "0x1111111111111111111111",
+				}
+				Expect(c.Update(ctx, updatedHost)).To(BeNil())
+
+				result, err := bmhr.Reconcile(ctx, newBMHRequest(host))
+				Expect(err).To(BeNil())
+				Expect(result).To(Equal(ctrl.Result{}))
+
+				updatedAgent := &v1beta1.Agent{}
+				Expect(c.Get(ctx, types.NamespacedName{Name: agent.Name, Namespace: agent.Namespace}, updatedAgent)).To(BeNil())
+				Expect(updatedAgent.Spec.InstallationDiskID).To(Equal("1"))
+			})
+
 			It("should not touch InstallationDiskID if the RootDeviceHints were not provided", func() {
 				updatedHost := &bmh_v1alpha1.BareMetalHost{}
 				err := c.Get(ctx, types.NamespacedName{Name: host.Name, Namespace: testNamespace}, updatedHost)