Expand iSCSI validations to include multipath

This commit expand the 2 iSCSI validations, one during the discovery phase, which checks that the iSCSI disk is not connected through the default network interface (the one used by the default gateway), and another during the networking stage, ensuring that the disk does not belong to the machine's network.
openshift · Jan 30, 2025 · 893ae98 · 893ae98
1 parent d0462ee
commit 893ae98
Show file tree

Hide file tree

Showing 4 changed files with 275 additions and 54 deletions.
diff --git a/internal/hardware/validator.go b/internal/hardware/validator.go
@@ -27,10 +27,12 @@ import (
 )
 
 const (
-	tooSmallDiskTemplate       = "Disk is too small (disk only has %s, but %s are required)"
-	wrongDriveTypeTemplate     = "Drive type is %s, it must be one of %s."
-	wrongMultipathTypeTemplate = "Multipath device has path of type %s, it must be %s"
-	wrongISCSINetworkTemplate  = "iSCSI host IP %s is the same as host IP, they must be different"
+	tooSmallDiskTemplate                    = "Disk is too small (disk only has %s, but %s are required)"
+	wrongDriveTypeTemplate                  = "Drive type is %s, it must be one of %s."
+	wrongMultipathTypeTemplate              = "Multipath device has path of type %s, it must be %s"
+	mixedTypesInMultipath                   = "Multipath device has paths of different types, but they must all be the same type"
+	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"
 )
 
 //go:generate mockgen -source=validator.go -package=hardware -destination=mock_validator.go
@@ -147,6 +149,7 @@ func (v *validator) DiskIsEligible(ctx context.Context, disk *models.Disk, infra
 	}
 
 	if disk.DriveType == models.DriveTypeMultipath {
+		fc, iscsi := false, false
 		for _, inventoryDisk := range inventory.Disks {
 			if strings.Contains(inventoryDisk.Holders, disk.Name) {
 				// We only allow multipath if all paths are FC/ iSCSI
@@ -155,6 +158,30 @@ func (v *validator) DiskIsEligible(ctx context.Context, disk *models.Disk, infra
 						fmt.Sprintf(wrongMultipathTypeTemplate, inventoryDisk.DriveType, strings.Join([]string{string(models.DriveTypeFC), string(models.DriveTypeISCSI)}, ", ")))
 					break
 				}
+				// We only allow multipath if all paths are of the same type
+				if !iscsi && inventoryDisk.DriveType == models.DriveTypeISCSI {
+					iscsi = true
+				} else if !fc && inventoryDisk.DriveType == models.DriveTypeFC {
+					fc = true
+				}
+				if iscsi && fc {
+					notEligibleReasons = append(notEligibleReasons, mixedTypesInMultipath)
+					break
+				}
+				// If errors are detected on iSCSI disks, multipath is not allowed
+				if !lo.Contains(notEligibleReasons, ErrsInIscsiDisableMultipathInstallation) {
+					if inventoryDisk.DriveType == models.DriveTypeISCSI {
+						// check if iSCSI boot drive is valid
+						if !v.IsValidStorageDeviceType(inventoryDisk, hostArchitecture, clusterVersion) {
+							notEligibleReasons = append(notEligibleReasons, ErrsInIscsiDisableMultipathInstallation)
+						}
+						// Check if network is configured properly to install on iSCSI boot drive
+						err = isISCSINetworkingValid(inventoryDisk, inventory)
+						if err != nil {
+							notEligibleReasons = append(notEligibleReasons, ErrsInIscsiDisableMultipathInstallation)
+						}
+					}
+				}
 			}
 		}
 	}

diff --git a/internal/hardware/validator_test.go b/internal/hardware/validator_test.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"errors"
+	"fmt"
 	"math"
 	"os"
 	"testing"
@@ -288,20 +289,100 @@ var _ = Describe("Disk eligibility", func() {
 		Expect(notEligibleReasons).To(BeEmpty())
 	})
 
