Reapply "Add the IP rule for a UDN only when it is advertised to the default VRF"

This reverts commit 1ea2739.

Signed-off-by: Jaime Caamaño Ruiz <[email protected]>

Author: jcaamano

go-controller/pkg/node/gateway_udn.go

@@ -89,6 +89,10 @@ type UserDefinedNetworkGateway struct {
 
 	// gwInterfaceIndex holds the link index of gateway interface
 	gwInterfaceIndex int
+
+	// save BGP state at the start of reconciliation loop run to handle it consistently throughout the run
+	isNetworkAdvertisedToDefaultVRF bool
+	isNetworkAdvertised             bool
 }
 
 func NewUserDefinedNetworkGateway(netInfo util.NetInfo, node *corev1.Node, nodeLister listers.NodeLister,
@@ -225,18 +229,18 @@ func (udng *UserDefinedNetworkGateway) AddNetwork() error {
 		return fmt.Errorf("could not add VRF %s routes for network %s, err: %v", vrfDeviceName, udng.GetNetworkName(), err)
 	}
 
-	isNetworkAdvertised := util.IsPodNetworkAdvertisedAtNode(udng.NetInfo, udng.node.Name)
+	udng.updateAdvertisementStatus()
 
 	// create the iprules for this network
-	if err = udng.updateUDNVRFIPRules(isNetworkAdvertised); err != nil {
+	if err = udng.updateUDNVRFIPRules(); err != nil {
 		return fmt.Errorf("failed to update IP rules for network %s: %w", udng.GetNetworkName(), err)
 	}
 
-	if err = udng.updateAdvertisedUDNIsolationRules(isNetworkAdvertised); err != nil {
+	if err = udng.updateAdvertisedUDNIsolationRules(); err != nil {
 		return fmt.Errorf("failed to update isolation rules for network %s: %w", udng.GetNetworkName(), err)
 	}
 
-	if err := udng.updateUDNVRFIPRoute(isNetworkAdvertised); err != nil {
+	if err := udng.updateUDNVRFIPRoute(); err != nil {
 		return fmt.Errorf("failed to update ip routes for network %s: %w", udng.GetNetworkName(), err)
 	}
 
@@ -314,18 +318,16 @@ func (udng *UserDefinedNetworkGateway) DelNetwork() error {
 		}
 	}
 
-	if util.IsPodNetworkAdvertisedAtNode(udng.NetInfo, udng.node.Name) {
-		err := udng.updateAdvertisedUDNIsolationRules(false)
-		if err != nil {
-			return fmt.Errorf("failed to remove advertised UDN isolation rules for network %s: %w", udng.GetNetworkName(), err)
-		}
+	err := udng.deleteAdvertisedUDNIsolationRules()
+	if err != nil {
+		return fmt.Errorf("failed to remove advertised UDN isolation rules for network %s: %w", udng.GetNetworkName(), err)
 	}
 
 	if err := udng.delMarkChain(); err != nil {
 		return err
 	}
 	// delete the management port interface for this network
-	err := udng.deleteUDNManagementPort()
+	err = udng.deleteUDNManagementPort()
 	if err != nil {
 		return err
 	}
@@ -483,8 +485,7 @@ func (udng *UserDefinedNetworkGateway) computeRoutesForUDN(mpLink netlink.Link)
 
 	// Route2: Add default route: default via 172.18.0.1 dev breth0 mtu 1400
 	// necessary for UDN CNI and host-networked pods default traffic to go to node's gatewayIP
-	isNetworkAdvertised := util.IsPodNetworkAdvertisedAtNode(udng.NetInfo, udng.node.Name)
-	defaultRoute, err := udng.getDefaultRoute(isNetworkAdvertised)
+	defaultRoute, err := udng.getDefaultRouteExceptIfVRFLite()
 	if err != nil {
 		return nil, fmt.Errorf("unable to add default route for network %s, err: %v", udng.GetNetworkName(), err)
 	}
@@ -585,15 +586,7 @@ func (udng *UserDefinedNetworkGateway) computeRoutesForUDN(mpLink netlink.Link)
 	return retVal, nil
 }
 
-func (udng *UserDefinedNetworkGateway) getDefaultRoute(isNetworkAdvertised bool) ([]netlink.Route, error) {
-	vrfs := udng.GetPodNetworkAdvertisedOnNodeVRFs(udng.node.Name)
-	// If the network is advertised on a non default VRF then we should only consider routes received from external BGP
-	// device and not send any traffic based on default route similar to one present in default VRF. This is more important
-	// for VRF-Lite usecase where we need traffic to leave from vlan device instead of default gateway interface.
-	if isNetworkAdvertised && !slices.Contains(vrfs, types.DefaultNetworkName) {
-		return nil, nil
-	}
-
+func (udng *UserDefinedNetworkGateway) getDefaultRoute() ([]netlink.Route, error) {
 	networkMTU := udng.NetInfo.MTU()
 	if networkMTU == 0 {
 		networkMTU = config.Default.MTU
@@ -618,6 +611,16 @@ func (udng *UserDefinedNetworkGateway) getDefaultRoute(isNetworkAdvertised bool)
 	return retVal, nil
 }
 
+func (udng *UserDefinedNetworkGateway) getDefaultRouteExceptIfVRFLite() ([]netlink.Route, error) {
+	// If the network is advertised on a non default VRF then we should only consider routes received from external BGP
+	// device and not send any traffic based on default route similar to one present in default VRF. This is more important
+	// for VRF-Lite usecase where we need traffic to leave from vlan device instead of default gateway interface.
+	if udng.isNetworkAdvertised && !udng.isNetworkAdvertisedToDefaultVRF {
+		return nil, nil
+	}
+	return udng.getDefaultRoute()
+}
+
 // getV4MasqueradeIP returns the V4 management port masqueradeIP for this network
 func (udng *UserDefinedNetworkGateway) getV4MasqueradeIP() (*net.IPNet, error) {
 	if !config.IPv4Mode {
@@ -644,21 +647,18 @@ func (udng *UserDefinedNetworkGateway) getV6MasqueradeIP() (*net.IPNet, error) {
 
 // constructUDNVRFIPRules constructs rules that redirect matching packets
 // into the corresponding UDN VRF routing table.
-// If the network is not advertised, an example of the rules we set for two
-// networks is:
+//
+// When a network is not advertised on the default VRF, an example of the rules
+// we set for it is:
 // 2000:	from all fwmark 0x1001 lookup 1007
 // 2000:	from all to 169.254.0.12 lookup 1007
-// 2000:	from all fwmark 0x1002 lookup 1009
-// 2000:	from all to 169.254.0.14 lookup 1009
-// If the network is advertised, an example of the rules we set for two
-// networks is:
+//
+// When a network is advertised on the default VRF, an example of the rules
+// we set for it is:
 // 2000:	from all fwmark 0x1001 lookup 1007
 // 2000:	from all to 10.132.0.0/14 lookup 1007
 // 2000:	from all to 169.254.0.12 lookup 1007
-// 2000:	from all fwmark 0x1001 lookup 1009
-// 2000:	from all to 10.134.0.0/14 lookup 1009
-// 2000:	from all to 169.254.0.14 lookup 1009
-func (udng *UserDefinedNetworkGateway) constructUDNVRFIPRules(isNetworkAdvertised bool) ([]netlink.Rule, []netlink.Rule, error) {
+func (udng *UserDefinedNetworkGateway) constructUDNVRFIPRules() ([]netlink.Rule, []netlink.Rule, error) {
 	var addIPRules []netlink.Rule
 	var delIPRules []netlink.Rule
 	var masqIPRules []netlink.Rule
@@ -691,12 +691,14 @@ func (udng *UserDefinedNetworkGateway) constructUDNVRFIPRules(isNetworkAdvertise
 		}
 	}
 	switch {
-	case !isNetworkAdvertised:
+	case udng.isNetworkAdvertisedToDefaultVRF:
+		// the network is advertised to the default VRF
 		addIPRules = append(addIPRules, masqIPRules...)
-		delIPRules = append(delIPRules, subnetIPRules...)
-	default:
 		addIPRules = append(addIPRules, subnetIPRules...)
+	default:
+		// network is not advertised on the default VRF
 		addIPRules = append(addIPRules, masqIPRules...)
+		delIPRules = append(delIPRules, subnetIPRules...)
 	}
 	return addIPRules, delIPRules, nil
 }
@@ -794,19 +796,20 @@ func (udng *UserDefinedNetworkGateway) doReconcile() error {
 		return fmt.Errorf("openflow manager with default bridge configuration has not been provided for network %s", udng.GetNetworkName())
 	}
 
+	udng.updateAdvertisementStatus()
+
 	// update bridge configuration
-	isNetworkAdvertised := util.IsPodNetworkAdvertisedAtNode(udng.NetInfo, udng.node.Name)
 	netConfig := udng.openflowManager.defaultBridge.GetNetworkConfig(udng.GetNetworkName())
 	if netConfig == nil {
 		return fmt.Errorf("missing bridge configuration for network %s", udng.GetNetworkName())
 	}
-	netConfig.Advertised.Store(isNetworkAdvertised)
+	netConfig.Advertised.Store(udng.isNetworkAdvertised)
 
-	if err := udng.updateUDNVRFIPRules(isNetworkAdvertised); err != nil {
+	if err := udng.updateUDNVRFIPRules(); err != nil {
 		return fmt.Errorf("error while updating ip rule for UDN %s: %s", udng.GetNetworkName(), err)
 	}
 
-	if err := udng.updateUDNVRFIPRoute(isNetworkAdvertised); err != nil {
+	if err := udng.updateUDNVRFIPRoute(); err != nil {
 		return fmt.Errorf("error while updating ip route for UDN %s: %s", udng.GetNetworkName(), err)
 	}
 
@@ -820,16 +823,16 @@ func (udng *UserDefinedNetworkGateway) doReconcile() error {
 	// let's sync these flows immediately
 	udng.openflowManager.requestFlowSync()
 
-	if err := udng.updateAdvertisedUDNIsolationRules(isNetworkAdvertised); err != nil {
+	if err := udng.updateAdvertisedUDNIsolationRules(); err != nil {
 		return fmt.Errorf("error while updating advertised UDN isolation rules for network %s: %w", udng.GetNetworkName(), err)
 	}
 	return nil
 }
 
 // updateUDNVRFIPRules updates IP rules for a network depending on whether the
-// network is advertised or not
-func (udng *UserDefinedNetworkGateway) updateUDNVRFIPRules(isNetworkAdvertised bool) error {
-	addIPRules, deleteIPRules, err := udng.constructUDNVRFIPRules(isNetworkAdvertised)
+// network is advertised to the default VRF or not
+func (udng *UserDefinedNetworkGateway) updateUDNVRFIPRules() error {
+	addIPRules, deleteIPRules, err := udng.constructUDNVRFIPRules()
 	if err != nil {
 		return fmt.Errorf("unable to get iprules for network %s, err: %v", udng.GetNetworkName(), err)
 	}
@@ -848,30 +851,40 @@ func (udng *UserDefinedNetworkGateway) updateUDNVRFIPRules(isNetworkAdvertised b
 }
 
 // Add or remove default route from a vrf device based on the network is
-// advertised on its own network or default network
-func (udng *UserDefinedNetworkGateway) updateUDNVRFIPRoute(isNetworkAdvertised bool) error {
-	vrfs := udng.GetPodNetworkAdvertisedOnNodeVRFs(udng.node.Name)
-	if isNetworkAdvertised && !slices.Contains(vrfs, types.DefaultNetworkName) {
+// advertised on its own network or the default network
+func (udng *UserDefinedNetworkGateway) updateUDNVRFIPRoute() error {
+	vrfName := util.GetNetworkVRFName(udng.NetInfo)
+
+	switch {
+	case udng.isNetworkAdvertised && !udng.isNetworkAdvertisedToDefaultVRF:
+		// Remove default route for networks advertised to non-default VRF
 		if err := udng.removeDefaultRouteFromVRF(); err != nil {
-			return fmt.Errorf("error while removing default route from VRF %s corresponding to network %s: %s",
-				util.GetNetworkVRFName(udng.NetInfo), udng.GetNetworkName(), err)
+			return fmt.Errorf("failed to remove default route from VRF %s for network %s: %v",
+				vrfName, udng.GetNetworkName(), err)
 		}
-	} else if !isNetworkAdvertised || slices.Contains(vrfs, types.DefaultNetworkName) {
-		defaultRoute, err := udng.getDefaultRoute(isNetworkAdvertised)
+
+	default:
+		// Add default route for networks that are either:
+		// - not advertised
+		// - advertised to default VRF
+		defaultRoute, err := udng.getDefaultRouteExceptIfVRFLite()
 		if err != nil {
-			return fmt.Errorf("unable to get default route for network %s, err: %v", udng.GetNetworkName(), err)
+			return fmt.Errorf("failed to get default route for network %s: %v",
+				udng.GetNetworkName(), err)
 		}
-		if err = udng.vrfManager.AddVRFRoutes(util.GetNetworkVRFName(udng.NetInfo), defaultRoute); err != nil {
-			return fmt.Errorf("error while adding default route to VRF %s corresponding to network %s, err: %v",
-				util.GetNetworkVRFName(udng.NetInfo), udng.GetNetworkName(), err)
+
+		if err = udng.vrfManager.AddVRFRoutes(vrfName, defaultRoute); err != nil {
+			return fmt.Errorf("failed to add default route to VRF %s for network %s: %v",
+				vrfName, udng.GetNetworkName(), err)
 		}
 	}
+
 	return nil
 }
 
 func (udng *UserDefinedNetworkGateway) removeDefaultRouteFromVRF() error {
 	vrfDeviceName := util.GetNetworkVRFName(udng.NetInfo)
-	defaultRoute, err := udng.getDefaultRoute(false)
+	defaultRoute, err := udng.getDefaultRoute()
 	if err != nil {
 		return fmt.Errorf("unable to get default route for network %s, err: %v", udng.GetNetworkName(), err)
 	}
@@ -900,39 +913,22 @@ func (udng *UserDefinedNetworkGateway) removeDefaultRouteFromVRF() error {
 //	   comment "advertised UDNs V4 subnets"
 //	   elements = { 10.10.0.0/16 comment "cluster_udn_l3network" }
 //	}
-func (udng *UserDefinedNetworkGateway) updateAdvertisedUDNIsolationRules(isNetworkAdvertised bool) error {
+func (udng *UserDefinedNetworkGateway) updateAdvertisedUDNIsolationRules() error {
+	switch {
+	case udng.isNetworkAdvertised:
+		return udng.addAdvertisedUDNIsolationRules()
+	default:
+		return udng.deleteAdvertisedUDNIsolationRules()
+	}
+}
+
+func (udng *UserDefinedNetworkGateway) addAdvertisedUDNIsolationRules() error {
 	nft, err := nodenft.GetNFTablesHelper()
 	if err != nil {
 		return fmt.Errorf("failed to get nftables helper: %v", err)
 	}
 	tx := nft.NewTransaction()
 
-	if !isNetworkAdvertised {
-		existingV4, err := nft.ListElements(context.TODO(), "set", nftablesAdvertisedUDNsSetV4)
-		if err != nil {
-			if !knftables.IsNotFound(err) {
-				return fmt.Errorf("could not list existing items in %s set: %w", nftablesAdvertisedUDNsSetV4, err)
-			}
-		}
-		existingV6, err := nft.ListElements(context.TODO(), "set", nftablesAdvertisedUDNsSetV6)
-		if err != nil {
-			if !knftables.IsNotFound(err) {
-				return fmt.Errorf("could not list existing items in %s set: %w", nftablesAdvertisedUDNsSetV6, err)
-			}
-		}
-
-		for _, elem := range append(existingV4, existingV6...) {
-			if elem.Comment != nil && *elem.Comment == udng.GetNetworkName() {
-				tx.Delete(elem)
-			}
-		}
-
-		if tx.NumOperations() == 0 {
-			return nil
-		}
-		return nft.Run(context.TODO(), tx)
-	}
-
 	for _, udnNet := range udng.Subnets() {
 		set := nftablesAdvertisedUDNsSetV4
 		if utilnet.IsIPv6CIDR(udnNet.CIDR) {
@@ -951,3 +947,41 @@ func (udng *UserDefinedNetworkGateway) updateAdvertisedUDNIsolationRules(isNetwo
 	}
 	return nft.Run(context.TODO(), tx)
 }
+
+func (udng *UserDefinedNetworkGateway) deleteAdvertisedUDNIsolationRules() error {
+	nft, err := nodenft.GetNFTablesHelper()
+	if err != nil {
+		return fmt.Errorf("failed to get nftables helper: %v", err)
+	}
+	tx := nft.NewTransaction()
+
+	existingV4, err := nft.ListElements(context.TODO(), "set", nftablesAdvertisedUDNsSetV4)
+	if err != nil {
+		if !knftables.IsNotFound(err) {
+			return fmt.Errorf("could not list existing items in %s set: %w", nftablesAdvertisedUDNsSetV4, err)
+		}
+	}
+	existingV6, err := nft.ListElements(context.TODO(), "set", nftablesAdvertisedUDNsSetV6)
+	if err != nil {
+		if !knftables.IsNotFound(err) {
+			return fmt.Errorf("could not list existing items in %s set: %w", nftablesAdvertisedUDNsSetV6, err)
+		}
+	}
+
+	for _, elem := range append(existingV4, existingV6...) {
+		if elem.Comment != nil && *elem.Comment == udng.GetNetworkName() {
+			tx.Delete(elem)
+		}
+	}
+
+	if tx.NumOperations() == 0 {
+		return nil
+	}
+	return nft.Run(context.TODO(), tx)
+}
+
+func (udng *UserDefinedNetworkGateway) updateAdvertisementStatus() {
+	vrfs := udng.GetPodNetworkAdvertisedOnNodeVRFs(udng.node.Name)
+	udng.isNetworkAdvertised = len(vrfs) > 0
+	udng.isNetworkAdvertisedToDefaultVRF = slices.Contains(vrfs, types.DefaultNetworkName)
+}