From 91eaa3a321a9f4665bfa3a5cb11df9622f5f26e2 Mon Sep 17 00:00:00 2001 From: Linoy Hadad Date: Thu, 27 Feb 2025 18:47:58 +0200 Subject: [PATCH] MGMT-20043: Modify MTU Validatin CMN to support multiple machine networks 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. --- internal/host/validations_test.go | 50 ++++++++++++++++++++++--------- internal/host/validator.go | 19 +++++------- 2 files changed, 44 insertions(+), 25 deletions(-) diff --git a/internal/host/validations_test.go b/internal/host/validations_test.go index b93c39db04..ac386019f6 100644 --- 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,7 +2749,6 @@ 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)) @@ -2745,19 +2756,30 @@ var _ = Describe("Validations test", func() { 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 index 0082fa215e..7c3a20d667 100644 --- 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) }