From ea94a5aca5cd29f9db93015bb4d1df209d4fb0da Mon Sep 17 00:00:00 2001 From: Patryk Diak Date: Tue, 3 Sep 2024 10:42:54 +0200 Subject: [PATCH 01/12] Layer2 UDN: Rename gateway switch ports In Layer2 networks there is no join switch, only the cluster switch. Ensure that the ports are named appropriately, this is important for the logical router policies created for local node access. Signed-off-by: Patryk Diak (cherry picked from commit 967923ec75b5ec50d80e158a33441ece2eb82fdb) --- go-controller/pkg/ovn/gateway.go | 21 ++++++++++++++++++- go-controller/pkg/ovn/multihoming_test.go | 10 ++------- ...econdary_layer2_network_controller_test.go | 2 +- 3 files changed, 23 insertions(+), 10 deletions(-) diff --git a/go-controller/pkg/ovn/gateway.go b/go-controller/pkg/ovn/gateway.go index f5b42121e1..9e0edcda0c 100644 --- a/go-controller/pkg/ovn/gateway.go +++ b/go-controller/pkg/ovn/gateway.go @@ -333,6 +333,15 @@ func (gw *GatewayManager) GatewayInit( gwSwitchPort := types.JoinSwitchToGWRouterPrefix + gatewayRouter gwRouterPort := types.GWRouterToJoinSwitchPrefix + gatewayRouter + // In Layer2 networks there is no join switch and the gw.joinSwitchName points to the cluster switch. + // Ensure that the ports are named appropriately, this is important for the logical router policies + // created for local node access. + // TODO(kyrtapz): Clean this up for clarity as part of https://github.com/ovn-org/ovn-kubernetes/issues/4689 + if gw.netInfo.TopologyType() == types.Layer2Topology { + gwSwitchPort = types.SwitchToRouterPrefix + gw.joinSwitchName + gwRouterPort = types.RouterToSwitchPrefix + gw.joinSwitchName + } + logicalSwitchPort := nbdb.LogicalSwitchPort{ Name: gwSwitchPort, Type: "router", @@ -1001,6 +1010,17 @@ func (gw *GatewayManager) Cleanup() error { var nextHops []net.IP gwRouterToJoinSwitchPortName := types.GWRouterToJoinSwitchPrefix + gw.gwRouterName + portName := types.JoinSwitchToGWRouterPrefix + gw.gwRouterName + + // In Layer2 networks there is no join switch and the gw.joinSwitchName points to the cluster switch. + // Ensure that the ports are named appropriately, this is important for the logical router policies + // created for local node access. + // TODO(kyrtapz): Clean this up for clarity as part of https://github.com/ovn-org/ovn-kubernetes/issues/4689 + if gw.netInfo.TopologyType() == types.Layer2Topology { + gwRouterToJoinSwitchPortName = types.RouterToSwitchPrefix + gw.joinSwitchName + portName = types.SwitchToRouterPrefix + gw.joinSwitchName + } + gwIPAddrs, err := libovsdbutil.GetLRPAddrs(gw.nbClient, gwRouterToJoinSwitchPortName) if err != nil && !errors.Is(err, libovsdbclient.ErrNotFound) { return fmt.Errorf( @@ -1018,7 +1038,6 @@ func (gw *GatewayManager) Cleanup() error { gw.policyRouteCleanup(nextHops) // Remove the patch port that connects join switch to gateway router - portName := types.JoinSwitchToGWRouterPrefix + gw.gwRouterName lsp := nbdb.LogicalSwitchPort{Name: portName} sw := nbdb.LogicalSwitch{Name: gw.joinSwitchName} err = libovsdbops.DeleteLogicalSwitchPorts(gw.nbClient, &sw, &lsp) diff --git a/go-controller/pkg/ovn/multihoming_test.go b/go-controller/pkg/ovn/multihoming_test.go index 641472cdb3..33bff1b13d 100644 --- a/go-controller/pkg/ovn/multihoming_test.go +++ b/go-controller/pkg/ovn/multihoming_test.go @@ -184,13 +184,7 @@ func (em *secondaryNetworkExpectationMachine) expectedLogicalSwitchesAndPorts(is data = append(data, mgmtPort) nodeslsps[switchName] = append(nodeslsps[switchName], mgmtPortUUID) - // there are multiple GRs in the cluster, thus their names must be scoped with the node name - gwRouterName := fmt.Sprintf( - "%s%s", - ovntypes.GWRouterPrefix, - ocInfo.bnc.GetNetworkScopedName(nodeName), - ) - networkSwitchToGWRouterLSPName := ovntypes.JoinSwitchToGWRouterPrefix + gwRouterName + networkSwitchToGWRouterLSPName := ovntypes.SwitchToRouterPrefix + switchName networkSwitchToGWRouterLSPUUID := networkSwitchToGWRouterLSPName + "-UUID" data = append(data, &nbdb.LogicalSwitchPort{ @@ -201,7 +195,7 @@ func (em *secondaryNetworkExpectationMachine) expectedLogicalSwitchesAndPorts(is "k8s.ovn.org/topology": ocInfo.bnc.TopologyType(), "k8s.ovn.org/network": ocInfo.bnc.GetNetworkName(), }, - Options: map[string]string{"router-port": ovntypes.GWRouterToJoinSwitchPrefix + gwRouterName}, + Options: map[string]string{"router-port": ovntypes.RouterToSwitchPrefix + switchName}, Type: "router", }) nodeslsps[switchName] = append(nodeslsps[switchName], networkSwitchToGWRouterLSPUUID) diff --git a/go-controller/pkg/ovn/secondary_layer2_network_controller_test.go b/go-controller/pkg/ovn/secondary_layer2_network_controller_test.go index 84e95981a1..2ed8cfc786 100644 --- a/go-controller/pkg/ovn/secondary_layer2_network_controller_test.go +++ b/go-controller/pkg/ovn/secondary_layer2_network_controller_test.go @@ -383,7 +383,7 @@ func expectedLayer2EgressEntities(netInfo util.NetInfo, gwConfig util.L3GatewayC ) gwRouterName := fmt.Sprintf("GR_%s_test-node", netInfo.GetNetworkName()) staticRouteOutputPort := ovntypes.GWRouterToExtSwitchPrefix + gwRouterName - gwRouterToNetworkSwitchPortName := ovntypes.GWRouterToJoinSwitchPrefix + gwRouterName + gwRouterToNetworkSwitchPortName := ovntypes.RouterToSwitchPrefix + netInfo.GetNetworkScopedName(ovntypes.OVNLayer2Switch) gwRouterToExtSwitchPortName := fmt.Sprintf("%s%s", ovntypes.GWRouterToExtSwitchPrefix, gwRouterName) masqSNAT := newMasqueradeManagementNATEntry(masqSNATUUID1, "169.254.169.14", layer2Subnet().String(), netInfo) From 7be00b7f976164d8417f9cf24ad290955cbe311e Mon Sep 17 00:00:00 2001 From: Surya Seetharaman Date: Tue, 3 Sep 2024 10:18:45 +0200 Subject: [PATCH 02/12] LGW: Add pod-route towards mpX in L2 network For some reason we were not adding the reroutes to mpX interface required in LGW in L2 topology on the GR. This commit fixes that. Signed-off-by: Surya Seetharaman --- .../pkg/ovn/base_network_controller.go | 22 +++++++++---------- go-controller/pkg/ovn/master.go | 2 +- .../secondary_layer2_network_controller.go | 2 +- .../secondary_layer3_network_controller.go | 2 +- test/e2e/network_segmentation.go | 6 ----- 5 files changed, 13 insertions(+), 21 deletions(-) diff --git a/go-controller/pkg/ovn/base_network_controller.go b/go-controller/pkg/ovn/base_network_controller.go index dd0107ee88..de05d348f4 100644 --- a/go-controller/pkg/ovn/base_network_controller.go +++ b/go-controller/pkg/ovn/base_network_controller.go @@ -615,7 +615,7 @@ func (bnc *BaseNetworkController) deleteNamespaceLocked(ns string) (*namespaceIn return nsInfo, nil } -func (bnc *BaseNetworkController) syncNodeManagementPort(node *kapi.Node, switchName string, hostSubnets []*net.IPNet, routeHostSubnets bool) ([]net.IP, error) { +func (bnc *BaseNetworkController) syncNodeManagementPort(node *kapi.Node, switchName, routerName string, hostSubnets []*net.IPNet) ([]net.IP, error) { macAddress, err := util.ParseNodeManagementPortMACAddresses(node, bnc.GetNetworkName()) if err != nil { return nil, err @@ -636,19 +636,25 @@ func (bnc *BaseNetworkController) syncNodeManagementPort(node *kapi.Node, switch if !utilnet.IsIPv6CIDR(hostSubnet) { v4Subnet = hostSubnet } - if config.Gateway.Mode == config.GatewayModeLocal && routeHostSubnets { + if config.Gateway.Mode == config.GatewayModeLocal { lrsr := nbdb.LogicalRouterStaticRoute{ Policy: &nbdb.LogicalRouterStaticRoutePolicySrcIP, IPPrefix: hostSubnet.String(), Nexthop: mgmtIfAddr.IP.String(), } + if bnc.IsSecondary() { + lrsr.ExternalIDs = map[string]string{ + ovntypes.NetworkExternalID: bnc.GetNetworkName(), + ovntypes.TopologyExternalID: bnc.TopologyType(), + } + } p := func(item *nbdb.LogicalRouterStaticRoute) bool { return item.IPPrefix == lrsr.IPPrefix && libovsdbops.PolicyEqualPredicate(lrsr.Policy, item.Policy) } - err := libovsdbops.CreateOrReplaceLogicalRouterStaticRouteWithPredicate(bnc.nbClient, bnc.GetNetworkScopedClusterRouterName(), + err := libovsdbops.CreateOrReplaceLogicalRouterStaticRouteWithPredicate(bnc.nbClient, routerName, &lrsr, p, &lrsr.Nexthop) if err != nil { - return nil, fmt.Errorf("error creating static route %+v on router %s: %v", lrsr, bnc.GetNetworkScopedClusterRouterName(), err) + return nil, fmt.Errorf("error creating static route %+v on router %s: %v", lrsr, routerName, err) } } } @@ -680,14 +686,6 @@ func (bnc *BaseNetworkController) syncNodeManagementPort(node *kapi.Node, switch return mgmtPortIPs, nil } -func (bnc *BaseNetworkController) syncNodeManagementPortRouteHostSubnets(node *kapi.Node, switchName string, hostSubnets []*net.IPNet) ([]net.IP, error) { - return bnc.syncNodeManagementPort(node, switchName, hostSubnets, true) -} - -func (bnc *BaseNetworkController) syncNodeManagementPortNoRouteHostSubnets(node *kapi.Node, switchName string, hostSubnets []*net.IPNet) ([]net.IP, error) { - return bnc.syncNodeManagementPort(node, switchName, hostSubnets, false) -} - // WatchNodes starts the watching of the nodes resource and calls back the appropriate handler logic func (bnc *BaseNetworkController) WatchNodes() error { if bnc.nodeHandler != nil { diff --git a/go-controller/pkg/ovn/master.go b/go-controller/pkg/ovn/master.go index 87ec5db269..1535403f25 100644 --- a/go-controller/pkg/ovn/master.go +++ b/go-controller/pkg/ovn/master.go @@ -130,7 +130,7 @@ func (oc *DefaultNetworkController) newClusterRouter() (*nbdb.LogicalRouter, err } func (oc *DefaultNetworkController) syncNodeManagementPortDefault(node *kapi.Node, switchName string, hostSubnets []*net.IPNet) error { - mgmtPortIPs, err := oc.syncNodeManagementPortRouteHostSubnets(node, switchName, hostSubnets) + mgmtPortIPs, err := oc.syncNodeManagementPort(node, switchName, oc.GetNetworkScopedClusterRouterName(), hostSubnets) if err == nil { return oc.setupUDNACLs(mgmtPortIPs) } diff --git a/go-controller/pkg/ovn/secondary_layer2_network_controller.go b/go-controller/pkg/ovn/secondary_layer2_network_controller.go index 455b934c69..9e848cca4d 100644 --- a/go-controller/pkg/ovn/secondary_layer2_network_controller.go +++ b/go-controller/pkg/ovn/secondary_layer2_network_controller.go @@ -428,7 +428,7 @@ func (oc *SecondaryLayer2NetworkController) addUpdateLocalNodeEvent(node *corev1 for _, subnet := range oc.Subnets() { hostSubnets = append(hostSubnets, subnet.CIDR) } - if _, err := oc.syncNodeManagementPortNoRouteHostSubnets(node, oc.GetNetworkScopedSwitchName(types.OVNLayer2Switch), hostSubnets); err != nil { + if _, err := oc.syncNodeManagementPort(node, oc.GetNetworkScopedSwitchName(types.OVNLayer2Switch), oc.GetNetworkScopedGWRouterName(node.Name), hostSubnets); err != nil { errs = append(errs, err) oc.mgmtPortFailed.Store(node.Name, true) } else { diff --git a/go-controller/pkg/ovn/secondary_layer3_network_controller.go b/go-controller/pkg/ovn/secondary_layer3_network_controller.go index d90521fad8..472bcb6ca5 100644 --- a/go-controller/pkg/ovn/secondary_layer3_network_controller.go +++ b/go-controller/pkg/ovn/secondary_layer3_network_controller.go @@ -653,7 +653,7 @@ func (oc *SecondaryLayer3NetworkController) addUpdateLocalNodeEvent(node *kapi.N errs = append(errs, err) oc.mgmtPortFailed.Store(node.Name, true) } else { - _, err = oc.syncNodeManagementPortRouteHostSubnets(node, oc.GetNetworkScopedSwitchName(node.Name), hostSubnets) + _, err = oc.syncNodeManagementPort(node, oc.GetNetworkScopedSwitchName(node.Name), oc.GetNetworkScopedClusterRouterName(), hostSubnets) if err != nil { errs = append(errs, err) oc.mgmtPortFailed.Store(node.Name, true) diff --git a/test/e2e/network_segmentation.go b/test/e2e/network_segmentation.go index 3f2ba74b05..a0c4e78513 100644 --- a/test/e2e/network_segmentation.go +++ b/test/e2e/network_segmentation.go @@ -752,12 +752,6 @@ var _ = Describe("Network Segmentation", func() { DescribeTable( "can be accessed to from the pods running in the Kubernetes cluster", func(netConfigParams networkAttachmentConfigParams, clientPodConfig podConfiguration) { - if netConfigParams.topology == "layer2" && IsGatewayModeLocal() { - const upstreamIssue = "https://github.com/ovn-org/ovn-kubernetes/issues/4686" - e2eskipper.Skipf( - "These tests are known to fail on Local Gateway deployments. Upstream issue: %s", upstreamIssue, - ) - } if netConfigParams.topology == "layer2" && !isInterconnectEnabled() { const upstreamIssue = "https://github.com/ovn-org/ovn-kubernetes/issues/4642" e2eskipper.Skipf( From a350b491daff768038997304e5f6595fe3897a97 Mon Sep 17 00:00:00 2001 From: Surya Seetharaman Date: Tue, 3 Sep 2024 14:38:51 +0200 Subject: [PATCH 03/12] Fix name of switch for L2 Co-Authored-by: Patryk Diak Signed-off-by: Surya Seetharaman --- .../pkg/ovn/secondary_layer3_network_controller_test.go | 2 +- go-controller/pkg/util/multi_network.go | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/go-controller/pkg/ovn/secondary_layer3_network_controller_test.go b/go-controller/pkg/ovn/secondary_layer3_network_controller_test.go index 4595d0fd14..027d550e0f 100644 --- a/go-controller/pkg/ovn/secondary_layer3_network_controller_test.go +++ b/go-controller/pkg/ovn/secondary_layer3_network_controller_test.go @@ -730,7 +730,7 @@ func expectedLogicalRouterPolicy(routerPolicyUUID1 string, netInfo util.NetInfo, priority = 1004 rerouteAction = "reroute" ) - networkScopedNodeName := netInfo.GetNetworkScopedName(nodeName) + networkScopedNodeName := netInfo.GetNetworkScopedSwitchName(nodeName) lrpName := fmt.Sprintf("%s%s", ovntypes.RouterToSwitchPrefix, networkScopedNodeName) return &nbdb.LogicalRouterPolicy{ UUID: routerPolicyUUID1, diff --git a/go-controller/pkg/util/multi_network.go b/go-controller/pkg/util/multi_network.go index a33097bda6..d1e5136a4b 100644 --- a/go-controller/pkg/util/multi_network.go +++ b/go-controller/pkg/util/multi_network.go @@ -328,6 +328,10 @@ func (nInfo *secondaryNetInfo) GetNetworkScopedGWRouterName(nodeName string) str } func (nInfo *secondaryNetInfo) GetNetworkScopedSwitchName(nodeName string) string { + // In Layer2Topology there is just one global switch + if nInfo.TopologyType() == types.Layer2Topology { + return fmt.Sprintf("%s%s", nInfo.getPrefix(), types.OVNLayer2Switch) + } return nInfo.GetNetworkScopedName(nodeName) } From 21db02afe2fa015960d138213f44bc991fd00fbf Mon Sep 17 00:00:00 2001 From: Surya Seetharaman Date: Thu, 5 Sep 2024 10:12:23 +0200 Subject: [PATCH 04/12] Expose GetNodeIfAddrAnnotation as a public function Signed-off-by: Surya Seetharaman --- go-controller/pkg/util/node_annotations.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/go-controller/pkg/util/node_annotations.go b/go-controller/pkg/util/node_annotations.go index 3f9af38b73..a497f8d3e3 100644 --- a/go-controller/pkg/util/node_annotations.go +++ b/go-controller/pkg/util/node_annotations.go @@ -725,7 +725,7 @@ type ParsedNodeEgressIPConfiguration struct { Capacity Capacity } -func getNodeIfAddrAnnotation(node *kapi.Node) (*primaryIfAddrAnnotation, error) { +func GetNodeIfAddrAnnotation(node *kapi.Node) (*primaryIfAddrAnnotation, error) { nodeIfAddrAnnotation, ok := node.Annotations[OvnNodeIfAddr] if !ok { return nil, newAnnotationNotSetError("%s annotation not found for node %q", OvnNodeIfAddr, node.Name) @@ -742,7 +742,7 @@ func getNodeIfAddrAnnotation(node *kapi.Node) (*primaryIfAddrAnnotation, error) // ParseNodePrimaryIfAddr returns the IPv4 / IPv6 values for the node's primary network interface func ParseNodePrimaryIfAddr(node *kapi.Node) (*ParsedNodeEgressIPConfiguration, error) { - nodeIfAddr, err := getNodeIfAddrAnnotation(node) + nodeIfAddr, err := GetNodeIfAddrAnnotation(node) if err != nil { return nil, err } @@ -932,7 +932,7 @@ func ParseCloudEgressIPConfig(node *kapi.Node) (*ParsedNodeEgressIPConfiguration // ParsedNodeEgressIPConfiguration.V[4|6].IP is used to verify if an egress IP matches node IP to disable its creation // use node IP instead of the value assigned from cloud egress CIDR config - nodeIfAddr, err := getNodeIfAddrAnnotation(node) + nodeIfAddr, err := GetNodeIfAddrAnnotation(node) if err != nil { return nil, err } @@ -1080,7 +1080,7 @@ func ParseNodeHostCIDRsExcludeOVNNetworks(node *kapi.Node) ([]string, error) { if err != nil { return nil, err } - ovnNetworks, err := getNodeIfAddrAnnotation(node) + ovnNetworks, err := GetNodeIfAddrAnnotation(node) if err != nil { return nil, err } From e535317940ba86510ec1c32b524dfaa505b28c3d Mon Sep 17 00:00:00 2001 From: Surya Seetharaman Date: Thu, 5 Sep 2024 13:51:49 +0200 Subject: [PATCH 05/12] Rename generateMatch to generateNodeIPMatch Signed-off-by: Surya Seetharaman --- .../ovn/gatewayrouter/policybasedroutes.go | 4 +-- .../gatewayrouter/policybasedroutes_test.go | 30 +++++++++---------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/go-controller/pkg/ovn/gatewayrouter/policybasedroutes.go b/go-controller/pkg/ovn/gatewayrouter/policybasedroutes.go index e054a67c18..0c72fe029b 100644 --- a/go-controller/pkg/ovn/gatewayrouter/policybasedroutes.go +++ b/go-controller/pkg/ovn/gatewayrouter/policybasedroutes.go @@ -50,7 +50,7 @@ func (pbr *PolicyBasedRoutesManager) Add(nodeName, mgmtPortIP string, hostIfCIDR // embed nodeName as comment so that it is easier to delete these rules later on. // logical router policy doesn't support external_ids to stash metadata networkScopedSwitchName := pbr.netInfo.GetNetworkScopedSwitchName(nodeName) - matchStr := generateMatch(networkScopedSwitchName, l3Prefix, hostIP) + matchStr := generateNodeIPMatch(networkScopedSwitchName, l3Prefix, hostIP) matches = matches.Insert(matchStr) } @@ -218,7 +218,7 @@ func (pbr *PolicyBasedRoutesManager) createPolicyBasedRoutes(match, priority, ne return nil } -func generateMatch(switchName, ipPrefix, hostIP string) string { +func generateNodeIPMatch(switchName, ipPrefix, hostIP string) string { return fmt.Sprintf(`inport == "%s%s" && %s.dst == %s /* %s */`, ovntypes.RouterToSwitchPrefix, switchName, ipPrefix, hostIP, switchName) } diff --git a/go-controller/pkg/ovn/gatewayrouter/policybasedroutes_test.go b/go-controller/pkg/ovn/gatewayrouter/policybasedroutes_test.go index b151465eed..eeb1e7d49f 100644 --- a/go-controller/pkg/ovn/gatewayrouter/policybasedroutes_test.go +++ b/go-controller/pkg/ovn/gatewayrouter/policybasedroutes_test.go @@ -159,7 +159,7 @@ func TestAdd(t *testing.T) { &nbdb.LogicalRouterPolicy{ UUID: "node-ip-lrp-uuid", Priority: nodeSubNetPrio, - Match: generateMatch(cdnL3Network.info.GetNetworkScopedSwitchName(node1Name), v4Prefix, node1HostIPv4Str), + Match: generateNodeIPMatch(cdnL3Network.info.GetNetworkScopedSwitchName(node1Name), v4Prefix, node1HostIPv4Str), Action: nbdb.LogicalRouterPolicyActionReroute, Nexthops: []string{node1CDNMgntIPv4Str}, }, @@ -186,14 +186,14 @@ func TestAdd(t *testing.T) { &nbdb.LogicalRouterPolicy{ UUID: "node-ip-lrp-uuid", Priority: nodeSubNetPrio, - Match: generateMatch(cdnL3Network.info.GetNetworkScopedSwitchName(node1Name), v4Prefix, node1HostIPv4Str), + Match: generateNodeIPMatch(cdnL3Network.info.GetNetworkScopedSwitchName(node1Name), v4Prefix, node1HostIPv4Str), Action: nbdb.LogicalRouterPolicyActionReroute, Nexthops: []string{node1CDNMgntIPv4Str}, }, &nbdb.LogicalRouterPolicy{ UUID: "node-ip2-lrp-uuid", Priority: nodeSubNetPrio, - Match: generateMatch(cdnL3Network.info.GetNetworkScopedSwitchName(node1Name), v4Prefix, node1HostOtherAddrIPv4Str), + Match: generateNodeIPMatch(cdnL3Network.info.GetNetworkScopedSwitchName(node1Name), v4Prefix, node1HostOtherAddrIPv4Str), Action: nbdb.LogicalRouterPolicyActionReroute, Nexthops: []string{node1CDNMgntIPv4Str}, }, @@ -234,7 +234,7 @@ func TestAdd(t *testing.T) { &nbdb.LogicalRouterPolicy{ UUID: "node-ip-lrp-uuid", Priority: nodeSubNetPrio, - Match: generateMatch(cdnL3Network.info.GetNetworkScopedSwitchName(node1Name), v6Prefix, node1HostIPv6Str), + Match: generateNodeIPMatch(cdnL3Network.info.GetNetworkScopedSwitchName(node1Name), v6Prefix, node1HostIPv6Str), Action: nbdb.LogicalRouterPolicyActionReroute, Nexthops: []string{node1CDNMgntIPv6Str}, }, @@ -267,14 +267,14 @@ func TestAdd(t *testing.T) { &nbdb.LogicalRouterPolicy{ UUID: "node-ip-lrp-v4-uuid", Priority: nodeSubNetPrio, - Match: generateMatch(cdnL3Network.info.GetNetworkScopedSwitchName(node1Name), v4Prefix, node1HostIPv4Str), + Match: generateNodeIPMatch(cdnL3Network.info.GetNetworkScopedSwitchName(node1Name), v4Prefix, node1HostIPv4Str), Action: nbdb.LogicalRouterPolicyActionReroute, Nexthops: []string{node1CDNMgntIPv4Str}, }, &nbdb.LogicalRouterPolicy{ UUID: "node-ip-lrp-v6-uuid", Priority: nodeSubNetPrio, - Match: generateMatch(cdnL3Network.info.GetNetworkScopedSwitchName(node1Name), v6Prefix, node1HostIPv6Str), + Match: generateNodeIPMatch(cdnL3Network.info.GetNetworkScopedSwitchName(node1Name), v6Prefix, node1HostIPv6Str), Action: nbdb.LogicalRouterPolicyActionReroute, Nexthops: []string{node1CDNMgntIPv6Str}, }, @@ -312,14 +312,14 @@ func TestAdd(t *testing.T) { &nbdb.LogicalRouterPolicy{ UUID: "node-ip-lrp-v4-uuid", Priority: nodeSubNetPrio, - Match: generateMatch(cdnL3Network.info.GetNetworkScopedSwitchName(node1Name), v4Prefix, node1HostIPv4Str), + Match: generateNodeIPMatch(cdnL3Network.info.GetNetworkScopedSwitchName(node1Name), v4Prefix, node1HostIPv4Str), Action: nbdb.LogicalRouterPolicyActionReroute, Nexthops: []string{node1CDNMgntIPv4Str}, }, &nbdb.LogicalRouterPolicy{ UUID: "node-ip-lrp-v6-uuid", Priority: nodeSubNetPrio, - Match: generateMatch(udnL3Network.info.GetNetworkScopedSwitchName(node1Name), v6Prefix, node1HostIPv6Str), + Match: generateNodeIPMatch(udnL3Network.info.GetNetworkScopedSwitchName(node1Name), v6Prefix, node1HostIPv6Str), Action: nbdb.LogicalRouterPolicyActionReroute, Nexthops: []string{node1UDNMgntIPv6Str}, ExternalIDs: map[string]string{ @@ -344,7 +344,7 @@ func TestAdd(t *testing.T) { &nbdb.LogicalRouterPolicy{ UUID: "node-ip-lrp-uuid", Priority: nodeSubNetPrio, - Match: generateMatch(cdnL3Network.info.GetNetworkScopedSwitchName(node1Name), v4Prefix, node1HostIPv4Str), + Match: generateNodeIPMatch(cdnL3Network.info.GetNetworkScopedSwitchName(node1Name), v4Prefix, node1HostIPv4Str), Action: nbdb.LogicalRouterPolicyActionReroute, Nexthops: []string{node1CDNMgntIPv4Str}, })}, @@ -357,7 +357,7 @@ func TestAdd(t *testing.T) { &nbdb.LogicalRouterPolicy{ UUID: "node-ip-lrp-uuid", Priority: nodeSubNetPrio, - Match: generateMatch(cdnL3Network.info.GetNetworkScopedSwitchName(node1Name), v4Prefix, node1HostIPv4Str), + Match: generateNodeIPMatch(cdnL3Network.info.GetNetworkScopedSwitchName(node1Name), v4Prefix, node1HostIPv4Str), Action: nbdb.LogicalRouterPolicyActionReroute, Nexthops: []string{node1CDNMgntIPv4Str}, }, @@ -378,7 +378,7 @@ func TestAdd(t *testing.T) { &nbdb.LogicalRouterPolicy{ UUID: "node-ip-lrp-uuid", Priority: nodeSubNetPrio, - Match: generateMatch(cdnL3Network.info.GetNetworkScopedSwitchName(node1Name), v4Prefix, node1HostIPv4Str), + Match: generateNodeIPMatch(cdnL3Network.info.GetNetworkScopedSwitchName(node1Name), v4Prefix, node1HostIPv4Str), Action: nbdb.LogicalRouterPolicyActionReroute, Nexthops: []string{node1CDNMgntIPv4Str}, })}, @@ -391,7 +391,7 @@ func TestAdd(t *testing.T) { &nbdb.LogicalRouterPolicy{ UUID: "node-ip-lrp-uuid", Priority: nodeSubNetPrio, - Match: generateMatch(cdnL3Network.info.GetNetworkScopedSwitchName(node1Name), v4Prefix, node1HostIPv4Str), + Match: generateNodeIPMatch(cdnL3Network.info.GetNetworkScopedSwitchName(node1Name), v4Prefix, node1HostIPv4Str), Action: nbdb.LogicalRouterPolicyActionReroute, Nexthops: []string{node1CDNMgntIPv4Str}, }, @@ -419,7 +419,7 @@ func TestAdd(t *testing.T) { &nbdb.LogicalRouterPolicy{ UUID: "node-ip-lrp-uuid", Priority: nodeSubNetPrio, - Match: generateMatch(cdnL3Network.info.GetNetworkScopedSwitchName(node1Name), v4Prefix, node1HostIPv4Str), + Match: generateNodeIPMatch(cdnL3Network.info.GetNetworkScopedSwitchName(node1Name), v4Prefix, node1HostIPv4Str), Action: nbdb.LogicalRouterPolicyActionReroute, Nexthops: []string{node1CDNMgntIPv4Str}, }), @@ -438,14 +438,14 @@ func TestAdd(t *testing.T) { &nbdb.LogicalRouterPolicy{ UUID: "node-ip-lrp-uuid", Priority: nodeSubNetPrio, - Match: generateMatch(cdnL3Network.info.GetNetworkScopedSwitchName(node1Name), v4Prefix, node1HostIPv4Str), + Match: generateNodeIPMatch(cdnL3Network.info.GetNetworkScopedSwitchName(node1Name), v4Prefix, node1HostIPv4Str), Action: nbdb.LogicalRouterPolicyActionReroute, Nexthops: []string{node1CDNMgntIPv4Str}, }, &nbdb.LogicalRouterPolicy{ UUID: "node-ip-lrp2-uuid", Priority: nodeSubNetPrio, - Match: generateMatch(udnL3Network.info.GetNetworkScopedSwitchName(node1Name), v4Prefix, node1HostIPv4Str), + Match: generateNodeIPMatch(udnL3Network.info.GetNetworkScopedSwitchName(node1Name), v4Prefix, node1HostIPv4Str), Action: nbdb.LogicalRouterPolicyActionReroute, Nexthops: []string{node1UDNMgntIPv4Str}, ExternalIDs: map[string]string{ From 9b3eacb6275aac368dce1ada7ef13de07f51366f Mon Sep 17 00:00:00 2001 From: Surya Seetharaman Date: Thu, 5 Sep 2024 13:52:49 +0200 Subject: [PATCH 06/12] Rename Add to AddSameNodeIPPolicy Signed-off-by: Surya Seetharaman --- go-controller/pkg/ovn/gateway.go | 2 +- go-controller/pkg/ovn/gatewayrouter/policybasedroutes.go | 2 +- go-controller/pkg/ovn/gatewayrouter/policybasedroutes_test.go | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/go-controller/pkg/ovn/gateway.go b/go-controller/pkg/ovn/gateway.go index 9e0edcda0c..87c3d2cf4a 100644 --- a/go-controller/pkg/ovn/gateway.go +++ b/go-controller/pkg/ovn/gateway.go @@ -1239,7 +1239,7 @@ func (gw *GatewayManager) syncGatewayLogicalNetwork( return fmt.Errorf("failed to extract the host IP addrs for network %q: %v", gw.netInfo.GetNetworkName(), err) } pbrMngr := gatewayrouter.NewPolicyBasedRoutesManager(gw.nbClient, routerName, gw.netInfo) - if err := pbrMngr.Add(node.Name, hostIfAddr.IP.String(), l3GatewayConfigIP, relevantHostIPs); err != nil { + if err := pbrMngr.AddSameNodeIPPolicy(node.Name, hostIfAddr.IP.String(), l3GatewayConfigIP, relevantHostIPs); err != nil { return fmt.Errorf("failed to configure the policy based routes for network %q: %v", gw.netInfo.GetNetworkName(), err) } } diff --git a/go-controller/pkg/ovn/gatewayrouter/policybasedroutes.go b/go-controller/pkg/ovn/gatewayrouter/policybasedroutes.go index 0c72fe029b..3899480656 100644 --- a/go-controller/pkg/ovn/gatewayrouter/policybasedroutes.go +++ b/go-controller/pkg/ovn/gatewayrouter/policybasedroutes.go @@ -31,7 +31,7 @@ func NewPolicyBasedRoutesManager(nbClient client.Client, clusterRouterName strin } } -func (pbr *PolicyBasedRoutesManager) Add(nodeName, mgmtPortIP string, hostIfCIDR *net.IPNet, otherHostAddrs []string) error { +func (pbr *PolicyBasedRoutesManager) AddSameNodeIPPolicy(nodeName, mgmtPortIP string, hostIfCIDR *net.IPNet, otherHostAddrs []string) error { if hostIfCIDR == nil { return fmt.Errorf(" host interface CIDR") } diff --git a/go-controller/pkg/ovn/gatewayrouter/policybasedroutes_test.go b/go-controller/pkg/ovn/gatewayrouter/policybasedroutes_test.go index eeb1e7d49f..f191b8863e 100644 --- a/go-controller/pkg/ovn/gatewayrouter/policybasedroutes_test.go +++ b/go-controller/pkg/ovn/gatewayrouter/policybasedroutes_test.go @@ -91,7 +91,7 @@ type test struct { expectErr bool } -func TestAdd(t *testing.T) { +func TestAddSameNodeIPPolicy(t *testing.T) { const ( node1Name = "node1" node1HostIPv4Str = "192.168.1.10" @@ -484,7 +484,7 @@ func TestAdd(t *testing.T) { if utilnet.IsIPv6(p.hostInfCIDR.IP) { mgntIP = targetNet.mgntIPv6 } - err = mgr.Add(p.nodeName, mgntIP, p.hostInfCIDR, p.otherHostInfAddrs) + err = mgr.AddSameNodeIPPolicy(p.nodeName, mgntIP, p.hostInfCIDR, p.otherHostInfAddrs) if tt.expectErr && err == nil { t.Fatalf("test: \"%s\", expected error but none occured", tt.desc) } From a3cc1bddf0fcc37a0ec91f7b3f30464c1e23cdc4 Mon Sep 17 00:00:00 2001 From: Surya Seetharaman Date: Thu, 5 Sep 2024 13:53:36 +0200 Subject: [PATCH 07/12] LGW: L2: Add a new LRP at 1500 for UDN Signed-off-by: Surya Seetharaman --- go-controller/pkg/ovn/gateway.go | 6 + .../ovn/gatewayrouter/policybasedroutes.go | 51 +++++- .../gatewayrouter/policybasedroutes_test.go | 152 +++++++++++++++++- go-controller/pkg/types/const.go | 1 + 4 files changed, 203 insertions(+), 7 deletions(-) diff --git a/go-controller/pkg/ovn/gateway.go b/go-controller/pkg/ovn/gateway.go index 87c3d2cf4a..77f2731a70 100644 --- a/go-controller/pkg/ovn/gateway.go +++ b/go-controller/pkg/ovn/gateway.go @@ -1242,6 +1242,12 @@ func (gw *GatewayManager) syncGatewayLogicalNetwork( if err := pbrMngr.AddSameNodeIPPolicy(node.Name, hostIfAddr.IP.String(), l3GatewayConfigIP, relevantHostIPs); err != nil { return fmt.Errorf("failed to configure the policy based routes for network %q: %v", gw.netInfo.GetNetworkName(), err) } + if gw.netInfo.TopologyType() == types.Layer2Topology && config.Gateway.Mode == config.GatewayModeLocal { + if err := pbrMngr.AddHostCIDRPolicy(node, hostIfAddr.IP.String(), subnet.String()); err != nil { + return fmt.Errorf("failed to configure the hostCIDR policy for L2 network %q on local gateway: %v", + gw.netInfo.GetNetworkName(), err) + } + } } return nil diff --git a/go-controller/pkg/ovn/gatewayrouter/policybasedroutes.go b/go-controller/pkg/ovn/gatewayrouter/policybasedroutes.go index 3899480656..b98c5e31fb 100644 --- a/go-controller/pkg/ovn/gatewayrouter/policybasedroutes.go +++ b/go-controller/pkg/ovn/gatewayrouter/policybasedroutes.go @@ -6,6 +6,7 @@ import ( "strconv" "strings" + v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/sets" utilnet "k8s.io/utils/net" @@ -44,7 +45,7 @@ func (pbr *PolicyBasedRoutesManager) AddSameNodeIPPolicy(nodeName, mgmtPortIP st if !isHostIPsValid(otherHostAddrs) { return fmt.Errorf("invalid other host address(es): %v", otherHostAddrs) } - l3Prefix := getIPPrefix(hostIfCIDR.IP) + l3Prefix := getIPCIDRPrefix(hostIfCIDR) matches := sets.New[string]() for _, hostIP := range append(otherHostAddrs, hostIfCIDR.IP.String()) { // embed nodeName as comment so that it is easier to delete these rules later on. @@ -66,6 +67,46 @@ func (pbr *PolicyBasedRoutesManager) AddSameNodeIPPolicy(nodeName, mgmtPortIP st return nil } +// AddHostCIDRPolicy adds the following policy in local-gateway-mode for UDN L2 topology to the GR +// 99 ip4.dst == 172.18.0.0/16 && ip4.src == 10.100.200.0/24 reroute 10.100.200.2 +// Since rtoe of GR is directly connected to the hostCIDR range in LGW even with the following +// reroute to mp0 src-ip route on GR that we add from syncNodeManagementPort: +// 10.100.200.0/24 10.100.200.2 src-ip +// the dst-ip based default OVN route takes precedence because the primary nodeCIDR range is a +// directly attached network to the OVN router and sends the traffic destined for other nodes to rtoe +// and via br-ex to outside in LGW which is not desired. +// Hence we need a LRP that sends all traffic destined to that primary nodeCIDR range that reroutes +// it to mp0 in LGW mode to override this directly attached network OVN route. +func (pbr *PolicyBasedRoutesManager) AddHostCIDRPolicy(node *v1.Node, mgmtPortIP, clusterPodSubnet string) error { + if mgmtPortIP == "" || net.ParseIP(mgmtPortIP) == nil { + return fmt.Errorf("invalid management port IP address: %q", mgmtPortIP) + } + // we only care about the primary node family since GR's port has that IP + // we don't care about secondary nodeIPs here which is why we are not using + // the hostCIDR annotation + primaryIfAddrs, err := util.GetNodeIfAddrAnnotation(node) + if err != nil { + return fmt.Errorf("failed to get primaryIP for node %s, err: %v", node.Name, err) + } + nodePrimaryStringPrefix := primaryIfAddrs.IPv4 + if utilnet.IsIPv6String(mgmtPortIP) { + nodePrimaryStringPrefix = primaryIfAddrs.IPv6 + } + _, nodePrimaryCIDRPrefix, err := net.ParseCIDR(nodePrimaryStringPrefix) + if nodePrimaryStringPrefix == "" || err != nil || nodePrimaryCIDRPrefix == nil { + return fmt.Errorf("invalid host CIDR prefix: prefixString: %q, prefixCIDR: %q, error: %v", + nodePrimaryStringPrefix, nodePrimaryCIDRPrefix, err) + } + ovnPrefix := getIPCIDRPrefix(nodePrimaryCIDRPrefix) + matchStr := generateHostCIDRMatch(ovnPrefix, nodePrimaryCIDRPrefix.String(), clusterPodSubnet) + if err := pbr.createPolicyBasedRoutes(matchStr, ovntypes.UDNHostCIDRPolicyPriority, mgmtPortIP); err != nil { + return fmt.Errorf("failed to add host-cidr policy route '%s' on host %q on %s "+ + "error: %v", matchStr, node.Name, pbr.clusterRouterName, err) + } + + return nil +} + // This function syncs logical router policies given various criteria // This function compares the following ovn-nbctl output: @@ -222,8 +263,12 @@ func generateNodeIPMatch(switchName, ipPrefix, hostIP string) string { return fmt.Sprintf(`inport == "%s%s" && %s.dst == %s /* %s */`, ovntypes.RouterToSwitchPrefix, switchName, ipPrefix, hostIP, switchName) } -func getIPPrefix(ip net.IP) string { - if utilnet.IsIPv6(ip) { +func generateHostCIDRMatch(ipPrefix, nodePrimaryCIDRPrefix, clusterPodSubnetPrefix string) string { + return fmt.Sprintf(`%s.dst == %s && %s.src == %s`, ipPrefix, nodePrimaryCIDRPrefix, ipPrefix, clusterPodSubnetPrefix) +} + +func getIPCIDRPrefix(cidr *net.IPNet) string { + if utilnet.IsIPv6CIDR(cidr) { return "ip6" } return "ip4" diff --git a/go-controller/pkg/ovn/gatewayrouter/policybasedroutes_test.go b/go-controller/pkg/ovn/gatewayrouter/policybasedroutes_test.go index f191b8863e..d90152717b 100644 --- a/go-controller/pkg/ovn/gatewayrouter/policybasedroutes_test.go +++ b/go-controller/pkg/ovn/gatewayrouter/policybasedroutes_test.go @@ -12,6 +12,8 @@ import ( libovsdbtest "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/testing/libovsdb" "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/types" "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/util" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" utilnet "k8s.io/utils/net" ) @@ -37,7 +39,7 @@ func (n network) copyNetworkAndSetLRPs(lrps ...*nbdb.LogicalRouterPolicy) networ return nCopy } -func (n network) generateTestData() []libovsdbtest.TestData { +func (n network) generateTestData(nodeName string) []libovsdbtest.TestData { data := make([]libovsdbtest.TestData, 0, 0) lrpUUIDs := make([]string, 0, len(n.initialLRPs)) for _, lrp := range n.initialLRPs { @@ -57,15 +59,19 @@ func (n network) generateTestData() []libovsdbtest.TestData { Name: n.info.GetNetworkScopedClusterRouterName(), Policies: lrpUUIDs, } + if n.info.TopologyType() == types.Layer2Topology { + lr.Name = n.info.GetNetworkScopedGWRouterName(nodeName) + lr.UUID = getLRUUID(n.info.GetNetworkScopedGWRouterName(nodeName)) + } return append(data, lr) } type networks []network -func (ns networks) generateTestData() []libovsdbtest.TestData { +func (ns networks) generateTestData(nodeName string) []libovsdbtest.TestData { data := make([]libovsdbtest.TestData, 0) for _, n := range ns { - data = append(data, n.generateTestData()...) + data = append(data, n.generateTestData(nodeName)...) } return data } @@ -460,7 +466,7 @@ func TestAddSameNodeIPPolicy(t *testing.T) { for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { dbSetup := libovsdbtest.TestSetup{ - NBData: tt.initialDB.generateTestData(), + NBData: tt.initialDB.generateTestData(node1Name), } nbdbClient, cleanup, err := libovsdbtest.NewNBTestHarness(dbSetup, nil) if err != nil { @@ -503,3 +509,141 @@ func TestAddSameNodeIPPolicy(t *testing.T) { }) } } + +func TestAddHostCIDRPolicy(t *testing.T) { + const ( + node1Name = "node1" + hostCIDRV4RangeStr = "192.168.1.0/24" + hostCIDRV6RangeStr = "fc00:f853:ccd:e793::/64" + node1HostIPv4Str = "192.168.1.10" + node1HostCIDR24IPv4Str = node1HostIPv4Str + "/24" + node1HostIPv6Str = "fc00:f853:ccd:e793::3" + node1HostCIDR64IPv6Str = node1HostIPv6Str + "/64" + joinSubnetIPv4Str = "100.10.1.0/24" + clusterSubnetIPv4Str = "10.128.0.0/16" + clusterSubnetIPv6Str = "2002:0:0:1234::/64" + node1UDNMgntIPv4Str = "10.200.1.2" + node1UDNMgntIPv6Str = "fd00:20:244::2" + v4Prefix = "ip4" + v6Prefix = "ip6" + udnNetworkName = "network1" + ) + + var ( + hostCIDRPolicyPrio, _ = strconv.Atoi(types.UDNHostCIDRPolicyPriority) + _, hostCIDRV4Range, _ = net.ParseCIDR(hostCIDRV4RangeStr) + _, hostCIDRV6Range, _ = net.ParseCIDR(hostCIDRV6RangeStr) + l2NetInfo, _ = util.NewNetInfo(&types2.NetConf{ + NetConf: cnitypes.NetConf{Name: udnNetworkName}, + Topology: types.Layer2Topology, + JoinSubnet: joinSubnetIPv4Str, // not required, but adding so NewNetInfo doesn't fail + Subnets: clusterSubnetIPv4Str + "," + clusterSubnetIPv6Str, // not required, but adding so NewNetInfo doesn't fail + }) + udnL2Network = network{ + initialLRPs: nil, + info: l2NetInfo, + mgntIPv4: node1UDNMgntIPv4Str, + mgntIPv6: node1UDNMgntIPv6Str, + } + node = &v1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: node1Name, + Annotations: map[string]string{ + "k8s.ovn.org/node-primary-ifaddr": fmt.Sprintf("{\"ipv4\": \"%s\", \"ipv6\": \"%s\"}", + node1HostCIDR24IPv4Str, node1HostCIDR64IPv6Str), + }, + }, + } + ) + + tests := []test{ + { + desc: "[udn][l2][ipv4][ipv6] add hostCIDR policy for L2", + addPolicies: []policy{ + { + targetNetwork: udnL2Network.info.GetNetworkName(), + hostInfCIDR: hostCIDRV4Range, + }, + { + targetNetwork: udnL2Network.info.GetNetworkName(), + hostInfCIDR: hostCIDRV6Range, + }, + }, + initialDB: networks{udnL2Network}, + expectedDB: []libovsdbtest.TestData{ + &nbdb.LogicalRouter{ + UUID: "udn-gr-uuid", + Name: udnL2Network.info.GetNetworkScopedGWRouterName(node1Name), + Policies: []string{"node-ip-lrp-v4-uuid", "node-ip-lrp-v6-uuid"}, + }, + &nbdb.LogicalRouterPolicy{ + UUID: "node-ip-lrp-v4-uuid", + Priority: hostCIDRPolicyPrio, + Match: generateHostCIDRMatch(v4Prefix, hostCIDRV4RangeStr, clusterSubnetIPv4Str), + Action: nbdb.LogicalRouterPolicyActionReroute, + Nexthops: []string{node1UDNMgntIPv4Str}, + ExternalIDs: map[string]string{ + types.NetworkExternalID: udnL2Network.info.GetNetworkName(), + types.TopologyExternalID: udnL2Network.info.TopologyType(), + }, + }, + &nbdb.LogicalRouterPolicy{ + UUID: "node-ip-lrp-v6-uuid", + Priority: hostCIDRPolicyPrio, + Match: generateHostCIDRMatch(v6Prefix, hostCIDRV6RangeStr, clusterSubnetIPv6Str), + Action: nbdb.LogicalRouterPolicyActionReroute, + Nexthops: []string{node1UDNMgntIPv6Str}, + ExternalIDs: map[string]string{ + types.NetworkExternalID: udnL2Network.info.GetNetworkName(), + types.TopologyExternalID: udnL2Network.info.TopologyType(), + }, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + dbSetup := libovsdbtest.TestSetup{ + NBData: tt.initialDB.generateTestData(node1Name), + } + nbdbClient, cleanup, err := libovsdbtest.NewNBTestHarness(dbSetup, nil) + if err != nil { + t.Errorf("libovsdb client error: %v", err) + return + } + t.Cleanup(cleanup.Cleanup) + netToMgr := map[string]*PolicyBasedRoutesManager{} + for _, net := range tt.initialDB { + netToMgr[net.info.GetNetworkName()] = NewPolicyBasedRoutesManager(nbdbClient, net.info.GetNetworkScopedGWRouterName(node1Name), net.info) + } + // verify all polices have a valid network name + for _, p := range tt.addPolicies { + mgr, ok := netToMgr[p.targetNetwork] + if !ok { + t.Errorf("policy defined a network %q but no associated network defined with this name", p.targetNetwork) + return + } + targetNet := tt.initialDB.getNetwork(p.targetNetwork) + mgntIP := targetNet.mgntIPv4 + clustersubnet := clusterSubnetIPv4Str + if utilnet.IsIPv6(p.hostInfCIDR.IP) { + mgntIP = targetNet.mgntIPv6 + clustersubnet = clusterSubnetIPv6Str + } + err = mgr.AddHostCIDRPolicy(node, mgntIP, clustersubnet) + if err != nil { + t.Fatal(fmt.Errorf("test: \"%s\" encountered error: %v", tt.desc, err)) + } + } + matcher := libovsdbtest.HaveData(tt.expectedDB) + success, err := matcher.Match(nbdbClient) + if !success { + t.Fatal(fmt.Errorf("test: \"%s\" didn't match expected with actual, err: %v", tt.desc, matcher.FailureMessage(nbdbClient))) + } + if err != nil { + t.Fatal(fmt.Errorf("test: \"%s\" encountered error: %v", tt.desc, err)) + } + }) + } +} diff --git a/go-controller/pkg/types/const.go b/go-controller/pkg/types/const.go index 7b021716e8..8b49d49b23 100644 --- a/go-controller/pkg/types/const.go +++ b/go-controller/pkg/types/const.go @@ -108,6 +108,7 @@ const ( MGMTPortPolicyPriority = "1005" NodeSubnetPolicyPriority = "1004" InterNodePolicyPriority = "1003" + UDNHostCIDRPolicyPriority = "99" HybridOverlaySubnetPriority = 1002 HybridOverlayReroutePriority = 501 DefaultNoRereoutePriority = 102 From d1e41e8c9e3d5003b76539ee5bb1709fed6822ca Mon Sep 17 00:00:00 2001 From: Surya Seetharaman Date: Sat, 7 Sep 2024 14:08:58 +0200 Subject: [PATCH 08/12] rename hostIfAddr to mgmtIfAddr Signed-off-by: Surya Seetharaman --- go-controller/pkg/ovn/gateway.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/go-controller/pkg/ovn/gateway.go b/go-controller/pkg/ovn/gateway.go index 77f2731a70..87aabbf96f 100644 --- a/go-controller/pkg/ovn/gateway.go +++ b/go-controller/pkg/ovn/gateway.go @@ -1226,24 +1226,24 @@ func (gw *GatewayManager) syncGatewayLogicalNetwork( routerName = gw.gwRouterName } for _, subnet := range hostSubnets { - hostIfAddr := util.GetNodeManagementIfAddr(subnet) - if hostIfAddr == nil { - return fmt.Errorf("host interface address not found for subnet %q on network %q", subnet, gw.netInfo.GetNetworkName()) + mgmtIfAddr := util.GetNodeManagementIfAddr(subnet) + if mgmtIfAddr == nil { + return fmt.Errorf("management interface address not found for subnet %q on network %q", subnet, gw.netInfo.GetNetworkName()) } - l3GatewayConfigIP, err := util.MatchFirstIPNetFamily(utilnet.IsIPv6(hostIfAddr.IP), l3GatewayConfig.IPAddresses) + l3GatewayConfigIP, err := util.MatchFirstIPNetFamily(utilnet.IsIPv6(mgmtIfAddr.IP), l3GatewayConfig.IPAddresses) if err != nil { return fmt.Errorf("failed to extract the gateway IP addr for network %q: %v", gw.netInfo.GetNetworkName(), err) } - relevantHostIPs, err := util.MatchAllIPStringFamily(utilnet.IsIPv6(hostIfAddr.IP), hostAddrs) + relevantHostIPs, err := util.MatchAllIPStringFamily(utilnet.IsIPv6(mgmtIfAddr.IP), hostAddrs) if err != nil && err != util.ErrorNoIP { return fmt.Errorf("failed to extract the host IP addrs for network %q: %v", gw.netInfo.GetNetworkName(), err) } pbrMngr := gatewayrouter.NewPolicyBasedRoutesManager(gw.nbClient, routerName, gw.netInfo) - if err := pbrMngr.AddSameNodeIPPolicy(node.Name, hostIfAddr.IP.String(), l3GatewayConfigIP, relevantHostIPs); err != nil { + if err := pbrMngr.AddSameNodeIPPolicy(node.Name, mgmtIfAddr.IP.String(), l3GatewayConfigIP, relevantHostIPs); err != nil { return fmt.Errorf("failed to configure the policy based routes for network %q: %v", gw.netInfo.GetNetworkName(), err) } if gw.netInfo.TopologyType() == types.Layer2Topology && config.Gateway.Mode == config.GatewayModeLocal { - if err := pbrMngr.AddHostCIDRPolicy(node, hostIfAddr.IP.String(), subnet.String()); err != nil { + if err := pbrMngr.AddHostCIDRPolicy(node, mgmtIfAddr.IP.String(), subnet.String()); err != nil { return fmt.Errorf("failed to configure the hostCIDR policy for L2 network %q on local gateway: %v", gw.netInfo.GetNetworkName(), err) } From bd58b4c89bb432a3f0288fd6d12b308bca3c97d1 Mon Sep 17 00:00:00 2001 From: Surya Seetharaman Date: Sat, 7 Sep 2024 17:23:41 +0200 Subject: [PATCH 09/12] Add LGW unit tests for secondary networks topologies Signed-off-by: Surya Seetharaman --- ...econdary_layer2_network_controller_test.go | 86 +++++++++++++++---- ...econdary_layer3_network_controller_test.go | 20 ++++- 2 files changed, 85 insertions(+), 21 deletions(-) diff --git a/go-controller/pkg/ovn/secondary_layer2_network_controller_test.go b/go-controller/pkg/ovn/secondary_layer2_network_controller_test.go index 2ed8cfc786..c6a0d58891 100644 --- a/go-controller/pkg/ovn/secondary_layer2_network_controller_test.go +++ b/go-controller/pkg/ovn/secondary_layer2_network_controller_test.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "net" + "strconv" "time" . "github.com/onsi/ginkgo" @@ -54,7 +55,7 @@ var _ = Describe("OVN Multi-Homed pod operations for layer2 network", func() { table.DescribeTable( "reconciles a new", - func(netInfo secondaryNetInfo, testConfig testConfiguration) { + func(netInfo secondaryNetInfo, testConfig testConfiguration, gatewayMode config.GatewayMode) { podInfo := dummyL2TestPod(ns, netInfo) if testConfig.configToOverride != nil { config.OVNKubernetesFeature = *testConfig.configToOverride @@ -62,6 +63,7 @@ var _ = Describe("OVN Multi-Homed pod operations for layer2 network", func() { config.Gateway.DisableSNATMultipleGWs = testConfig.gatewayConfig.DisableSNATMultipleGWs } } + config.Gateway.Mode = gatewayMode app.Action = func(ctx *cli.Context) error { By(fmt.Sprintf("creating a network attachment definition for network: %s", netInfo.netName)) nad, err := newNetworkAttachmentDefinition( @@ -73,6 +75,19 @@ var _ = Describe("OVN Multi-Homed pod operations for layer2 network", func() { By("setting up the OVN DB without any entities in it") Expect(netInfo.setupOVNDependencies(&initialDB)).To(Succeed()) + if netInfo.isPrimary { + networkConfig, err := util.NewNetInfo(netInfo.netconf()) + Expect(err).NotTo(HaveOccurred()) + + initialDB.NBData = append( + initialDB.NBData, + &nbdb.LogicalRouter{ + Name: fmt.Sprintf("GR_%s_%s", networkConfig.GetNetworkName(), nodeName), + ExternalIDs: standardNonDefaultNetworkExtIDs(networkConfig), + }, + ) + } + const nodeIPv4CIDR = "192.168.126.202/24" By(fmt.Sprintf("Creating a node named %q, with IP: %s", nodeName, nodeIPv4CIDR)) testNode, err := newNodeWithSecondaryNets(nodeName, nodeIPv4CIDR, netInfo) @@ -158,25 +173,37 @@ var _ = Describe("OVN Multi-Homed pod operations for layer2 network", func() { table.Entry("pod on a user defined secondary network", dummySecondaryLayer2UserDefinedNetwork("100.200.0.0/16"), nonICClusterTestConfiguration(), + config.GatewayModeShared, ), table.Entry("pod on a user defined primary network", dummyPrimaryLayer2UserDefinedNetwork("100.200.0.0/16"), nonICClusterTestConfiguration(), + config.GatewayModeShared, ), table.Entry("pod on a user defined secondary network on an IC cluster", dummySecondaryLayer2UserDefinedNetwork("100.200.0.0/16"), icClusterTestConfiguration(), + config.GatewayModeShared, ), table.Entry("pod on a user defined primary network on an IC cluster", dummyPrimaryLayer2UserDefinedNetwork("100.200.0.0/16"), icClusterTestConfiguration(), + config.GatewayModeShared, + ), + + table.Entry("pod on a user defined primary network on an IC cluster; LGW", + dummyPrimaryLayer2UserDefinedNetwork("100.200.0.0/16"), + icClusterTestConfiguration(), + config.GatewayModeLocal, ), + table.Entry("pod on a user defined primary network on an IC cluster with per-pod SNATs enabled", dummyPrimaryLayer2UserDefinedNetwork("100.200.0.0/16"), icClusterWithDisableSNATTestConfiguration(), + config.GatewayModeShared, ), ) @@ -372,14 +399,16 @@ func dummyL2TestPodAdditionalNetworkIP() string { func expectedLayer2EgressEntities(netInfo util.NetInfo, gwConfig util.L3GatewayConfig, nodeName string) []libovsdbtest.TestData { const ( - nat1 = "nat1-UUID" - nat2 = "nat2-UUID" - nat3 = "nat3-UUID" - perPodSNAT = "pod-snat-UUID" - sr1 = "sr1-UUID" - sr2 = "sr2-UUID" - routerPolicyUUID1 = "lrp1-UUID" - masqSNATUUID1 = "masq-snat1-UUID" + nat1 = "nat1-UUID" + nat2 = "nat2-UUID" + nat3 = "nat3-UUID" + perPodSNAT = "pod-snat-UUID" + sr1 = "sr1-UUID" + sr2 = "sr2-UUID" + lrsr1 = "lrsr1-UUID" + routerPolicyUUID1 = "lrp1-UUID" + hostCIDRPolicyUUID = "host-cidr-policy-UUID" + masqSNATUUID1 = "masq-snat1-UUID" ) gwRouterName := fmt.Sprintf("GR_%s_test-node", netInfo.GetNetworkName()) staticRouteOutputPort := ovntypes.GWRouterToExtSwitchPrefix + gwRouterName @@ -393,17 +422,18 @@ func expectedLayer2EgressEntities(netInfo util.NetInfo, gwConfig util.L3GatewayC } else { nat = append(nat, nat1, nat2, nat3, masqSNATUUID1) } + gr := &nbdb.LogicalRouter{ + Name: gwRouterName, + UUID: gwRouterName + "-UUID", + Nat: nat, + Ports: []string{gwRouterToNetworkSwitchPortName + "-UUID", gwRouterToExtSwitchPortName + "-UUID"}, + StaticRoutes: []string{sr1, sr2}, + ExternalIDs: gwRouterExternalIDs(netInfo, gwConfig), + Options: gwRouterOptions(gwConfig), + Policies: []string{routerPolicyUUID1}, + } expectedEntities := []libovsdbtest.TestData{ - &nbdb.LogicalRouter{ - Name: gwRouterName, - UUID: gwRouterName + "-UUID", - Nat: nat, - Ports: []string{gwRouterToNetworkSwitchPortName + "-UUID", gwRouterToExtSwitchPortName + "-UUID"}, - StaticRoutes: []string{sr1, sr2}, - ExternalIDs: gwRouterExternalIDs(netInfo, gwConfig), - Options: gwRouterOptions(gwConfig), - Policies: []string{routerPolicyUUID1}, - }, + gr, expectedGWToNetworkSwitchRouterPort(gwRouterToNetworkSwitchPortName, netInfo, gwRouterJoinIPAddress(), layer2SubnetGWAddr()), expectedGRStaticRoute(sr1, dummyMasqueradeSubnet().String(), nextHopMasqueradeIP().String(), nil, &staticRouteOutputPort, netInfo), expectedGRStaticRoute(sr2, ipv4DefaultRoute().String(), nodeGateway().IP.String(), nil, &staticRouteOutputPort, netInfo), @@ -412,6 +442,17 @@ func expectedLayer2EgressEntities(netInfo util.NetInfo, gwConfig util.L3GatewayC masqSNAT, expectedLogicalRouterPolicy(routerPolicyUUID1, netInfo, nodeName, nodeIP().IP.String(), managementPortIP(layer2Subnet()).String()), } + if config.Gateway.Mode == config.GatewayModeLocal { + l2LGWLRP := expectedLogicalRouterPolicy(hostCIDRPolicyUUID, netInfo, nodeName, nodeCIDR().String(), managementPortIP(layer2Subnet()).String()) + l2LGWLRP.Match = fmt.Sprintf(`ip4.dst == %s && ip4.src == %s`, nodeCIDR().String(), layer2Subnet().String()) + l2LGWLRP.Priority, _ = strconv.Atoi(ovntypes.UDNHostCIDRPolicyPriority) + expectedEntities = append(expectedEntities, l2LGWLRP) + gr.Policies = append(gr.Policies, hostCIDRPolicyUUID) + lrsr := expectedGRStaticRoute(lrsr1, layer2Subnet().String(), managementPortIP(layer2Subnet()).String(), + &nbdb.LogicalRouterStaticRoutePolicySrcIP, nil, netInfo) + expectedEntities = append(expectedEntities, lrsr) + gr.StaticRoutes = append(gr.StaticRoutes, lrsr1) + } expectedEntities = append(expectedEntities, expectedExternalSwitchAndLSPs(netInfo, gwConfig, nodeName)...) if config.Gateway.DisableSNATMultipleGWs { @@ -488,3 +529,10 @@ func nodeIP() *net.IPNet { Mask: net.CIDRMask(24, 32), } } + +func nodeCIDR() *net.IPNet { + return &net.IPNet{ + IP: net.ParseIP("192.168.126.0"), + Mask: net.CIDRMask(24, 32), + } +} diff --git a/go-controller/pkg/ovn/secondary_layer3_network_controller_test.go b/go-controller/pkg/ovn/secondary_layer3_network_controller_test.go index 027d550e0f..6795531931 100644 --- a/go-controller/pkg/ovn/secondary_layer3_network_controller_test.go +++ b/go-controller/pkg/ovn/secondary_layer3_network_controller_test.go @@ -86,7 +86,7 @@ var _ = Describe("OVN Multi-Homed pod operations", func() { table.DescribeTable( "reconciles a new", - func(netInfo secondaryNetInfo, testConfig testConfiguration) { + func(netInfo secondaryNetInfo, testConfig testConfiguration, gwMode config.GatewayMode) { podInfo := dummyTestPod(ns, netInfo) if testConfig.configToOverride != nil { config.OVNKubernetesFeature = *testConfig.configToOverride @@ -94,6 +94,7 @@ var _ = Describe("OVN Multi-Homed pod operations", func() { config.Gateway.DisableSNATMultipleGWs = testConfig.gatewayConfig.DisableSNATMultipleGWs } } + config.Gateway.Mode = gwMode app.Action = func(ctx *cli.Context) error { nad, err := newNetworkAttachmentDefinition( ns, @@ -195,22 +196,32 @@ var _ = Describe("OVN Multi-Homed pod operations", func() { table.Entry("pod on a user defined secondary network", dummySecondaryLayer3UserDefinedNetwork("192.168.0.0/16", "192.168.1.0/24"), nonICClusterTestConfiguration(), + config.GatewayModeShared, ), table.Entry("pod on a user defined primary network", dummyPrimaryLayer3UserDefinedNetwork("192.168.0.0/16", "192.168.1.0/24"), nonICClusterTestConfiguration(), + config.GatewayModeShared, ), table.Entry("pod on a user defined secondary network on an IC cluster", dummySecondaryLayer3UserDefinedNetwork("192.168.0.0/16", "192.168.1.0/24"), icClusterTestConfiguration(), + config.GatewayModeShared, ), table.Entry("pod on a user defined primary network on an IC cluster", dummyPrimaryLayer3UserDefinedNetwork("192.168.0.0/16", "192.168.1.0/24"), icClusterTestConfiguration(), + config.GatewayModeShared, + ), + table.Entry("pod on a user defined primary network on an IC cluster; LGW", + dummyPrimaryLayer3UserDefinedNetwork("192.168.0.0/16", "192.168.1.0/24"), + icClusterTestConfiguration(), + config.GatewayModeLocal, ), table.Entry("pod on a user defined primary network on an IC cluster with per-pod SNATs enabled", dummyPrimaryLayer3UserDefinedNetwork("192.168.0.0/16", "192.168.1.0/24"), icClusterWithDisableSNATTestConfiguration(), + config.GatewayModeShared, ), ) @@ -690,6 +701,7 @@ func expectedLayer3EgressEntities(netInfo util.NetInfo, gwConfig util.L3GatewayC routerPolicyUUID2 = "lrpol2-UUID" staticRouteUUID1 = "sr1-UUID" staticRouteUUID2 = "sr2-UUID" + staticRouteUUID3 = "sr3-UUID" masqSNATUUID1 = "masq-snat1-UUID" ) masqIPAddr := dummyMasqueradeIP().IP.String() @@ -705,6 +717,10 @@ func expectedLayer3EgressEntities(netInfo util.NetInfo, gwConfig util.L3GatewayC } gatewayChassisUUID := fmt.Sprintf("%s-%s-UUID", rtosLRPName, gwConfig.ChassisID) + lrsrNextHop := gwRouterJoinIPAddress().IP.String() + if config.Gateway.Mode == config.GatewayModeLocal { + lrsrNextHop = managementPortIP(nodeSubnet).String() + } expectedEntities := []libovsdbtest.TestData{ &nbdb.LogicalRouter{ Name: clusterRouterName, @@ -716,7 +732,7 @@ func expectedLayer3EgressEntities(netInfo util.NetInfo, gwConfig util.L3GatewayC Nat: []string{masqSNATUUID1}, }, &nbdb.LogicalRouterPort{UUID: rtosLRPUUID, Name: rtosLRPName, Networks: []string{"192.168.1.1/24"}, MAC: "0a:58:c0:a8:01:01", GatewayChassis: []string{gatewayChassisUUID}}, - expectedGRStaticRoute(staticRouteUUID1, nodeSubnet.String(), gwRouterJoinIPAddress().IP.String(), &nbdb.LogicalRouterStaticRoutePolicySrcIP, nil, netInfo), + expectedGRStaticRoute(staticRouteUUID1, nodeSubnet.String(), lrsrNextHop, &nbdb.LogicalRouterStaticRoutePolicySrcIP, nil, netInfo), expectedGRStaticRoute(staticRouteUUID2, gwRouterJoinIPAddress().IP.String(), gwRouterJoinIPAddress().IP.String(), nil, nil, netInfo), expectedLogicalRouterPolicy(routerPolicyUUID1, netInfo, nodeName, nodeIP, managementPortIP(nodeSubnet).String()), expectedLogicalRouterPolicy(routerPolicyUUID2, netInfo, nodeName, masqIPAddr, managementPortIP(nodeSubnet).String()), From f61fbbffcb327a9c62957d51636be37a0717724d Mon Sep 17 00:00:00 2001 From: Surya Seetharaman Date: Tue, 17 Sep 2024 21:09:06 +0200 Subject: [PATCH 10/12] Make error logs better after this PR: unable to get logical router GR_l2.network_ovn-worker2: object not found before this PR: object not found Signed-off-by: Surya Seetharaman --- go-controller/pkg/libovsdb/ops/router.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/go-controller/pkg/libovsdb/ops/router.go b/go-controller/pkg/libovsdb/ops/router.go index c0436914eb..aa8841a751 100644 --- a/go-controller/pkg/libovsdb/ops/router.go +++ b/go-controller/pkg/libovsdb/ops/router.go @@ -644,7 +644,7 @@ func CreateOrReplaceLogicalRouterStaticRouteWithPredicate(nbClient libovsdbclien lr := &nbdb.LogicalRouter{Name: routerName} router, err := GetLogicalRouter(nbClient, lr) if err != nil { - return err + return fmt.Errorf("unable to get logical router %s: %w", routerName, err) } newPredicate := func(item *nbdb.LogicalRouterStaticRoute) bool { for _, routeUUID := range router.StaticRoutes { @@ -656,7 +656,7 @@ func CreateOrReplaceLogicalRouterStaticRouteWithPredicate(nbClient libovsdbclien } routes, err := FindLogicalRouterStaticRoutesWithPredicate(nbClient, newPredicate) if err != nil { - return err + return fmt.Errorf("unable to get logical router static routes with predicate on router %s: %w", routerName, err) } var ops []libovsdb.Operation @@ -697,7 +697,7 @@ func CreateOrReplaceLogicalRouterStaticRouteWithPredicate(nbClient libovsdbclien ops, err = CreateOrUpdateLogicalRouterStaticRoutesWithPredicateOps(nbClient, ops, routerName, lrsr, nil, fields...) if err != nil { - return err + return fmt.Errorf("unable to get create or update logical router static routes on router %s: %w", routerName, err) } _, err = TransactAndCheck(nbClient, ops) return err From b0dd59be6e056893125499e3d26f86a8d0437043 Mon Sep 17 00:00:00 2001 From: Surya Seetharaman Date: Wed, 18 Sep 2024 11:24:17 +0200 Subject: [PATCH 11/12] L2: syncNodeManagementPort should be called after gwManager syncNodeManagementPort should be called after the gwManager because we want to add the routes to the GR which is only created when the gwManager is run Signed-off-by: Surya Seetharaman --- .../secondary_layer2_network_controller.go | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/go-controller/pkg/ovn/secondary_layer2_network_controller.go b/go-controller/pkg/ovn/secondary_layer2_network_controller.go index 9e848cca4d..3de2b71291 100644 --- a/go-controller/pkg/ovn/secondary_layer2_network_controller.go +++ b/go-controller/pkg/ovn/secondary_layer2_network_controller.go @@ -419,22 +419,6 @@ func (oc *SecondaryLayer2NetworkController) addUpdateLocalNodeEvent(node *corev1 var errs []error if util.IsNetworkSegmentationSupportEnabled() && oc.IsPrimaryNetwork() { - if nSyncs.syncMgmtPort { - // Layer 2 networks have a single, large subnet, that's the one - // associated to the controller. Take the management port IP from - // there. - subnets := oc.Subnets() - hostSubnets := make([]*net.IPNet, 0, len(subnets)) - for _, subnet := range oc.Subnets() { - hostSubnets = append(hostSubnets, subnet.CIDR) - } - if _, err := oc.syncNodeManagementPort(node, oc.GetNetworkScopedSwitchName(types.OVNLayer2Switch), oc.GetNetworkScopedGWRouterName(node.Name), hostSubnets); err != nil { - errs = append(errs, err) - oc.mgmtPortFailed.Store(node.Name, true) - } else { - oc.mgmtPortFailed.Delete(node.Name) - } - } if nSyncs.syncGw { gwManager := oc.gatewayManagerForNode(node.Name) oc.gatewayManagers.Store(node.Name, gwManager) @@ -469,6 +453,22 @@ func (oc *SecondaryLayer2NetworkController) addUpdateLocalNodeEvent(node *corev1 } } } + if nSyncs.syncMgmtPort { + // Layer 2 networks have a single, large subnet, that's the one + // associated to the controller. Take the management port IP from + // there. + subnets := oc.Subnets() + hostSubnets := make([]*net.IPNet, 0, len(subnets)) + for _, subnet := range oc.Subnets() { + hostSubnets = append(hostSubnets, subnet.CIDR) + } + if _, err := oc.syncNodeManagementPort(node, oc.GetNetworkScopedSwitchName(types.OVNLayer2Switch), oc.GetNetworkScopedGWRouterName(node.Name), hostSubnets); err != nil { + errs = append(errs, err) + oc.mgmtPortFailed.Store(node.Name, true) + } else { + oc.mgmtPortFailed.Delete(node.Name) + } + } } errs = append(errs, oc.BaseSecondaryLayer2NetworkController.addUpdateLocalNodeEvent(node)) From 12b9838a5caa4803950d0cb5054ebe8e890e72eb Mon Sep 17 00:00:00 2001 From: Surya Seetharaman Date: Wed, 18 Sep 2024 11:31:27 +0200 Subject: [PATCH 12/12] fix retries for node events in udn See https://github.com/ovn-org/ovn-kubernetes/blob/master/go-controller/pkg/ovn/default_network_controller.go#L920 In UDN L3/L2 controllers we were missing checking for mgmtPortFailed and accounting for that as part of nodeupdates. So retry for any errors from syncManagementPort was not happening. Signed-off-by: Surya Seetharaman --- .../pkg/ovn/secondary_layer2_network_controller.go | 10 ++++++---- .../pkg/ovn/secondary_layer3_network_controller.go | 1 + 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/go-controller/pkg/ovn/secondary_layer2_network_controller.go b/go-controller/pkg/ovn/secondary_layer2_network_controller.go index 3de2b71291..eab7bf5443 100644 --- a/go-controller/pkg/ovn/secondary_layer2_network_controller.go +++ b/go-controller/pkg/ovn/secondary_layer2_network_controller.go @@ -159,16 +159,18 @@ func (h *secondaryLayer2NetworkControllerEventHandler) UpdateResource(oldObj, ne if newNodeIsLocalZoneNode { var nodeSyncsParam *nodeSyncs if h.oc.isLocalZoneNode(oldNode) { - // determine what actually changed in this update - syncMgmtPort := macAddressChanged(oldNode, newNode, h.oc.NetInfo.GetNetworkName()) || nodeSubnetChanged - + // determine what actually changed in this update and combine that with what failed previously + _, mgmtUpdateFailed := h.oc.mgmtPortFailed.Load(newNode.Name) + shouldSyncMgmtPort := mgmtUpdateFailed || + macAddressChanged(oldNode, newNode, h.oc.NetInfo.GetNetworkName()) || + nodeSubnetChanged _, gwUpdateFailed := h.oc.gatewaysFailed.Load(newNode.Name) shouldSyncGW := gwUpdateFailed || gatewayChanged(oldNode, newNode) || hostCIDRsChanged(oldNode, newNode) || nodeGatewayMTUSupportChanged(oldNode, newNode) - nodeSyncsParam = &nodeSyncs{syncMgmtPort: syncMgmtPort, syncGw: shouldSyncGW} + nodeSyncsParam = &nodeSyncs{syncMgmtPort: shouldSyncMgmtPort, syncGw: shouldSyncGW} } else { klog.Infof("Node %s moved from the remote zone %s to local zone %s.", newNode.Name, util.GetNodeZone(oldNode), util.GetNodeZone(newNode)) diff --git a/go-controller/pkg/ovn/secondary_layer3_network_controller.go b/go-controller/pkg/ovn/secondary_layer3_network_controller.go index 472bcb6ca5..fe696305f0 100644 --- a/go-controller/pkg/ovn/secondary_layer3_network_controller.go +++ b/go-controller/pkg/ovn/secondary_layer3_network_controller.go @@ -168,6 +168,7 @@ func (h *secondaryLayer3NetworkControllerEventHandler) UpdateResource(oldObj, ne _, nodeSync := h.oc.addNodeFailed.Load(newNode.Name) _, failed := h.oc.nodeClusterRouterPortFailed.Load(newNode.Name) clusterRtrSync := failed || nodeChassisChanged(oldNode, newNode) || nodeSubnetChanged + _, failed = h.oc.mgmtPortFailed.Load(newNode.Name) syncMgmtPort := failed || macAddressChanged(oldNode, newNode, h.oc.GetNetworkName()) || nodeSubnetChanged _, syncZoneIC := h.oc.syncZoneICFailed.Load(newNode.Name) syncZoneIC = syncZoneIC || zoneClusterChanged