diff --git a/pkg/agent/cniserver/secondary.go b/pkg/agent/cniserver/secondary.go index 05d0b624403..ae01aed0dff 100644 --- a/pkg/agent/cniserver/secondary.go +++ b/pkg/agent/cniserver/secondary.go @@ -46,8 +46,11 @@ func (pc *podConfigurator) ConfigureSriovSecondaryInterface( containerIface := result.Interfaces[1] klog.InfoS("Configured SR-IOV interface", "Pod", klog.KRef(podNamespace, podName), "interface", containerInterfaceName, "hostInterface", hostIface) - if err = pc.ifConfigurator.advertiseContainerAddr(containerNetNS, containerIface.Name, result); err != nil { - klog.ErrorS(err, "Failed to advertise IP address for SR-IOV interface", "container", containerID, "interface", containerInterfaceName) + if result.IPs != nil { + if err = pc.ifConfigurator.advertiseContainerAddr(containerNetNS, containerIface.Name, result); err != nil { + klog.ErrorS(err, "Failed to advertise IP address for SR-IOV interface", + "container", containerID, "interface", containerInterfaceName) + } } return nil } @@ -84,8 +87,11 @@ func (pc *podConfigurator) ConfigureVLANSecondaryInterface( } klog.InfoS("Configured VLAN interface", "Pod", klog.KRef(podNamespace, podName), "interface", containerInterfaceName, "hostInterface", hostIface) - 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) + 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 diff --git a/pkg/agent/secondarynetwork/podwatch/controller.go b/pkg/agent/secondarynetwork/podwatch/controller.go index ab2e95146c8..86d684bb833 100644 --- a/pkg/agent/secondarynetwork/podwatch/controller.go +++ b/pkg/agent/secondarynetwork/podwatch/controller.go @@ -164,7 +164,7 @@ func (pc *PodController) deletePodSecondaryNetwork(podCNIInfo *cnipodcache.CNICo IFName: iface, } if err := pc.ipamAllocator.SecondaryNetworkRelease(podOwner); err != nil { - return fmt.Errorf("Failed to clean-up whereabouts IPAM %v", err) + return fmt.Errorf("failed to clean up IPAM: %v", err) } // Delete map entry for secNetInstIface, secNetInstConfig @@ -231,10 +231,13 @@ func (pc *PodController) handleAddUpdatePod(obj interface{}) error { } err = pc.configurePodSecondaryNetwork(pod, networklist, podCNIInfo) - // We do not return error to retry, if at least one secondary network is configured. - if (err != nil) && (len(podCNIInfo.Interfaces) == 0) { - // Return error to requeue and retry. - return err + 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 + } + // We do not return error to retry, if at least one secondary network is configured. } return nil } @@ -309,28 +312,42 @@ func (pc *PodController) configureSecondaryInterface( } } - podOwner := &crdv1a2.PodOwner{ - Name: pod.Name, - Namespace: pod.Namespace, - ContainerID: podCNIInfo.ContainerID, - IFName: network.InterfaceRequest, - } - ipamResult, err := pc.ipamAllocator.SecondaryNetworkAllocate(podOwner, &networkConfig.NetworkConfig) - if err != nil { - return fmt.Errorf("secondary network IPAM failed: %v", err) - } - result := &ipamResult.Result - for _, ip := range result.IPs { - ip.Interface = current.Int(1) + var result *current.Result + var vlanID uint16 + var ifConfigErr error + if networkConfig.IPAM != nil { + podOwner := &crdv1a2.PodOwner{ + Name: pod.Name, + Namespace: pod.Namespace, + ContainerID: podCNIInfo.ContainerID, + IFName: network.InterfaceRequest, + } + ipamResult, err := pc.ipamAllocator.SecondaryNetworkAllocate(podOwner, &networkConfig.NetworkConfig) + if err != nil { + return fmt.Errorf("secondary network IPAM failed: %v", err) + } + defer func() { + if ifConfigErr != nil { + // Interface creation failed. Free allocated IP address + if err := pc.ipamAllocator.SecondaryNetworkRelease(podOwner); err != nil { + klog.ErrorS(err, "IPAM de-allocation failed: ", err) + } + } + }() + result = &ipamResult.Result + for _, ip := range result.IPs { + ip.Interface = current.Int(1) + } + vlanID = ipamResult.VLANID + } else { + result = ¤t.Result{} } var ovsPortUUID string - var ifConfigErr error switch networkConfig.NetworkType { case sriovNetworkType: ifConfigErr = pc.configureSriovAsSecondaryInterface(pod, network, podCNIInfo, int(networkConfig.MTU), result) case vlanNetworkType: - vlanID := ipamResult.VLANID if networkConfig.VLAN > 0 { // Let VLAN ID in the CNI network configuration override the IPPool subnet // VLAN. @@ -341,14 +358,10 @@ func (pc *PodController) configureSecondaryInterface( podCNIInfo.ContainerID, podCNIInfo.ContainerNetNS, network.InterfaceRequest, int(networkConfig.MTU), vlanID, result) } - if ifConfigErr != nil { - // Interface creation failed. Free allocated IP address - if err := pc.ipamAllocator.SecondaryNetworkRelease(podOwner); err != nil { - klog.ErrorS(err, "IPAM de-allocation failed: ", err) - } 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) @@ -425,8 +438,10 @@ func validateNetworkConfig(cniConfig []byte) (*SecondaryNetworkConfig, error) { if networkConfig.MTU < 0 { return &networkConfig, fmt.Errorf("invalid MTU %d", networkConfig.MTU) } - if networkConfig.IPAM.Type != ipam.AntreaIPAMType { - return &networkConfig, fmt.Errorf("unsupported IPAM type %s", networkConfig.IPAM.Type) + if networkConfig.IPAM != nil { + if networkConfig.IPAM.Type != ipam.AntreaIPAMType { + return &networkConfig, fmt.Errorf("unsupported IPAM type %s", networkConfig.IPAM.Type) + } } if networkConfig.MTU == 0 { diff --git a/pkg/agent/secondarynetwork/podwatch/controller_test.go b/pkg/agent/secondarynetwork/podwatch/controller_test.go index 493e3e6364b..71f2e16ddfe 100644 --- a/pkg/agent/secondarynetwork/podwatch/controller_test.go +++ b/pkg/agent/secondarynetwork/podwatch/controller_test.go @@ -70,6 +70,14 @@ const ( } }` + netAttachNoIPAMTemplate = `{ + "cniVersion": "{{.CNIVersion}}", + "type": "{{.CNIType}}", + "networkType": "{{.NetworkType}}", + "mtu": {{.MTU}}, + "vlan": {{.VLAN}} +}` + defaultCNIVersion = "0.3.0" defaultMTU = 1500 sriovDeviceID = "sriov-device-id" @@ -82,10 +90,10 @@ const ( ) func testNetwork(name string, networkType cnipodcache.NetworkType) *netdefv1.NetworkAttachmentDefinition { - return testNetworkExt(name, "", "", string(networkType), "", 0, 0) + return testNetworkExt(name, "", "", string(networkType), "", 0, 0, false) } -func testNetworkExt(name, cniVersion, cniType, networkType, ipamType string, mtu int, vlan int) *netdefv1.NetworkAttachmentDefinition { +func testNetworkExt(name, cniVersion, cniType, networkType, ipamType string, mtu, vlan int, noIPAM bool) *netdefv1.NetworkAttachmentDefinition { if cniVersion == "" { cniVersion = defaultCNIVersion } @@ -103,7 +111,13 @@ func testNetworkExt(name, cniVersion, cniType, networkType, ipamType string, mtu MTU int VLAN int }{cniVersion, cniType, networkType, ipamType, mtu, vlan} - tmpl := template.Must(template.New("test").Parse(netAttachTemplate)) + + var tmpl *template.Template + if !noIPAM { + tmpl = template.Must(template.New("test").Parse(netAttachTemplate)) + } else { + tmpl = template.Must(template.New("test").Parse(netAttachNoIPAMTemplate)) + } var b bytes.Buffer tmpl.Execute(&b, &data) return &netdefv1.NetworkAttachmentDefinition{ @@ -216,7 +230,6 @@ func TestPodControllerRun(t *testing.T) { InterfaceRequest: interfaceName, }) network := testNetwork(networkName, sriovNetworkType) - ipamResult := testIPAMResult("148.14.24.100/24") var interfaceConfigured int32 @@ -288,6 +301,7 @@ func TestConfigurePodSecondaryNetwork(t *testing.T) { ipamType string mtu int vlan int + noIPAM bool doNotCreateNetwork bool interfaceCreated bool expectedErr string @@ -316,7 +330,6 @@ func TestConfigurePodSecondaryNetwork(t *testing.T) { { name: "default MTU", networkType: vlanNetworkType, - vlan: 0, interfaceCreated: true, expectedCalls: func(mockIPAM *podwatchtesting.MockIPAMAllocator, mockIC *podwatchtesting.MockInterfaceConfigurator) { mockIPAM.EXPECT().SecondaryNetworkAllocate(podOwner, gomock.Any()).Return(testIPAMResult("148.14.24.100/24"), nil) @@ -332,6 +345,24 @@ func TestConfigurePodSecondaryNetwork(t *testing.T) { ).Return(ovsPortUUID, nil) }, }, + { + name: "no IPAM", + networkType: vlanNetworkType, + noIPAM: true, + interfaceCreated: true, + expectedCalls: func(mockIPAM *podwatchtesting.MockIPAMAllocator, mockIC *podwatchtesting.MockInterfaceConfigurator) { + mockIC.EXPECT().ConfigureVLANSecondaryInterface( + podName, + testNamespace, + containerID, + containerNetNs(containerID), + interfaceName, + 1500, + uint16(0), + gomock.Any(), + ).Return(ovsPortUUID, nil) + }, + }, { name: "SRIOV network", networkType: sriovNetworkType, @@ -427,6 +458,24 @@ func TestConfigurePodSecondaryNetwork(t *testing.T) { }, expectedErr: "interface creation failure", }, + { + name: "interface failure with no IPAM", + networkType: vlanNetworkType, + noIPAM: true, + expectedCalls: func(mockIPAM *podwatchtesting.MockIPAMAllocator, mockIC *podwatchtesting.MockInterfaceConfigurator) { + mockIC.EXPECT().ConfigureVLANSecondaryInterface( + podName, + testNamespace, + containerID, + containerNetNs(containerID), + interfaceName, + 1500, + uint16(0), + gomock.Any(), + ).Return("", errors.New("interface creation failure")) + }, + expectedErr: "interface creation failure", + }, } for _, tc := range tests { @@ -435,7 +484,7 @@ func TestConfigurePodSecondaryNetwork(t *testing.T) { pc, mockIPAM, interfaceConfigurator := testPodController(ctrl) savedCNIConfig := *cniConfigInfo - network1 := testNetworkExt(networkName, tc.cniVersion, tc.cniType, string(tc.networkType), tc.ipamType, tc.mtu, tc.vlan) + 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{}) } @@ -499,7 +548,7 @@ func TestPodControllerAddPod(t *testing.T) { savedCNIConfig := *cniConfig network1 := testNetwork("net1", sriovNetworkType) testVLAN := 100 - network2 := testNetworkExt("net2", "", "", string(vlanNetworkType), "", defaultMTU, testVLAN) + network2 := testNetworkExt("net2", "", "", string(vlanNetworkType), "", defaultMTU, testVLAN, false) podOwner1 := &crdv1a2.PodOwner{ Name: podName,