Add ignored interfaces names when getting interface by IP (antrea-io#…

…3219) This is to resolve an issue that caused by two interfaces configured with the same IP address (different masks) on the host. The issue occurred in AKE setup with NetworkPolicyOnly mode, and antrea-gw0 is configured with Node's IP with 32bit mask. The changes include, 1. Provide a set of ignored interface names (e.g., antrea-gw0) when getting the Node's transport interface with Node's IP in agentInitializer. 2. Use Node's transport interface name to get the accurate interface in Egress feature instead of Node's IP. Signed-off-by: wenyingd <[email protected]>
NamanAg30 · Feb 15, 2022 · 397750d · 397750d
1 parent c3621fb
commit 397750d
Show file tree

Hide file tree

Showing 13 changed files with 51 additions and 44 deletions.
diff --git a/cmd/antrea-agent/agent.go b/cmd/antrea-agent/agent.go
@@ -301,14 +301,6 @@ func run(o *Options) error {
 	}
 
 	var egressController *egress.EgressController
-	var nodeTransportIP net.IP
-	if nodeConfig.NodeTransportIPv4Addr != nil {
-		nodeTransportIP = nodeConfig.NodeTransportIPv4Addr.IP
-	} else if nodeConfig.NodeTransportIPv6Addr != nil {
-		nodeTransportIP = nodeConfig.NodeTransportIPv6Addr.IP
-	} else {
-		return fmt.Errorf("invalid Node Transport IPAddr in Node config: %v", nodeConfig)
-	}
 
 	var externalIPPoolController *externalippool.ExternalIPPoolController
 	var externalIPController *serviceexternalip.ServiceExternalIPController
@@ -320,7 +312,14 @@ func run(o *Options) error {
 			crdClient, externalIPPoolInformer,
 		)
 		localIPDetector = ipassigner.NewLocalIPDetector()
-
+		var nodeTransportIP net.IP
+		if nodeConfig.NodeTransportIPv4Addr != nil {
+			nodeTransportIP = nodeConfig.NodeTransportIPv4Addr.IP
+		} else if nodeConfig.NodeTransportIPv6Addr != nil {
+			nodeTransportIP = nodeConfig.NodeTransportIPv6Addr.IP
+		} else {
+			return fmt.Errorf("invalid Node Transport IPAddr in Node config: %v", nodeConfig)
+		}
 		memberlistCluster, err = memberlist.NewCluster(nodeTransportIP, o.config.ClusterMembershipPort,
 			nodeConfig.Name, nodeInformer, externalIPPoolInformer,
 		)
