diff --git a/go-controller/pkg/libovsdb/ops/router.go b/go-controller/pkg/libovsdb/ops/router.go index c0436914eb8..aa8841a7511 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 diff --git a/go-controller/pkg/ovn/base_network_controller.go b/go-controller/pkg/ovn/base_network_controller.go index dd0107ee888..de05d348f48 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/gateway.go b/go-controller/pkg/ovn/gateway.go index f5b42121e11..87aabbf96f4 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) @@ -1207,22 +1226,28 @@ 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.Add(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, 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) + } + } } return nil diff --git a/go-controller/pkg/ovn/gatewayrouter/policybasedroutes.go b/go-controller/pkg/ovn/gatewayrouter/policybasedroutes.go index e054a67c18a..b98c5e31fbb 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" @@ -31,7 +32,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") } @@ -44,13 +45,13 @@ func (pbr *PolicyBasedRoutesManager) Add(nodeName, mgmtPortIP string, hostIfCIDR 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. // 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) } @@ -66,6 +67,46 @@ func (pbr *PolicyBasedRoutesManager) Add(nodeName, mgmtPortIP string, hostIfCIDR 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: @@ -218,12 +259,16 @@ 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) } -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 b151465eedf..d90152717ba 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 } @@ -91,7 +97,7 @@ type test struct { expectErr bool } -func TestAdd(t *testing.T) { +func TestAddSameNodeIPPolicy(t *testing.T) { const ( node1Name = "node1" node1HostIPv4Str = "192.168.1.10" @@ -159,7 +165,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 +192,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 +240,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 +273,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 +318,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 +350,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 +363,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 +384,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 +397,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 +425,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 +444,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{ @@ -460,7 +466,7 @@ func TestAdd(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 { @@ -484,7 +490,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) } @@ -503,3 +509,141 @@ func TestAdd(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/ovn/master.go b/go-controller/pkg/ovn/master.go index 87ec5db2696..1535403f25c 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/multihoming_test.go b/go-controller/pkg/ovn/multihoming_test.go index 641472cdb3c..33bff1b13dc 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.go b/go-controller/pkg/ovn/secondary_layer2_network_controller.go index 455b934c69a..eab7bf54436 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)) @@ -419,22 +421,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.syncNodeManagementPortNoRouteHostSubnets(node, oc.GetNetworkScopedSwitchName(types.OVNLayer2Switch), 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 +455,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)) 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 84e95981a1b..c6a0d58891c 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,18 +399,20 @@ 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 - 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) @@ -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.go b/go-controller/pkg/ovn/secondary_layer3_network_controller.go index d90521fad8a..fe696305f08 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 @@ -653,7 +654,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/go-controller/pkg/ovn/secondary_layer3_network_controller_test.go b/go-controller/pkg/ovn/secondary_layer3_network_controller_test.go index 4595d0fd14e..6795531931c 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()), @@ -730,7 +746,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/types/const.go b/go-controller/pkg/types/const.go index 7b021716e81..8b49d49b239 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 diff --git a/go-controller/pkg/util/multi_network.go b/go-controller/pkg/util/multi_network.go index a33097bda68..d1e5136a4b9 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) } diff --git a/go-controller/pkg/util/node_annotations.go b/go-controller/pkg/util/node_annotations.go index 3f9af38b73b..a497f8d3e33 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 } diff --git a/test/e2e/network_segmentation.go b/test/e2e/network_segmentation.go index 3f2ba74b055..a0c4e78513a 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(