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

…orks With the recent addition of the external LB feature in CMN, enabling multiple machine networks, this PR adjusts the MTU validation to accommodate scenarios with multiple machine networks.
openshift · Feb 27, 2025 · 91eaa3a · 91eaa3a
1 parent 444bfdf
commit 91eaa3a
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)
 					}