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 29, 2025 · 8cc5d7e · 8cc5d7e
1 parent 1eaabc5
commit 8cc5d7e
Show file tree

Hide file tree

Showing 4 changed files with 252 additions and 55 deletions.
diff --git a/internal/hardware/validator.go b/internal/hardware/validator.go
@@ -27,10 +27,11 @@ 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"
+	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
@@ -155,6 +156,20 @@ 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
 				}
+				// 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"
@@ -293,15 +294,83 @@ var _ = Describe("Disk eligibility", func() {
 		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())
 	})