From 397750dfe3d69920f427d1b0fca0a2e8d50daec4 Mon Sep 17 00:00:00 2001 From: Wenying Dong Date: Tue, 15 Feb 2022 09:52:47 -0800 Subject: [PATCH] Add ignored interfaces names when getting interface by IP (#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 --- cmd/antrea-agent/agent.go | 21 +++++++++---------- pkg/agent/agent.go | 11 +++++++++- pkg/agent/agent_linux.go | 2 +- pkg/agent/agent_test.go | 3 ++- pkg/agent/agent_windows.go | 2 +- .../controller/egress/egress_controller.go | 4 ++-- .../serviceexternalip/controller.go | 5 ++--- pkg/agent/ipassigner/ip_assigner_linux.go | 13 +++--------- pkg/agent/ipassigner/ip_assigner_windows.go | 4 +--- pkg/agent/util/net.go | 15 ++++++++----- pkg/agent/util/net_test.go | 4 +++- .../agent/ip_assigner_linux_test.go | 8 +++---- test/integration/agent/route_test.go | 3 ++- 13 files changed, 51 insertions(+), 44 deletions(-) diff --git a/cmd/antrea-agent/agent.go b/cmd/antrea-agent/agent.go index dd23164cb7c..e7fc702e736 100644 --- 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 index 015f250855a..1711251117c 100644 --- 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 index 33c189f59de..26cf1c0e206 100644 --- 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 index a5a6345ac58..9a2d30ac909 100644 --- 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 index 5be3fdff579..c43b715874b 100644 --- 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 index 29f317030a2..4addbc95cec 100644 --- 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 index 7c28e236eba..f8ac8c38d17 100644 --- 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 index b82ceb6cb97..04ed9d1af69 100644 --- 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 index 4730939db0e..00caa69176d 100644 --- 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 index a89e8528019..81719628e9f 100644 --- 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 index be4e3b5e012..cb04c2d8f7c 100644 --- 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 index 9d10b090e9a..8e308390c83 100644 --- 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 index ab72879d1ef..c8abbe47417 100644 --- 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")