MGMT-20043: Modify MTU Validatin CMN to support multiple machine netw…

…orks
openshift · Feb 27, 2025 · 831149b · 831149b
1 parent 444bfdf
commit 831149b
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 25 deletions.
diff --git a/internal/host/validations_test.go b/internal/host/validations_test.go
@@ -2699,7 +2699,25 @@ var _ = Describe("Validations test", func() {
 			cluster *common.Cluster
 			host    *models.Host
 		)
-		DescribeTable("MTU validations across CMN and UMN", func(mtuReports []*models.MtuReport, validationStatus ValidationStatus, isUMN, isDay2 bool) {
+
+		mNet := []*models.MachineNetwork{
+			{
+				Cidr:      "1.2.3.1/24",
+				ClusterID: clusterID,
+			},
+		}
+
+		multipleMNets := []*models.MachineNetwork{
+			{
+				Cidr:      "1.2.3.1/24",
+				ClusterID: clusterID,
+			},
+			{
+				Cidr:      "1.2.4.1/24",
+				ClusterID: clusterID,
+			},
+		}
+		DescribeTable("MTU validations across CMN and UMN", func(mtuReports []*models.MtuReport, mNets []*models.MachineNetwork, validationStatus ValidationStatus, isUMN, isDay2 bool) {
 			mockProviderRegistry.EXPECT().IsHostSupported(commontesting.EqPlatformType(models.PlatformTypeVsphere), gomock.Any()).Return(false, nil).AnyTimes()
 
 			if isDay2 {
@@ -2714,13 +2732,7 @@ var _ = Describe("Validations test", func() {
 			}
 
 			cluster.UserManagedNetworking = &isUMN
-			mNet := []*models.MachineNetwork{
-				{
-					Cidr:      "1.2.3.1/24",
-					ClusterID: clusterID,
-				},
-			}
-			cluster.MachineNetworks = mNet
+			cluster.MachineNetworks = mNets
 
 			connectivityReport := &models.ConnectivityReport{}
 			connectivityReport.RemoteHosts = append(connectivityReport.RemoteHosts, &models.ConnectivityRemoteHost{
@@ -2737,27 +2749,37 @@ var _ = Describe("Validations test", func() {
 			mockAndRefreshStatus(host)
 
 			refreshedHost := &hostutil.GetHostFromDB(*host.ID, host.InfraEnvID, db).Host
-			fmt.Println(host.ValidationsInfo)
 			status, _, ok := getValidationResult(refreshedHost.ValidationsInfo, IsMtuValid)
 			Expect(ok).To(BeTrue(), fmt.Sprintf("debuuging info: status %s, isOK: %t host: %#v cluster: %#v", status, ok, host, cluster))
 			Expect(status).To(Equal(validationStatus))
 		},
 			Entry("isMtuValidInMachineNetwork (CMN) - Happy flow - MTU reports in the machine network are successful", []*models.MtuReport{
 				{MtuSuccessful: true, RemoteIPAddress: "1.2.3.5", OutgoingNic: "eth0"},
 				{MtuSuccessful: false, RemoteIPAddress: "1.2.4.5", OutgoingNic: "eth0"},
-			}, ValidationSuccess, false, false),
+			}, mNet, ValidationSuccess, false, false),
 			Entry("isMtuValidInMachineNetwork (CMN) - Bad flow - MTU reports in the machine network are unsuccessful", []*models.MtuReport{
 				{MtuSuccessful: false, RemoteIPAddress: "1.2.3.5", OutgoingNic: "eth0"},
-			}, ValidationFailure, false, false),
+			}, mNet, ValidationFailure, false, false),
 			Entry("isMtuValidAllInterfaces (UMN) - Happy flow - all Mtu reports successful", []*models.MtuReport{
 				{MtuSuccessful: true, RemoteIPAddress: "1.2.3.5", OutgoingNic: "eth0"},
 				{MtuSuccessful: true, RemoteIPAddress: "1.2.4.5", OutgoingNic: "eth0"},
-			}, ValidationSuccess, true, false),
+			}, mNet, ValidationSuccess, true, false),
 			Entry("isMtuValidAllInterfaces (UMN) - Bad flow - all MTU reports are unsuccessful", []*models.MtuReport{
 				{MtuSuccessful: false, RemoteIPAddress: "1.2.3.5", OutgoingNic: "eth0"},
 				{MtuSuccessful: false, RemoteIPAddress: "1.2.4.5", OutgoingNic: "eth0"},
-			}, ValidationFailure, true, false),
-			Entry("Day 2 cluster - the validation should be skipped", []*models.MtuReport{}, ValidationSuccess, false, true),
+			}, mNet, ValidationFailure, true, false),
+			// Reports are successful when remote IPs are within the machine networks but unsuccessful when the remote IP is outside the machine network → validation should pass.
+			Entry("isMtuValidInMachineNetwork (CMN) - Happy flow - MTU reports in the machine networks are successful, outside machine networks are unsuccessful", []*models.MtuReport{
+				{MtuSuccessful: true, RemoteIPAddress: "1.2.3.5", OutgoingNic: "eth0"},
+				{MtuSuccessful: true, RemoteIPAddress: "1.2.4.5", OutgoingNic: "eth0"},
+				{MtuSuccessful: false, RemoteIPAddress: "1.2.5.5", OutgoingNic: "eth0"},
+			}, multipleMNets, ValidationSuccess, false, false),
+			// Although the host IP is "1.2.3.4/24", which belongs to the machine network "1.2.3.1/24", the unsuccessful report with the remote IP in the second machine network causes the validation to fail.
+			Entry("isMtuValidInMachineNetwork (CMN) - Bad flow - MTU report in at least one of the machine networks are unsuccessful", []*models.MtuReport{
+				{MtuSuccessful: true, RemoteIPAddress: "1.2.3.5", OutgoingNic: "eth0"},
+				{MtuSuccessful: false, RemoteIPAddress: "1.2.4.5", OutgoingNic: "eth0"},
+			}, multipleMNets, ValidationFailure, false, false),
+			Entry("Day 2 cluster - the validation should be skipped", []*models.MtuReport{}, mNet, ValidationSuccess, false, true),
 		)
 	})
 

