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 <[email protected]>
antrea-io · Jan 31, 2024 · 24cb4d4 · 24cb4d4
1 parent 003eb71
commit 24cb4d4
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 32 deletions.
diff --git 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
@@ -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
@@ -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 := &current.Interface{
+			Name:    interfaceConfig.InterfaceName,
+			Mac:     mac,
+			Sandbox: "",
+		}
+		containerIface := &current.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]