diff --git a/pkg/agent/controller/egress/egress_controller.go b/pkg/agent/controller/egress/egress_controller.go index 14e41b63cc2..d6fc6e7da95 100644 --- a/pkg/agent/controller/egress/egress_controller.go +++ b/pkg/agent/controller/egress/egress_controller.go @@ -658,8 +658,10 @@ func (c *EgressController) syncEgress(egressName string) error { } if desiredNode == c.nodeName { - // Ensure the Egress IP is assigned to the system. - if err := c.ipAssigner.AssignIP(desiredEgressIP); err != nil { + // Ensure the Egress IP is assigned to the system. Force advertising the IP if it was previously assigned to + // another Node in the Egress API. This could force refreshing other peers' neighbor cache when the Egress IP is + // obtained by this Node and another Node at the same time in some situations, e.g. split brain. + if err := c.ipAssigner.AssignIP(desiredEgressIP, egress.Status.EgressNode != c.nodeName); err != nil { return err } } else { diff --git a/pkg/agent/controller/egress/egress_controller_test.go b/pkg/agent/controller/egress/egress_controller_test.go index 2563bd1c97f..71d54740a27 100644 --- a/pkg/agent/controller/egress/egress_controller_test.go +++ b/pkg/agent/controller/egress/egress_controller_test.go @@ -588,7 +588,9 @@ func TestSyncEgress(t *testing.T) { mockOFClient.EXPECT().InstallPodSNATFlows(uint32(1), net.ParseIP(fakeLocalEgressIP1), uint32(1)) mockOFClient.EXPECT().InstallPodSNATFlows(uint32(2), net.ParseIP(fakeLocalEgressIP1), uint32(1)) mockRouteClient.EXPECT().AddSNATRule(net.ParseIP(fakeLocalEgressIP1), uint32(1)) - mockIPAssigner.EXPECT().AssignIP(fakeLocalEgressIP2).Times(2) + mockIPAssigner.EXPECT().AssignIP(fakeLocalEgressIP2, true) + // forceAdvertise depends on how fast the Egress status update is reflected in the informer cache, which doesn't really matter. + mockIPAssigner.EXPECT().AssignIP(fakeLocalEgressIP2, gomock.Any()) mockOFClient.EXPECT().InstallSNATMarkFlows(net.ParseIP(fakeLocalEgressIP2), uint32(2)) mockOFClient.EXPECT().InstallPodSNATFlows(uint32(3), net.ParseIP(fakeLocalEgressIP2), uint32(2)) mockRouteClient.EXPECT().AddSNATRule(net.ParseIP(fakeLocalEgressIP2), uint32(2)) @@ -631,7 +633,7 @@ func TestSyncEgress(t *testing.T) { }, }, expectedCalls: func(mockOFClient *openflowtest.MockClient, mockRouteClient *routetest.MockInterface, mockIPAssigner *ipassignertest.MockIPAssigner) { - mockIPAssigner.EXPECT().AssignIP(fakeLocalEgressIP1) + mockIPAssigner.EXPECT().AssignIP(fakeLocalEgressIP1, true) mockOFClient.EXPECT().InstallSNATMarkFlows(net.ParseIP(fakeLocalEgressIP1), uint32(1)) mockOFClient.EXPECT().InstallPodSNATFlows(uint32(1), net.ParseIP(fakeLocalEgressIP1), uint32(1)) mockOFClient.EXPECT().InstallPodSNATFlows(uint32(2), net.ParseIP(fakeLocalEgressIP1), uint32(1)) @@ -673,7 +675,7 @@ func TestSyncEgress(t *testing.T) { }, }, expectedCalls: func(mockOFClient *openflowtest.MockClient, mockRouteClient *routetest.MockInterface, mockIPAssigner *ipassignertest.MockIPAssigner) { - mockIPAssigner.EXPECT().AssignIP(fakeLocalEgressIP1) + mockIPAssigner.EXPECT().AssignIP(fakeLocalEgressIP1, true) mockOFClient.EXPECT().InstallSNATMarkFlows(net.ParseIP(fakeLocalEgressIP1), uint32(1)) mockOFClient.EXPECT().InstallPodSNATFlows(uint32(1), net.ParseIP(fakeLocalEgressIP1), uint32(1)) mockOFClient.EXPECT().InstallPodSNATFlows(uint32(2), net.ParseIP(fakeLocalEgressIP1), uint32(1)) diff --git a/pkg/agent/controller/serviceexternalip/controller.go b/pkg/agent/controller/serviceexternalip/controller.go index 10e8695cb17..31d108eb8fb 100644 --- a/pkg/agent/controller/serviceexternalip/controller.go +++ b/pkg/agent/controller/serviceexternalip/controller.go @@ -393,7 +393,7 @@ func (c *ServiceExternalIPController) assignIP(ip string, service apimachineryty c.assignedIPsMutex.Lock() defer c.assignedIPsMutex.Unlock() if _, ok := c.assignedIPs[ip]; !ok { - if err := c.ipAssigner.AssignIP(ip); err != nil { + if err := c.ipAssigner.AssignIP(ip, true); err != nil { return err } c.assignedIPs[ip] = sets.New[string](service.String()) diff --git a/pkg/agent/controller/serviceexternalip/controller_test.go b/pkg/agent/controller/serviceexternalip/controller_test.go index 6fd1244c313..35546e240af 100644 --- a/pkg/agent/controller/serviceexternalip/controller_test.go +++ b/pkg/agent/controller/serviceexternalip/controller_test.go @@ -228,7 +228,7 @@ func TestCreateService(t *testing.T) { serviceToCreate: servicePolicyCluster, healthyNodes: []string{fakeNode1, fakeNode2}, expectedCalls: func(mockIPAssigner *ipassignertest.MockIPAssigner) { - mockIPAssigner.EXPECT().AssignIP(fakeServiceExternalIP1) + mockIPAssigner.EXPECT().AssignIP(fakeServiceExternalIP1, true) }, expectedExternalIPStates: map[apimachinerytypes.NamespacedName]externalIPState{ keyFor(servicePolicyCluster): { @@ -269,7 +269,7 @@ func TestCreateService(t *testing.T) { serviceToCreate: servicePolicyLocal, healthyNodes: []string{fakeNode1, fakeNode2}, expectedCalls: func(mockIPAssigner *ipassignertest.MockIPAssigner) { - mockIPAssigner.EXPECT().AssignIP(fakeServiceExternalIP1) + mockIPAssigner.EXPECT().AssignIP(fakeServiceExternalIP1, true) }, expectedExternalIPStates: map[apimachinerytypes.NamespacedName]externalIPState{ keyFor(servicePolicyLocal): { @@ -454,7 +454,7 @@ func TestUpdateService(t *testing.T) { healthyNodes: []string{fakeNode1, fakeNode2}, expectedCalls: func(mockIPAssigner *ipassignertest.MockIPAssigner) { mockIPAssigner.EXPECT().UnassignIP(fakeServiceExternalIP1) - mockIPAssigner.EXPECT().AssignIP(fakeServiceExternalIP2) + mockIPAssigner.EXPECT().AssignIP(fakeServiceExternalIP2, true) }, expectError: false, }, @@ -472,7 +472,7 @@ func TestUpdateService(t *testing.T) { }, healthyNodes: []string{fakeNode1, fakeNode2}, expectedCalls: func(mockIPAssigner *ipassignertest.MockIPAssigner) { - mockIPAssigner.EXPECT().AssignIP(fakeServiceExternalIP2) + mockIPAssigner.EXPECT().AssignIP(fakeServiceExternalIP2, true) }, expectError: false, }, diff --git a/pkg/agent/ipassigner/ip_assigner.go b/pkg/agent/ipassigner/ip_assigner.go index 62e6c6c3627..8bfc681fd7b 100644 --- a/pkg/agent/ipassigner/ip_assigner.go +++ b/pkg/agent/ipassigner/ip_assigner.go @@ -19,7 +19,7 @@ import "k8s.io/apimachinery/pkg/util/sets" // IPAssigner provides methods to assign or unassign IP. type IPAssigner interface { // AssignIP ensures the provided IP is assigned to the system. - AssignIP(ip string) error + AssignIP(ip string, forceAdvertise bool) error // UnassignIP ensures the provided IP is not assigned to the system. UnassignIP(ip string) error // AssignedIPs return the IPs that are assigned to the system by this IPAssigner. diff --git a/pkg/agent/ipassigner/ip_assigner_linux.go b/pkg/agent/ipassigner/ip_assigner_linux.go index 9411953ee01..fa6946338a0 100644 --- a/pkg/agent/ipassigner/ip_assigner_linux.go +++ b/pkg/agent/ipassigner/ip_assigner_linux.go @@ -28,6 +28,8 @@ import ( "antrea.io/antrea/pkg/agent/ipassigner/responder" "antrea.io/antrea/pkg/agent/util" + "antrea.io/antrea/pkg/agent/util/arping" + "antrea.io/antrea/pkg/agent/util/ndp" "antrea.io/antrea/pkg/agent/util/sysctl" ) @@ -71,11 +73,11 @@ func NewIPAssigner(nodeTransportInterface string, dummyDeviceName string) (IPAss return nil, err } if dummyDeviceName == "" || arpIgnore > 0 { - arpResonder, err := responder.NewARPResponder(externalInterface) + arpResponder, err := responder.NewARPResponder(externalInterface) if err != nil { return nil, fmt.Errorf("failed to create ARP responder for link %s: %v", externalInterface.Name, err) } - a.arpResponder = arpResonder + a.arpResponder = arpResponder } } if ipv6 != nil { @@ -141,7 +143,7 @@ func (a *ipAssigner) loadIPAddresses() (sets.Set[string], error) { } // AssignIP ensures the provided IP is assigned to the dummy device and the ARP/NDP responders. -func (a *ipAssigner) AssignIP(ip string) error { +func (a *ipAssigner) AssignIP(ip string, forceAdvertise bool) error { parsedIP := net.ParseIP(ip) if parsedIP == nil { return fmt.Errorf("invalid IP %s", ip) @@ -151,6 +153,9 @@ func (a *ipAssigner) AssignIP(ip string) error { if a.assignedIPs.Has(ip) { klog.V(2).InfoS("The IP is already assigned", "ip", ip) + if forceAdvertise { + a.advertise(parsedIP) + } return nil } @@ -177,11 +182,26 @@ func (a *ipAssigner) AssignIP(ip string) error { return fmt.Errorf("failed to assign IP %v to NDP responder: %v", ip, err) } } - + // Always advertise the IP when the IP is newly assigned to this Node. + a.advertise(parsedIP) a.assignedIPs.Insert(ip) return nil } +func (a *ipAssigner) advertise(ip net.IP) { + if utilnet.IsIPv4(ip) { + klog.V(2).InfoS("Sending gratuitous ARP", "ip", ip) + if err := arping.GratuitousARPOverIface(ip, a.externalInterface); err != nil { + klog.ErrorS(err, "Failed to send gratuitous ARP", "ip", ip) + } + } else { + klog.V(2).InfoS("Sending neighbor advertisement", "ip", ip) + if err := ndp.NeighborAdvertisement(ip, a.externalInterface); err != nil { + klog.ErrorS(err, "Failed to send neighbor advertisement", "ip", ip) + } + } +} + // UnassignIP ensures the provided IP is not assigned to the dummy device. func (a *ipAssigner) UnassignIP(ip string) error { parsedIP := net.ParseIP(ip) @@ -271,6 +291,7 @@ func (a *ipAssigner) InitIPs(ips sets.Set[string]) error { if err != nil { return err } + a.advertise(ip) } a.assignedIPs = ips.Union(nil) return nil diff --git a/pkg/agent/ipassigner/responder/arp_responder.go b/pkg/agent/ipassigner/responder/arp_responder.go index 06f572b709e..a2cef75a9bf 100644 --- a/pkg/agent/ipassigner/responder/arp_responder.go +++ b/pkg/agent/ipassigner/responder/arp_responder.go @@ -20,7 +20,6 @@ import ( "sync" "github.com/mdlayher/arp" - "github.com/mdlayher/ethernet" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/klog/v2" utilnet "k8s.io/utils/net" @@ -47,15 +46,6 @@ func NewARPResponder(iface *net.Interface) (*arpResponder, error) { }, nil } -// advertise sends an gratuitous ARP packet for the IP. -func (r *arpResponder) advertise(ip net.IP) error { - pkt, err := arp.NewPacket(arp.OperationRequest, r.iface.HardwareAddr, ip, ethernet.Broadcast, ip) - if err != nil { - return err - } - return r.conn.WriteTo(pkt, ethernet.Broadcast) -} - func (r *arpResponder) InterfaceName() string { return r.iface.Name } @@ -66,10 +56,6 @@ func (r *arpResponder) AddIP(ip net.IP) error { } if r.addIP(ip) { klog.InfoS("Assigned IP to ARP responder", "ip", ip, "interface", r.iface.Name) - err := r.advertise(ip) - if err != nil { - klog.ErrorS(err, "Failed to advertise", "ip", ip, "interface", r.iface.Name) - } } return nil } diff --git a/pkg/agent/ipassigner/responder/arp_responder_test.go b/pkg/agent/ipassigner/responder/arp_responder_test.go index 44c62850cf4..748b00658f9 100644 --- a/pkg/agent/ipassigner/responder/arp_responder_test.go +++ b/pkg/agent/ipassigner/responder/arp_responder_test.go @@ -77,64 +77,6 @@ func newFakeNetworkInterface() *net.Interface { } } -func TestARPResponder_Advertise(t *testing.T) { - tests := []struct { - name string - iface *net.Interface - ip net.IP - expectError bool - expectedBytes []byte - }{ - { - name: "GratuitousARP for IPv4", - iface: newFakeNetworkInterface(), - ip: net.ParseIP("192.168.10.1").To4(), - expectError: false, - expectedBytes: []byte{ - // ethernet header (16 bytes) - 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, // 6 bytes: destination hardware address - 0x00, 0x11, 0x22, 0x33, 0x44, 0x55, // 6 bytes: source hardware address - 0x08, 0x06, // 2 bytes: ethernet type - // arp payload (46 bytes) - 0x00, 0x01, // 2 bytes: hardware type - 0x08, 0x00, // 2 bytes: protocol type - 0x06, // 1 byte : hardware address length - 0x04, // 1 byte : protocol length - 0x00, 0x01, // 2 bytes: operation - 0x00, 0x11, 0x22, 0x33, 0x44, 0x55, // 6 bytes: source hardware address - 0xc0, 0xa8, 0x0a, 0x01, // 4 bytes: source protocol address - 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, // 6 bytes: target hardware address - 0xc0, 0xa8, 0x0a, 0x01, // 4 bytes: target protocol address - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // 18 bytes: padding - }, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - conn := &fakePacketConn{ - buffer: bytes.NewBuffer(nil), - addr: packet.Addr{ - HardwareAddr: tt.iface.HardwareAddr, - }, - } - fakeARPClient, err := newFakeARPClient(tt.iface, conn) - require.NoError(t, err) - - r := arpResponder{ - iface: tt.iface, - conn: fakeARPClient, - } - err = r.advertise(tt.ip) - if tt.expectError { - assert.Error(t, err) - } else { - assert.NoError(t, err) - assert.Equal(t, tt.expectedBytes, conn.buffer.Bytes()) - } - }) - } -} - func TestARPResponder_HandleARPRequest(t *testing.T) { tests := []struct { name string diff --git a/pkg/agent/ipassigner/responder/ndp_responder.go b/pkg/agent/ipassigner/responder/ndp_responder.go index d4afa237cdb..103843fecdf 100644 --- a/pkg/agent/ipassigner/responder/ndp_responder.go +++ b/pkg/agent/ipassigner/responder/ndp_responder.go @@ -73,21 +73,6 @@ func (r *ndpResponder) InterfaceName() string { return r.iface.Name } -// advertise sends Neighbor Advertisement for the IP. -func (r *ndpResponder) advertise(ip net.IP) error { - na := &ndp.NeighborAdvertisement{ - Override: true, - TargetAddress: ip, - Options: []ndp.Option{ - &ndp.LinkLayerAddress{ - Direction: ndp.Target, - Addr: r.iface.HardwareAddr, - }, - }, - } - return r.conn.WriteTo(na, nil, net.IPv6linklocalallnodes) -} - func (r *ndpResponder) handleNeighborSolicitation() error { pkt, _, srcIP, err := r.conn.ReadFrom() if err != nil { @@ -173,9 +158,6 @@ func (r *ndpResponder) AddIP(ip net.IP) error { }(); err != nil { return err } - if err := r.advertise(ip); err != nil { - klog.ErrorS(err, "Failed to advertise", "ip", ip, "interface", r.iface.Name) - } return nil } diff --git a/pkg/agent/ipassigner/responder/ndp_responder_test.go b/pkg/agent/ipassigner/responder/ndp_responder_test.go index bc45af42dea..21b0c329d95 100644 --- a/pkg/agent/ipassigner/responder/ndp_responder_test.go +++ b/pkg/agent/ipassigner/responder/ndp_responder_test.go @@ -52,60 +52,6 @@ func (c *fakeNDPConn) LeaveGroup(ip net.IP) error { return c.leaveGroup(ip) } -func TestNDPResponder_Advertise(t *testing.T) { - buffer := bytes.NewBuffer(nil) - fakeConn := &fakeNDPConn{ - writeTo: func(msg ndp.Message, _ *ipv6.ControlMessage, _ net.IP) error { - bs, err := ndp.MarshalMessage(msg) - assert.NoError(t, err) - buffer.Write(bs) - return nil - }, - } - responder := &ndpResponder{ - iface: newFakeNetworkInterface(), - conn: fakeConn, - } - err := responder.advertise(net.ParseIP("fe80::250:56ff:fea7:e29d")) - assert.NoError(t, err) - // Neighbor Advertisement Message Format - RFC 4861 Section 4.4. - // 0 1 2 3 - // 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 - // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ - // | Type | Code | Checksum | - // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ - // |R|S|O| Reserved | - // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ - // | | - // + + - // | | - // + Target Address + - // | | - // + + - // | | - // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ - // | Options ... - // +-+-+-+-+-+-+-+-+-+-+-+- - // - // Options formats - Source/Target Link-layer Address. RFC 4861 Section 4.6.1. - // 0 1 2 3 - // 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 - // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ - // | Type | Length | Link-Layer Address ... - // +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ - expectedBytes := []byte{ - 0x88, // type - 136 for Neighbor Advertisement - 0x00, // code - 0x00, 0x00, // checksum - 0x20, 0x00, 0x00, 0x00, // flags and reserved bits. Override bit is set. - 0xfe, 0x80, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0x50, 0x56, 0xff, 0xfe, 0xa7, 0xe2, 0x9d, // IPv6 address - 0x02, // option - 2 for Target Link-layer Address - 0x01, // length (units of 8 octets including type and length fields) - 0x00, 0x11, 0x22, 0x33, 0x44, 0x55, // hardware address - } - assert.Equal(t, expectedBytes, buffer.Bytes()) -} - func TestNDPResponder_handleNeighborSolicitation(t *testing.T) { tests := []struct { name string diff --git a/pkg/agent/ipassigner/testing/mock_ipassigner.go b/pkg/agent/ipassigner/testing/mock_ipassigner.go index 28ee313e0ec..d79ef782a9e 100644 --- a/pkg/agent/ipassigner/testing/mock_ipassigner.go +++ b/pkg/agent/ipassigner/testing/mock_ipassigner.go @@ -49,17 +49,17 @@ func (m *MockIPAssigner) EXPECT() *MockIPAssignerMockRecorder { } // AssignIP mocks base method -func (m *MockIPAssigner) AssignIP(arg0 string) error { +func (m *MockIPAssigner) AssignIP(arg0 string, arg1 bool) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "AssignIP", arg0) + ret := m.ctrl.Call(m, "AssignIP", arg0, arg1) ret0, _ := ret[0].(error) return ret0 } // AssignIP indicates an expected call of AssignIP -func (mr *MockIPAssignerMockRecorder) AssignIP(arg0 interface{}) *gomock.Call { +func (mr *MockIPAssignerMockRecorder) AssignIP(arg0, arg1 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AssignIP", reflect.TypeOf((*MockIPAssigner)(nil).AssignIP), arg0) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AssignIP", reflect.TypeOf((*MockIPAssigner)(nil).AssignIP), arg0, arg1) } // AssignedIPs mocks base method diff --git a/test/e2e/egress_test.go b/test/e2e/egress_test.go index 35b5274b6d4..99a16de0c7e 100644 --- a/test/e2e/egress_test.go +++ b/test/e2e/egress_test.go @@ -643,6 +643,17 @@ func testEgressMigration(t *testing.T, data *TestData, triggerFunc, revertFunc f if egress.Status.EgressNode != fromNode { fromNode, toNode = nodeName(1), nodeName(0) } + var checkIPNeighbor func(string) + if observerNode := nodeName(2); observerNode != "" { + checkIPNeighbor, err = setupIPNeighborChecker(data, t, observerNode, fromNode, toNode, egress.Spec.EgressIP) + require.NoError(t, err) + } else { + checkIPNeighbor = func(_ string) { + t.Logf("The cluster didn't have enough Nodes, skip IP neighbor check") + } + } + + checkIPNeighbor(fromNode) // Trigger Egress IP migration. The EgressIP should be moved to the other Node. triggerFunc(externalIPPoolTwoNodes.Name, fromNode) @@ -655,11 +666,13 @@ func testEgressMigration(t *testing.T, data *TestData, triggerFunc, revertFunc f } _, err = data.checkEgressState(egress.Name, egress.Spec.EgressIP, toNode, otherNodeToCheck, timeout) assert.NoError(t, err) + checkIPNeighbor(toNode) // Revert the operation. The EgressIP should be moved back. revertFunc(externalIPPoolTwoNodes.Name, fromNode) _, err = data.checkEgressState(egress.Name, egress.Spec.EgressIP, fromNode, toNode, timeout) assert.NoError(t, err) + checkIPNeighbor(fromNode) } func (data *TestData) checkEgressState(egressName, expectedIP, expectedNode, otherNode string, timeout time.Duration) (*v1alpha2.Egress, error) { @@ -716,6 +729,62 @@ func hasIP(data *TestData, nodeName string, ip string) (bool, error) { return strings.Contains(stdout, ip+"/32") || strings.Contains(stdout, ip+"/128"), nil } +func setupIPNeighborChecker(data *TestData, t *testing.T, observerNode, node1, node2, ip string) (checkIPNeighbor func(string), err error) { + transportInterface, err := data.GetTransportInterface() + require.NoError(t, err) + + macAddress1, err := data.GetNodeMACAddress(node1, transportInterface) + require.NoError(t, err) + macAddress2, err := data.GetNodeMACAddress(node2, transportInterface) + require.NoError(t, err) + nodeToMACAddress := map[string]string{node1: macAddress1, node2: macAddress2} + + antreaPodName, err := data.getAntreaPodOnNode(observerNode) + require.NoError(t, err) + + // The Egress IP may not be in the same subnet as the primary IP of the transport interface. + // Adding a direct route for the Egress IP so the Node will query its MAC address, instead of trying to connect via + // its gateway. + cmd := []string{"ip", "route", "replace", ip, "dev", transportInterface} + _, _, err = data.RunCommandFromPod(antreaNamespace, antreaPodName, agentContainerName, cmd) + require.NoError(t, err, "Failed to add a direct route for Egress IP %s on Node %s", ip, observerNode) + + t.Cleanup(func() { + cmd := []string{"ip", "route", "del", ip, "dev", transportInterface} + _, _, err = data.RunCommandFromPod(antreaNamespace, antreaPodName, agentContainerName, cmd) + require.NoError(t, err, "Failed to delete the direct route for Egress IP %s on Node %s", ip, observerNode) + }) + + checkIPNeighbor = func(expectNode string) { + check := func(allowEmpty bool) { + showIPNeighCmd := []string{"ip", "neighbor", "show", ip, "dev", transportInterface} + // stdout example: + // 172.18.0.1 lladdr 02:42:c2:60:91:66 STALE + stdout, _, err := data.RunCommandFromPod(antreaNamespace, antreaPodName, agentContainerName, showIPNeighCmd) + require.NoError(t, err, "Failed to query lladdr for Egress IP %s on Node %s", ip, observerNode) + if stdout == "" || strings.Contains(stdout, "FAILED") { + if !allowEmpty { + t.Errorf("Didn't get lladdr for Egress IP %s on Node %s", ip, observerNode) + } + return + } + fields := strings.Fields(stdout) + require.Len(t, fields, 4) + llAddr := fields[2] + assert.Equal(t, nodeToMACAddress[expectNode], llAddr, "lladdr for Egress IP %s didn't match the MAC address of Node %s", ip, expectNode) + } + // Before the Node actually connects to the Egress IP, we expect that the lladdr either matches the Egress Node's MAC address or is empty. + check(true) + // The protocol must be present when using wget with IPv6 address. + cmd := []string{"wget", fmt.Sprintf("http://%s", net.JoinHostPort(ip, "80")), "-T", "1", "-t", "1"} + // We don't care whether it succeeds, just make it connect to the Egress IP to learn its MAC address. + data.RunCommandFromPod(antreaNamespace, antreaPodName, agentContainerName, cmd) + // After the Node tries to connect to the Egress IP, we expect that the lladdr matches the Egress Node's MAC address. + check(false) + } + return checkIPNeighbor, nil +} + func (data *TestData) createExternalIPPool(t *testing.T, generateName string, ipRange v1alpha2.IPRange, matchExpressions []metav1.LabelSelectorRequirement, matchLabels map[string]string) *v1alpha2.ExternalIPPool { pool := &v1alpha2.ExternalIPPool{ ObjectMeta: metav1.ObjectMeta{GenerateName: generateName}, diff --git a/test/e2e/framework.go b/test/e2e/framework.go index 7b6395e6cd7..13f5b592d99 100644 --- a/test/e2e/framework.go +++ b/test/e2e/framework.go @@ -2276,24 +2276,44 @@ func (data *TestData) GetMulticastInterfaces(antreaNamespace string) ([]string, return agentConf.Multicast.MulticastInterfaces, nil } -func GetTransportInterface(data *TestData) (string, error) { - cmd := fmt.Sprintf("ip -br addr show | grep %s", clusterInfo.nodes[0].ipv4Addr) - if testOptions.providerName == "kind" { - cmd = "/bin/sh -c " + cmd - } - _, stdout, stderr, err := data.RunCommandOnNode(nodeName(0), cmd) +func (data *TestData) GetTransportInterface() (string, error) { + // It assumes all Nodes have the same transport interface name. + nodeName := nodeName(0) + nodeIP := nodeIP(0) + antreaPod, err := data.getAntreaPodOnNode(nodeName) if err != nil { - return "", err + return "", fmt.Errorf("failed to get Antrea Pod on Node %s: %v", nodeName, err) } - if stdout == "" || stderr != "" { - return "", fmt.Errorf("failed to get transport interface, stdout: %s, stderr: %s", stdout, stderr) + cmd := []string{"ip", "-br", "addr", "show"} + stdout, stderr, err := data.RunCommandFromPod(antreaNamespace, antreaPod, agentContainerName, cmd) + if stdout == "" || stderr != "" || err != nil { + return "", fmt.Errorf("failed to show ip address, stdout: %s, stderr: %s, err: %v", stdout, stderr, err) } // Example stdout: // eth0@if461 UP 172.18.0.2/16 fc00:f853:ccd:e793::2/64 fe80::42:acff:fe12:2/64 // eno1 UP 10.176.3.138/22 fe80::e643:4bff:fe43:a30e/64 - fields := strings.Fields(stdout) - name, _, _ := strings.Cut(fields[0], "@") - return name, nil + lines := strings.Split(strings.TrimSpace(stdout), "\n") + for _, line := range lines { + if strings.Contains(line, nodeIP+"/") { + fields := strings.Fields(line) + name, _, _ := strings.Cut(fields[0], "@") + return name, nil + } + } + return "", fmt.Errorf("no interface was assigned with Node IP %s", nodeIP) +} + +func (data *TestData) GetNodeMACAddress(node, device string) (string, error) { + antreaPod, err := data.getAntreaPodOnNode(node) + if err != nil { + return "", fmt.Errorf("failed to get Antrea Pod on Node %s: %v", node, err) + } + cmd := []string{"cat", fmt.Sprintf("/sys/class/net/%s/address", device)} + stdout, stderr, err := data.RunCommandFromPod(antreaNamespace, antreaPod, agentContainerName, cmd) + if stdout == "" || stderr != "" || err != nil { + return "", fmt.Errorf("failed to get MAC address, stdout: %s, stderr: %s, err: %v", stdout, stderr, err) + } + return strings.TrimSpace(stdout), nil } // mutateAntreaConfigMap will perform the specified updates on the antrea-agent config and the diff --git a/test/e2e/multicast_test.go b/test/e2e/multicast_test.go index 36ffeef84e9..d1428c54933 100644 --- a/test/e2e/multicast_test.go +++ b/test/e2e/multicast_test.go @@ -714,7 +714,7 @@ func computeMulticastInterfaces(t *testing.T, data *TestData) (map[int][]string, if err != nil { return nil, err } - transportInterface, err := GetTransportInterface(data) + transportInterface, err := data.GetTransportInterface() if err != nil { t.Fatalf("Error getting transport interfaces: %v", err) } diff --git a/test/integration/agent/ip_assigner_linux_test.go b/test/integration/agent/ip_assigner_linux_test.go index 24d798131ef..77368e18130 100644 --- a/test/integration/agent/ip_assigner_linux_test.go +++ b/test/integration/agent/ip_assigner_linux_test.go @@ -40,7 +40,7 @@ func TestIPAssigner(t *testing.T) { require.NoError(t, err, "Failed to find the dummy device") defer netlink.LinkDel(dummyDevice) - err = ipAssigner.AssignIP("x") + err = ipAssigner.AssignIP("x", false) assert.Error(t, err, "Assigning an invalid IP should fail") ip1 := "10.10.10.10" @@ -49,7 +49,7 @@ func TestIPAssigner(t *testing.T) { desiredIPs := sets.New[string](ip1, ip2, ip3) for ip := range desiredIPs { - errAssign := ipAssigner.AssignIP(ip) + errAssign := ipAssigner.AssignIP(ip, false) cmd := exec.Command("ip", "addr") out, err := cmd.CombinedOutput() if err != nil {