diff --git a/internal/host/validator.go b/internal/host/validator.go
@@ -346,19 +346,16 @@ func (v *validator) isMtuValidAllInterfaces(c *validationContext, connectivityRe
 
 // isMtuValidInMachineNetwork - intended for CMN, focusing exclusively on the MTU reports for the machine network.
 func (v *validator) isMtuValidInMachineNetwork(c *validationContext, connectivityReport *models.ConnectivityReport) (ValidationStatus, string) {
-	for _, machineNet := range c.cluster.MachineNetworks {
-		_, mNetwork, err := net.ParseCIDR(string(machineNet.Cidr))
-		if err != nil {
-			return ValidationError, "Internal error - failed to parse machine network CIDR"
-		}
-		for _, r := range connectivityReport.RemoteHosts {
-			for _, mtuReport := range r.MtuReport {
-				sourceIP, err := network.FindSourceIPInMachineNetwork(mtuReport.OutgoingNic, mNetwork, c.inventory.Interfaces)
+	for _, r := range connectivityReport.RemoteHosts {
+		for _, mtuReport := range r.MtuReport {
+			remoteIP := net.ParseIP(mtuReport.RemoteIPAddress)
+			for _, machineNet := range c.cluster.MachineNetworks {
+				_, mNetwork, err := net.ParseCIDR(string(machineNet.Cidr))
 				if err != nil {
-					return ValidationError, err.Error()
+					return ValidationError, "Internal error - failed to parse machine network CIDR"
 				}
-				remoteIP := net.ParseIP(mtuReport.RemoteIPAddress)
-				if mNetwork.Contains(remoteIP) && sourceIP != "" {
+				// We are only interested in the remote IP within machine networks. If it is not part of any machine network, we don't need to consider it.
+				if mNetwork.Contains(remoteIP) {
 					if !mtuReport.MtuSuccessful {
 						return ValidationFailure, fmt.Sprintf("MTU is broken. Interface: %s, remote IP address: %s", mtuReport.OutgoingNic, mtuReport.RemoteIPAddress)
 					}