diff --git a/pkg/agent/agent_test.go b/pkg/agent/agent_test.go index 11c449aee54..27968c1f0da 100644 --- a/pkg/agent/agent_test.go +++ b/pkg/agent/agent_test.go @@ -91,10 +91,12 @@ func TestInitstore(t *testing.T) { ovsPort1 := ovsconfig.OVSPortData{UUID: uuid1, Name: "p1", IFName: "p1", OFPort: 11, ExternalIDs: convertExternalIDMap(cniserver.BuildOVSPortExternalIDs( - interfacestore.NewContainerInterface("p1", uuid1, "pod1", "ns1", p1NetMAC, []net.IP{p1NetIP}, 0)))} + interfacestore.NewContainerInterface("p1", uuid1, "pod1", "ns1", "eth0", p1NetMAC, []net.IP{p1NetIP}, 0)))} ovsPort2 := ovsconfig.OVSPortData{UUID: uuid2, Name: "p2", IFName: "p2", OFPort: 12, ExternalIDs: convertExternalIDMap(cniserver.BuildOVSPortExternalIDs( - interfacestore.NewContainerInterface("p2", uuid2, "pod2", "ns2", p2NetMAC, []net.IP{p2NetIP}, 0)))} + interfacestore.NewContainerInterface("p2", uuid2, "pod2", "ns2", "eth0", p2NetMAC, []net.IP{p2NetIP}, 0), + )), + } initOVSPorts := []ovsconfig.OVSPortData{ovsPort1, ovsPort2} mockOVSBridgeClient.EXPECT().GetPortList().Return(initOVSPorts, ovsconfig.NewTransactionError(fmt.Errorf("Failed to list OVS ports"), true)) diff --git a/pkg/agent/cniserver/pod_configuration.go b/pkg/agent/cniserver/pod_configuration.go index 0009021e0e7..351b13c19bd 100644 --- a/pkg/agent/cniserver/pod_configuration.go +++ b/pkg/agent/cniserver/pod_configuration.go @@ -51,11 +51,14 @@ const ( ovsExternalIDContainerID = "container-id" ovsExternalIDPodName = "pod-name" ovsExternalIDPodNamespace = "pod-namespace" + ovsExternalIDIFDev = "if-dev" ) const ( defaultOVSInterfaceType int = iota //nolint suppress deadcode check for windows internalOVSInterfaceType + + defaultIFDevName = "eth0" ) var ( @@ -72,6 +75,9 @@ type podConfigurator struct { // podUpdateNotifier is used for notifying updates of local Pods to other components which may benefit from this // information, i.e. NetworkPolicyController, EgressController. podUpdateNotifier channel.Notifier + // isSecondaryNetwork is true if this instance of podConfigurator is used to configure + // Pod secondary network interfaces. + isSecondaryNetwork bool } func newPodConfigurator( @@ -116,10 +122,8 @@ func buildContainerConfig( containerIface *current.Interface, ips []*current.IPConfig, vlanID uint16) *interfacestore.InterfaceConfig { - containerIPs, err := parseContainerIPs(ips) - if err != nil { - klog.Errorf("Failed to find container %s IP", containerID) - } + // A secondary interface can be created without IPs. Ignore the IP parsing error here. + containerIPs, _ := parseContainerIPs(ips) // containerIface.Mac should be a valid MAC string, otherwise it should throw error before containerMAC, _ := net.ParseMAC(containerIface.Mac) return interfacestore.NewContainerInterface( @@ -127,6 +131,7 @@ func buildContainerConfig( containerID, podName, podNamespace, + containerIface.Name, containerMAC, containerIPs, vlanID) @@ -141,6 +146,10 @@ func BuildOVSPortExternalIDs(containerConfig *interfacestore.InterfaceConfig) ma externalIDs[ovsExternalIDIP] = getContainerIPsString(containerConfig.IPs) externalIDs[ovsExternalIDPodName] = containerConfig.PodName externalIDs[ovsExternalIDPodNamespace] = containerConfig.PodNamespace + if containerConfig.IFDev != defaultIFDevName { + // Save interface name for a secondary interface. + externalIDs[ovsExternalIDIFDev] = containerConfig.IFDev + } externalIDs[interfacestore.AntreaInterfaceTypeKey] = interfacestore.AntreaContainer return externalIDs } @@ -168,10 +177,14 @@ func ParseOVSPortInterfaceConfig(portData *ovsconfig.OVSPortData, portConfig *in klog.V(2).Infof("OVS port %s has no %s in external_ids", portData.Name, ovsExternalIDContainerID) return nil } - containerIPStrs := strings.Split(portData.ExternalIDs[ovsExternalIDIP], ",") + var containerIPs []net.IP - for _, ipStr := range containerIPStrs { - containerIPs = append(containerIPs, net.ParseIP(ipStr)) + // A secondary interface may not have an IP assigned. + if portData.ExternalIDs[ovsExternalIDIP] != "" { + containerIPStrs := strings.Split(portData.ExternalIDs[ovsExternalIDIP], ",") + for _, ipStr := range containerIPStrs { + containerIPs = append(containerIPs, net.ParseIP(ipStr)) + } } containerMAC, err := net.ParseMAC(portData.ExternalIDs[ovsExternalIDMAC]) @@ -181,12 +194,14 @@ func ParseOVSPortInterfaceConfig(portData *ovsconfig.OVSPortData, portConfig *in } podName, _ := portData.ExternalIDs[ovsExternalIDPodName] podNamespace, _ := portData.ExternalIDs[ovsExternalIDPodNamespace] + ifDev, _ := portData.ExternalIDs[ovsExternalIDIFDev] interfaceConfig := interfacestore.NewContainerInterface( portData.Name, containerID, podName, podNamespace, + ifDev, containerMAC, containerIPs, portData.VLANID) @@ -226,8 +241,11 @@ func (pc *podConfigurator) configureInterfacesCommon( } }() - if err := pc.routeClient.AddLocalAntreaFlexibleIPAMPodRule(containerConfig.IPs); err != nil { - return err + // Not needed for a secondary network interface. + if !pc.isSecondaryNetwork { + if err := pc.routeClient.AddLocalAntreaFlexibleIPAMPodRule(containerConfig.IPs); err != nil { + return err + } } // Note that the IP address should be advertised after Pod OpenFlow entries are installed, otherwise the packet might @@ -236,9 +254,11 @@ func (pc *podConfigurator) configureInterfacesCommon( // Do not return an error and fail the interface creation. klog.ErrorS(err, "Failed to advertise IP address for container", "container", containerID) } + // Mark the manipulation as success to cancel deferred operations. success = true - klog.Infof("Configured interfaces for container %s", containerID) + klog.InfoS("Configured container interface", "Pod", klog.KRef(podNamespace, podName), + "container", containerID, "interface", containerIface.Name, "hostInterface", hostIface.Name) return nil } @@ -489,28 +509,36 @@ func (pc *podConfigurator) connectInterfaceToOVSCommon(ovsPortName, netNS string } }() - // GetOFPort will wait for up to 1 second for OVSDB to report the OFPort number. var ofPort int32 - ofPort, err = pc.ovsBridgeClient.GetOFPort(ovsPortName, false) - if err != nil { - return fmt.Errorf("failed to get of_port of OVS port %s: %v", ovsPortName, err) - } - klog.V(2).Infof("Setting up Openflow entries for container %s", containerID) - if err = pc.ofClient.InstallPodFlows(ovsPortName, containerConfig.IPs, containerConfig.MAC, uint32(ofPort), containerConfig.VLANID, nil); err != nil { - return fmt.Errorf("failed to add Openflow entries for container %s: %v", containerID, err) + // Not needed for a secondary network interface. + if !pc.isSecondaryNetwork { + // GetOFPort will wait for up to 1 second for OVSDB to report the OFPort number. + ofPort, err = pc.ovsBridgeClient.GetOFPort(ovsPortName, false) + if err != nil { + return fmt.Errorf("failed to get of_port of OVS port %s: %v", ovsPortName, err) + } + klog.V(2).InfoS("Setting up Openflow entries for Pod interface", "container", containerID, "port", ovsPortName) + if err = pc.ofClient.InstallPodFlows(ovsPortName, containerConfig.IPs, containerConfig.MAC, uint32(ofPort), containerConfig.VLANID, nil); err != nil { + return fmt.Errorf("failed to add Openflow entries for container %s: %v", containerID, err) + } } + containerConfig.OVSPortConfig = &interfacestore.OVSPortConfig{PortUUID: portUUID, OFPort: ofPort} // Add containerConfig into local cache pc.ifaceStore.AddInterface(containerConfig) - // Notify the Pod update event to required components. - event := agenttypes.PodUpdate{ - PodName: containerConfig.PodName, - PodNamespace: containerConfig.PodNamespace, - ContainerID: containerConfig.ContainerID, - NetNS: netNS, - IsAdd: true, - } - pc.podUpdateNotifier.Notify(event) + + // Not needed for a secondary network interface. + if !pc.isSecondaryNetwork { + // Notify the Pod update event to required components. + event := agenttypes.PodUpdate{ + PodName: containerConfig.PodName, + PodNamespace: containerConfig.PodNamespace, + ContainerID: containerConfig.ContainerID, + NetNS: netNS, + IsAdd: true, + } + pc.podUpdateNotifier.Notify(event) + } return nil } @@ -518,29 +546,32 @@ func (pc *podConfigurator) connectInterfaceToOVSCommon(ovsPortName, netNS string func (pc *podConfigurator) disconnectInterfaceFromOVS(containerConfig *interfacestore.InterfaceConfig) error { containerID := containerConfig.ContainerID klog.V(2).Infof("Deleting Openflow entries for container %s", containerID) - if err := pc.ofClient.UninstallPodFlows(containerConfig.InterfaceName); err != nil { - return fmt.Errorf("failed to delete Openflow entries for container %s: %v", containerID, err) - // We should not delete OVS port if Pod flows deletion fails, otherwise - // it is possible a new Pod will reuse the reclaimed ofport number, and - // the OVS flows added for the new Pod can conflict with the stale - // flows of the deleted Pod. + if !pc.isSecondaryNetwork { + if err := pc.ofClient.UninstallPodFlows(containerConfig.InterfaceName); err != nil { + return fmt.Errorf("failed to delete Openflow entries for container %s: %v", containerID, err) + // We should not delete OVS port if Pod flows deletion fails, otherwise + // it is possible a new Pod will reuse the reclaimed ofport number, and + // the OVS flows added for the new Pod can conflict with the stale + // flows of the deleted Pod. + } } - klog.V(2).Infof("Deleting OVS port %s for container %s", containerConfig.PortUUID, containerID) // TODO: handle error and introduce garbage collection for failure on deletion if err := pc.ovsBridgeClient.DeletePort(containerConfig.PortUUID); err != nil { - return fmt.Errorf("failed to delete OVS port for container %s: %v", containerID, err) + return fmt.Errorf("failed to delete OVS port for container %s interface %s: %v", containerID, containerConfig.InterfaceName, err) } // Remove container configuration from cache. pc.ifaceStore.DeleteInterface(containerConfig) - event := agenttypes.PodUpdate{ - PodName: containerConfig.PodName, - PodNamespace: containerConfig.PodNamespace, - ContainerID: containerConfig.ContainerID, - IsAdd: false, - } - pc.podUpdateNotifier.Notify(event) - klog.Infof("Removed interfaces for container %s", containerID) + if !pc.isSecondaryNetwork { + event := agenttypes.PodUpdate{ + PodName: containerConfig.PodName, + PodNamespace: containerConfig.PodNamespace, + ContainerID: containerConfig.ContainerID, + IsAdd: false, + } + pc.podUpdateNotifier.Notify(event) + } + klog.InfoS("Deleted container OVS port", "container", containerID, "interface", containerConfig.InterfaceName) return nil } diff --git a/pkg/agent/cniserver/pod_configuration_linux.go b/pkg/agent/cniserver/pod_configuration_linux.go index c692f541731..75dd9fd6d5e 100644 --- a/pkg/agent/cniserver/pod_configuration_linux.go +++ b/pkg/agent/cniserver/pod_configuration_linux.go @@ -25,18 +25,13 @@ import ( "antrea.io/antrea/pkg/agent/interfacestore" ) -// connectInterfaceToOVS connects an existing interface to ovs br-int. +// connectInterfaceToOVS connects an existing interface to the OVS bridge. func (pc *podConfigurator) connectInterfaceToOVS( - podName string, - podNamespace string, - containerID string, - netNS string, - hostIface *current.Interface, - containerIface *current.Interface, + podName, podNamespace, containerID, netNS string, + hostIface, containerIface *current.Interface, ips []*current.IPConfig, vlanID uint16, - containerAccess *containerAccessArbitrator, -) (*interfacestore.InterfaceConfig, error) { + containerAccess *containerAccessArbitrator) (*interfacestore.InterfaceConfig, error) { // Use the outer veth interface name as the OVS port name. ovsPortName := hostIface.Name containerConfig := buildContainerConfig(ovsPortName, containerID, podName, podNamespace, containerIface, ips, vlanID) diff --git a/pkg/agent/cniserver/pod_configuration_linux_test.go b/pkg/agent/cniserver/pod_configuration_linux_test.go index 67b5440088a..cc78e068c09 100644 --- a/pkg/agent/cniserver/pod_configuration_linux_test.go +++ b/pkg/agent/cniserver/pod_configuration_linux_test.go @@ -25,6 +25,8 @@ import ( "github.com/stretchr/testify/require" "go.uber.org/mock/gomock" + "antrea.io/antrea/pkg/agent/cniserver/ipam" + ipamtest "antrea.io/antrea/pkg/agent/cniserver/ipam/testing" "antrea.io/antrea/pkg/agent/interfacestore" openflowtest "antrea.io/antrea/pkg/agent/openflow/testing" routetest "antrea.io/antrea/pkg/agent/route/testing" @@ -387,6 +389,73 @@ func TestParseOVSPortInterfaceConfig(t *testing.T) { }, }, }, + { + name: "secondary", + portData: &ovsconfig.OVSPortData{ + Name: portName, + ExternalIDs: map[string]string{ + ovsExternalIDContainerID: containerID, + ovsExternalIDIP: containerIPs, + ovsExternalIDMAC: containerMACStr, + ovsExternalIDPodName: podName, + ovsExternalIDPodNamespace: testPodNamespace, + ovsExternalIDIFDev: "eth1", + }, + }, + portConfig: &interfacestore.OVSPortConfig{ + PortUUID: portUUID, + OFPort: ofPort, + }, + ifaceConfig: &interfacestore.InterfaceConfig{ + Type: interfacestore.ContainerInterface, + InterfaceName: portName, + IPs: parsedIPs, + MAC: containerMAC, + OVSPortConfig: &interfacestore.OVSPortConfig{ + PortUUID: portUUID, + OFPort: ofPort, + }, + ContainerInterfaceConfig: &interfacestore.ContainerInterfaceConfig{ + ContainerID: containerID, + PodName: podName, + PodNamespace: testPodNamespace, + IFDev: "eth1", + }, + }, + }, + { + name: "secondary-no-ip", + portData: &ovsconfig.OVSPortData{ + Name: portName, + ExternalIDs: map[string]string{ + ovsExternalIDContainerID: containerID, + ovsExternalIDIP: "", + ovsExternalIDMAC: containerMACStr, + ovsExternalIDPodName: podName, + ovsExternalIDPodNamespace: testPodNamespace, + ovsExternalIDIFDev: "eth1", + }, + }, + portConfig: &interfacestore.OVSPortConfig{ + PortUUID: portUUID, + OFPort: ofPort, + }, + ifaceConfig: &interfacestore.InterfaceConfig{ + Type: interfacestore.ContainerInterface, + InterfaceName: portName, + MAC: containerMAC, + OVSPortConfig: &interfacestore.OVSPortConfig{ + PortUUID: portUUID, + OFPort: ofPort, + }, + ContainerInterfaceConfig: &interfacestore.ContainerInterfaceConfig{ + ContainerID: containerID, + PodName: podName, + PodNamespace: testPodNamespace, + IFDev: "eth1", + }, + }, + }, } { t.Run(tc.name, func(t *testing.T) { iface := ParseOVSPortInterfaceConfig(tc.portData, tc.portConfig) @@ -403,7 +472,8 @@ func TestCheckHostInterface(t *testing.T) { interfaces := []*current.Interface{containerIntf, {Name: hostIfaceName}} containeIPs := ipamResult.IPs ifaceMAC, _ := net.ParseMAC("01:02:03:04:05:06") - containerInterface := interfacestore.NewContainerInterface(hostIfaceName, containerID, "pod1", testPodNamespace, ifaceMAC, []net.IP{containerIP}, 1) + containerInterface := interfacestore.NewContainerInterface(hostIfaceName, containerID, + "pod1", testPodNamespace, "eth0", ifaceMAC, []net.IP{containerIP}, 1) containerInterface.OVSPortConfig = &interfacestore.OVSPortConfig{ PortUUID: generateUUID(), OFPort: int32(10), @@ -453,7 +523,6 @@ func TestCheckHostInterface(t *testing.T) { } func TestConfigureSriovSecondaryInterface(t *testing.T) { - controller := gomock.NewController(t) containerID := generateUUID() containerNS := "containerNS" @@ -463,6 +532,7 @@ func TestConfigureSriovSecondaryInterface(t *testing.T) { configureLinkErr error advertiseErr error expectedErr error + intfConfig *interfacestore.InterfaceConfig }{ { name: "sriov-vf-not-set", @@ -478,23 +548,142 @@ func TestConfigureSriovSecondaryInterface(t *testing.T) { advertiseErr: fmt.Errorf("unable to advertise on the sriov link"), // When advertiseContainerAddr returns an error, it is logged, but does not // cause ConfigureSriovSecondaryInterface to also return an error. - + intfConfig: &interfacestore.InterfaceConfig{ + Type: interfacestore.ContainerInterface, + InterfaceName: "vf2", + ContainerInterfaceConfig: &interfacestore.ContainerInterfaceConfig{ + ContainerID: containerID, + PodName: podName, + PodNamespace: testPodNamespace, + IFDev: "eth0", + }, + }, }, { name: "success", podSriovVFDeviceID: "vf3", + intfConfig: &interfacestore.InterfaceConfig{ + Type: interfacestore.ContainerInterface, + InterfaceName: "vf3", + ContainerInterfaceConfig: &interfacestore.ContainerInterfaceConfig{ + ContainerID: containerID, + PodName: podName, + PodNamespace: testPodNamespace, + IFDev: "eth0", + }, + }, }, } { t.Run(tc.name, func(t *testing.T) { ifaceConfigurator := newTestInterfaceConfigurator() ifaceConfigurator.configureContainerLinkError = tc.configureLinkErr ifaceConfigurator.advertiseContainerAddrError = tc.advertiseErr - podConfigurator := createPodConfigurator(controller, ifaceConfigurator) + podConfigurator, _ := NewSecondaryInterfaceConfigurator(nil, interfacestore.NewInterfaceStore()) + podConfigurator.ifConfigurator = ifaceConfigurator err := podConfigurator.ConfigureSriovSecondaryInterface(podName, testPodNamespace, containerID, containerNS, containerIfaceName, mtu, tc.podSriovVFDeviceID, ¤t.Result{}) assert.Equal(t, tc.expectedErr, err) + if tc.expectedErr != nil { + assert.Equal(t, 0, podConfigurator.ifaceStore.Len()) + return + } + + assert.Equal(t, 1, podConfigurator.ifaceStore.Len()) + intfConfig, _ := podConfigurator.ifaceStore.GetContainerInterface(containerID) + assert.Equal(t, tc.intfConfig, intfConfig) + assert.NoError(t, podConfigurator.DeleteSriovSecondaryInterface(intfConfig)) + assert.Equal(t, 0, podConfigurator.ifaceStore.Len()) }) } } +func newTestContainerInterfaceConfig(podName, containerID, ifDev string, vlan int) *interfacestore.InterfaceConfig { + hostIfaceName := util.GenerateContainerHostVethName(podName, testPodNamespace, containerID, ifDev) + fakePortUUID := generateUUID() + podMAC, _ := net.ParseMAC("01:02:03:04:05:06") + podIP := net.ParseIP("1.1.1.11") + podIPv6 := net.ParseIP("3ffe:ffff:10:1ff::111") + containerConfig := interfacestore.NewContainerInterface( + hostIfaceName, containerID, podName, testPodNamespace, + ifDev, podMAC, []net.IP{podIP, podIPv6}, uint16(vlan)) + containerConfig.OVSPortConfig = &interfacestore.OVSPortConfig{PortUUID: fakePortUUID, OFPort: 0} + return containerConfig +} + +func TestConfigureVLANSecondaryInterface(t *testing.T) { + controller := gomock.NewController(t) + mockOVSBridgeClient := ovsconfigtest.NewMockOVSBridgeClient(controller) + ifaceStore := interfacestore.NewInterfaceStore() + pc, err := NewSecondaryInterfaceConfigurator(mockOVSBridgeClient, ifaceStore) + require.NoError(t, err, "No error expected in podConfigurator constructor") + testIfaceConfigurator := newTestInterfaceConfigurator() + pc.ifConfigurator = testIfaceConfigurator + + containerNS := "containerNS" + podName := "pod1" + containerID := generateUUID() + containerCfg1 := newTestContainerInterfaceConfig(podName, containerID, "eth1", 100) + containerCfg2 := newTestContainerInterfaceConfig(podName, containerID, "eth2", 0) + // Secondary interface with no IP. + containerCfg2.IPs = nil + + result := ipamtest.GenerateIPAMResult([]string{"1.1.1.11/24,1.1.1.1,4", "3ffe:ffff:10:1ff::111/64,3ffe:ffff:10::1,6"}, routes, dns) + ipamResult := &ipam.IPAMResult{Result: *result, VLANID: 100} + testIfaceConfigurator.hostIfaceName = containerCfg1.InterfaceName + testIfaceConfigurator.containerMAC = containerCfg1.MAC.String() + + mockOVSBridgeClient.EXPECT().CreateAccessPort( + containerCfg1.InterfaceName, containerCfg1.InterfaceName, + gomock.Any(), uint16(100)).Return(containerCfg1.PortUUID, nil).Times(1) + assert.NoError(t, pc.ConfigureVLANSecondaryInterface(podName, testPodNamespace, containerID, containerNS, "eth1", 1500, ipamResult)) + assert.Equal(t, 1, ifaceStore.Len()) + intfConfig, _ := ifaceStore.GetContainerInterface(containerID) + assert.Equal(t, containerCfg1, intfConfig) + + testIfaceConfigurator.hostIfaceName = containerCfg2.InterfaceName + testIfaceConfigurator.containerMAC = containerCfg2.MAC.String() + ipamResult = &ipam.IPAMResult{} + mockOVSBridgeClient.EXPECT().CreatePort( + containerCfg2.InterfaceName, containerCfg2.InterfaceName, + gomock.Any()).Return(containerCfg2.PortUUID, nil).Times(1) + assert.NoError(t, pc.ConfigureVLANSecondaryInterface(podName, testPodNamespace, containerID, containerNS, "eth2", 1600, ipamResult)) + assert.Equal(t, 2, ifaceStore.Len()) + intfConfigs := ifaceStore.GetContainerInterfacesByPod(podName, testPodNamespace) + assert.Len(t, intfConfigs, 2) + for _, intfConfig := range intfConfigs { + if intfConfig.InterfaceName == containerCfg1.InterfaceName { + assert.Equal(t, containerCfg1, intfConfig) + } else { + assert.Equal(t, containerCfg2, intfConfig) + } + } +} + +func TestDeleteVLANSecondaryInterface(t *testing.T) { + controller := gomock.NewController(t) + mockOVSBridgeClient := ovsconfigtest.NewMockOVSBridgeClient(controller) + ifaceStore := interfacestore.NewInterfaceStore() + pc, err := NewSecondaryInterfaceConfigurator(mockOVSBridgeClient, ifaceStore) + require.Nil(t, err, "No error expected in podConfigurator constructor") + + podName := "pod1" + containerID := generateUUID() + containerCfg1 := newTestContainerInterfaceConfig(podName, containerID, "eth1", 100) + containerCfg2 := newTestContainerInterfaceConfig(podName, containerID, "eth2", 0) + // Secondary interface with no IP. + containerCfg2.IPs = nil + ifaceStore.AddInterface(containerCfg1) + ifaceStore.AddInterface(containerCfg2) + + mockOVSBridgeClient.EXPECT().DeletePort(containerCfg1.PortUUID).Return(nil) + assert.NoError(t, pc.DeleteVLANSecondaryInterface(containerCfg1)) + intfConfig, _ := ifaceStore.GetContainerInterface(containerID) + assert.Equal(t, containerCfg2, intfConfig) + + mockOVSBridgeClient.EXPECT().DeletePort(containerCfg2.PortUUID).Return(nil) + assert.NoError(t, pc.DeleteVLANSecondaryInterface(containerCfg2)) + _, found := ifaceStore.GetContainerInterface(containerID) + assert.False(t, found, "Interface should not be in the local cache anymore") +} + func createPodConfigurator(controller *gomock.Controller, testIfaceConfigurator *fakeInterfaceConfigurator) *podConfigurator { gwMAC, _ := net.ParseMAC("00:00:11:11:11:11") mockOVSBridgeClient = ovsconfigtest.NewMockOVSBridgeClient(controller) diff --git a/pkg/agent/cniserver/pod_configuration_windows.go b/pkg/agent/cniserver/pod_configuration_windows.go index 6c574ad0306..8842a6bf416 100644 --- a/pkg/agent/cniserver/pod_configuration_windows.go +++ b/pkg/agent/cniserver/pod_configuration_windows.go @@ -62,18 +62,13 @@ func (pc *podConfigurator) connectInterfaceToOVSAsync(ifConfig *interfacestore.I }) } -// connectInterfaceToOVS connects an existing interface to OVS br-int. +// connectInterfaceToOVS connects an existing interface to the OVS bridge. func (pc *podConfigurator) connectInterfaceToOVS( - podName string, - podNamespace string, - containerID string, - netNS string, - hostIface *current.Interface, - containerIface *current.Interface, + podName, podNamespace, containerID, netNS string, + hostIface, containerIface *current.Interface, ips []*current.IPConfig, vlanID uint16, - containerAccess *containerAccessArbitrator, -) (*interfacestore.InterfaceConfig, error) { + containerAccess *containerAccessArbitrator) (*interfacestore.InterfaceConfig, error) { // Use the outer veth interface name as the OVS port name. ovsPortName := hostIface.Name containerConfig := buildContainerConfig(ovsPortName, containerID, podName, podNamespace, containerIface, ips, vlanID) diff --git a/pkg/agent/cniserver/secondary.go b/pkg/agent/cniserver/secondary.go index bfe94751e99..a4176baba15 100644 --- a/pkg/agent/cniserver/secondary.go +++ b/pkg/agent/cniserver/secondary.go @@ -20,11 +20,17 @@ import ( current "github.com/containernetworking/cni/pkg/types/100" "k8s.io/klog/v2" + "antrea.io/antrea/pkg/agent/cniserver/ipam" + "antrea.io/antrea/pkg/agent/interfacestore" "antrea.io/antrea/pkg/ovs/ovsconfig" ) -func NewSecondaryInterfaceConfigurator(ovsBridgeClient ovsconfig.OVSBridgeClient) (*podConfigurator, error) { - return newPodConfigurator(ovsBridgeClient, nil, nil, nil, nil, ovsconfig.OVSDatapathSystem, false, false, nil) +func NewSecondaryInterfaceConfigurator(ovsBridgeClient ovsconfig.OVSBridgeClient, interfaceStore interfacestore.InterfaceStore) (*podConfigurator, error) { + pc, err := newPodConfigurator(ovsBridgeClient, nil, nil, interfaceStore, nil, ovsconfig.OVSDatapathSystem, false, false, nil) + if err == nil { + pc.isSecondaryNetwork = true + } + return pc, err } // ConfigureSriovSecondaryInterface configures a SR-IOV secondary interface for a Pod. @@ -42,9 +48,13 @@ func (pc *podConfigurator) ConfigureSriovSecondaryInterface( if err != nil { return err } - hostIface := result.Interfaces[0] containerIface := result.Interfaces[1] - klog.InfoS("Configured SR-IOV interface", "Pod", klog.KRef(podNamespace, podName), "interface", containerInterfaceName, "hostInterface", hostIface) + klog.InfoS("Configured SR-IOV interface", "Pod", klog.KRef(podNamespace, podName), "interface", containerInterfaceName) + + // Use podSriovVFDeviceID as the interface name in the interface store. + hostInterfaceName := podSriovVFDeviceID + containerConfig := buildContainerConfig(hostInterfaceName, containerID, podName, podNamespace, containerIface, result.IPs, 0) + pc.ifaceStore.AddInterface(containerConfig) if result.IPs != nil { if err = pc.ifConfigurator.advertiseContainerAddr(containerNetNS, containerIface.Name, result); err != nil { @@ -55,55 +65,35 @@ func (pc *podConfigurator) ConfigureSriovSecondaryInterface( return nil } +// DeleteSriovSecondaryInterface deletes a SRIOV secondary interface. +func (pc *podConfigurator) DeleteSriovSecondaryInterface(interfaceConfig *interfacestore.InterfaceConfig) error { + pc.ifaceStore.DeleteInterface(interfaceConfig) + klog.InfoS("Deleted SR-IOV interface", "Pod", klog.KRef(interfaceConfig.PodNamespace, interfaceConfig.PodName), + "interface", interfaceConfig.IFDev) + return nil + +} + // ConfigureVLANSecondaryInterface configures a VLAN secondary interface on the secondary network // OVS bridge, and returns the OVS port UUID. func (pc *podConfigurator) ConfigureVLANSecondaryInterface( podName, podNamespace string, containerID, containerNetNS, containerInterfaceName string, - mtu int, vlanID uint16, - result *current.Result) (string, error) { - // TODO: revisit the possibility of reusing configureInterfaces(), connectInterfaceToOVS() - // removeInterfaces() code, and using InterfaceStore to store secondary interface info. - if err := pc.ifConfigurator.configureContainerLink(podName, podNamespace, containerID, containerNetNS, containerInterfaceName, mtu, "", "", result, nil); err != nil { - return "", err - } - hostIface := result.Interfaces[0] - containerIface := result.Interfaces[1] - - success := false - defer func() { - if !success { - if err := pc.ifConfigurator.removeContainerLink(containerID, hostIface.Name); err != nil { - klog.ErrorS(err, "failed to roll back veth creation", "container", containerID, "interface", containerInterfaceName) - } - } - }() - - // Use the outer veth interface name as the OVS port name. - ovsPortName := hostIface.Name - ovsPortUUID, err := pc.createOVSPort(ovsPortName, nil, vlanID) - if err != nil { - return "", fmt.Errorf("failed to add OVS port for container %s: %v", containerID, err) - } - klog.InfoS("Configured VLAN interface", "Pod", klog.KRef(podNamespace, podName), "interface", containerInterfaceName, "hostInterface", hostIface) - - if result.IPs != nil { - if err := pc.ifConfigurator.advertiseContainerAddr(containerNetNS, containerIface.Name, result); err != nil { - klog.ErrorS(err, "Failed to advertise IP address for VLAN interface", - "container", containerID, "interface", containerInterfaceName) - } - } - success = true - return ovsPortUUID, nil + mtu int, ipamResult *ipam.IPAMResult) error { + return pc.configureInterfacesCommon(podName, podNamespace, containerID, containerNetNS, + containerInterfaceName, mtu, "", ipamResult, nil) } // DeleteVLANSecondaryInterface deletes a VLAN secondary interface. -func (pc *podConfigurator) DeleteVLANSecondaryInterface(containerID, hostInterfaceName, ovsPortUUID string) error { - if err := pc.ovsBridgeClient.DeletePort(ovsPortUUID); err != nil { - return fmt.Errorf("failed to delete OVS port for container %s: %v", containerID, err) - } - if err := pc.ifConfigurator.removeContainerLink(containerID, hostInterfaceName); err != nil { +func (pc *podConfigurator) DeleteVLANSecondaryInterface(interfaceConfig *interfacestore.InterfaceConfig) error { + if err := pc.disconnectInterfaceFromOVS(interfaceConfig); err != nil { return err } + if err := pc.ifConfigurator.removeContainerLink(interfaceConfig.ContainerID, interfaceConfig.InterfaceName); err != nil { + klog.ErrorS(err, "Failed to delete container interface link", + "Pod", klog.KRef(interfaceConfig.PodNamespace, interfaceConfig.PodName), + "interface", interfaceConfig.IFDev) + // No retry for interface deletion. + } return nil } diff --git a/pkg/agent/cniserver/server_linux_test.go b/pkg/agent/cniserver/server_linux_test.go index 52791d129b8..856024dd32c 100644 --- a/pkg/agent/cniserver/server_linux_test.go +++ b/pkg/agent/cniserver/server_linux_test.go @@ -141,6 +141,7 @@ func TestRemoveInterface(t *testing.T) { containerID, podName, testPodNamespace, + "eth0", containerMAC, []net.IP{containerIP}, 0) @@ -435,7 +436,7 @@ func TestCmdDel(t *testing.T) { response: &cnipb.CniCmdResponse{ Error: &cnipb.Error{ Code: cnipb.ErrorCode_CONFIG_INTERFACE_FAILURE, - Message: fmt.Sprintf("failed to delete OVS port for container %s: failed to delete port", testPodInfraContainerID), + Message: fmt.Sprintf("failed to delete OVS port for container %s interface A-1-b0d460: failed to delete port", testPodInfraContainerID), }, }, }, @@ -473,7 +474,9 @@ func TestCmdDel(t *testing.T) { cniserver := newMockCNIServer(t, controller, ipamMock, tc.ipamType, tc.enableSecondaryNetworkIPAM, tc.isChaining) requestMsg, hostInterfaceName := createCNIRequestAndInterfaceName(t, testPodNameA, tc.cniType, ipamResult, tc.ipamType, true) containerID := requestMsg.CniArgs.ContainerId - containerIfaceConfig := interfacestore.NewContainerInterface(hostInterfaceName, containerID, testPodNameA, testPodNamespace, containerVethMac, []net.IP{net.ParseIP("10.1.2.100")}, 0) + containerIfaceConfig := interfacestore.NewContainerInterface(hostInterfaceName, containerID, + testPodNameA, testPodNamespace, "eth0", + containerVethMac, []net.IP{net.ParseIP("10.1.2.100")}, 0) containerIfaceConfig.OVSPortConfig = &interfacestore.OVSPortConfig{PortUUID: ovsPortID, OFPort: ovsPort} ifaceStore.AddInterface(containerIfaceConfig) testIfaceConfigurator := newTestInterfaceConfigurator() @@ -536,7 +539,9 @@ func TestCmdCheck(t *testing.T) { } podArgs := cniservertest.GenerateCNIArgs(name, testPodNamespace, testPodInfraContainerID) requestMsg, containerID := newRequest(podArgs, networkCfg, "", t) - containerIfaceConfig := interfacestore.NewContainerInterface(hostInterfaceName, containerID, name, testPodNamespace, containerVethMac, []net.IP{net.ParseIP("10.1.2.100")}, 0) + containerIfaceConfig := interfacestore.NewContainerInterface(hostInterfaceName, containerID, + name, testPodNamespace, "eth0", + containerVethMac, []net.IP{net.ParseIP("10.1.2.100")}, 0) containerIfaceConfig.OVSPortConfig = &interfacestore.OVSPortConfig{PortUUID: ovsPortID, OFPort: ovsPort} ifaceStore.AddInterface(containerIfaceConfig) return requestMsg, hostInterfaceName diff --git a/pkg/agent/cniserver/server_test.go b/pkg/agent/cniserver/server_test.go index 97acdef7028..b1f0f4b6699 100644 --- a/pkg/agent/cniserver/server_test.go +++ b/pkg/agent/cniserver/server_test.go @@ -676,46 +676,65 @@ func TestBuildOVSPortExternalIDs(t *testing.T) { containerIP1 := net.ParseIP("10.1.2.100") containerIP2 := net.ParseIP("2001:fd1a::2") containerIPs := []net.IP{containerIP1, containerIP2} - containerConfig := interfacestore.NewContainerInterface("pod1-abcd", containerID, "test-1", "t1", containerMAC, containerIPs, 0) - externalIds := BuildOVSPortExternalIDs(containerConfig) - parsedIP, existed := externalIds[ovsExternalIDIP] + containerConfig := interfacestore.NewContainerInterface("pod1-abcd", containerID, + "test-1", "t1", "eth0", containerMAC, containerIPs, 0) + externalIDs := BuildOVSPortExternalIDs(containerConfig) + _, existed := externalIDs[ovsExternalIDIFDev] + assert.False(t, existed, "External IDs should not include interface name eth0") + parsedIP, existed := externalIDs[ovsExternalIDIP] parsedIPStr := parsedIP.(string) if !existed || !strings.Contains(parsedIPStr, "10.1.2.100") || !strings.Contains(parsedIPStr, "2001:fd1a::2") { - t.Errorf("Failed to parse container configuration") + t.Errorf("Failed to store IPs to external IDs") } - parsedMac, existed := externalIds[ovsExternalIDMAC] + parsedMac, existed := externalIDs[ovsExternalIDMAC] if !existed || parsedMac != containerMAC.String() { - t.Errorf("Failed to parse container configuration") + t.Errorf("Failed to store MAC to external IDs") } - parsedID, existed := externalIds[ovsExternalIDContainerID] + parsedID, existed := externalIDs[ovsExternalIDContainerID] if !existed || parsedID != containerID { - t.Errorf("Failed to parse container configuration") + t.Errorf("Failed to store container ID to external IDs") } - portExternalIDs := make(map[string]string) - for k, v := range externalIds { - val := v.(string) - portExternalIDs[k] = val - } - mockPort := &ovsconfig.OVSPortData{ - Name: "testPort", - ExternalIDs: portExternalIDs, - } - portConfig := &interfacestore.OVSPortConfig{ - PortUUID: "12345678", - OFPort: int32(1), - } - ifaceConfig := ParseOVSPortInterfaceConfig(mockPort, portConfig) - assert.Equal(t, len(containerIPs), len(ifaceConfig.IPs)) - for _, ip1 := range containerIPs { - existed := false - for _, ip2 := range ifaceConfig.IPs { - if ip2.Equal(ip1) { - existed = true - break + + testConfigParsingFn := func() { + portExternalIDs := make(map[string]string) + for k, v := range externalIDs { + val := v.(string) + portExternalIDs[k] = val + } + mockPort := &ovsconfig.OVSPortData{ + Name: "testPort", + ExternalIDs: portExternalIDs, + } + portConfig := &interfacestore.OVSPortConfig{ + PortUUID: "12345678", + OFPort: int32(1), + } + ifaceConfig := ParseOVSPortInterfaceConfig(mockPort, portConfig) + assert.Equal(t, len(containerIPs), len(ifaceConfig.IPs)) + for _, ip1 := range containerIPs { + existed := false + for _, ip2 := range ifaceConfig.IPs { + if ip2.Equal(ip1) { + existed = true + break + } } + assert.Truef(t, existed, "IP %s should exist in the restored InterfaceConfig", ip1.String()) } - assert.True(t, existed, fmt.Sprintf("IP %s should exist in the restored InterfaceConfig", ip1.String())) } + testConfigParsingFn() + + // Secondary interface with no IP. + containerIPs = nil + containerConfig = interfacestore.NewContainerInterface("pod1-abcd", containerID, + "test-1", "t1", "eth1", containerMAC, containerIPs, 0) + externalIDs = BuildOVSPortExternalIDs(containerConfig) + parsedIFDev, existed := externalIDs[ovsExternalIDIFDev] + assert.True(t, existed && parsedIFDev == "eth1") + parsedIP, existed = externalIDs[ovsExternalIDIP] + assert.True(t, existed && parsedIP.(string) == "") + testConfigParsingFn() + } func translateRawPrevResult(prevResult *current.Result, cniVersion string) (map[string]interface{}, error) { diff --git a/pkg/agent/cniserver/server_windows_test.go b/pkg/agent/cniserver/server_windows_test.go index 600adb5bf8e..ba562bae762 100644 --- a/pkg/agent/cniserver/server_windows_test.go +++ b/pkg/agent/cniserver/server_windows_test.go @@ -624,7 +624,7 @@ func TestCmdDel(t *testing.T) { server.podConfigurator.ifConfigurator.(*ifConfigurator).addEndpoint(hnsEndpoint) } if tc.ifaceExists { - containerIface := interfacestore.NewContainerInterface(ovsPortName, containerID, testPodNameA, testPodNamespace, containerMAC, []net.IP{net.ParseIP("10.1.2.100")}, 0) + containerIface := interfacestore.NewContainerInterface(ovsPortName, containerID, testPodNameA, testPodNamespace, "", containerMAC, []net.IP{net.ParseIP("10.1.2.100")}, 0) containerIface.OVSPortConfig = &interfacestore.OVSPortConfig{ OFPort: 100, PortUUID: ovsPortID, @@ -687,7 +687,7 @@ func TestCmdCheck(t *testing.T) { return &result } wrapperContainerInterface := func(ifaceName, containerID, podName, ovsPortID string, mac net.HardwareAddr, containerIP net.IP) *interfacestore.InterfaceConfig { - containerIface := interfacestore.NewContainerInterface(ifaceName, containerID, podName, testPodNamespace, mac, []net.IP{containerIP}, 0) + containerIface := interfacestore.NewContainerInterface(ifaceName, containerID, podName, testPodNamespace, "", mac, []net.IP{containerIP}, 0) containerIface.OVSPortConfig = &interfacestore.OVSPortConfig{ PortUUID: ovsPortID, OFPort: 10, diff --git a/pkg/agent/interfacestore/interface_cache.go b/pkg/agent/interfacestore/interface_cache.go index 4eb4c49bf0e..bba8cf2068a 100644 --- a/pkg/agent/interfacestore/interface_cache.go +++ b/pkg/agent/interfacestore/interface_cache.go @@ -85,7 +85,7 @@ func getInterfaceKey(obj interface{}) (string, error) { interfaceConfig := obj.(*InterfaceConfig) var key string if interfaceConfig.Type == ContainerInterface { - key = util.GenerateContainerInterfaceKey(interfaceConfig.ContainerID) + key = util.GenerateContainerInterfaceKey(interfaceConfig.ContainerID, interfaceConfig.IFDev) } else if interfaceConfig.Type == IPSecTunnelInterface { // IPsec tunnel interface for a Node. key = util.GenerateNodeTunnelInterfaceKey(interfaceConfig.NodeName) @@ -266,7 +266,8 @@ func interfaceIPIndexFunc(obj interface{}) ([]string, error) { func interfaceOFPortIndexFunc(obj interface{}) ([]string, error) { interfaceConfig := obj.(*InterfaceConfig) - if interfaceConfig.OFPort < 0 { + // OVSPortConfig can be nil for a secondary SR-IOV interface. + if interfaceConfig.OVSPortConfig == nil || interfaceConfig.OFPort < 0 { // If interfaceConfig OFport is not valid, we return empty key. return []string{}, nil } diff --git a/pkg/agent/interfacestore/interface_cache_test.go b/pkg/agent/interfacestore/interface_cache_test.go index c338fb2fe09..bfbf2cacca6 100644 --- a/pkg/agent/interfacestore/interface_cache_test.go +++ b/pkg/agent/interfacestore/interface_cache_test.go @@ -20,6 +20,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "antrea.io/antrea/pkg/agent/util" "antrea.io/antrea/pkg/ovs/ovsconfig" @@ -28,6 +29,7 @@ import ( var ( podMAC, _ = net.ParseMAC("11:22:33:44:55:66") podIP = net.ParseIP("1.2.3.4") + podIPv6 = net.ParseIP("2001:db8::1") gwIP = net.ParseIP("1.2.3.1") hostIP = net.ParseIP("2.2.2.2") ipsecTunnelIP = net.ParseIP("2.2.2.3") @@ -37,6 +39,7 @@ var ( func TestNewInterfaceStore(t *testing.T) { t.Run("testContainerInterface", testContainerInterface) + t.Run("testSecondaryInterface", testSecondaryInterface) t.Run("testGatewayInterface", testGatewayInterface) t.Run("testTunnelInterface", testTunnelInterface) t.Run("testUplinkInterface", testUplinkInterface) @@ -45,17 +48,17 @@ func TestNewInterfaceStore(t *testing.T) { func testContainerInterface(t *testing.T) { store := NewInterfaceStore() - containerInterface := NewContainerInterface("ns0p0c0", "c0", "p0", "ns0", podMAC, []net.IP{podIP}, 2) + containerInterface := NewContainerInterface("ns0p0c0", "c0", "p0", "ns0", "eth0", podMAC, []net.IP{podIP, podIPv6}, 2) containerInterface.OVSPortConfig = &OVSPortConfig{ OFPort: 12, PortUUID: "1234567890", } - containerInterfaceKey := util.GenerateContainerInterfaceKey(containerInterface.ContainerID) + containerInterfaceKey := util.GenerateContainerInterfaceKey(containerInterface.ContainerID, containerInterface.IFDev) store.Initialize([]*InterfaceConfig{containerInterface}) assert.Equal(t, 1, store.Len()) storedIface, exists := store.GetInterface(containerInterfaceKey) assert.True(t, exists) - assert.True(t, reflect.DeepEqual(storedIface, containerInterface)) + assert.Equal(t, containerInterface, storedIface) // The name of Container InterfaceConfig is not the key in InterfaceStore _, exists = store.GetInterface(containerInterface.InterfaceName) assert.False(t, exists) @@ -65,21 +68,27 @@ func testContainerInterface(t *testing.T) { assert.True(t, exists) _, exists = store.GetInterfaceByIP(podIP.String()) assert.True(t, exists) + _, exists = store.GetInterfaceByIP(podIPv6.String()) + assert.True(t, exists) _, exists = store.GetInterfaceByOFPort(uint32(containerInterface.OVSPortConfig.OFPort)) assert.True(t, exists) ifaces := store.GetContainerInterfacesByPod(containerInterface.PodName, containerInterface.PodNamespace) assert.Equal(t, 1, len(ifaces)) - assert.True(t, reflect.DeepEqual(ifaces[0], containerInterface)) + assert.Equal(t, containerInterface, ifaces[0]) ifaceNames := store.GetInterfaceKeysByType(ContainerInterface) assert.Equal(t, 1, len(ifaceNames)) assert.Equal(t, containerInterfaceKey, ifaceNames[0]) assert.Equal(t, 1, store.GetContainerInterfaceNum()) + store.DeleteInterface(containerInterface) assert.Equal(t, 0, store.GetContainerInterfaceNum()) _, exists = store.GetContainerInterface(containerInterface.ContainerID) assert.False(t, exists) _, exists = store.GetInterfaceByIP(containerInterface.IPs[0].String()) assert.False(t, exists) + _, exists = store.GetInterfaceByIP(containerInterface.IPs[1].String()) + assert.False(t, exists) + containerInterface.IPs = nil store.AddInterface(containerInterface) assert.Equal(t, 1, store.GetContainerInterfaceNum()) @@ -87,6 +96,47 @@ func testContainerInterface(t *testing.T) { assert.False(t, exists) } +func testSecondaryInterface(t *testing.T) { + store := NewInterfaceStore() + // Seondary interface without an IP. + containerInterface1 := NewContainerInterface("c0-eth1", "c0", "p0", "ns0", "eth1", podMAC, nil, 2) + containerInterface2 := NewContainerInterface("c0-eth2", "c0", "p0", "ns0", "eth2", podMAC, []net.IP{podIP}, 0) + store.Initialize([]*InterfaceConfig{containerInterface1, containerInterface2}) + assert.Equal(t, 2, store.Len()) + + for _, containerInterface := range []*InterfaceConfig{containerInterface1, containerInterface2} { + interfaceKey := util.GenerateContainerInterfaceKey(containerInterface.ContainerID, containerInterface.IFDev) + storedIface, exists := store.GetInterface(interfaceKey) + assert.True(t, exists) + assert.Equal(t, containerInterface, storedIface) + _, exists = store.GetInterface(containerInterface.InterfaceName) + assert.False(t, exists) + _, exists = store.GetInterfaceByName(containerInterface.InterfaceName) + assert.True(t, exists) + _, exists = store.GetContainerInterface(containerInterface.ContainerID) + assert.True(t, exists) + if containerInterface.IPs != nil { + storedIface, exists = store.GetInterfaceByIP(podIP.String()) + require.True(t, exists) + assert.Equal(t, containerInterface, storedIface) + } + ifaces := store.GetContainerInterfacesByPod(containerInterface.PodName, containerInterface.PodNamespace) + assert.Equal(t, 2, len(ifaces)) + ifaceNames := store.GetInterfaceKeysByType(ContainerInterface) + assert.Equal(t, 2, len(ifaceNames)) + assert.Equal(t, 2, store.GetContainerInterfaceNum()) + + store.DeleteInterface(containerInterface) + assert.Equal(t, 1, store.GetContainerInterfaceNum()) + if containerInterface.IPs != nil { + _, exists = store.GetInterfaceByIP(containerInterface.IPs[0].String()) + assert.False(t, exists) + } + store.AddInterface(containerInterface) + assert.Equal(t, 2, store.GetContainerInterfaceNum()) + } +} + func testGatewayInterface(t *testing.T) { gatewayInterface := NewGatewayInterface("antrea-gw0", util.GenerateRandomMAC()) gatewayInterface.IPs = []net.IP{gwIP} diff --git a/pkg/agent/interfacestore/types.go b/pkg/agent/interfacestore/types.go index 90715c1a2bc..9d0ca2afe57 100644 --- a/pkg/agent/interfacestore/types.go +++ b/pkg/agent/interfacestore/types.go @@ -64,6 +64,8 @@ type ContainerInterfaceConfig struct { ContainerID string PodName string PodNamespace string + // Interface name inside container. + IFDev string } type TunnelInterfaceConfig struct { @@ -133,13 +135,15 @@ func NewContainerInterface( containerID string, podName string, podNamespace string, + ifDev string, mac net.HardwareAddr, ips []net.IP, vlanID uint16) *InterfaceConfig { containerConfig := &ContainerInterfaceConfig{ ContainerID: containerID, PodName: podName, - PodNamespace: podNamespace} + PodNamespace: podNamespace, + IFDev: ifDev} return &InterfaceConfig{ InterfaceName: interfaceName, Type: ContainerInterface, diff --git a/pkg/agent/secondarynetwork/cnipodcache/cache.go b/pkg/agent/secondarynetwork/cnipodcache/cache.go deleted file mode 100644 index 0eba91af76e..00000000000 --- a/pkg/agent/secondarynetwork/cnipodcache/cache.go +++ /dev/null @@ -1,109 +0,0 @@ -// Copyright 2021 Antrea Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http:// www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package cnipodcache - -import ( - "sync" - - "k8s.io/client-go/tools/cache" - - "antrea.io/antrea/pkg/agent/util" - "antrea.io/antrea/pkg/util/k8s" -) - -const ( - podIndex = "pod" -) - -type CNIPodInfoCache struct { - // Mutex to protect CNIConfigInfo.PodCNIDeleted which is written by CNIServer, and read by - // the secondary network Pod controller. - sync.RWMutex - cache cache.Indexer -} - -// Add CNIPodInfo to local cache store. -func (c *CNIPodInfoCache) AddCNIConfigInfo(cniConfig *CNIConfigInfo) { - c.cache.Add(cniConfig) -} - -// Delete CNIPodInfo from local cache store. -func (c *CNIPodInfoCache) DeleteCNIConfigInfo(cniConfig *CNIConfigInfo) { - c.cache.Delete(cniConfig) -} - -func (c *CNIPodInfoCache) SetPodCNIDeleted(cniConfig *CNIConfigInfo) { - c.Lock() - defer c.Unlock() - cniConfig.PodCNIDeleted = true -} - -// Retrieve a valid CNI cache (PodCNIDeleted is not true) entry for the given Pod name and namespace. -func (c *CNIPodInfoCache) GetValidCNIConfigInfoPerPod(podName, podNamespace string) *CNIConfigInfo { - podObjs, _ := c.cache.ByIndex(podIndex, k8s.NamespacedName(podNamespace, podName)) - c.RLock() - defer c.RUnlock() - for i := range podObjs { - var cniPodConfig *CNIConfigInfo - cniPodConfig = podObjs[i].(*CNIConfigInfo) - if cniPodConfig.PodCNIDeleted != true { - return cniPodConfig - } - } - return nil -} - -// Retrieve all CNIConfigInfo from cacheStore for the given podName and its Namespace -// NOTE: In an ideal scenario, there should be one cache entry per Pod name and namespace. -func (c *CNIPodInfoCache) GetAllCNIConfigInfoPerPod(podName, podNamespace string) []*CNIConfigInfo { - podObjs, _ := c.cache.ByIndex(podIndex, k8s.NamespacedName(podNamespace, podName)) - CNIPodConfigs := make([]*CNIConfigInfo, len(podObjs)) - for i := range podObjs { - CNIPodConfigs[i] = podObjs[i].(*CNIConfigInfo) - } - return CNIPodConfigs -} - -func (c *CNIPodInfoCache) GetCNIConfigInfoByContainerID(podName, podNamespace, containerID string) *CNIConfigInfo { - podObjs, _ := c.cache.ByIndex(podIndex, k8s.NamespacedName(podNamespace, podName)) - for i := range podObjs { - var cniPodConfig *CNIConfigInfo - cniPodConfig = podObjs[i].(*CNIConfigInfo) - if cniPodConfig.ContainerID == containerID { - return cniPodConfig - } - } - return nil -} - -func podIndexFunc(obj interface{}) ([]string, error) { - podConfig := obj.(*CNIConfigInfo) - return []string{k8s.NamespacedName(podConfig.PodNamespace, podConfig.PodName)}, nil -} - -func getCNIPodInfoKey(obj interface{}) (string, error) { - podConfig := obj.(*CNIConfigInfo) - var key string - key = util.GenerateContainerInterfaceKey(podConfig.ContainerID) - return key, nil -} - -func NewCNIPodInfoStore() *CNIPodInfoCache { - return &CNIPodInfoCache{ - cache: cache.NewIndexer(getCNIPodInfoKey, cache.Indexers{ - podIndex: podIndexFunc, - }), - } -} diff --git a/pkg/agent/secondarynetwork/cnipodcache/types.go b/pkg/agent/secondarynetwork/cnipodcache/types.go deleted file mode 100644 index 657ce431c2d..00000000000 --- a/pkg/agent/secondarynetwork/cnipodcache/types.go +++ /dev/null @@ -1,44 +0,0 @@ -// Copyright 2021 Antrea Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package cnipodcache - -type CNIConfigInfo struct { - PodName string - PodNamespace string - ContainerID string - ContainerNetNS string - PodCNIDeleted bool - // Interfaces is a map that stores the secondary interface information with interface - // name to be the key. - Interfaces map[string]*InterfaceInfo -} - -type NetworkType string - -type InterfaceInfo struct { - NetworkType NetworkType - HostInterfaceName string - // OVS port UUID for a VLAN interface. - OVSPortUUID string -} - -type CNIPodInfoStore interface { - AddCNIConfigInfo(cniConfig *CNIConfigInfo) - DeleteCNIConfigInfo(cniConfig *CNIConfigInfo) - GetValidCNIConfigInfoPerPod(podName, podNamespace string) *CNIConfigInfo - GetAllCNIConfigInfoPerPod(podName, podNamespace string) []*CNIConfigInfo - GetCNIConfigInfoByContainerID(podName, podNamespace, containerID string) *CNIConfigInfo - SetPodCNIDeleted(CNIConfig *CNIConfigInfo) -} diff --git a/pkg/agent/secondarynetwork/podwatch/controller.go b/pkg/agent/secondarynetwork/podwatch/controller.go index fd6d9f8c500..4fa884b9373 100644 --- a/pkg/agent/secondarynetwork/podwatch/controller.go +++ b/pkg/agent/secondarynetwork/podwatch/controller.go @@ -24,6 +24,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/wait" clientset "k8s.io/client-go/kubernetes" "k8s.io/client-go/tools/cache" @@ -38,7 +39,7 @@ import ( "antrea.io/antrea/pkg/agent/cniserver" "antrea.io/antrea/pkg/agent/cniserver/ipam" cnitypes "antrea.io/antrea/pkg/agent/cniserver/types" - cnipodcache "antrea.io/antrea/pkg/agent/secondarynetwork/cnipodcache" + "antrea.io/antrea/pkg/agent/interfacestore" "antrea.io/antrea/pkg/agent/types" crdv1a2 "antrea.io/antrea/pkg/apis/crd/v1alpha2" "antrea.io/antrea/pkg/ovs/ovsconfig" @@ -57,7 +58,6 @@ const ( const ( networkAttachDefAnnotationKey = "k8s.v1.cni.cncf.io/networks" cniPath = "/opt/cni/bin/" - defaultSecondaryInterfaceName = "eth1" startIfaceIndex = 1 endIfaceIndex = 101 @@ -67,8 +67,9 @@ const ( type InterfaceConfigurator interface { ConfigureSriovSecondaryInterface(podName, podNamespace, containerID, containerNetNS, containerInterfaceName string, mtu int, podSriovVFDeviceID string, result *current.Result) error - ConfigureVLANSecondaryInterface(podName, podNamespace, containerID, containerNetNS, containerInterfaceName string, mtu int, vlanID uint16, result *current.Result) (string, error) - DeleteVLANSecondaryInterface(containerID, hostInterfaceName, ovsPortUUID string) error + DeleteSriovSecondaryInterface(interfaceConfig *interfacestore.InterfaceConfig) error + ConfigureVLANSecondaryInterface(podName, podNamespace, containerID, containerNetNS, containerInterfaceName string, mtu int, ipamResult *ipam.IPAMResult) error + DeleteVLANSecondaryInterface(interfaceConfig *interfacestore.InterfaceConfig) error } type IPAMAllocator interface { @@ -76,18 +77,25 @@ type IPAMAllocator interface { SecondaryNetworkRelease(podOwner *crdv1a2.PodOwner) error } -type PodController struct { +type podCNIInfo struct { + containerID string + netNS string +} + +type podController struct { kubeClient clientset.Interface netAttachDefClient netdefclient.K8sCniCncfIoV1Interface queue workqueue.RateLimitingInterface podInformer cache.SharedIndexInformer nodeName string podUpdateSubscriber channel.Subscriber - podCache cnipodcache.CNIPodInfoStore ovsBridgeClient ovsconfig.OVSBridgeClient + interfaceStore interfacestore.InterfaceStore interfaceConfigurator InterfaceConfigurator ipamAllocator IPAMAllocator - vfDeviceIDUsageMap sync.Map + // Map from "namespace/pod" to podCNIInfo. + cniCache sync.Map + vfDeviceIDUsageMap sync.Map } func NewPodController( @@ -97,20 +105,22 @@ func NewPodController( nodeName string, podUpdateSubscriber channel.Subscriber, ovsBridgeClient ovsconfig.OVSBridgeClient, -) (*PodController, error) { - interfaceConfigurator, err := cniserver.NewSecondaryInterfaceConfigurator(ovsBridgeClient) +) (*podController, error) { + ifaceStore := interfacestore.NewInterfaceStore() + interfaceConfigurator, err := cniserver.NewSecondaryInterfaceConfigurator(ovsBridgeClient, ifaceStore) if err != nil { return nil, fmt.Errorf("failed to create SecondaryInterfaceConfigurator: %v", err) } - pc := PodController{ - kubeClient: kubeClient, - netAttachDefClient: netAttachDefClient, - queue: workqueue.NewNamedRateLimitingQueue(workqueue.NewItemExponentialFailureRateLimiter(minRetryDelay, maxRetryDelay), "podcontroller"), + pc := podController{ + kubeClient: kubeClient, + netAttachDefClient: netAttachDefClient, + queue: workqueue.NewNamedRateLimitingQueue( + workqueue.NewItemExponentialFailureRateLimiter(minRetryDelay, maxRetryDelay), "podcontroller"), podInformer: podInformer, nodeName: nodeName, podUpdateSubscriber: podUpdateSubscriber, - podCache: cnipodcache.NewCNIPodInfoStore(), ovsBridgeClient: ovsBridgeClient, + interfaceStore: ifaceStore, interfaceConfigurator: interfaceConfigurator, ipamAllocator: ipam.GetSecondaryNetworkAllocator(), } @@ -134,54 +144,19 @@ func podKeyGet(podName, podNamespace string) string { return podNamespace + "/" + podName } -func generatePodSecondaryIfaceName(podCNIInfo *cnipodcache.CNIConfigInfo) (string, error) { - // Assign default interface name, if podCNIInfo.Interfaces is empty. - if len(podCNIInfo.Interfaces) == 0 { - return defaultSecondaryInterfaceName, nil - } else { - // Generate new interface name (eth1,eth2..eth100) and return to caller. - for ifaceIndex := startIfaceIndex; ifaceIndex < endIfaceIndex; ifaceIndex++ { - ifName := fmt.Sprintf("%s%d", "eth", ifaceIndex) - _, exist := podCNIInfo.Interfaces[ifName] - if !exist { - return ifName, nil - } +func allocatePodSecondaryIfaceName(usedIFNames sets.Set[string]) (string, error) { + // Generate new interface name (eth1,eth2..eth100) and return to caller. + for ifaceIndex := startIfaceIndex; ifaceIndex < endIfaceIndex; ifaceIndex++ { + ifName := fmt.Sprintf("%s%d", "eth", ifaceIndex) + if !usedIFNames.Has(ifName) { + usedIFNames.Insert(ifName) + return ifName, nil } } return "", fmt.Errorf("no more interface names") } -func (pc *PodController) deletePodSecondaryNetwork(podCNIInfo *cnipodcache.CNIConfigInfo) error { - // Release IPAM allocation for all secondary networks of the Pod which is getting removed. - // NOTE: SR-IOV VF interface clean-up, upon Pod delete will be handled by SR-IOV device - // plugin. Not handled here. - for iface, interfaceInfo := range podCNIInfo.Interfaces { - klog.InfoS("Deleting secondary interface", - "Pod", klog.KRef(podCNIInfo.PodNamespace, podCNIInfo.PodName), "interface", iface) - if interfaceInfo.NetworkType == vlanNetworkType { - if err := pc.interfaceConfigurator.DeleteVLANSecondaryInterface(podCNIInfo.ContainerID, - interfaceInfo.HostInterfaceName, interfaceInfo.OVSPortUUID); err != nil { - return err - } - } - - podOwner := &crdv1a2.PodOwner{ - Name: podCNIInfo.PodName, - Namespace: podCNIInfo.PodNamespace, - ContainerID: podCNIInfo.ContainerID, - IFName: iface, - } - if err := pc.ipamAllocator.SecondaryNetworkRelease(podOwner); err != nil { - return fmt.Errorf("failed to clean up IPAM: %v", err) - } - - // Delete map entry for secNetInstIface, secNetInstConfig - delete(podCNIInfo.Interfaces, iface) - } - return nil -} - -func (pc *PodController) enqueuePod(obj interface{}) { +func (pc *podController) enqueuePod(obj interface{}) { var err error pod, isPod := obj.(*corev1.Pod) if !isPod { @@ -201,32 +176,32 @@ func (pc *PodController) enqueuePod(obj interface{}) { } // processCNIUpdate will be called when CNIServer publishes a Pod update event. -func (pc *PodController) processCNIUpdate(e interface{}) { +func (pc *podController) processCNIUpdate(e interface{}) { event := e.(types.PodUpdate) + podKey := podKeyGet(event.PodName, event.PodNamespace) if event.IsAdd { - // Go cache the CNI server info at CNIConfigInfo cache, for podWatch usage - cniInfo := &cnipodcache.CNIConfigInfo{PodName: event.PodName, PodNamespace: event.PodNamespace, - ContainerID: event.ContainerID, ContainerNetNS: event.NetNS, PodCNIDeleted: false} - pc.podCache.AddCNIConfigInfo(cniInfo) + pc.cniCache.Store(podKey, &podCNIInfo{containerID: event.ContainerID, netNS: event.NetNS}) } else { - containerInfo := pc.podCache.GetCNIConfigInfoByContainerID(event.PodName, event.PodNamespace, event.ContainerID) - if containerInfo != nil { - // Update PodCNIDeleted = true. - // This is to let Podwatch controller know that the CNI server cleaned up this Pod's primary network configuration. - pc.podCache.SetPodCNIDeleted(containerInfo) - } + pc.cniCache.Delete(podKey) } pc.queue.Add(podKeyGet(event.PodName, event.PodNamespace)) } // handleAddUpdatePod handles Pod Add, Update events and updates annotation if required. -func (pc *PodController) handleAddUpdatePod(obj interface{}) error { - var err error - var podCNIInfo *cnipodcache.CNIConfigInfo - pod := obj.(*corev1.Pod) +func (pc *podController) handleAddUpdatePod(pod *corev1.Pod, podCNIInfo *podCNIInfo, + storedInterfaces []*interfacestore.InterfaceConfig) error { + if len(storedInterfaces) > 0 { + // We do not support secondary network update at the moment. Return as long as one + // secondary interface has been created for the Pod. + klog.V(1).InfoS("Secondary network already configured on this Pod and update not supported, skipping update", + "Pod", klog.KObj(pod)) + return nil + } + if len(pod.Status.PodIPs) == 0 { - // Primary network configuration is not complete yet. - // Note: Return nil here to unqueue Pod add event. Secondary network configuration will be handled with Pod update event. + // Primary network configuration is not complete yet. Return nil here to unqueue the + // Pod event. Secondary network configuration will be handled with the following Pod + // update events. return nil } @@ -236,18 +211,6 @@ func (pc *PodController) handleAddUpdatePod(obj interface{}) error { klog.V(2).InfoS("Pod does not have a NetworkAttachmentDefinition", "Pod", klog.KObj(pod)) return nil } - // Retrieve Pod specific cache entry which has "PodCNIDeleted = false" - if podCNIInfo = pc.podCache.GetValidCNIConfigInfoPerPod(pod.Name, pod.Namespace); podCNIInfo == nil { - return nil - } - // Valid cache entry retrieved from cache and we received a Pod add or update event. - // Avoid processing Pod annotation, if we already have at least one secondary network successfully configured on this Pod. - // We do not support/handle Annotation updates yet. - if len(podCNIInfo.Interfaces) > 0 { - klog.V(1).InfoS("Secondary network already configured on this Pod and annotation update not supported, skipping update", "Pod", klog.KObj(pod)) - return nil - } - // Parse Pod annotation and proceed with the secondary network configuration. networklist, err := netdefutils.ParseNetworkAnnotation(secondaryNetwork, pod.Namespace) if err != nil { @@ -257,57 +220,93 @@ func (pc *PodController) handleAddUpdatePod(obj interface{}) error { return nil } - err = pc.configurePodSecondaryNetwork(pod, networklist, podCNIInfo) - if err != nil { - klog.ErrorS(err, "Error when configuring secondary network", "Pod", klog.KObj(pod)) - if len(podCNIInfo.Interfaces) == 0 { - // Return error to requeue and retry. - return err + return pc.configurePodSecondaryNetwork(pod, networklist, podCNIInfo) +} + +func (pc *podController) removeInterfaces(interfaces []*interfacestore.InterfaceConfig) error { + var savedErr error + for _, interfaceConfig := range interfaces { + podName := interfaceConfig.PodName + podNamespace := interfaceConfig.PodNamespace + klog.V(1).InfoS("Deleting secondary interface", + "Pod", klog.KRef(podNamespace, podName), "interface", interfaceConfig.IFDev) + + var err error + // Since only VLAN and SR-IOV interfaces are supported by now, we judge the + // interface type by checking interfaceConfig.OVSPortConfig is set or not. + if interfaceConfig.OVSPortConfig != nil { + err = pc.interfaceConfigurator.DeleteVLANSecondaryInterface(interfaceConfig) + } else { + err = pc.deleteSriovSecondaryInterface(interfaceConfig) + } + if err != nil { + klog.ErrorS(err, "Error when deleting secondary interface", + "Pod", klog.KRef(podNamespace, podName), "interface", interfaceConfig.IFDev) + savedErr = err + continue + } + + podOwner := &crdv1a2.PodOwner{ + Name: interfaceConfig.PodName, + Namespace: interfaceConfig.PodNamespace, + ContainerID: interfaceConfig.ContainerID, + IFName: interfaceConfig.IFDev} + if err = pc.ipamAllocator.SecondaryNetworkRelease(podOwner); err != nil { + klog.ErrorS(err, "Error when releasing IPAM allocation", + "Pod", klog.KRef(podNamespace, podName), "interface", interfaceConfig.IFDev) + savedErr = err } - // We do not return error to retry, if at least one secondary network is configured. } - return nil + return savedErr } -func (pc *PodController) handleRemovePod(key string) error { - var err error - pod := strings.Split(key, "/") - // Read the CNI info (stored during Pod creation by cniserver) from cache. - // Delete CNI info shared in cache for a specific Pod which is getting removed/deleted. - podCNIInfo := pc.podCache.GetAllCNIConfigInfoPerPod(pod[1], pod[0]) - for _, info := range podCNIInfo { - // Delete all secondary interfaces and release IPAM. - if err = pc.deletePodSecondaryNetwork(info); err != nil { - klog.ErrorS(err, "Error when deleting secondary network", "Pod", klog.KRef(info.PodNamespace, info.PodName)) - // Return error to requeue Pod delete. +func (pc *podController) syncPod(key string) error { + var pod *corev1.Pod + var cniInfo *podCNIInfo + podExists := false + + if cniObj, cniAdded := pc.cniCache.Load(key); cniAdded { + podObj, ok, err := pc.podInformer.GetIndexer().GetByKey(key) + if err != nil { return err - } else { - // Delete cache entry from podCNIInfo. - pc.podCache.DeleteCNIConfigInfo(info) - // Delete Pod specific VF cache (if one exists) - pc.deleteVFDeviceIDListPerPod(info.PodName, info.PodNamespace) + } + if ok { + pod = podObj.(*corev1.Pod) + cniInfo = cniObj.(*podCNIInfo) + podExists = true } } - return nil -} -func (pc *PodController) syncPod(key string) error { - obj, exists, err := pc.podInformer.GetIndexer().GetByKey(key) - if err != nil { - return err - } else if exists { - return pc.handleAddUpdatePod(obj) - } else { - return pc.handleRemovePod(key) + namespacePod := strings.Split(key, "/") + podNamespace := namespacePod[0] + podName := namespacePod[1] + storedInterfaces := pc.interfaceStore.GetContainerInterfacesByPod(podName, podNamespace) + if len(storedInterfaces) != 0 { + // Pod or its primary interface has been deleted. Remove secondary interfaces too. + if !podExists || + // Interfaces created for a previous Pod with the same Namespace/name are + // not deleted yet. First delete them before processing the new Pod's + // secondary networks. + storedInterfaces[0].ContainerID != cniInfo.containerID { + if err := pc.removeInterfaces(storedInterfaces); err != nil { + return err + } + } + } + + if !podExists { + pc.deleteVFDeviceIDListPerPod(podName, podNamespace) + return nil } + return pc.handleAddUpdatePod(pod, cniInfo, storedInterfaces) } -func (pc *PodController) Worker() { +func (pc *podController) Worker() { for pc.processNextWorkItem() { } } -func (pc *PodController) processNextWorkItem() bool { +func (pc *podController) processNextWorkItem() bool { obj, quit := pc.queue.Get() if quit { return false @@ -324,33 +323,22 @@ func (pc *PodController) processNextWorkItem() bool { } // Configure Secondary Network Interface. -func (pc *PodController) configureSecondaryInterface( +func (pc *podController) configureSecondaryInterface( pod *corev1.Pod, network *netdefv1.NetworkSelectionElement, - podCNIInfo *cnipodcache.CNIConfigInfo, + podCNIInfo *podCNIInfo, networkConfig *SecondaryNetworkConfig) error { - // Generate a new interface name, if the secondary interface name was not provided in the - // Pod annotation. - if len(network.InterfaceRequest) == 0 { - var err error - if network.InterfaceRequest, err = generatePodSecondaryIfaceName(podCNIInfo); err != nil { - klog.ErrorS(err, "Cannot generate interface name", "Pod", klog.KObj(pod)) - // do not return error: no need to requeue - return nil - } - } - - var result *current.Result - var vlanID uint16 + var ipamResult *ipam.IPAMResult var ifConfigErr error if networkConfig.IPAM != nil { + var err error podOwner := &crdv1a2.PodOwner{ Name: pod.Name, Namespace: pod.Namespace, - ContainerID: podCNIInfo.ContainerID, + ContainerID: podCNIInfo.containerID, IFName: network.InterfaceRequest, } - ipamResult, err := pc.ipamAllocator.SecondaryNetworkAllocate(podOwner, &networkConfig.NetworkConfig) + ipamResult, err = pc.ipamAllocator.SecondaryNetworkAllocate(podOwner, &networkConfig.NetworkConfig) if err != nil { return fmt.Errorf("secondary network IPAM failed: %v", err) } @@ -362,84 +350,98 @@ func (pc *PodController) configureSecondaryInterface( } } }() - result = &ipamResult.Result - for _, ip := range result.IPs { + for _, ip := range ipamResult.IPs { ip.Interface = current.Int(1) } - vlanID = ipamResult.VLANID } else { - result = ¤t.Result{} + ipamResult = &ipam.IPAMResult{} } - var ovsPortUUID string switch networkConfig.NetworkType { case sriovNetworkType: - ifConfigErr = pc.configureSriovAsSecondaryInterface(pod, network, podCNIInfo, int(networkConfig.MTU), result) + ifConfigErr = pc.configureSriovAsSecondaryInterface(pod, network, podCNIInfo, int(networkConfig.MTU), &ipamResult.Result) case vlanNetworkType: if networkConfig.VLAN > 0 { // Let VLAN ID in the CNI network configuration override the IPPool subnet // VLAN. - vlanID = uint16(networkConfig.VLAN) + ipamResult.VLANID = uint16(networkConfig.VLAN) } - ovsPortUUID, ifConfigErr = pc.interfaceConfigurator.ConfigureVLANSecondaryInterface( - podCNIInfo.PodName, podCNIInfo.PodNamespace, - podCNIInfo.ContainerID, podCNIInfo.ContainerNetNS, network.InterfaceRequest, - int(networkConfig.MTU), vlanID, result) - } - if ifConfigErr != nil { - return ifConfigErr + ifConfigErr = pc.interfaceConfigurator.ConfigureVLANSecondaryInterface( + pod.Name, pod.Namespace, + podCNIInfo.containerID, podCNIInfo.netNS, network.InterfaceRequest, + int(networkConfig.MTU), ipamResult) } + return ifConfigErr +} - // Update Pod CNI cache with the network config which was successfully configured. - if podCNIInfo.Interfaces == nil { - podCNIInfo.Interfaces = make(map[string]*cnipodcache.InterfaceInfo) - } - hostInterfaceName := "" - if len(result.Interfaces) > 0 { - // In mock tests, result.Interfaces can be nil - hostInterfaceName = result.Interfaces[0].Name +func (pc *podController) configurePodSecondaryNetwork(pod *corev1.Pod, networkList []*netdefv1.NetworkSelectionElement, podCNIInfo *podCNIInfo) error { + usedIFNames := sets.New[string]() + for _, network := range networkList { + if network.InterfaceRequest != "" { + usedIFNames.Insert(network.InterfaceRequest) + } } - interfaceInfo := cnipodcache.InterfaceInfo{ - NetworkType: networkConfig.NetworkType, - HostInterfaceName: hostInterfaceName, - OVSPortUUID: ovsPortUUID} - podCNIInfo.Interfaces[network.InterfaceRequest] = &interfaceInfo - return nil -} -func (pc *PodController) configurePodSecondaryNetwork(pod *corev1.Pod, networklist []*netdefv1.NetworkSelectionElement, podCNIInfo *cnipodcache.CNIConfigInfo) error { - for _, network := range networklist { + var savedErr error + interfacesConfigured := 0 + for _, network := range networkList { klog.V(2).InfoS("Secondary Network attached to Pod", "network", network, "Pod", klog.KObj(pod)) netDefCRD, err := pc.netAttachDefClient.NetworkAttachmentDefinitions(network.Namespace).Get(context.TODO(), network.Name, metav1.GetOptions{}) if err != nil { - // NetworkAttachmentDefinition not found at this time. Return error to re-queue and re-try. - return fmt.Errorf("failed to get NetworkAttachmentDefinition: %v", err) + klog.ErrorS(err, "Failed to get NetworkAttachmentDefinition", + "network", network, "Pod", klog.KRef(pod.Namespace, pod.Name)) + savedErr = err + continue } + cniConfig, err := netdefutils.GetCNIConfig(netDefCRD, "") if err != nil { - // NetworkAttachmentDefinition Spec.Config parsing failed. Return error to re-queue and re-try. - return fmt.Errorf("NetworkAttachmentDefinition CNI config spec read error: %v", err) + klog.ErrorS(err, "Failed to parse NetworkAttachmentDefinition", + "network", network, "Pod", klog.KRef(pod.Namespace, pod.Name)) + // NetworkAttachmentDefinition Spec.Config parsing failed. Do not retry. + continue } + networkConfig, err := validateNetworkConfig(cniConfig) if err != nil { if networkConfig != nil && networkConfig.Type != cniserver.AntreaCNIType { // Ignore non-Antrea CNI type. klog.InfoS("Not Antrea CNI type in NetworkAttachmentDefinition, ignoring", "NetworkAttachmentDefinition", klog.KObj(netDefCRD), "Pod", klog.KRef(pod.Namespace, pod.Name)) - continue + } else { + klog.ErrorS(err, "NetworkConfig validation failed", + "NetworkAttachmentDefinition", klog.KObj(netDefCRD), "Pod", klog.KRef(pod.Namespace, pod.Name)) } - klog.ErrorS(err, "NetworkConfig validation failed", - "NetworkAttachmentDefinition", klog.KObj(netDefCRD), "Pod", klog.KRef(pod.Namespace, pod.Name)) continue } - // secondary network information retrieved from API server. Proceed to configure secondary interface now. + + // Generate a new interface name, if the secondary interface name was not provided in the + // Pod annotation. + if network.InterfaceRequest == "" { + var err error + if network.InterfaceRequest, err = allocatePodSecondaryIfaceName(usedIFNames); err != nil { + klog.ErrorS(err, "Cannot generate interface name", "Pod", klog.KRef(pod.Namespace, pod.Name)) + // Do not return error: no need to requeue. + continue + } + } + + // Secondary network information retrieved from API server. Proceed to configure secondary interface now. if err = pc.configureSecondaryInterface(pod, network, podCNIInfo, networkConfig); err != nil { klog.ErrorS(err, "Secondary interface configuration failed", - "Pod", klog.KRef(pod.Namespace, pod.Name), "networkType", networkConfig.NetworkType) - // Secondary interface configuration failed. return error to re-queue and re-try. - return fmt.Errorf("secondary interface configuration failed: %v", err) + "Pod", klog.KRef(pod.Namespace, pod.Name), "interface", network.InterfaceRequest, + "networkType", networkConfig.NetworkType) + savedErr = err + } else { + interfacesConfigured++ } } + + if savedErr != nil && interfacesConfigured == 0 { + // As we do not support secondary network update, do not return error to + // retry, if at least one secondary network is configured. + return savedErr + } return nil } @@ -479,7 +481,7 @@ func validateNetworkConfig(cniConfig []byte) (*SecondaryNetworkConfig, error) { return &networkConfig, nil } -func (pc *PodController) Run(stopCh <-chan struct{}) { +func (pc *podController) Run(stopCh <-chan struct{}) { defer func() { klog.InfoS("Shutting down", "controller", controllerName) pc.queue.ShutDown() diff --git a/pkg/agent/secondarynetwork/podwatch/controller_test.go b/pkg/agent/secondarynetwork/podwatch/controller_test.go index 63572d4353c..feb1b61154a 100644 --- a/pkg/agent/secondarynetwork/podwatch/controller_test.go +++ b/pkg/agent/secondarynetwork/podwatch/controller_test.go @@ -45,9 +45,10 @@ import ( "k8s.io/client-go/util/workqueue" "antrea.io/antrea/pkg/agent/cniserver/ipam" - "antrea.io/antrea/pkg/agent/cniserver/types" - "antrea.io/antrea/pkg/agent/secondarynetwork/cnipodcache" + cnitypes "antrea.io/antrea/pkg/agent/cniserver/types" + "antrea.io/antrea/pkg/agent/interfacestore" podwatchtesting "antrea.io/antrea/pkg/agent/secondarynetwork/podwatch/testing" + "antrea.io/antrea/pkg/agent/types" crdv1a2 "antrea.io/antrea/pkg/apis/crd/v1alpha2" ) @@ -86,10 +87,9 @@ const ( podIP = "1.2.3.4" networkName = "net" interfaceName = "eth2" - ovsPortUUID = "12345678-e29b-41d4-a716-446655440000" ) -func testNetwork(name string, networkType cnipodcache.NetworkType) *netdefv1.NetworkAttachmentDefinition { +func testNetwork(name string, networkType networkType) *netdefv1.NetworkAttachmentDefinition { return testNetworkExt(name, "", "", string(networkType), "", 0, 0, false) } @@ -134,7 +134,7 @@ func containerNetNs(container string) string { return fmt.Sprintf("/var/run/netns/%s", container) } -func testPod(name string, container string, podIP string, networks ...netdefv1.NetworkSelectionElement) (*corev1.Pod, *cnipodcache.CNIConfigInfo) { +func testPod(name string, container string, podIP string, networks ...netdefv1.NetworkSelectionElement) (*corev1.Pod, *podCNIInfo) { annotations := make(map[string]string) if len(networks) > 0 { annotation, _ := json.Marshal(networks) @@ -167,26 +167,25 @@ func testPod(name string, container string, podIP string, networks ...netdefv1.N }, } } - cniConfig := &cnipodcache.CNIConfigInfo{ - PodName: name, - PodNamespace: testNamespace, - ContainerID: container, - ContainerNetNS: containerNetNs(container), - PodCNIDeleted: false, + cniInfo := &podCNIInfo{ + containerID: container, + netNS: containerNetNs(container), } - return pod, cniConfig + return pod, cniInfo } -func testIPAMResult(cidr string) *ipam.IPAMResult { +func testIPAMResult(cidr string, vlan int) *ipam.IPAMResult { _, ipNet, _ := net.ParseCIDR(cidr) return &ipam.IPAMResult{ Result: current.Result{ IPs: []*current.IPConfig{ { - Address: *ipNet, + Address: *ipNet, + Interface: current.Int(1), }, }, }, + VLANID: uint16(vlan), } } @@ -201,7 +200,6 @@ func TestPodControllerRun(t *testing.T) { client := fake.NewSimpleClientset() netdefclient := netdefclientfake.NewSimpleClientset().K8sCniCncfIoV1() informerFactory := informers.NewSharedInformerFactory(client, resyncPeriod) - podCache := cnipodcache.NewCNIPodInfoStore() interfaceConfigurator := podwatchtesting.NewMockInterfaceConfigurator(ctrl) mockIPAM := podwatchtesting.NewMockIPAMAllocator(ctrl) podController, _ := NewPodController( @@ -210,9 +208,10 @@ func TestPodControllerRun(t *testing.T) { informerFactory.Core().V1().Pods().Informer(), testNode, nil, nil) - podController.podCache = podCache podController.interfaceConfigurator = interfaceConfigurator podController.ipamAllocator = mockIPAM + cniCache := &podController.cniCache + interfaceStore := podController.interfaceStore stopCh := make(chan struct{}) informerFactory.Start(stopCh) @@ -225,12 +224,33 @@ func TestPodControllerRun(t *testing.T) { podController.Run(stopCh) }() - pod, cniConfig := testPod(podName, containerID, podIP, netdefv1.NetworkSelectionElement{ + pod, cniInfo := testPod(podName, containerID, podIP, netdefv1.NetworkSelectionElement{ Name: networkName, InterfaceRequest: interfaceName, }) + podKey := podKeyGet(pod.Name, pod.Namespace) network := testNetwork(networkName, sriovNetworkType) - ipamResult := testIPAMResult("148.14.24.100/24") + ipamResult := testIPAMResult("148.14.24.100/24", 0) + podOwner := &crdv1a2.PodOwner{ + Name: pod.Name, + Namespace: pod.Namespace, + ContainerID: containerID, + IFName: interfaceName} + containerConfig := interfacestore.NewContainerInterface(interfaceName, containerID, + pod.Name, pod.Namespace, interfaceName, nil, nil, 0) + + // CNI Add event. + event := types.PodUpdate{ + IsAdd: true, + PodName: pod.Name, + PodNamespace: pod.Namespace, + ContainerID: containerID, + NetNS: cniInfo.netNS, + } + podController.processCNIUpdate(event) + cniObj, _ := cniCache.Load(podKey) + assert.NotNil(t, cniObj) + assert.Equal(t, cniInfo, cniObj.(*podCNIInfo)) var interfaceConfigured int32 interfaceConfigurator.EXPECT().ConfigureSriovSecondaryInterface( @@ -244,11 +264,11 @@ func TestPodControllerRun(t *testing.T) { &ipamResult.Result, ).Do(func(string, string, string, string, string, int, string, *current.Result) { atomic.AddInt32(&interfaceConfigured, 1) + interfaceStore.AddInterface(containerConfig) }) - mockIPAM.EXPECT().SecondaryNetworkAllocate(gomock.Any(), gomock.Any()).Return(ipamResult, nil) + mockIPAM.EXPECT().SecondaryNetworkAllocate(podOwner, gomock.Any()).Return(ipamResult, nil) - podCache.AddCNIConfigInfo(cniConfig) - // the NetworkAttachmentDefinition must be created before the Pod: if handleAddUpdatePod + // The NetworkAttachmentDefinition must be created before the Pod: if handleAddUpdatePod // runs before the NetworkAttachmentDefinition has been created, it will return an // error. The Pod will then be requeued, but the Poll below will timeout before the Pod has // a chance to be processed again. Rather than increase the timeout or change the queue's @@ -259,21 +279,70 @@ func TestPodControllerRun(t *testing.T) { _, err = client.CoreV1().Pods(testNamespace).Create(context.Background(), pod, metav1.CreateOptions{}) require.NoError(t, err, "error when creating test Pod") - // unfortunately, we cannot use the podcache being updated by the controller as a signal - // here: the podcache is not thread-safe and is only meant to be accessed by the controller - // event handlers (with the exception of the operations meant to be performed by the CNI server). + // Wait for ConfigureSriovSecondaryInterface is called. + assert.NoError(t, wait.Poll(10*time.Millisecond, 1*time.Second, func() (bool, error) { + return atomic.LoadInt32(&interfaceConfigured) == 1, nil + })) + _, exists := podController.vfDeviceIDUsageMap.Load(podKey) + assert.True(t, exists) + + podController.processCNIUpdate(event) + interfaceConfigurator.EXPECT().ConfigureSriovSecondaryInterface( + podName, + testNamespace, + containerID, + containerNetNs(containerID), + interfaceName, + defaultMTU, + "", + &ipamResult.Result, + ).Do(func(string, string, string, string, string, int, string, *current.Result) { + atomic.AddInt32(&interfaceConfigured, 1) + interfaceStore.AddInterface(containerConfig) + }) + mockIPAM.EXPECT().SecondaryNetworkAllocate(podOwner, gomock.Any()).Return(ipamResult, nil) + + interfaceStore.DeleteInterface(containerConfig) + // Since interface is not saved to the interface store, interface creation should be + // triggered again. + podController.processCNIUpdate(event) assert.NoError(t, wait.Poll(10*time.Millisecond, 1*time.Second, func() (bool, error) { - return atomic.LoadInt32(&interfaceConfigured) > 0, nil + return atomic.LoadInt32(&interfaceConfigured) == 2, nil })) - mockIPAM.EXPECT().SecondaryNetworkRelease(gomock.Any()) + interfaceConfigurator.EXPECT().DeleteSriovSecondaryInterface(containerConfig). + Do(func(*interfacestore.InterfaceConfig) { + atomic.AddInt32(&interfaceConfigured, -1) + }) + mockIPAM.EXPECT().SecondaryNetworkRelease(podOwner) + require.NoError(t, client.CoreV1().Pods(testNamespace).Delete(context.Background(), + podName, metav1.DeleteOptions{}), "error when deleting test Pod") + + assert.NoError(t, wait.Poll(10*time.Millisecond, 1*time.Second, func() (bool, error) { + return atomic.LoadInt32(&interfaceConfigured) == 1, nil + })) + _, exists = podController.vfDeviceIDUsageMap.Load(podKey) + assert.False(t, exists) - require.NotNil(t, podCache.GetCNIConfigInfoByContainerID(podName, testNamespace, containerID) == nil) - require.NoError(t, client.CoreV1().Pods(testNamespace).Delete(context.Background(), podName, metav1.DeleteOptions{}), "error when deleting test Pod") + interfaceConfigurator.EXPECT().DeleteSriovSecondaryInterface(containerConfig). + Do(func(*interfacestore.InterfaceConfig) { + atomic.AddInt32(&interfaceConfigured, -1) + }) + mockIPAM.EXPECT().SecondaryNetworkRelease(podOwner) + // CNI Del event. + event.IsAdd = false + // Interfac is not deleted from the interface store, so CNI Del should trigger interface + // deletion again. + podController.processCNIUpdate(event) + _, exists = cniCache.Load(podKey) + assert.False(t, exists) assert.NoError(t, wait.Poll(10*time.Millisecond, 1*time.Second, func() (bool, error) { - return podCache.GetCNIConfigInfoByContainerID(podName, testNamespace, containerID) == nil, nil + return atomic.LoadInt32(&interfaceConfigured) == 0, nil })) + interfaceStore.DeleteInterface(containerConfig) + podController.processCNIUpdate(event) + close(stopCh) wg.Wait() } @@ -297,24 +366,22 @@ func TestConfigurePodSecondaryNetwork(t *testing.T) { name string cniVersion string cniType string - networkType cnipodcache.NetworkType + networkType networkType ipamType string mtu int vlan int noIPAM bool doNotCreateNetwork bool - interfaceCreated bool expectedErr string expectedCalls func(mockIPAM *podwatchtesting.MockIPAMAllocator, mockIC *podwatchtesting.MockInterfaceConfigurator) }{ { - name: "VLAN network", - networkType: vlanNetworkType, - mtu: 1600, - vlan: 101, - interfaceCreated: true, + name: "VLAN network", + networkType: vlanNetworkType, + mtu: 1600, + vlan: 101, expectedCalls: func(mockIPAM *podwatchtesting.MockIPAMAllocator, mockIC *podwatchtesting.MockInterfaceConfigurator) { - mockIPAM.EXPECT().SecondaryNetworkAllocate(podOwner, gomock.Any()).Return(testIPAMResult("148.14.24.100/24"), nil) + mockIPAM.EXPECT().SecondaryNetworkAllocate(podOwner, gomock.Any()).Return(testIPAMResult("148.14.24.100/24", 0), nil) mockIC.EXPECT().ConfigureVLANSecondaryInterface( podName, testNamespace, @@ -322,17 +389,17 @@ func TestConfigurePodSecondaryNetwork(t *testing.T) { containerNetNs(containerID), interfaceName, 1600, - uint16(101), - gomock.Any(), - ).Return(ovsPortUUID, nil) + testIPAMResult("148.14.24.100/24", 101), + ) }, }, { - name: "default MTU", - networkType: vlanNetworkType, - interfaceCreated: true, + name: "VLAN in IPPool", + networkType: vlanNetworkType, + vlan: 0, expectedCalls: func(mockIPAM *podwatchtesting.MockIPAMAllocator, mockIC *podwatchtesting.MockInterfaceConfigurator) { - mockIPAM.EXPECT().SecondaryNetworkAllocate(podOwner, gomock.Any()).Return(testIPAMResult("148.14.24.100/24"), nil) + // IPAM returns the VLAN ID in the IPPool subnet. + mockIPAM.EXPECT().SecondaryNetworkAllocate(podOwner, gomock.Any()).Return(testIPAMResult("148.14.24.100/24", 101), nil) mockIC.EXPECT().ConfigureVLANSecondaryInterface( podName, testNamespace, @@ -340,17 +407,16 @@ func TestConfigurePodSecondaryNetwork(t *testing.T) { containerNetNs(containerID), interfaceName, 1500, - uint16(0), - gomock.Any(), - ).Return(ovsPortUUID, nil) + testIPAMResult("148.14.24.100/24", 101), + ) }, }, { - name: "no IPAM", - networkType: vlanNetworkType, - noIPAM: true, - interfaceCreated: true, + name: "network VLAN overrides IPPool VLAN", + networkType: vlanNetworkType, + vlan: 101, expectedCalls: func(mockIPAM *podwatchtesting.MockIPAMAllocator, mockIC *podwatchtesting.MockInterfaceConfigurator) { + mockIPAM.EXPECT().SecondaryNetworkAllocate(podOwner, gomock.Any()).Return(testIPAMResult("148.14.24.100/24", 102), nil) mockIC.EXPECT().ConfigureVLANSecondaryInterface( podName, testNamespace, @@ -358,18 +424,32 @@ func TestConfigurePodSecondaryNetwork(t *testing.T) { containerNetNs(containerID), interfaceName, 1500, - uint16(0), - gomock.Any(), - ).Return(ovsPortUUID, nil) + testIPAMResult("148.14.24.100/24", 101), + ) }, }, { - name: "SRIOV network", - networkType: sriovNetworkType, - mtu: 1500, - interfaceCreated: true, + name: "no IPAM", + networkType: vlanNetworkType, + noIPAM: true, + expectedCalls: func(mockIPAM *podwatchtesting.MockIPAMAllocator, mockIC *podwatchtesting.MockInterfaceConfigurator) { + mockIC.EXPECT().ConfigureVLANSecondaryInterface( + podName, + testNamespace, + containerID, + containerNetNs(containerID), + interfaceName, + 1500, + &ipam.IPAMResult{}, + ) + }, + }, + { + name: "SRIOV network", + networkType: sriovNetworkType, + mtu: 1500, expectedCalls: func(mockIPAM *podwatchtesting.MockIPAMAllocator, mockIC *podwatchtesting.MockInterfaceConfigurator) { - mockIPAM.EXPECT().SecondaryNetworkAllocate(podOwner, gomock.Any()).Return(testIPAMResult("148.14.24.100/24"), nil) + mockIPAM.EXPECT().SecondaryNetworkAllocate(podOwner, gomock.Any()).Return(testIPAMResult("148.14.24.100/24", 0), nil) mockIC.EXPECT().ConfigureSriovSecondaryInterface( podName, testNamespace, @@ -378,8 +458,8 @@ func TestConfigurePodSecondaryNetwork(t *testing.T) { interfaceName, 1500, sriovDeviceID, - gomock.Any(), - ).Return(nil) + &testIPAMResult("148.14.24.100/24", 0).Result, + ) }, }, { @@ -388,7 +468,7 @@ func TestConfigurePodSecondaryNetwork(t *testing.T) { mtu: 1500, vlan: 100, doNotCreateNetwork: true, - expectedErr: "failed to get NetworkAttachmentDefinition:", + expectedErr: "\"net\" not found", }, { name: "unsupported CNI version", @@ -433,7 +513,7 @@ func TestConfigurePodSecondaryNetwork(t *testing.T) { networkType: sriovNetworkType, mtu: 1500, expectedCalls: func(mockIPAM *podwatchtesting.MockIPAMAllocator, mockIC *podwatchtesting.MockInterfaceConfigurator) { - mockIPAM.EXPECT().SecondaryNetworkAllocate(podOwner, gomock.Any()).Return(testIPAMResult("148.14.24.100/24"), errors.New("failure")) + mockIPAM.EXPECT().SecondaryNetworkAllocate(podOwner, gomock.Any()).Return(testIPAMResult("148.14.24.100/24", 0), errors.New("failure")) }, expectedErr: "secondary network IPAM failed", }, @@ -443,7 +523,7 @@ func TestConfigurePodSecondaryNetwork(t *testing.T) { mtu: 1600, vlan: 101, expectedCalls: func(mockIPAM *podwatchtesting.MockIPAMAllocator, mockIC *podwatchtesting.MockInterfaceConfigurator) { - mockIPAM.EXPECT().SecondaryNetworkAllocate(podOwner, gomock.Any()).Return(testIPAMResult("148.14.24.100/24"), nil) + mockIPAM.EXPECT().SecondaryNetworkAllocate(podOwner, gomock.Any()).Return(testIPAMResult("148.14.24.100/24", 0), nil) mockIC.EXPECT().ConfigureVLANSecondaryInterface( podName, testNamespace, @@ -451,10 +531,9 @@ func TestConfigurePodSecondaryNetwork(t *testing.T) { containerNetNs(containerID), interfaceName, 1600, - uint16(101), - gomock.Any(), - ).Return("", errors.New("interface creation failure")) - mockIPAM.EXPECT().SecondaryNetworkRelease(podOwner).Return(nil) + testIPAMResult("148.14.24.100/24", 101), + ).Return(errors.New("interface creation failure")) + mockIPAM.EXPECT().SecondaryNetworkRelease(podOwner) }, expectedErr: "interface creation failure", }, @@ -470,9 +549,8 @@ func TestConfigurePodSecondaryNetwork(t *testing.T) { containerNetNs(containerID), interfaceName, 1500, - uint16(0), - gomock.Any(), - ).Return("", errors.New("interface creation failure")) + &ipam.IPAMResult{}, + ).Return(errors.New("interface creation failure")) }, expectedErr: "interface creation failure", }, @@ -480,58 +558,58 @@ func TestConfigurePodSecondaryNetwork(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - pod, cniConfigInfo := testPod(podName, containerID, podIP, element1) - pc, mockIPAM, interfaceConfigurator := testPodController(ctrl) - savedCNIConfig := *cniConfigInfo + pod, cniInfo := testPod(podName, containerID, podIP, element1) + pc, mockIPAM, interfaceConfigurator := testPodControllerStart(ctrl) - network1 := testNetworkExt(networkName, tc.cniVersion, tc.cniType, string(tc.networkType), tc.ipamType, tc.mtu, tc.vlan, tc.noIPAM) if !tc.doNotCreateNetwork { - pc.netAttachDefClient.NetworkAttachmentDefinitions(testNamespace).Create(context.Background(), network1, metav1.CreateOptions{}) + network1 := testNetworkExt(networkName, tc.cniVersion, tc.cniType, + string(tc.networkType), tc.ipamType, tc.mtu, tc.vlan, tc.noIPAM) + pc.netAttachDefClient.NetworkAttachmentDefinitions(testNamespace).Create(context.Background(), + network1, metav1.CreateOptions{}) } if tc.expectedCalls != nil { tc.expectedCalls(mockIPAM, interfaceConfigurator) } - err := pc.configurePodSecondaryNetwork(pod, []*netdefv1.NetworkSelectionElement{&element1}, cniConfigInfo) + err := pc.configurePodSecondaryNetwork(pod, []*netdefv1.NetworkSelectionElement{&element1}, cniInfo) if tc.expectedErr == "" { assert.Nil(t, err) } else { assert.True(t, strings.Contains(err.Error(), tc.expectedErr)) } - - if tc.interfaceCreated { - info := cnipodcache.InterfaceInfo{ - NetworkType: tc.networkType, - } - if tc.networkType == vlanNetworkType { - info.OVSPortUUID = ovsPortUUID - } - savedCNIConfig.Interfaces = map[string]*cnipodcache.InterfaceInfo{interfaceName: &info} - } - assert.Equal(t, &savedCNIConfig, cniConfigInfo) }) } } func TestPodControllerAddPod(t *testing.T) { - pod, cniConfig := testPod(podName, containerID, podIP, netdefv1.NetworkSelectionElement{ + pod, _ := testPod(podName, containerID, podIP, netdefv1.NetworkSelectionElement{ Name: networkName, InterfaceRequest: interfaceName, }) + podKey := podKeyGet(podName, testNamespace) - t.Run("missing network", func(t *testing.T) { - ctrl := gomock.NewController(t) - podController, _, _ := testPodController(ctrl) - podController.podCache.AddCNIConfigInfo(cniConfig) - _, err := podController.kubeClient.CoreV1().Pods(testNamespace).Create(context.Background(), pod, metav1.CreateOptions{}) + // Create Pod and wait for Informer cache updated. + createPodFn := func(pc *podController, pod *corev1.Pod) { + _, err := pc.kubeClient.CoreV1().Pods(testNamespace).Create(context.Background(), + pod, metav1.CreateOptions{}) require.NoError(t, err, "error when creating test Pod") - assert.Error(t, podController.handleAddUpdatePod(pod)) - }) + assert.NoError(t, wait.Poll(10*time.Millisecond, 1*time.Second, func() (bool, error) { + _, ok, err := pc.podInformer.GetIndexer().GetByKey(podKey) + return ok, err + })) + } + deletePodFn := func(pc *podController, podName string) { + require.NoError(t, pc.kubeClient.CoreV1().Pods(testNamespace).Delete(context.Background(), + podName, metav1.DeleteOptions{}), "error when deleting test Pod") + assert.NoError(t, wait.Poll(10*time.Millisecond, 1*time.Second, func() (bool, error) { + _, ok, err := pc.podInformer.GetIndexer().GetByKey(podKey) + return !ok, err + })) + } t.Run("multiple network interfaces", func(t *testing.T) { ctrl := gomock.NewController(t) - podController, mockIPAM, interfaceConfigurator := testPodController(ctrl) - + podController, mockIPAM, interfaceConfigurator := testPodControllerStart(ctrl) pod, cniConfig := testPod( podName, containerID, @@ -545,27 +623,38 @@ func TestPodControllerAddPod(t *testing.T) { InterfaceRequest: "eth11", }, ) - savedCNIConfig := *cniConfig network1 := testNetwork("net1", sriovNetworkType) testVLAN := 100 network2 := testNetworkExt("net2", "", "", string(vlanNetworkType), "", defaultMTU, testVLAN, false) - podOwner1 := &crdv1a2.PodOwner{ - Name: podName, - Namespace: testNamespace, - ContainerID: containerID, - IFName: "eth10"} - podOwner2 := &crdv1a2.PodOwner{ - Name: podName, - Namespace: testNamespace, - ContainerID: containerID, - IFName: "eth11"} - networkConfig1 := types.NetworkConfig{ + podOwner1 := &crdv1a2.PodOwner{Name: podName, Namespace: testNamespace, + ContainerID: containerID, IFName: "eth10"} + podOwner2 := &crdv1a2.PodOwner{Name: podName, Namespace: testNamespace, + ContainerID: containerID, IFName: "eth11"} + containerConfig1 := interfacestore.NewContainerInterface("interface1", containerID, + pod.Name, pod.Namespace, "eth10", nil, nil, 0) + containerConfig2 := interfacestore.NewContainerInterface("interface2", containerID, + pod.Name, pod.Namespace, "eth11", nil, nil, 0) + // VLAN interface should have OVSPortConfig. + containerConfig2.OVSPortConfig = &interfacestore.OVSPortConfig{} + + staleContainerID := containerID + "-stale" + stalePodOwner1 := &crdv1a2.PodOwner{Name: podName, Namespace: testNamespace, + ContainerID: staleContainerID, IFName: "eth1"} + stalePodOwner2 := &crdv1a2.PodOwner{Name: podName, Namespace: testNamespace, + ContainerID: staleContainerID, IFName: "eth2"} + staleConfig1 := interfacestore.NewContainerInterface("interface1", staleContainerID, + pod.Name, pod.Namespace, "eth1", nil, nil, 0) + staleConfig2 := interfacestore.NewContainerInterface("interface2", staleContainerID, + pod.Name, pod.Namespace, "eth2", nil, nil, 0) + staleConfig1.OVSPortConfig = &interfacestore.OVSPortConfig{} + + networkConfig1 := cnitypes.NetworkConfig{ CNIVersion: "0.3.0", Name: "net1", Type: "antrea", MTU: 1500, - IPAM: &types.IPAMConfig{ + IPAM: &cnitypes.IPAMConfig{ Type: "antrea", IPPools: []string{"ipv4-pool-1", "ipv6-pool-1"}, }, @@ -573,6 +662,20 @@ func TestPodControllerAddPod(t *testing.T) { networkConfig2 := networkConfig1 networkConfig2.Name = "net2" + podController.interfaceStore.AddInterface(staleConfig1) + podController.interfaceStore.AddInterface(staleConfig2) + // Stale interfaces in the interface store should be deleted first. + mockIPAM.EXPECT().SecondaryNetworkRelease(stalePodOwner1) + mockIPAM.EXPECT().SecondaryNetworkRelease(stalePodOwner2) + interfaceConfigurator.EXPECT().DeleteVLANSecondaryInterface(staleConfig1) + interfaceConfigurator.EXPECT().DeleteSriovSecondaryInterface(staleConfig2) + + podController.cniCache.Store(podKey, cniConfig) + createPodFn(podController, pod) + assert.NoError(t, podController.syncPod(podKey)) + podController.interfaceStore.DeleteInterface(staleConfig1) + podController.interfaceStore.DeleteInterface(staleConfig2) + interfaceConfigurator.EXPECT().ConfigureSriovSecondaryInterface( podName, testNamespace, @@ -581,7 +684,7 @@ func TestPodControllerAddPod(t *testing.T) { "eth10", interfaceDefaultMTU, gomock.Any(), - gomock.Any(), + &testIPAMResult("148.14.24.100/24", 0).Result, ) interfaceConfigurator.EXPECT().ConfigureVLANSecondaryInterface( podName, @@ -590,90 +693,69 @@ func TestPodControllerAddPod(t *testing.T) { containerNetNs(containerID), "eth11", defaultMTU, - uint16(testVLAN), - gomock.Any(), - ).Return(ovsPortUUID, nil) - - mockIPAM.EXPECT().SecondaryNetworkAllocate(podOwner1, &networkConfig1).Return(testIPAMResult("148.14.24.100/24"), nil) - mockIPAM.EXPECT().SecondaryNetworkAllocate(podOwner2, &networkConfig2).Return(testIPAMResult("148.14.24.101/24"), nil) + testIPAMResult("148.14.24.101/24", 100), + ) + mockIPAM.EXPECT().SecondaryNetworkAllocate(podOwner1, &networkConfig1).Return(testIPAMResult("148.14.24.100/24", 0), nil) + mockIPAM.EXPECT().SecondaryNetworkAllocate(podOwner2, &networkConfig2).Return(testIPAMResult("148.14.24.101/24", 0), nil) - podController.podCache.AddCNIConfigInfo(cniConfig) - _, err := podController.kubeClient.CoreV1().Pods(testNamespace).Create(context.Background(), pod, metav1.CreateOptions{}) - require.NoError(t, err, "error when creating test Pod") - _, err = podController.netAttachDefClient.NetworkAttachmentDefinitions(testNamespace).Create(context.Background(), network1, metav1.CreateOptions{}) + _, err := podController.netAttachDefClient.NetworkAttachmentDefinitions(testNamespace).Create(context.Background(), + network1, metav1.CreateOptions{}) require.NoError(t, err, "error when creating test NetworkAttachmentDefinition") - _, err = podController.netAttachDefClient.NetworkAttachmentDefinitions(testNamespace).Create(context.Background(), network2, metav1.CreateOptions{}) + _, err = podController.netAttachDefClient.NetworkAttachmentDefinitions(testNamespace).Create(context.Background(), + network2, metav1.CreateOptions{}) require.NoError(t, err, "error when creating test NetworkAttachmentDefinition") - assert.NoError(t, podController.handleAddUpdatePod(pod)) + assert.NoError(t, podController.syncPod(podKey)) - infos := podController.podCache.GetAllCNIConfigInfoPerPod(podName, testNamespace) - assert.Equal(t, 1, len(infos)) - savedCNIConfig.Interfaces = map[string]*cnipodcache.InterfaceInfo{ - "eth10": { - NetworkType: sriovNetworkType, - }, - "eth11": { - OVSPortUUID: ovsPortUUID, - NetworkType: vlanNetworkType, - }, - } - assert.Equal(t, &savedCNIConfig, infos[0]) + podController.interfaceStore.AddInterface(containerConfig1) + podController.interfaceStore.AddInterface(containerConfig2) + mockIPAM.EXPECT().SecondaryNetworkRelease(podOwner1) + mockIPAM.EXPECT().SecondaryNetworkRelease(podOwner2) + interfaceConfigurator.EXPECT().DeleteSriovSecondaryInterface(containerConfig1) + interfaceConfigurator.EXPECT().DeleteVLANSecondaryInterface(containerConfig2) - mockIPAM.EXPECT().SecondaryNetworkRelease(podOwner1).Return(nil) - mockIPAM.EXPECT().SecondaryNetworkRelease(podOwner2).Return(nil) - interfaceConfigurator.EXPECT().DeleteVLANSecondaryInterface( - containerID, - gomock.Any(), - ovsPortUUID).Return(nil) - assert.NoError(t, podController.handleRemovePod(testNamespace+"/"+podName)) - infos = podController.podCache.GetAllCNIConfigInfoPerPod(podName, testNamespace) - assert.Equal(t, 0, len(infos)) + deletePodFn(podController, pod.Name) + assert.NoError(t, podController.syncPod(podKey)) }) t.Run("no network interfaces", func(t *testing.T) { ctrl := gomock.NewController(t) - podController, _, _ := testPodController(ctrl) - + podController, _, _ := testPodControllerStart(ctrl) pod, cniConfig := testPod(podName, containerID, podIP) - podController.podCache.AddCNIConfigInfo(cniConfig) - _, err := podController.kubeClient.CoreV1().Pods(testNamespace).Create(context.Background(), pod, metav1.CreateOptions{}) - require.NoError(t, err, "error when creating test Pod") - assert.NoError(t, podController.handleAddUpdatePod(pod)) + podController.cniCache.Store(podKey, cniConfig) + createPodFn(podController, pod) + assert.NoError(t, podController.syncPod(podKey)) }) - t.Run("missing podcache entry", func(t *testing.T) { + t.Run("missing CNI cache entry", func(t *testing.T) { ctrl := gomock.NewController(t) - podController, _, _ := testPodController(ctrl) - + podController, _, _ := testPodControllerStart(ctrl) network := testNetwork(networkName, sriovNetworkType) - _, err := podController.kubeClient.CoreV1().Pods(testNamespace).Create(context.Background(), pod, metav1.CreateOptions{}) - require.NoError(t, err, "error when creating test Pod") - _, err = podController.netAttachDefClient.NetworkAttachmentDefinitions(testNamespace).Create(context.Background(), network, metav1.CreateOptions{}) + _, err := podController.netAttachDefClient.NetworkAttachmentDefinitions(testNamespace).Create(context.Background(), + network, metav1.CreateOptions{}) require.NoError(t, err, "error when creating test NetworkAttachmentDefinition") - assert.NoError(t, podController.handleAddUpdatePod(pod)) + createPodFn(podController, pod) + assert.NoError(t, podController.syncPod(podKey)) }) t.Run("missing Status.PodIPs", func(t *testing.T) { ctrl := gomock.NewController(t) - podController, _, _ := testPodController(ctrl) - + podController, _, _ := testPodControllerStart(ctrl) pod, cniConfig := testPod(podName, containerID, "") network := testNetwork(networkName, sriovNetworkType) - podController.podCache.AddCNIConfigInfo(cniConfig) - _, err := podController.kubeClient.CoreV1().Pods(testNamespace).Create(context.Background(), pod, metav1.CreateOptions{}) - require.NoError(t, err, "error when creating test Pod") - _, err = podController.netAttachDefClient.NetworkAttachmentDefinitions(testNamespace).Create(context.Background(), network, metav1.CreateOptions{}) + _, err := podController.netAttachDefClient.NetworkAttachmentDefinitions(testNamespace).Create(context.Background(), + network, metav1.CreateOptions{}) require.NoError(t, err, "error when creating test NetworkAttachmentDefinition") - assert.NoError(t, podController.handleAddUpdatePod(pod)) + createPodFn(podController, pod) + podController.cniCache.Store(podKey, cniConfig) + assert.NoError(t, podController.syncPod(podKey)) }) t.Run("different Namespace for Pod and NetworkAttachmentDefinition", func(t *testing.T) { ctrl := gomock.NewController(t) - podController, mockIPAM, interfaceConfigurator := testPodController(ctrl) - + podController, mockIPAM, interfaceConfigurator := testPodControllerStart(ctrl) networkNamespace := "nsB" network := testNetwork(networkName, sriovNetworkType) network.Namespace = networkNamespace @@ -694,20 +776,19 @@ func TestPodControllerAddPod(t *testing.T) { sriovDeviceID, gomock.Any(), ) - mockIPAM.EXPECT().SecondaryNetworkAllocate(gomock.Any(), gomock.Any()).Return(testIPAMResult("148.14.24.100/24"), nil) + mockIPAM.EXPECT().SecondaryNetworkAllocate(gomock.Any(), gomock.Any()).Return(testIPAMResult("148.14.24.100/24", 0), nil) - podController.podCache.AddCNIConfigInfo(cniConfig) - _, err := podController.kubeClient.CoreV1().Pods(testNamespace).Create(context.Background(), pod, metav1.CreateOptions{}) - require.NoError(t, err, "error when creating test Pod") - _, err = podController.netAttachDefClient.NetworkAttachmentDefinitions(networkNamespace).Create(context.Background(), network, metav1.CreateOptions{}) + _, err := podController.netAttachDefClient.NetworkAttachmentDefinitions(networkNamespace).Create(context.Background(), + network, metav1.CreateOptions{}) require.NoError(t, err, "error when creating test NetworkAttachmentDefinition") - assert.NoError(t, podController.handleAddUpdatePod(pod)) + createPodFn(podController, pod) + podController.cniCache.Store(podKey, cniConfig) + assert.NoError(t, podController.syncPod(podKey)) }) t.Run("no interface name", func(t *testing.T) { ctrl := gomock.NewController(t) - podController, mockIPAM, interfaceConfigurator := testPodController(ctrl) - + podController, mockIPAM, interfaceConfigurator := testPodControllerStart(ctrl) pod, cniConfig := testPod( podName, containerID, @@ -744,49 +825,20 @@ func TestPodControllerAddPod(t *testing.T) { gomock.Any(), ) - mockIPAM.EXPECT().SecondaryNetworkAllocate(gomock.Any(), gomock.Any()).Return(testIPAMResult("148.14.24.100/24"), nil) - mockIPAM.EXPECT().SecondaryNetworkAllocate(gomock.Any(), gomock.Any()).Return(testIPAMResult("148.14.24.101/24"), nil) - - podController.podCache.AddCNIConfigInfo(cniConfig) - _, err := podController.kubeClient.CoreV1().Pods(testNamespace).Create(context.Background(), pod, metav1.CreateOptions{}) - require.NoError(t, err, "error when creating test Pod") - _, err = podController.netAttachDefClient.NetworkAttachmentDefinitions(testNamespace).Create(context.Background(), network, metav1.CreateOptions{}) - require.NoError(t, err, "error when creating test NetworkAttachmentDefinition") - assert.NoError(t, podController.handleAddUpdatePod(pod)) - }) - - t.Run("error when creating interface", func(t *testing.T) { - ctrl := gomock.NewController(t) - podController, mockIPAM, interfaceConfigurator := testPodController(ctrl) - - network := testNetwork(networkName, sriovNetworkType) - - interfaceConfigurator.EXPECT().ConfigureSriovSecondaryInterface( - podName, - testNamespace, - containerID, - containerNetNs(containerID), - interfaceName, - defaultMTU, - gomock.Any(), - gomock.Any(), - ).Return(fmt.Errorf("error when creating interface")) - - mockIPAM.EXPECT().SecondaryNetworkAllocate(gomock.Any(), gomock.Any()).Return(testIPAMResult("148.14.24.100/24"), nil) - mockIPAM.EXPECT().SecondaryNetworkRelease(gomock.Any()) + mockIPAM.EXPECT().SecondaryNetworkAllocate(gomock.Any(), gomock.Any()).Return(testIPAMResult("148.14.24.100/24", 0), nil) + mockIPAM.EXPECT().SecondaryNetworkAllocate(gomock.Any(), gomock.Any()).Return(testIPAMResult("148.14.24.101/24", 0), nil) - podController.podCache.AddCNIConfigInfo(cniConfig) - _, err := podController.kubeClient.CoreV1().Pods(testNamespace).Create(context.Background(), pod, metav1.CreateOptions{}) - require.NoError(t, err, "error when creating test Pod") - _, err = podController.netAttachDefClient.NetworkAttachmentDefinitions(testNamespace).Create(context.Background(), network, metav1.CreateOptions{}) + _, err := podController.netAttachDefClient.NetworkAttachmentDefinitions(testNamespace).Create(context.Background(), + network, metav1.CreateOptions{}) require.NoError(t, err, "error when creating test NetworkAttachmentDefinition") - assert.Error(t, podController.handleAddUpdatePod(pod)) + createPodFn(podController, pod) + podController.cniCache.Store(podKey, cniConfig) + assert.NoError(t, podController.syncPod(podKey)) }) t.Run("invalid CNI config", func(t *testing.T) { ctrl := gomock.NewController(t) - podController, _, _ := testPodController(ctrl) - + podController, _, _ := testPodControllerStart(ctrl) pod, cniConfig := testPod( podName, containerID, @@ -805,69 +857,82 @@ func TestPodControllerAddPod(t *testing.T) { }, } - podController.podCache.AddCNIConfigInfo(cniConfig) - _, err := podController.kubeClient.CoreV1().Pods(testNamespace).Create(context.Background(), pod, metav1.CreateOptions{}) - require.NoError(t, err, "error when creating test Pod") - _, err = podController.netAttachDefClient.NetworkAttachmentDefinitions(testNamespace).Create(context.Background(), network, metav1.CreateOptions{}) + _, err := podController.netAttachDefClient.NetworkAttachmentDefinitions(testNamespace).Create(context.Background(), + network, metav1.CreateOptions{}) require.NoError(t, err, "error when creating test NetworkAttachmentDefinition") - // we don't expect an error here, no requeueing - assert.Error(t, podController.handleAddUpdatePod(pod)) + createPodFn(podController, pod) + podController.cniCache.Store(podKey, cniConfig) + // We don't expect an error here, no requeueing. + assert.NoError(t, podController.syncPod(podKey)) }) t.Run("invalid networks annotation", func(t *testing.T) { ctrl := gomock.NewController(t) - podController, _, _ := testPodController(ctrl) - + podController, _, _ := testPodControllerStart(ctrl) pod, cniConfig := testPod(podName, containerID, podIP) pod.Annotations = map[string]string{ networkAttachDefAnnotationKey: "", } network := testNetwork(networkName, sriovNetworkType) - podController.podCache.AddCNIConfigInfo(cniConfig) - _, err := podController.kubeClient.CoreV1().Pods(testNamespace).Create(context.Background(), pod, metav1.CreateOptions{}) - require.NoError(t, err, "error when creating test Pod") - _, err = podController.netAttachDefClient.NetworkAttachmentDefinitions(testNamespace).Create(context.Background(), network, metav1.CreateOptions{}) + _, err := podController.netAttachDefClient.NetworkAttachmentDefinitions(testNamespace).Create(context.Background(), + network, metav1.CreateOptions{}) require.NoError(t, err, "error when creating test NetworkAttachmentDefinition") - // we don't expect an error here, no requeueing - assert.NoError(t, podController.handleAddUpdatePod(pod)) + createPodFn(podController, pod) + podController.cniCache.Store(podKey, cniConfig) + // We don't expect an error here, no requeueing. + assert.NoError(t, podController.syncPod(podKey)) }) - t.Run("Error when adding VF deviceID cache per Pod", func(t *testing.T) { + t.Run("updating deviceID cache per Pod", func(t *testing.T) { ctrl := gomock.NewController(t) - network := testNetwork(networkName, sriovNetworkType) podController, _, _ := testPodController(ctrl) - podController.podCache.AddCNIConfigInfo(cniConfig) - _, err := podController.kubeClient.CoreV1().Pods(testNamespace).Create(context.Background(), pod, metav1.CreateOptions{}) - require.NoError(t, err, "error when creating test Pod") - _, err = podController.netAttachDefClient.NetworkAttachmentDefinitions(testNamespace).Create(context.Background(), network, metav1.CreateOptions{}) - require.NoError(t, err, "error when creating test NetworkAttachmentDefinition") - - _, err = podController.assignUnusedSriovVFDeviceIDPerPod(podName, testNamespace, interfaceName) + _, err := podController.assignUnusedSriovVFDeviceID(podName, testNamespace, interfaceName) + _, exists := podController.vfDeviceIDUsageMap.Load(podKey) + assert.True(t, exists) require.NoError(t, err, "error while assigning unused VfDevice ID") - + podController.releaseSriovVFDeviceID(podName, testNamespace, interfaceName) + _, exists = podController.vfDeviceIDUsageMap.Load(podKey) + assert.True(t, exists) podController.deleteVFDeviceIDListPerPod(podName, testNamespace) - require.NoError(t, err, "error deleting cache") - + _, exists = podController.vfDeviceIDUsageMap.Load(podKey) + assert.False(t, exists) }) } -func testPodController(ctrl *gomock.Controller) (*PodController, *podwatchtesting.MockIPAMAllocator, *podwatchtesting.MockInterfaceConfigurator) { +func testPodController(ctrl *gomock.Controller) ( + *podController, *podwatchtesting.MockIPAMAllocator, + *podwatchtesting.MockInterfaceConfigurator) { client := fake.NewSimpleClientset() netdefclient := netdefclientfake.NewSimpleClientset().K8sCniCncfIoV1() informerFactory := informers.NewSharedInformerFactory(client, resyncPeriod) - podCache := cnipodcache.NewCNIPodInfoStore() interfaceConfigurator := podwatchtesting.NewMockInterfaceConfigurator(ctrl) mockIPAM := podwatchtesting.NewMockIPAMAllocator(ctrl) - // PodController object without event handlers - return &PodController{ - kubeClient: client, - netAttachDefClient: netdefclient, - queue: workqueue.NewNamedRateLimitingQueue(workqueue.NewItemExponentialFailureRateLimiter(minRetryDelay, maxRetryDelay), "podcontroller"), + + // podController without event handlers. + return &podController{ + kubeClient: client, + netAttachDefClient: netdefclient, + queue: workqueue.NewNamedRateLimitingQueue( + workqueue.NewItemExponentialFailureRateLimiter(minRetryDelay, maxRetryDelay), + "podcontroller"), podInformer: informerFactory.Core().V1().Pods().Informer(), nodeName: testNode, - podCache: podCache, interfaceConfigurator: interfaceConfigurator, ipamAllocator: mockIPAM, + interfaceStore: interfacestore.NewInterfaceStore(), }, mockIPAM, interfaceConfigurator } + +// Create a test podController and start informerFactory. +func testPodControllerStart(ctrl *gomock.Controller) ( + *podController, *podwatchtesting.MockIPAMAllocator, + *podwatchtesting.MockInterfaceConfigurator) { + podController, mockIPAM, interfaceConfigurator := testPodController(ctrl) + informerFactory := informers.NewSharedInformerFactory(podController.kubeClient, resyncPeriod) + podController.podInformer = informerFactory.Core().V1().Pods().Informer() + stopCh := make(chan struct{}) + informerFactory.Start(stopCh) + informerFactory.WaitForCacheSync(stopCh) + return podController, mockIPAM, interfaceConfigurator +} diff --git a/pkg/agent/secondarynetwork/podwatch/sriov.go b/pkg/agent/secondarynetwork/podwatch/sriov.go index ba7c3490218..8e63551a973 100644 --- a/pkg/agent/secondarynetwork/podwatch/sriov.go +++ b/pkg/agent/secondarynetwork/podwatch/sriov.go @@ -32,7 +32,7 @@ import ( grpcinsecure "google.golang.org/grpc/credentials/insecure" podresourcesv1alpha1 "k8s.io/kubelet/pkg/apis/podresources/v1alpha1" - cnipodcache "antrea.io/antrea/pkg/agent/secondarynetwork/cnipodcache" + "antrea.io/antrea/pkg/agent/interfacestore" ) const ( @@ -47,7 +47,7 @@ var ( getPodContainerDeviceIDsFn = getPodContainerDeviceIDs ) -type KubeletPodResources struct { +type kubeletPodResources struct { resources []*podresourcesv1alpha1.PodResources } @@ -73,7 +73,6 @@ func getPodContainerDeviceIDs(podName string, podNamespace string) ([]string, er if err != nil { return []string{}, fmt.Errorf("error getting the gRPC client for Pod resources: %v", err) } - defer conn.Close() client := podresourcesv1alpha1.NewPodResourcesListerClient(conn) @@ -87,7 +86,7 @@ func getPodContainerDeviceIDs(podName string, podNamespace string) ([]string, er } var podDeviceIDs []string - var kpr KubeletPodResources + var kpr kubeletPodResources kpr.resources = podResources.GetPodResources() for _, pr := range kpr.resources { if pr.Name == podName && pr.Namespace == podNamespace { @@ -109,8 +108,8 @@ func getPodContainerDeviceIDs(podName string, podNamespace string) ([]string, er // which is still not associated with a network device name. // NOTE: buildVFDeviceIDListPerPod is called only if a Pod specific VF to Interface mapping cache // was not build earlier. Sample initial entry per Pod: "{18:01.1,""},{18:01.2,""},{18:01.3,""}" -func (pc *PodController) buildVFDeviceIDListPerPod(podName, podNamespace string) ([]podSriovVFDeviceIDInfo, error) { - podKey := podNamespace + "/" + podName +func (pc *podController) buildVFDeviceIDListPerPod(podName, podNamespace string) ([]podSriovVFDeviceIDInfo, error) { + podKey := podKeyGet(podName, podNamespace) deviceCache, cacheFound := pc.vfDeviceIDUsageMap.Load(podKey) if cacheFound { return deviceCache.([]podSriovVFDeviceIDInfo), nil @@ -129,8 +128,8 @@ func (pc *PodController) buildVFDeviceIDListPerPod(podName, podNamespace string) return vfDeviceIDInfoCache, nil } -func (pc *PodController) deleteVFDeviceIDListPerPod(podName, podNamespace string) { - podKey := podNamespace + "/" + podName +func (pc *podController) deleteVFDeviceIDListPerPod(podName, podNamespace string) { + podKey := podKeyGet(podName, podNamespace) _, cacheFound := pc.vfDeviceIDUsageMap.Load(podKey) if cacheFound { pc.vfDeviceIDUsageMap.Delete(podKey) @@ -139,7 +138,21 @@ func (pc *PodController) deleteVFDeviceIDListPerPod(podName, podNamespace string return } -func (pc *PodController) assignUnusedSriovVFDeviceIDPerPod(podName, podNamespace, interfaceName string) (string, error) { +func (pc *podController) releaseSriovVFDeviceID(podName, podNamespace, interfaceName string) { + podKey := podKeyGet(podName, podNamespace) + obj, cacheFound := pc.vfDeviceIDUsageMap.Load(podKey) + if !cacheFound { + return + } + cache := obj.([]podSriovVFDeviceIDInfo) + for idx := 0; idx < len(cache); idx++ { + if cache[idx].ifName == interfaceName { + cache[idx].ifName = "" + } + } +} + +func (pc *podController) assignUnusedSriovVFDeviceID(podName, podNamespace, interfaceName string) (string, error) { var cache []podSriovVFDeviceIDInfo cache, err := pc.buildVFDeviceIDListPerPod(podName, podNamespace) if err != nil { @@ -156,17 +169,25 @@ func (pc *PodController) assignUnusedSriovVFDeviceIDPerPod(podName, podNamespace } // Configure SRIOV VF as a Secondary Network Interface. -func (pc *PodController) configureSriovAsSecondaryInterface(pod *corev1.Pod, network *netdefv1.NetworkSelectionElement, containerInfo *cnipodcache.CNIConfigInfo, mtu int, result *current.Result) error { - podSriovVFDeviceID, err := pc.assignUnusedSriovVFDeviceIDPerPod(pod.Name, pod.Namespace, network.InterfaceRequest) +func (pc *podController) configureSriovAsSecondaryInterface(pod *corev1.Pod, network *netdefv1.NetworkSelectionElement, podCNIInfo *podCNIInfo, mtu int, result *current.Result) error { + podSriovVFDeviceID, err := pc.assignUnusedSriovVFDeviceID(pod.Name, pod.Namespace, network.InterfaceRequest) if err != nil { return err } - if err = pc.interfaceConfigurator.ConfigureSriovSecondaryInterface( - containerInfo.PodName, containerInfo.PodNamespace, containerInfo.ContainerID, - containerInfo.ContainerNetNS, network.InterfaceRequest, - mtu, podSriovVFDeviceID, result); err != nil { + pod.Name, pod.Namespace, podCNIInfo.containerID, podCNIInfo.netNS, + network.InterfaceRequest, mtu, podSriovVFDeviceID, result); err != nil { return fmt.Errorf("SRIOV Interface creation failed: %v", err) } return nil } + +func (pc *podController) deleteSriovSecondaryInterface(interfaceConfig *interfacestore.InterfaceConfig) error { + // NOTE: SR-IOV VF interface clean-up will be handled by SR-IOV device plugin. The interface + // is not deleted here. + if err := pc.interfaceConfigurator.DeleteSriovSecondaryInterface(interfaceConfig); err != nil { + return err + } + pc.releaseSriovVFDeviceID(interfaceConfig.PodName, interfaceConfig.PodNamespace, interfaceConfig.IFDev) + return nil +} diff --git a/pkg/agent/secondarynetwork/podwatch/testing/mock_podwatch.go b/pkg/agent/secondarynetwork/podwatch/testing/mock_podwatch.go index 149efe2dff6..b831261bc4f 100644 --- a/pkg/agent/secondarynetwork/podwatch/testing/mock_podwatch.go +++ b/pkg/agent/secondarynetwork/podwatch/testing/mock_podwatch.go @@ -1,4 +1,4 @@ -// Copyright 2023 Antrea Authors +// Copyright 2024 Antrea Authors // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -28,6 +28,7 @@ import ( ipam "antrea.io/antrea/pkg/agent/cniserver/ipam" types "antrea.io/antrea/pkg/agent/cniserver/types" + interfacestore "antrea.io/antrea/pkg/agent/interfacestore" v1alpha2 "antrea.io/antrea/pkg/apis/crd/v1alpha2" types100 "github.com/containernetworking/cni/pkg/types/100" gomock "go.uber.org/mock/gomock" @@ -71,32 +72,45 @@ func (mr *MockInterfaceConfiguratorMockRecorder) ConfigureSriovSecondaryInterfac } // ConfigureVLANSecondaryInterface mocks base method. -func (m *MockInterfaceConfigurator) ConfigureVLANSecondaryInterface(arg0, arg1, arg2, arg3, arg4 string, arg5 int, arg6 uint16, arg7 *types100.Result) (string, error) { +func (m *MockInterfaceConfigurator) ConfigureVLANSecondaryInterface(arg0, arg1, arg2, arg3, arg4 string, arg5 int, arg6 *ipam.IPAMResult) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ConfigureVLANSecondaryInterface", arg0, arg1, arg2, arg3, arg4, arg5, arg6, arg7) - ret0, _ := ret[0].(string) - ret1, _ := ret[1].(error) - return ret0, ret1 + ret := m.ctrl.Call(m, "ConfigureVLANSecondaryInterface", arg0, arg1, arg2, arg3, arg4, arg5, arg6) + ret0, _ := ret[0].(error) + return ret0 } // ConfigureVLANSecondaryInterface indicates an expected call of ConfigureVLANSecondaryInterface. -func (mr *MockInterfaceConfiguratorMockRecorder) ConfigureVLANSecondaryInterface(arg0, arg1, arg2, arg3, arg4, arg5, arg6, arg7 any) *gomock.Call { +func (mr *MockInterfaceConfiguratorMockRecorder) ConfigureVLANSecondaryInterface(arg0, arg1, arg2, arg3, arg4, arg5, arg6 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ConfigureVLANSecondaryInterface", reflect.TypeOf((*MockInterfaceConfigurator)(nil).ConfigureVLANSecondaryInterface), arg0, arg1, arg2, arg3, arg4, arg5, arg6) +} + +// DeleteSriovSecondaryInterface mocks base method. +func (m *MockInterfaceConfigurator) DeleteSriovSecondaryInterface(arg0 *interfacestore.InterfaceConfig) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DeleteSriovSecondaryInterface", arg0) + ret0, _ := ret[0].(error) + return ret0 +} + +// DeleteSriovSecondaryInterface indicates an expected call of DeleteSriovSecondaryInterface. +func (mr *MockInterfaceConfiguratorMockRecorder) DeleteSriovSecondaryInterface(arg0 any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ConfigureVLANSecondaryInterface", reflect.TypeOf((*MockInterfaceConfigurator)(nil).ConfigureVLANSecondaryInterface), arg0, arg1, arg2, arg3, arg4, arg5, arg6, arg7) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteSriovSecondaryInterface", reflect.TypeOf((*MockInterfaceConfigurator)(nil).DeleteSriovSecondaryInterface), arg0) } // DeleteVLANSecondaryInterface mocks base method. -func (m *MockInterfaceConfigurator) DeleteVLANSecondaryInterface(arg0, arg1, arg2 string) error { +func (m *MockInterfaceConfigurator) DeleteVLANSecondaryInterface(arg0 *interfacestore.InterfaceConfig) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "DeleteVLANSecondaryInterface", arg0, arg1, arg2) + ret := m.ctrl.Call(m, "DeleteVLANSecondaryInterface", arg0) ret0, _ := ret[0].(error) return ret0 } // DeleteVLANSecondaryInterface indicates an expected call of DeleteVLANSecondaryInterface. -func (mr *MockInterfaceConfiguratorMockRecorder) DeleteVLANSecondaryInterface(arg0, arg1, arg2 any) *gomock.Call { +func (mr *MockInterfaceConfiguratorMockRecorder) DeleteVLANSecondaryInterface(arg0 any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteVLANSecondaryInterface", reflect.TypeOf((*MockInterfaceConfigurator)(nil).DeleteVLANSecondaryInterface), arg0, arg1, arg2) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteVLANSecondaryInterface", reflect.TypeOf((*MockInterfaceConfigurator)(nil).DeleteVLANSecondaryInterface), arg0) } // MockIPAMAllocator is a mock of IPAMAllocator interface. diff --git a/pkg/agent/secondarynetwork/podwatch/types.go b/pkg/agent/secondarynetwork/podwatch/types.go index 329c6706e3e..3be1c226308 100644 --- a/pkg/agent/secondarynetwork/podwatch/types.go +++ b/pkg/agent/secondarynetwork/podwatch/types.go @@ -16,21 +16,18 @@ package podwatch import ( "antrea.io/antrea/pkg/agent/cniserver/types" - "antrea.io/antrea/pkg/agent/secondarynetwork/cnipodcache" ) -type RouteInfo struct { - Dst string `json:"dst,omitempty"` -} +type networkType string const ( - sriovNetworkType cnipodcache.NetworkType = "sriov" - vlanNetworkType cnipodcache.NetworkType = "vlan" + sriovNetworkType networkType = "sriov" + vlanNetworkType networkType = "vlan" ) type SecondaryNetworkConfig struct { types.NetworkConfig - NetworkType cnipodcache.NetworkType `json:"networkType,omitempty"` + NetworkType networkType `json:"networkType,omitempty"` // VLAN ID of the OVS port. Applicable only to the VLAN network type. If a // non-zero VLAN is specified, it will override the VLAN in the Antrea // IPAM IPPool subnet. diff --git a/pkg/agent/util/net.go b/pkg/agent/util/net.go index 099242d3cfa..c830c0a4aa1 100644 --- a/pkg/agent/util/net.go +++ b/pkg/agent/util/net.go @@ -72,15 +72,15 @@ func generateInterfaceName(key string, name string, useHead bool) string { } // GenerateContainerInterfaceKey generates a unique string for a Pod's -// interface as: container/. +// interface as: "c//". // We must use ContainerID instead of PodNamespace + PodName because there could // be more than one container associated with the same Pod at some point. // For example, when deleting a StatefulSet Pod with 0 second grace period, the // Pod will be removed from the Kubernetes API very quickly and a new Pod will // be created immediately, and kubelet may process the deletion of the previous // Pod and the addition of the new Pod simultaneously. -func GenerateContainerInterfaceKey(containerID string) string { - return fmt.Sprintf("container/%s", containerID) +func GenerateContainerInterfaceKey(containerID, ifDev string) string { + return fmt.Sprintf("c/%s/%s", containerID, ifDev) } // GenerateNodeTunnelInterfaceKey generates a unique string for a Node's