+	It("Check that mixed types under multipath isn't eligible", func() {
+		testDisk.Name = "dm-0"
+		testDisk.DriveType = models.DriveTypeMultipath
+		allDisks := []*models.Disk{&testDisk, {Name: "sda", DriveType: models.DriveTypeFC, Holders: "dm-0"}, {Name: "sdb", DriveType: models.DriveTypeISCSI, Holders: "dm-0"}}
+		inventory.Disks = allDisks
+
+		notEligibleReasons, err := hwvalidator.DiskIsEligible(ctx, &testDisk, infraEnv, &cluster, &host, inventory)
+
+		Expect(err).ToNot(HaveOccurred())
+		Expect(notEligibleReasons).To(ContainElement(mixedTypesInMultipath))
+	})
+
 	It("Check that Multipath iSCSI is eligible", func() {
 		testDisk.Name = "dm-0"
 		testDisk.DriveType = models.DriveTypeMultipath
 		allDisks := []*models.Disk{&testDisk, {Name: "sda", DriveType: models.DriveTypeISCSI, Holders: "dm-0"}, {Name: "sdb", DriveType: models.DriveTypeISCSI, Holders: "dm-0"}}
 		inventory.Disks = allDisks
+		cluster.OpenshiftVersion = "4.15.0"
+		hostInventory, _ := common.UnmarshalInventory(host.Inventory)
 
+		// Add a default IPv6 route
+		inventory.Routes = append(hostInventory.Routes, &models.Route{
+			Family:      int32(common.IPv6),
+			Interface:   "eth0",
+			Gateway:     "fe80:db8::1",
+			Destination: "::",
+			Metric:      600,
+		})
+		inventory.Interfaces = hostInventory.Interfaces
+
+		operatorsMock.EXPECT().GetRequirementsBreakdownForHostInCluster(gomock.Any(), gomock.Any(), gomock.Any()).Return([]*models.OperatorHostRequirements{}, nil).AnyTimes()
+
+		By("Check Multipath iSCSI is not eligible when host IPv4 address isn't set")
 		notEligibleReasons, err := hwvalidator.DiskIsEligible(ctx, &testDisk, infraEnv, &cluster, &host, inventory)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(notEligibleReasons).To(ContainElement(ErrsInIscsiDisableMultipathInstallation))
 
+		By("Check Multipath iSCSI is eligible when host IPv4 address is not part of default network interface")
+		for _, disk := range allDisks {
+			disk.Iscsi = &models.Iscsi{HostIPAddress: "4.5.6.7"}
+		}
+		notEligibleReasons, err = hwvalidator.DiskIsEligible(ctx, &testDisk, infraEnv, &cluster, &host, inventory)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(notEligibleReasons).To(BeEmpty(), fmt.Sprintf("Debug info: inventory: %s", host.Inventory))
+
+		By("Check Multipath iSCSI is not eligible when host IPv4 address is part of default network interface")
+		for _, disk := range allDisks {
+			disk.Iscsi = &models.Iscsi{HostIPAddress: "1.2.3.4"}
+		}
+		notEligibleReasons, err = hwvalidator.DiskIsEligible(ctx, &testDisk, infraEnv, &cluster, &host, inventory)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(notEligibleReasons).To(ContainElement(ErrsInIscsiDisableMultipathInstallation))
+
+		By("Check Multipath iSCSI is eligible when host IPv6 address is not part of default network interface")
+		for _, disk := range allDisks {
+			disk.Iscsi = &models.Iscsi{HostIPAddress: "1002:db8::10"}
+		}
+		notEligibleReasons, err = hwvalidator.DiskIsEligible(ctx, &testDisk, infraEnv, &cluster, &host, inventory)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(notEligibleReasons).To(BeEmpty())
+
+		By("Check Multipath iSCSI is not eligible when host IPv6 address is part of default network interface")
+		for _, disk := range allDisks {
+			disk.Iscsi = &models.Iscsi{HostIPAddress: "1001:db8::10"}
+		}
+		notEligibleReasons, err = hwvalidator.DiskIsEligible(ctx, &testDisk, infraEnv, &cluster, &host, inventory)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(notEligibleReasons).To(ContainElement(ErrsInIscsiDisableMultipathInstallation))
+
+		By("Check Multipath iSCSI on older version is not eligible")
+		for _, disk := range allDisks {
+			disk.Iscsi = &models.Iscsi{HostIPAddress: "4.5.6.7"}
+		}
+		cluster.OpenshiftVersion = "4.14.1"
+		notEligibleReasons, err = hwvalidator.DiskIsEligible(ctx, &testDisk, infraEnv, &cluster, &host, inventory)
+		Expect(err).ToNot(HaveOccurred())
+		Expect(notEligibleReasons).To(ContainElement(ErrsInIscsiDisableMultipathInstallation))
+
+		By("Check Multipath iSCSI is eligible on day2 cluster")
+		for _, disk := range allDisks {
+			disk.Iscsi = &models.Iscsi{HostIPAddress: "4.5.6.7"}
+		}
+		cluster.Kind = swag.String(models.ClusterKindAddHostsCluster)
+		cluster.OpenshiftVersion = ""
+		infraEnv.OpenshiftVersion = "4.16"
+		notEligibleReasons, err = hwvalidator.DiskIsEligible(ctx, &testDisk, infraEnv, &cluster, &host, inventory)
 		Expect(err).ToNot(HaveOccurred())
 		Expect(notEligibleReasons).To(BeEmpty())
 
 		By("Check infra env Multipath iSCSI is eligible")
+		for _, disk := range allDisks {
+			disk.Iscsi = &models.Iscsi{HostIPAddress: "4.5.6.7"}
+		}
 		notEligibleReasons, err = hwvalidator.DiskIsEligible(ctx, &testDisk, infraEnv, nil, &host, inventory)
-
 		Expect(err).ToNot(HaveOccurred())
 		Expect(notEligibleReasons).To(BeEmpty())
 	})