MGMT-18513: Add machine networks validation when iSCSI boot disk is in use

internal/host/hostcommands/install_cmd.go

@@ -382,20 +382,9 @@ func appendISCSIArgs(installerArgs []string, installationDisk *models.Disk, inve
 		return nil, fmt.Errorf("Cannot parse iSCSI host IP %s: %w", installationDisk.Iscsi.HostIPAddress, err)
 	}
 
-	nic, ok := lo.Find(inventory.Interfaces, func(nic *models.Interface) bool {
-		ips := nic.IPV4Addresses
-		if iSCSIHostIP.Is6() {
-			ips = nic.IPV6Addresses
-		}
-		_, ok := lo.Find(ips, func(ip string) bool {
-			prefix, err := netip.ParsePrefix(ip)
-			return err == nil && iSCSIHostIP.Compare(prefix.Addr()) == 0
-		})
-		return ok
-	})
-
-	if !ok {
-		return nil, fmt.Errorf("Cannot find the interface belonging to iSCSI host IP %s", iSCSIHostIP.String())
+	nic, err := network.FindInterfaceByIP(iSCSIHostIP, inventory.Interfaces)
+	if err != nil {
+		return nil, fmt.Errorf("Cannot find the interface belonging to iSCSI host IP: %w", err)
 	}
 
 	dhcp := "dhcp"

internal/host/hostcommands/install_cmd_test.go

@@ -866,7 +866,7 @@ var _ = Describe("construct host install arguments", func() {
 		inventory, _ := common.UnmarshalInventory(host.Inventory)
 		_, err := constructHostInstallerArgs(cluster, host, inventory, infraEnv, log)
 		Expect(err).To(HaveOccurred())
-		Expect(err.Error()).To(Equal("Cannot find the interface belonging to iSCSI host IP 10.56.21.80"))
+		Expect(err.Error()).To(Equal("Cannot find the interface belonging to iSCSI host IP: Cannot find the network interface on which the IP 10.56.21.80 is set"))
 	})
 	It("iSCSI installation disk - OCI - IPv4 address is left un-configured on machine networks interface", func() {
 		cluster.MachineNetworks = []*models.MachineNetwork{{Cidr: "10.56.21.0/25"}}

internal/host/refresh_status_preprocessor.go

@@ -310,6 +310,10 @@ func newValidations(v *validator) []validation {
 			id:        NoIPCollisionsInNetwork,
 			condition: v.noIPCollisionsInNetwork,
 		},
+		{
+			id:        NoIscsiNicBelongsToMachineCidr,
+			condition: v.noIscsiNicBelongsToMachineCidr,
+		},
 	}
 }
 

internal/host/statemachine.go

@@ -954,6 +954,7 @@ func NewHostStateMachine(sm stateswitch.StateMachine, th TransitionHandler) stat
 		If(NoSkipInstallationDisk),
 		If(NoSkipMissingDisk),
 		If(NoIPCollisionsInNetwork),
+		If(NoIscsiNicBelongsToMachineCidr),
 		/*
 					 * MGMT-15213: The release domain is not resolved correctly when there is a mirror or proxy.  In this case
 					 * validation might fail, but the installation may succeed.

internal/host/transition_test.go

@@ -6556,6 +6556,7 @@ var allValidationIDs = []validationID{
 	NoSkipMissingDisk,
 	NoIPCollisionsInNetwork,
 	IsReleaseDomainNameResolvedCorrectly,
+	NoIscsiNicBelongsToMachineCidr,
 }
 
 var allConditions = []conditionId{
@@ -6708,6 +6709,19 @@ var _ = Describe("State machine test - refresh transition", func() {
 			Expect(stateMachine.Run(TransitionTypeRefresh, testState, &refreshHostArgs)).To(Succeed())
 			Expect(string(testState.State())).To(Equal(models.HostStatusKnown))
 		})
+
+		It("Moves from known to insufficient when  validation fails", func() {
+
+			refreshHostArgs.conditions[string(NoIscsiNicBelongsToMachineCidr)] = false
+
+			Expect(stateMachine.Run(TransitionTypeRefresh, testState, &refreshHostArgs)).To(Succeed())
+			Expect(string(testState.State())).To(Equal(models.HostStatusInsufficient))
+
+			refreshHostArgs.conditions[string(NoIscsiNicBelongsToMachineCidr)] = true
+
+			Expect(stateMachine.Run(TransitionTypeRefresh, testState, &refreshHostArgs)).To(Succeed())
+			Expect(string(testState.State())).To(Equal(models.HostStatusKnown))
+		})
 	})
 
 })

internal/host/validation_id.go

@@ -52,6 +52,7 @@ const (
 	NoSkipInstallationDisk                         = validationID(models.HostValidationIDNoSkipInstallationDisk)
 	NoSkipMissingDisk                              = validationID(models.HostValidationIDNoSkipMissingDisk)
 	NoIPCollisionsInNetwork                        = validationID(models.HostValidationIDNoIPCollisionsInNetwork)
+	NoIscsiNicBelongsToMachineCidr                 = validationID(models.HostValidationIDNoIscsiNicBelongsToMachineCidr)
 )
 
 func (v validationID) category() (string, error) {
@@ -75,7 +76,8 @@ func (v validationID) category() (string, error) {
 		IsDNSWildcardNotConfigured,
 		NonOverlappingSubnets,
 		IsReleaseDomainNameResolvedCorrectly,
-		NoIPCollisionsInNetwork:
+		NoIPCollisionsInNetwork,
+		NoIscsiNicBelongsToMachineCidr:
 		return "network", nil
 	case HasInventory,
 		HasMinCPUCores,

internal/host/validations_test.go

@@ -2339,4 +2339,255 @@ var _ = Describe("Validations test", func() {
 			Expect(ValidationError).To(Equal(status))
 		})
 	})
+	Context("No iSCSI NIC belongs to machine networks", func() {
+		var (
+			cluster common.Cluster
+			host    models.Host
+		)
+		BeforeEach(func() {
+			cluster = hostutil.GenerateTestCluster(clusterID)
+			hostId, infraEnvId := strfmt.UUID(uuid.New().String()), strfmt.UUID(uuid.New().String())
+
+			mockProviderRegistry.EXPECT().IsHostSupported(commontesting.EqPlatformType(models.PlatformTypeVsphere), gomock.Any()).Return(false, nil).AnyTimes()
+
+			host = hostutil.GenerateTestHostByKind(hostId, infraEnvId, &clusterID, models.HostStatusKnown, models.HostKindHost, models.HostRoleMaster)
+			host.Inventory = hostutil.GenerateMasterInventory()
+		})
+
+		It("No installation disk", func() {
+			var inventory models.Inventory
+			err := json.Unmarshal([]byte(host.Inventory), &inventory)
+			Expect(err).ShouldNot(HaveOccurred())
+
+			host.InstallationDiskID = "not-foo"
+			inventory.Disks = []*models.Disk{
+				{ID: "foo", DriveType: models.DriveTypeSSD},
+			}
+
+			inventoryByte, _ := json.Marshal(inventory)
+			host.Inventory = string(inventoryByte)
+
+			Expect(db.Create(&cluster).Error).ToNot(HaveOccurred())
+			Expect(db.Create(&host).Error).ShouldNot(HaveOccurred())
+			mockAndRefreshStatus(&host)
+
+			refreshedHost := hostutil.GetHostFromDB(*host.ID, host.InfraEnvID, db).Host
+
+			_, _, ok := getValidationResult(refreshedHost.ValidationsInfo, NoIscsiNicBelongsToMachineCidr)
+			Expect(ok).To(BeFalse())
+		})
+		It("Not iSCSI drive", func() {
+			var inventory models.Inventory
+			err := json.Unmarshal([]byte(host.Inventory), &inventory)
+			Expect(err).ShouldNot(HaveOccurred())
+
+			host.InstallationDiskID = "install-disk"
+			inventory.Disks = []*models.Disk{
+				{ID: "install-disk", DriveType: models.DriveTypeSSD},
+			}
+
+			inventoryByte, _ := json.Marshal(inventory)
+			host.Inventory = string(inventoryByte)
+
+			Expect(db.Create(&cluster).Error).ToNot(HaveOccurred())
+			Expect(db.Create(&host).Error).ShouldNot(HaveOccurred())
+			mockAndRefreshStatus(&host)
+
+			refreshedHost := hostutil.GetHostFromDB(*host.ID, host.InfraEnvID, db).Host
+
+			_, _, ok := getValidationResult(refreshedHost.ValidationsInfo, NoIscsiNicBelongsToMachineCidr)
+			Expect(ok).To(BeFalse())
+		})
+		It("iSCSI drive - no network interface set with host IP", func() {
+			var inventory models.Inventory
+			err := json.Unmarshal([]byte(host.Inventory), &inventory)
+			Expect(err).ShouldNot(HaveOccurred())
+
+			host.InstallationDiskID = "install-disk"
+			inventory.Disks = []*models.Disk{
+				{
+					ID:        "install-disk",
+					DriveType: models.DriveTypeISCSI,
+					Iscsi: &models.Iscsi{
+						HostIPAddress: "99.99.99.99",
+					}},
+			}
+
+			inventoryByte, _ := json.Marshal(inventory)
+			host.Inventory = string(inventoryByte)
+
+			Expect(db.Create(&cluster).Error).ToNot(HaveOccurred())
+			Expect(db.Create(&host).Error).ShouldNot(HaveOccurred())
+			mockAndRefreshStatus(&host)
+
+			refreshedHost := hostutil.GetHostFromDB(*host.ID, host.InfraEnvID, db).Host
+
+			status, message, ok := getValidationResult(refreshedHost.ValidationsInfo, NoIscsiNicBelongsToMachineCidr)
+			Expect(ok).To(BeTrue())
+			Expect(status).To(Equal(ValidationError))
+			Expect(message).To(Equal("Cannot find network interface associated to iSCSI host IP address"))
+		})
+
+		DescribeTable(
+			"iSCSI drive - malformed iSCSI properties",
+			func(iSCSI *models.Iscsi, expectedMessage string) {
+				var inventory models.Inventory
+				err := json.Unmarshal([]byte(host.Inventory), &inventory)
+				Expect(err).ShouldNot(HaveOccurred())
+
+				host.InstallationDiskID = "install-disk"
+				inventory.Disks = []*models.Disk{
+					{
+						ID:        "install-disk",
+						DriveType: models.DriveTypeISCSI,
+						Iscsi:     iSCSI,
+					},
+				}
+
+				inventoryByte, _ := json.Marshal(inventory)
+				host.Inventory = string(inventoryByte)
+
+				Expect(db.Create(&cluster).Error).ToNot(HaveOccurred())
+				Expect(db.Create(&host).Error).ShouldNot(HaveOccurred())
+				mockAndRefreshStatus(&host)
+
+				refreshedHost := hostutil.GetHostFromDB(*host.ID, host.InfraEnvID, db).Host
+
+				status, message, ok := getValidationResult(refreshedHost.ValidationsInfo, NoIscsiNicBelongsToMachineCidr)
+				Expect(ok).To(BeTrue())
+				Expect(status).To(Equal(ValidationError))
+				Expect(message).To(Equal(expectedMessage))
+			},
+			Entry("iSCSI is nil", nil, "iSCSI installation disk is missing host IP address"),
+			Entry("Host IP address is empty", &models.Iscsi{}, "Cannot find network interface associated to iSCSI host IP address"),
+			Entry("Host IP address is invalid", &models.Iscsi{HostIPAddress: "invalid"}, "Cannot find network interface associated to iSCSI host IP address"),
+		)
+		DescribeTable(
+			"iSCSI drive - machine networks",
+			func(iSCSIHostIPAdress string, machineNetworks []*models.MachineNetwork, interfaces []*models.Interface, expectedStatus ValidationStatus, expectedMessage string) {
+				var inventory models.Inventory
+				err := json.Unmarshal([]byte(host.Inventory), &inventory)
+				Expect(err).ShouldNot(HaveOccurred())
+
+				host.InstallationDiskID = "install-disk"
+				inventory.Disks = []*models.Disk{
+					{
+						ID:        "install-disk",
+						DriveType: models.DriveTypeISCSI,
+						Iscsi: &models.Iscsi{
+							HostIPAddress: iSCSIHostIPAdress,
+						},
+					},
+				}
+				inventory.Interfaces = interfaces
+
+				inventoryByte, _ := json.Marshal(inventory)
+				host.Inventory = string(inventoryByte)
+				cluster.MachineNetworks = machineNetworks
+				Expect(db.Create(&cluster).Error).ToNot(HaveOccurred())
+				Expect(db.Create(&host).Error).ShouldNot(HaveOccurred())
+				mockAndRefreshStatus(&host)
+
+				refreshedHost := hostutil.GetHostFromDB(*host.ID, host.InfraEnvID, db).Host
+
+				status, message, ok := getValidationResult(refreshedHost.ValidationsInfo, NoIscsiNicBelongsToMachineCidr)
+				Expect(ok).To(BeTrue())
+				Expect(message).To(Equal(expectedMessage))
+				Expect(status).To(Equal(expectedStatus))
+			},
+			Entry("Machine networks is nil", "192.168.1.1", nil, nil, ValidationPending, "Missing inventory or machine network CIDR"),
+			Entry("Machine networks is empty", "192.168.1.1", []*models.MachineNetwork{}, nil, ValidationPending, "Missing inventory or machine network CIDR"),
+			Entry("Machine networks IPv4 overlaps with host IPv4 address", "192.168.1.1",
+				[]*models.MachineNetwork{
+					{
+						Cidr:      "192.168.1.0/24",
+						ClusterID: clusterID,
+					},
+				},
+				[]*models.Interface{
+					{
+						IPV4Addresses: []string{"192.168.1.1/24"},
+					},
+				},
+				ValidationFailure, "Network interface connected to iSCSI disk cannot belong to machine network CIDRs"),
+			Entry("Machine networks IPv6 overlaps with host IPv6 address", "2001:0db8:85a3::8a2e:0370:7334",
+				[]*models.MachineNetwork{
+					{
+						Cidr:      "2001:0db8:85a3::/48",
+						ClusterID: clusterID,
+					},
+				},
+				[]*models.Interface{
+					{
+						IPV6Addresses: []string{"2001:0db8:85a3::8a2e:0370:7334/48"},
+					},
+				},
+				ValidationFailure, "Network interface connected to iSCSI disk cannot belong to machine network CIDRs"),
+			Entry("Machine networks IPv6 uses the same interface as host IPv4 address", "192.168.1.1",
+				[]*models.MachineNetwork{
+					{
+						Cidr:      "2001:0db8:85a3::/48",
+						ClusterID: clusterID,
+					},
+				},
+				[]*models.Interface{
+					{
+						IPV4Addresses: []string{"192.168.1.1/24"},
+						IPV6Addresses: []string{"2001:0db8:85a3::8a2e:0370:7334/48"},
+					},
+				},
+				ValidationFailure, "Network interface connected to iSCSI disk cannot belong to machine network CIDRs"),
+			Entry("Machine networks IPv4 uses the same interface as host IPv6 address", "2001:0db8:85a3::8a2e:0370:7334",
+				[]*models.MachineNetwork{
+					{
+						Cidr:      "192.168.1.1/24",
+						ClusterID: clusterID,
+					},
+				},
+				[]*models.Interface{
+					{
+						IPV4Addresses: []string{"192.168.1.1/24"},
+						IPV6Addresses: []string{"2001:0db8:85a3::8a2e:0370:7334/48"},
+					},
+				},
+				ValidationFailure, "Network interface connected to iSCSI disk cannot belong to machine network CIDRs"),
+			Entry("Machine networks IPv4 and host IPv4 address use different interfaces", "192.168.1.1",
+				[]*models.MachineNetwork{
+					{
+						Cidr:      "192.168.2.1/24",
+						ClusterID: clusterID,
+					},
+				},
+				[]*models.Interface{
+					{
+						IPV4Addresses: []string{"192.168.1.1/24"},
+					},
+					{
+						IPV4Addresses: []string{"192.168.2.1/24"},
+					},
+				},
+				ValidationSuccess, "Network interface connected to iSCSI disk does not belong to machine network CIDRs"),
+			Entry("Machine networks IPv4/IPv6 and host IPv4 address use different interfaces", "192.168.1.1",
+				[]*models.MachineNetwork{
+					{
+						Cidr:      "192.168.2.1/24",
+						ClusterID: clusterID,
+					},
+					{
+						Cidr:      "2001:0db8:85a3::/48",
+						ClusterID: clusterID,
+					},
+				},
+				[]*models.Interface{
+					{
+						IPV4Addresses: []string{"192.168.1.1/24"},
+					},
+					{
+						IPV4Addresses: []string{"192.168.2.1/24"},
+						IPV6Addresses: []string{"2001:0db8:85a3::8a2e:0370:7334/48"},
+					},
+				},
+				ValidationSuccess, "Network interface connected to iSCSI disk does not belong to machine network CIDRs"),
+		)
+	})
 })