From 24cb4d4dd024572a1b9f5bca04c00ecbed479fdd Mon Sep 17 00:00:00 2001 From: Jianjun Shen Date: Wed, 31 Jan 2024 09:39:39 -0800 Subject: [PATCH] Refactor Windows CNI interface configuration (#5947) Define separate configureInterfaces funcs for Linux and Windows respectively. The Windows implementation first checks if the container interface has already been created for the infra container, and if yes returns the interfaces based on the information in the interface store. Signed-off-by: Jianjun Shen --- pkg/agent/cniserver/pod_configuration.go | 41 ++++--------------- .../cniserver/pod_configuration_linux.go | 9 ++++ .../cniserver/pod_configuration_windows.go | 39 ++++++++++++++++++ 3 files changed, 57 insertions(+), 32 deletions(-) diff --git a/pkg/agent/cniserver/pod_configuration.go b/pkg/agent/cniserver/pod_configuration.go index 908ee6eb30c..d545eb73691 100644 --- a/pkg/agent/cniserver/pod_configuration.go +++ b/pkg/agent/cniserver/pod_configuration.go @@ -194,29 +194,19 @@ func ParseOVSPortInterfaceConfig(portData *ovsconfig.OVSPortData, portConfig *in return interfaceConfig } -func (pc *podConfigurator) configureInterfaces( - podName string, - podNamespace string, - containerID string, - containerNetNS string, - containerIFDev string, - mtu int, - sriovVFDeviceID string, - result *ipam.IPAMResult, - createOVSPort bool, - containerAccess *containerAccessArbitrator, -) error { - err := pc.ifConfigurator.configureContainerLink(podName, podNamespace, containerID, containerNetNS, containerIFDev, mtu, sriovVFDeviceID, "", &result.Result, containerAccess) +func (pc *podConfigurator) configureInterfacesCommon( + podName, podNamespace, containerID, containerNetNS string, + containerIFDev string, mtu int, sriovVFDeviceID string, + result *ipam.IPAMResult, containerAccess *containerAccessArbitrator) error { + err := pc.ifConfigurator.configureContainerLink( + podName, podNamespace, containerID, containerNetNS, + containerIFDev, mtu, sriovVFDeviceID, "", &result.Result, containerAccess) if err != nil { return err } + hostIface := result.Interfaces[0] containerIface := result.Interfaces[1] - - if !createOVSPort { - return nil - } - // Delete veth pair if any failure occurs in later manipulation. success := false defer func() { @@ -225,19 +215,6 @@ func (pc *podConfigurator) configureInterfaces( } }() - // Check if the OVS configurations for the container exists or not. If yes, return immediately. This check is - // used on Windows, as kubelet on Windows will call CNI Add for infrastructure container for multiple times - // to query IP of Pod. But there should be only one OVS port created for the same Pod (identified by its sandbox - // container ID). And if the OVS port is added more than once, OVS will return an error. - // See https://github.com/kubernetes/kubernetes/issues/57253#issuecomment-358897721. - _, found := pc.ifaceStore.GetContainerInterface(containerID) - if found { - klog.V(2).Infof("Found an existing OVS port for container %s, returning", containerID) - // Mark the operation as successful, otherwise the container link might be removed by mistake. - success = true - return nil - } - var containerConfig *interfacestore.InterfaceConfig if containerConfig, err = pc.connectInterfaceToOVS(podName, podNamespace, containerID, containerNetNS, hostIface, containerIface, result.IPs, result.VLANID, containerAccess); err != nil { @@ -255,7 +232,7 @@ func (pc *podConfigurator) configureInterfaces( // Note that the IP address should be advertised after Pod OpenFlow entries are installed, otherwise the packet might // be dropped by OVS. - if err = pc.ifConfigurator.advertiseContainerAddr(containerNetNS, containerIface.Name, &result.Result); err != nil { + if err := pc.ifConfigurator.advertiseContainerAddr(containerNetNS, containerIface.Name, &result.Result); err != nil { // Do not return an error and fail the interface creation. klog.ErrorS(err, "Failed to advertise IP address for container", "container", containerID) } diff --git a/pkg/agent/cniserver/pod_configuration_linux.go b/pkg/agent/cniserver/pod_configuration_linux.go index 1ca96ff2c33..c692f541731 100644 --- a/pkg/agent/cniserver/pod_configuration_linux.go +++ b/pkg/agent/cniserver/pod_configuration_linux.go @@ -21,6 +21,7 @@ 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" ) @@ -42,6 +43,14 @@ func (pc *podConfigurator) connectInterfaceToOVS( return containerConfig, pc.connectInterfaceToOVSCommon(ovsPortName, netNS, containerConfig) } +func (pc *podConfigurator) configureInterfaces( + podName, podNamespace, containerID, containerNetNS string, + containerIFDev string, mtu int, sriovVFDeviceID string, + result *ipam.IPAMResult, createOVSPort bool, containerAccess *containerAccessArbitrator) error { + return pc.configureInterfacesCommon(podName, podNamespace, containerID, containerNetNS, + containerIFDev, mtu, sriovVFDeviceID, result, containerAccess) +} + func (pc *podConfigurator) reconcileMissingPods(ifConfigs []*interfacestore.InterfaceConfig, containerAccess *containerAccessArbitrator) { for i := range ifConfigs { // This should not happen since OVSDB is persisted on the Node. diff --git a/pkg/agent/cniserver/pod_configuration_windows.go b/pkg/agent/cniserver/pod_configuration_windows.go index 9259b156d79..6c574ad0306 100644 --- a/pkg/agent/cniserver/pod_configuration_windows.go +++ b/pkg/agent/cniserver/pod_configuration_windows.go @@ -23,6 +23,7 @@ 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/agent/types" "antrea.io/antrea/pkg/agent/util" @@ -102,6 +103,44 @@ func (pc *podConfigurator) connectInterfaceToOVS( return containerConfig, pc.connectInterfaceToOVSAsync(containerConfig, containerAccess) } +func (pc *podConfigurator) configureInterfaces( + podName, podNamespace, containerID, containerNetNS string, + containerIFDev string, mtu int, sriovVFDeviceID string, + result *ipam.IPAMResult, createOVSPort bool, containerAccess *containerAccessArbitrator) error { + if !createOVSPort { + return pc.ifConfigurator.configureContainerLink( + podName, podNamespace, containerID, containerNetNS, + containerIFDev, mtu, sriovVFDeviceID, "", + &result.Result, containerAccess) + } + // Check if the OVS configurations for the container exists or not. If yes, return + // immediately. This check is used on Windows, as kubelet on Windows will call CNI ADD + // multiple times for the infrastructure container to query IP of the Pod. But there should + // be only one OVS port created for the same Pod (identified by its sandbox container ID), + // and if the OVS port is added more than once, OVS will return an error. + // See: https://github.com/kubernetes/kubernetes/issues/57253#issuecomment-358897721. + interfaceConfig, found := pc.ifaceStore.GetContainerInterface(containerID) + if found { + klog.V(2).Infof("Found an existing OVS port for container %s, returning", containerID) + mac := interfaceConfig.MAC.String() + hostIface := ¤t.Interface{ + Name: interfaceConfig.InterfaceName, + Mac: mac, + Sandbox: "", + } + containerIface := ¤t.Interface{ + Name: containerIFDev, + Mac: mac, + Sandbox: containerNetNS, + } + result.Interfaces = []*current.Interface{hostIface, containerIface} + return nil + } + + return pc.configureInterfacesCommon(podName, podNamespace, containerID, containerNetNS, + containerIFDev, mtu, sriovVFDeviceID, result, containerAccess) +} + func (pc *podConfigurator) reconcileMissingPods(ifConfigs []*interfacestore.InterfaceConfig, containerAccess *containerAccessArbitrator) { for i := range ifConfigs { ifaceConfig := ifConfigs[i]