@@ -330,7 +329,7 @@ func run(o *Options) error {
 	}
 	if features.DefaultFeatureGate.Enabled(features.Egress) {
 		egressController, err = egress.NewEgressController(
-			ofClient, antreaClientProvider, crdClient, ifaceStore, routeClient, nodeConfig.Name, nodeTransportIP,
+			ofClient, antreaClientProvider, crdClient, ifaceStore, routeClient, nodeConfig.Name, nodeConfig.NodeTransportInterfaceName,
 			memberlistCluster, egressInformer, nodeInformer, localIPDetector,
 		)
 		if err != nil {
@@ -340,7 +339,7 @@ func run(o *Options) error {
 	if features.DefaultFeatureGate.Enabled(features.ServiceExternalIP) {
 		externalIPController, err = serviceexternalip.NewServiceExternalIPController(
 			nodeConfig.Name,
-			nodeTransportIP,
+			nodeConfig.NodeTransportInterfaceName,
 			k8sClient,
 			memberlistCluster,
 			serviceInformer,

diff --git a/pkg/agent/agent.go b/pkg/agent/agent.go
@@ -28,6 +28,7 @@ import (
 	"github.com/containernetworking/plugins/pkg/ip"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	apitypes "k8s.io/apimachinery/pkg/types"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/wait"
 	clientset "k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/util/retry"
@@ -47,6 +48,7 @@ import (
 	"antrea.io/antrea/pkg/ovs/ovsconfig"
 	"antrea.io/antrea/pkg/ovs/ovsctl"
 	"antrea.io/antrea/pkg/util/env"
+	utilip "antrea.io/antrea/pkg/util/ip"
 	"antrea.io/antrea/pkg/util/k8s"
 )
 
@@ -796,7 +798,7 @@ func (i *Initializer) initNodeLocalConfig() error {
 	if err != nil {
 		return fmt.Errorf("failed to obtain local IP addresses from K8s: %w", err)
 	}
-	nodeIPv4Addr, nodeIPv6Addr, nodeInterface, err = getIPNetDeviceFromIP(ipAddrs)
+	nodeIPv4Addr, nodeIPv6Addr, nodeInterface, err = i.getNodeInterfaceFromIP(ipAddrs)
 	if err != nil {
 		return fmt.Errorf("failed to get local IPNet device with IP %v: %v", ipAddrs, err)
 	}
@@ -1117,3 +1119,10 @@ func (i *Initializer) patchNodeAnnotations(nodeName, key string, value interface
 	}
 	return nil
 }
+
+// getNodeInterfaceFromIP returns the IPv4/IPv6 configuration, and the associated interface according the give nodeIPs.
+// When searching the Node interface, antrea-gw0 is ignored because it is configured with the same address as Node IP
+// with NetworkPolicyOnly mode on public cloud setup, e.g., AKS.
+func (i *Initializer) getNodeInterfaceFromIP(nodeIPs *utilip.DualStackIPs) (v4IPNet *net.IPNet, v6IPNet *net.IPNet, iface *net.Interface, err error) {
+	return getIPNetDeviceFromIP(nodeIPs, sets.NewString(i.hostGateway))
+}
diff --git a/pkg/agent/agent_linux.go b/pkg/agent/agent_linux.go
@@ -47,7 +47,7 @@ func (i *Initializer) prepareOVSBridge() error {
 	klog.Infof("Preparing OVS bridge for AntreaFlexibleIPAM")
 	// Get uplink network configuration.
 	// TODO(gran): support IPv6
-	_, _, adapter, err := util.GetIPNetDeviceFromIP(&utilip.DualStackIPs{IPv4: i.nodeConfig.NodeIPv4Addr.IP})
+	_, _, adapter, err := i.getNodeInterfaceFromIP(&utilip.DualStackIPs{IPv4: i.nodeConfig.NodeIPv4Addr.IP})
 	if err != nil {
 		return err
 	}

diff --git a/pkg/agent/agent_test.go b/pkg/agent/agent_test.go
@@ -28,6 +28,7 @@ import (
 	"github.com/stretchr/testify/require"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/client-go/kubernetes/fake"
 
 	"antrea.io/antrea/pkg/agent/cniserver"
@@ -422,7 +423,7 @@ func TestInitNodeLocalConfig(t *testing.T) {
 
 func mockGetIPNetDeviceFromIP(ipNet *net.IPNet, ipDevice *net.Interface) func() {
 	prevGetIPNetDeviceFromIP := getIPNetDeviceFromIP
-	getIPNetDeviceFromIP = func(localIP *ip.DualStackIPs) (*net.IPNet, *net.IPNet, *net.Interface, error) {
+	getIPNetDeviceFromIP = func(localIP *ip.DualStackIPs, ignoredHostInterfaces sets.String) (*net.IPNet, *net.IPNet, *net.Interface, error) {
 		return ipNet, nil, ipDevice, nil
 	}
 	return func() { getIPNetDeviceFromIP = prevGetIPNetDeviceFromIP }

diff --git a/pkg/agent/agent_windows.go b/pkg/agent/agent_windows.go
@@ -51,7 +51,7 @@ func (i *Initializer) prepareHostNetwork() error {
 	// Get uplink network configuration. The uplink interface is the one used for transporting Pod traffic across Nodes.
 	// Use the interface specified with "transportInterface" in the configuration if configured, otherwise the interface
 	// configured with NodeIP is used as uplink.
-	_, _, adapter, err := util.GetIPNetDeviceFromIP(&ip.DualStackIPs{IPv4: i.nodeConfig.NodeTransportIPv4Addr.IP})
+	_, _, adapter, err := i.getNodeInterfaceFromIP(&ip.DualStackIPs{IPv4: i.nodeConfig.NodeTransportIPv4Addr.IP})
 	if err != nil {
 		return err
 	}

diff --git a/pkg/agent/controller/egress/egress_controller.go b/pkg/agent/controller/egress/egress_controller.go
@@ -151,7 +151,7 @@ func NewEgressController(
 	ifaceStore interfacestore.InterfaceStore,
 	routeClient route.Interface,
 	nodeName string,
-	nodeTransportIP net.IP,
+	nodeTransportInterface string,
 	cluster *memberlist.Cluster,
 	egressInformer crdinformers.EgressInformer,
 	nodeInformer coreinformers.NodeInformer,
@@ -176,7 +176,7 @@ func NewEgressController(
 		idAllocator:          newIDAllocator(minEgressMark, maxEgressMark),
 		cluster:              cluster,
 	}
-	ipAssigner, err := ipassigner.NewIPAssigner(nodeTransportIP, egressDummyDevice)
+	ipAssigner, err := ipassigner.NewIPAssigner(nodeTransportInterface, egressDummyDevice)
 	if err != nil {
 		return nil, fmt.Errorf("initializing egressIP assigner failed: %v", err)
 	}

diff --git a/pkg/agent/controller/serviceexternalip/controller.go b/pkg/agent/controller/serviceexternalip/controller.go
@@ -17,7 +17,6 @@ package serviceexternalip
 import (
 	"context"
 	"fmt"
-	"net"
 	"sync"
 	"time"
 
@@ -86,7 +85,7 @@ type ServiceExternalIPController struct {
 
 func NewServiceExternalIPController(
 	nodeName string,
-	nodeTransportIP net.IP,
+	nodeTransportInterface string,
 	client kubernetes.Interface,
 	cluster memberlist.Interface,
 	serviceInformer coreinformers.ServiceInformer,
@@ -107,7 +106,7 @@ func NewServiceExternalIPController(
 		externalIPStates:      make(map[apimachinerytypes.NamespacedName]externalIPState),
 		localIPDetector:       localIPDetector,
 	}
-	ipAssigner, err := ipassigner.NewIPAssigner(nodeTransportIP, ingressDummyDevice)
+	ipAssigner, err := ipassigner.NewIPAssigner(nodeTransportInterface, ingressDummyDevice)
 	if err != nil {
 		return nil, fmt.Errorf("initializing service external IP assigner failed: %v", err)
 	}

diff --git a/pkg/agent/ipassigner/ip_assigner_linux.go b/pkg/agent/ipassigner/ip_assigner_linux.go
@@ -26,7 +26,6 @@ import (
 	"antrea.io/antrea/pkg/agent/util"
 	"antrea.io/antrea/pkg/agent/util/arping"
 	"antrea.io/antrea/pkg/agent/util/ndp"
-	"antrea.io/antrea/pkg/util/ip"
 )
 
 // ipAssigner creates a dummy device and assigns IPs to it.
@@ -47,16 +46,10 @@ type ipAssigner struct {
 }
 
 // NewIPAssigner returns an *ipAssigner.
-func NewIPAssigner(nodeTransportIPAddr net.IP, dummyDeviceName string) (*ipAssigner, error) {
-	nodeTransportIPs := new(ip.DualStackIPs)
-	if nodeTransportIPAddr.To4() == nil {
-		nodeTransportIPs.IPv6 = nodeTransportIPAddr
-	} else {
-		nodeTransportIPs.IPv4 = nodeTransportIPAddr
-	}
-	_, _, externalInterface, err := util.GetIPNetDeviceFromIP(nodeTransportIPs)
+func NewIPAssigner(nodeTransportInterface string, dummyDeviceName string) (*ipAssigner, error) {
+	_, _, externalInterface, err := util.GetIPNetDeviceByName(nodeTransportInterface)
 	if err != nil {
-		return nil, fmt.Errorf("get IPNetDevice from ip %v error: %+v", nodeTransportIPAddr, err)
+		return nil, fmt.Errorf("get IPNetDevice from name %s error: %+v", nodeTransportInterface, err)
 	}
 
 	dummyDevice, err := ensureDummyDevice(dummyDeviceName)

diff --git a/pkg/agent/ipassigner/ip_assigner_windows.go b/pkg/agent/ipassigner/ip_assigner_windows.go
@@ -15,15 +15,13 @@
 package ipassigner
 
 import (
-	"net"
-
 	"k8s.io/apimachinery/pkg/util/sets"
 )
 
 type ipAssigner struct {
 }
 
-func NewIPAssigner(nodeTransportIPAddr net.IP, dummyDeviceName string) (*ipAssigner, error) {
+func NewIPAssigner(nodeTransportInterface string, dummyDeviceName string) (*ipAssigner, error) {
 	return nil, nil
 }
 

diff --git a/pkg/agent/util/net.go b/pkg/agent/util/net.go
@@ -111,8 +111,9 @@ func dialUnix(address string) (net.Conn, error) {
 	return net.Dial("unix", address)
 }
 
-// GetIPNetDeviceFromIP returns local IPs/masks and associated device from IP.
-func GetIPNetDeviceFromIP(localIPs *ip.DualStackIPs) (v4IPNet *net.IPNet, v6IPNet *net.IPNet, iface *net.Interface, err error) {
+// GetIPNetDeviceFromIP returns local IPs/masks and associated device from IP, and ignores the interfaces which have
+// names in the ignoredInterfaces.
+func GetIPNetDeviceFromIP(localIPs *ip.DualStackIPs, ignoredInterfaces sets.String) (v4IPNet *net.IPNet, v6IPNet *net.IPNet, iface *net.Interface, err error) {
 	linkList, err := net.Interfaces()
 	if err != nil {
 		return nil, nil, nil, err
@@ -129,19 +130,23 @@ func GetIPNetDeviceFromIP(localIPs *ip.DualStackIPs) (v4IPNet *net.IPNet, v6IPNe
 		return nil
 	}
 	for i := range linkList {
-		addrList, err := linkList[i].Addrs()
+		link := linkList[i]
+		if ignoredInterfaces.Has(link.Name) {
+			continue
+		}
+		addrList, err := link.Addrs()
 		if err != nil {
 			continue
 		}
 		for _, addr := range addrList {
 			if ipNet, ok := addr.(*net.IPNet); ok {
 				if ipNet.IP.Equal(localIPs.IPv4) {
-					if err := saveIface(&linkList[i]); err != nil {
+					if err := saveIface(&link); err != nil {
 						return nil, nil, nil, err
 					}
 					v4IPNet = ipNet
 				} else if ipNet.IP.Equal(localIPs.IPv6) {
-					if err := saveIface(&linkList[i]); err != nil {
+					if err := saveIface(&link); err != nil {
 						return nil, nil, nil, err
 					}
 					v6IPNet = ipNet

diff --git a/pkg/agent/util/net_test.go b/pkg/agent/util/net_test.go
@@ -20,6 +20,8 @@ import (
 	"strings"
 	"testing"
 
+	"k8s.io/apimachinery/pkg/util/sets"
+
 	"antrea.io/antrea/pkg/util/ip"
 )
 
@@ -58,7 +60,7 @@ func TestGetDefaultLocalNodeAddr(t *testing.T) {
 	localAddr := conn.LocalAddr().(*net.UDPAddr).IP
 
 	nodeIPs := &ip.DualStackIPs{IPv4: localAddr}
-	_, _, dev, err := GetIPNetDeviceFromIP(nodeIPs)
+	_, _, dev, err := GetIPNetDeviceFromIP(nodeIPs, sets.NewString())
 	if err != nil {
 		t.Error(err)
 	}

diff --git a/test/integration/agent/ip_assigner_linux_test.go b/test/integration/agent/ip_assigner_linux_test.go
@@ -30,10 +30,10 @@ import (
 const dummyDeviceName = "antrea-dummy0"
 
 func TestIPAssigner(t *testing.T) {
-	nodeIPAddr := nodeIPv4.IP
-	require.NotNil(t, nodeIPAddr, "Get Node IP failed")
+	nodeLinkName := nodeIntf.Name
+	require.NotNil(t, nodeLinkName, "Get Node link failed")
 
-	ipAssigner, err := ipassigner.NewIPAssigner(nodeIPAddr, dummyDeviceName)
+	ipAssigner, err := ipassigner.NewIPAssigner(nodeLinkName, dummyDeviceName)
 	require.NoError(t, err, "Initializing IP assigner failed")
 
 	dummyDevice, err := netlink.LinkByName(dummyDeviceName)
@@ -65,7 +65,7 @@ func TestIPAssigner(t *testing.T) {
 	assert.Equal(t, desiredIPs, actualIPs, "Actual IPs don't match")
 
 	// NewIPAssigner should load existing IPs correctly.
-	newIPAssigner, err := ipassigner.NewIPAssigner(nodeIPAddr, dummyDeviceName)
+	newIPAssigner, err := ipassigner.NewIPAssigner(nodeLinkName, dummyDeviceName)
 	require.NoError(t, err, "Initializing new IP assigner failed")
 	assert.Equal(t, desiredIPs, newIPAssigner.AssignedIPs(), "Assigned IPs don't match")
 

diff --git a/test/integration/agent/route_test.go b/test/integration/agent/route_test.go
@@ -31,6 +31,7 @@ import (
 	"github.com/stretchr/testify/require"
 	"github.com/vishvananda/netlink"
 	"golang.org/x/net/nettest"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/wait"
 
 	"antrea.io/antrea/pkg/agent/config"
@@ -56,7 +57,7 @@ var (
 		conn, _ := net.Dial("udp", "8.8.8.8:80")
 		defer conn.Close()
 		return &utilip.DualStackIPs{IPv4: conn.LocalAddr().(*net.UDPAddr).IP}
-	}())
+	}(), sets.NewString())
 	nodeLink, _  = netlink.LinkByName(nodeIntf.Name)
 	localPeerIP  = ip.NextIP(nodeIPv4.IP)
 	remotePeerIP = net.ParseIP("50.50.50.1")