Merge pull request #4633 from martinkennelly/node-ip-udn

Node IP hander: ignore mgnt port IPs
ovn-org · Aug 23, 2024 · 2ebd782 · 2ebd782
2 parents f77b1be + 71f242b
commit 2ebd782
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 11 deletions.
diff --git a/go-controller/pkg/node/gateway_localnet_linux_test.go b/go-controller/pkg/node/gateway_localnet_linux_test.go
@@ -76,7 +76,8 @@ func startNodePortWatcher(n *nodePortWatcher, fakeClient *util.OVNNodeClientset,
 	n.nodeIPManager = newAddressManagerInternal(fakeNodeName, k, fakeMgmtPortConfig, n.watchFactory, nil, false)
 	localHostNetEp := "192.168.18.15/32"
 	ip, ipnet, _ := net.ParseCIDR(localHostNetEp)
-	n.nodeIPManager.addAddr(net.IPNet{IP: ip, Mask: ipnet.Mask})
+	ipFullNet := net.IPNet{IP: ip, Mask: ipnet.Mask}
+	n.nodeIPManager.cidrs.Insert(ipFullNet.String())
 
 	// Add or delete iptables rules from FORWARD chain based on DisableForwarding. This is
 	// to imitate addition or deletion of iptales rules done in newNodePortWatcher().
@@ -125,7 +126,8 @@ func startNodePortWatcherWithRetry(n *nodePortWatcher, fakeClient *util.OVNNodeC
 	n.nodeIPManager = newAddressManagerInternal(fakeNodeName, k, fakeMgmtPortConfig, n.watchFactory, nil, false)
 	localHostNetEp := "192.168.18.15/32"
 	ip, ipnet, _ := net.ParseCIDR(localHostNetEp)
-	n.nodeIPManager.addAddr(net.IPNet{IP: ip, Mask: ipnet.Mask})
+	ipFullNet := net.IPNet{IP: ip, Mask: ipnet.Mask}
+	n.nodeIPManager.cidrs.Insert(ipFullNet.String())
 
 	nodePortWatcherRetry := n.newRetryFrameworkForTests(factory.ServiceForFakeNodePortWatcherType, stopChan, wg)
 	if _, err := nodePortWatcherRetry.WatchResource(); err != nil {

diff --git a/go-controller/pkg/node/node_ip_handler_linux.go b/go-controller/pkg/node/node_ip_handler_linux.go
@@ -69,10 +69,10 @@ func newAddressManagerInternal(nodeName string, k kube.Interface, config *manage
 
 // updates the address manager with a new IP
 // returns true if there was an update
-func (c *addressManager) addAddr(ipnet net.IPNet) bool {
+func (c *addressManager) addAddr(ipnet net.IPNet, linkIndex int) bool {
 	c.Lock()
 	defer c.Unlock()
-	if !c.cidrs.Has(ipnet.String()) && c.isValidNodeIP(ipnet.IP) {
+	if !c.cidrs.Has(ipnet.String()) && c.isValidNodeIP(ipnet.IP, linkIndex) {
 		klog.Infof("Adding IP: %s, to node IP manager", ipnet)
 		c.cidrs.Insert(ipnet.String())
 		return true
@@ -83,10 +83,10 @@ func (c *addressManager) addAddr(ipnet net.IPNet) bool {
 
 // removes IP from address manager
 // returns true if there was an update
-func (c *addressManager) delAddr(ipnet net.IPNet) bool {
+func (c *addressManager) delAddr(ipnet net.IPNet, linkIndex int) bool {
 	c.Lock()
 	defer c.Unlock()
-	if c.cidrs.Has(ipnet.String()) && c.isValidNodeIP(ipnet.IP) {
+	if c.cidrs.Has(ipnet.String()) && c.isValidNodeIP(ipnet.IP, linkIndex) {
 		klog.Infof("Removing IP: %s, from node IP manager", ipnet)
 		c.cidrs.Delete(ipnet.String())
 		return true
@@ -151,9 +151,9 @@ func (c *addressManager) runInternal(stopChan <-chan struct{}, subscribe subscri
 			}
 			addrChanged := false
 			if a.NewAddr {
-				addrChanged = c.addAddr(a.LinkAddress)
+				addrChanged = c.addAddr(a.LinkAddress, a.LinkIndex)
 			} else {
-				addrChanged = c.delAddr(a.LinkAddress)
+				addrChanged = c.delAddr(a.LinkAddress, a.LinkIndex)
 			}
 
 			c.handleNodePrimaryAddrChange()
@@ -368,7 +368,7 @@ func (c *addressManager) nodePrimaryAddrChanged() (bool, error) {
 
 // detects if the IP is valid for a node
 // excludes things like local IPs, mgmt port ip, special masquerade IP and Egress IPs for non-ovs type interfaces
-func (c *addressManager) isValidNodeIP(addr net.IP) bool {
+func (c *addressManager) isValidNodeIP(addr net.IP, linkIndex int) bool {
 	if addr == nil {
 		return false
 	}
@@ -378,7 +378,7 @@ func (c *addressManager) isValidNodeIP(addr net.IP) bool {
 	if addr.IsLoopback() {
 		return false
 	}
-
+	// check CDN management port
 	if utilnet.IsIPv4(addr) {
 		if c.mgmtPortConfig.ipv4 != nil && c.mgmtPortConfig.ipv4.ifAddr.IP.Equal(addr) {
 			return false
@@ -388,6 +388,16 @@ func (c *addressManager) isValidNodeIP(addr net.IP) bool {
 			return false
 		}
 	}
+	if util.IsNetworkSegmentationSupportEnabled() {
+		// check CDN + UDN management ports
+		if mpLink, err := util.GetNetLinkOps().LinkByIndex(linkIndex); err != nil {
+			klog.Errorf("Unable to determine if link is an OVN management port for address %s and link index %d: %v", addr.String(), linkIndex, err)
+		} else {
+			if strings.HasPrefix(mpLink.Attrs().Name, types.K8sMgmtIntfNamePrefix) {
+				return false
+			}
+		}
+	}
 
 	if util.IsAddressReservedForInternalUse(addr) {
 		return false
@@ -427,7 +437,7 @@ func (c *addressManager) sync() {
 
 	currAddresses := sets.New[string]()
 	for _, addr := range addrs {
-		if !c.isValidNodeIP(addr.IP) {
+		if !c.isValidNodeIP(addr.IP, addr.LinkIndex) {
 			klog.V(5).Infof("Skipping non-useable IP address for host: %s", addr.String())
 			continue
 		}

diff --git a/go-controller/pkg/node/node_ip_handler_linux_test.go b/go-controller/pkg/node/node_ip_handler_linux_test.go
@@ -2,6 +2,7 @@ package node
 
 import (
 	"context"
+	"fmt"
 	"net"
 	"sync"
 	"sync/atomic"
@@ -14,6 +15,7 @@ import (
 	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/factory"
 	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/kube"
 	ovntest "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/testing"
+	ovntypes "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/types"
 	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/util"
 
 	"github.com/containernetworking/plugins/pkg/ns"
@@ -175,6 +177,7 @@ var _ = Describe("Node IP Handler tests", func() {
 		dummyBrName              = "breth0"
 		dummyBrInternalIPv4      = "10.1.1.10"
 		dummyAdditionalIPv4CIDR  = "192.168.2.2/24"
+		dummyAdditionalIPv4CIDR2 = "192.168.3.2/24"
 		dummyBrUniqLocalIPv6CIDR = "fd53:6043:6000:e0e0:1::6001/80"
 		dummyMasqIPv4            = "169.254.169.2"
 		dummyMasqIPv4CIDR        = dummyMasqIPv4 + "/29"
@@ -302,6 +305,19 @@ var _ = Describe("Node IP Handler tests", func() {
 					nodeHasAddress(tc.fakeClient, nodeName, ovntest.MustParseIPNet(dummyMasqIPv6CIDR))
 			}, 3).Should(BeFalse())
 		})
+
+		ovntest.OnSupportedPlatformsIt("doesn't allow OVN management port IPs", func() {
+			config.OVNKubernetesFeature.EnableMultiNetwork = true
+			config.OVNKubernetesFeature.EnableNetworkSegmentation = true
+			Expect(tc.ns.Do(func(netNS ns.NetNS) error {
+				mpLink := ovntest.AddLink(fmt.Sprintf("%s1234", ovntypes.K8sMgmtIntfNamePrefix))
+				return netlink.AddrAdd(mpLink, &netlink.Addr{LinkIndex: mpLink.Attrs().Index, Scope: unix.RT_SCOPE_UNIVERSE,
+					IPNet: ovntest.MustParseIPNet(dummyAdditionalIPv4CIDR)})
+			})).ShouldNot(HaveOccurred())
+			Consistently(func() bool {
+				return nodeHasAddress(tc.fakeClient, nodeName, ovntest.MustParseIPNet(dummyAdditionalIPv4CIDR))
+			}, 2).Should(BeFalse())
+		})
 	})
 })