Reorganize the code for configuring Windows networking into dedicated…

… package (#5159)

This commits reorganizes Windows-related code for configuring Windows networking
into a dedicated package `pkg/agent/util/winnet`, enhancing clarity and making it
easier for developers to locate and understand the Windows-specific functionality.
Besides, an interface implemented by the Windows-specific networking utility
functions is added to the package, facilitating unit testing and enhancing code
maintainability.

Furthermore, additional unit tests are added for the file
`pkg/agent/route/route_windows.go`, in order to enhance coverage.

Signed-off-by: Hongliang Liu <[email protected]>

hack/update-codegen-dockerized.sh

@@ -59,6 +59,7 @@ MOCKGEN_TARGETS=(
   "pkg/agent/util/iptables Interface testing mock_iptables_linux.go" # Must specify linux.go suffix, otherwise compilation would fail on windows platform as source file has linux build tag.
   "pkg/agent/util/netlink Interface testing mock_netlink_linux.go"
   "pkg/agent/wireguard Interface testing mock_wireguard.go"
+  "pkg/agent/util/winnet Interface testing mock_net_windows.go"
   "pkg/antctl AntctlClient ."
   "pkg/controller/networkpolicy EndpointQuerier,PolicyRuleQuerier testing"
   "pkg/controller/querier ControllerQuerier testing"

pkg/agent/agent_windows.go

@@ -30,15 +30,17 @@ import (
 	"antrea.io/antrea/pkg/agent/interfacestore"
 	"antrea.io/antrea/pkg/agent/util"
 	antreasyscall "antrea.io/antrea/pkg/agent/util/syscall"
+	"antrea.io/antrea/pkg/agent/util/winnet"
 	"antrea.io/antrea/pkg/apis/crd/v1alpha1"
 	"antrea.io/antrea/pkg/ovs/ovsconfig"
 	"antrea.io/antrea/pkg/ovs/ovsctl"
 	utilip "antrea.io/antrea/pkg/util/ip"
 )
 
 var (
+	winnetUtil winnet.Interface = &winnet.Handle{}
 	// setInterfaceMTU is meant to be overridden for testing
-	setInterfaceMTU = util.SetInterfaceMTU
+	setInterfaceMTU = winnetUtil.SetNetAdapterMTU
 
 	// setInterfaceARPAnnounce is meant to be overridden for testing.
 	setInterfaceARPAnnounce = func(ifaceName string, value int) error { return nil }
@@ -57,11 +59,11 @@ func (i *Initializer) prepareHNSNetworkAndOVSExtension() error {
 	hnsNetwork, err := hcsshim.GetHNSNetworkByName(util.LocalHNSNetwork)
 	if err == nil {
 		// Enable OVS Extension on the HNS Network.
-		if err = util.EnableHNSNetworkExtension(hnsNetwork.Id, util.OVSExtensionID); err != nil {
+		if err = util.EnableHNSNetworkExtension(hnsNetwork.Id, winnet.OVSExtensionID); err != nil {
 			return err
 		}
 		// Enable RSC for existing vSwitch.
-		if err = util.EnableRSCOnVSwitch(util.LocalHNSNetwork); err != nil {
+		if err = winnetUtil.EnableRSCOnVSwitch(util.LocalHNSNetwork); err != nil {
 			return err
 		}
 		// Save the uplink adapter name to check if the OVS uplink port has been created in prepareOVSBridge stage.
@@ -87,7 +89,7 @@ func (i *Initializer) prepareHNSNetworkAndOVSExtension() error {
 	// adapter was not found" if no adapter is provided and no physical adapter is available on the host.
 	// If the discovered adapter is virtual, it likely means the physical adapter is already attached to another
 	// HNSNetwork. For example, docker may create HNSNetworks which attach to the physical adapter.
-	isVirtual, err := util.IsVirtualAdapter(adapter.Name)
+	isVirtual, err := winnetUtil.IsVirtualNetAdapter(adapter.Name)
 	if err != nil {
 		return err
 	}
@@ -107,7 +109,7 @@ func (i *Initializer) prepareHNSNetworkAndOVSExtension() error {
 		klog.InfoS("No default gateway found on interface", "interface", adapter.Name)
 	}
 	i.nodeConfig.UplinkNetConfig.Gateway = defaultGW
-	dnsServers, err := util.GetDNServersByInterfaceIndex(adapter.Index)
+	dnsServers, err := winnetUtil.GetDNServersByNetAdapterIndex(adapter.Index)
 	if err != nil {
 		return err
 	}
@@ -128,12 +130,12 @@ func (i *Initializer) prepareHNSNetworkAndOVSExtension() error {
 func (i *Initializer) prepareVMNetworkAndOVSExtension() error {
 	klog.V(2).Info("Setting up VM network")
 	// Check whether VM Switch is created
-	exists, err := util.VMSwitchExists()
+	exists, err := winnetUtil.VMSwitchExists(util.LocalVMSwitch)
 	if err != nil {
 		return err
 	}
 	if exists {
-		vmSwitchIFName, err := util.GetVMSwitchInterfaceName()
+		vmSwitchIFName, err := winnetUtil.GetVMSwitchNetAdapterName(util.LocalVMSwitch)
 		if err != nil {
 			return err
 		}
@@ -168,20 +170,29 @@ func (i *Initializer) prepareVMNetworkAndOVSExtension() error {
 	}()
 
 	klog.V(2).InfoS("Creating VM switch", "uplinkIFName", uplinkIFName)
-	if err = util.CreateVMSwitch(uplinkIFName); err != nil {
+	if err = winnetUtil.AddVMSwitch(uplinkIFName, util.LocalVMSwitch); err != nil {
 		return fmt.Errorf("failed to create VM switch for interface %s: %v", uplinkIFName, err)
 	}
+	enabled, err := winnetUtil.IsVMSwitchOVSExtensionEnabled(util.LocalVMSwitch)
+	if err != nil {
+		return err
+	}
+	if !enabled {
+		if err = winnetUtil.EnableVMSwitchOVSExtension(util.LocalVMSwitch); err != nil {
+			return err
+		}
+	}
 
 	defer func() {
 		if !success {
-			if err = util.RemoveVMSwitch(); err != nil {
+			if err = winnetUtil.RemoveVMSwitch(util.LocalVMSwitch); err != nil {
 				klog.ErrorS(err, "Failed to remove VMSwitch")
 			}
 		}
 	}()
 
 	uplinkMACStr := strings.Replace(uplinkIface.HardwareAddr.String(), ":", "", -1)
-	if err = util.RenameVMNetworkAdapter(util.LocalVMSwitch, uplinkMACStr, hostIFName, true); err != nil {
+	if err = winnetUtil.RenameVMNetworkAdapter(util.LocalVMSwitch, uplinkMACStr, hostIFName, true); err != nil {
 		return fmt.Errorf("failed to rename VMNetworkAdapter as %s: %v", hostIFName, err)
 	}
 
@@ -298,7 +309,7 @@ func (i *Initializer) prepareOVSBridgeOnHNSNetwork() error {
 	// connection are routed to the selected backend Pod via the bridge interface; if we do not enable IP forwarding on
 	// the bridge interface, the packet will be discarded on the bridge interface as the destination of the packet
 	// is not the Node.
-	if err = util.EnableIPForwarding(brName); err != nil {
+	if err = winnetUtil.EnableIPForwarding(brName); err != nil {
 		return err
 	}
 	// Set the uplink with "no-flood" config, so that the IP of local Pods and "antrea-gw0" will not be leaked to the
@@ -395,11 +406,11 @@ func (i *Initializer) saveHostRoutes() error {
 	// IPv6 is not supported on Windows currently. Please refer to https://github.com/antrea-io/antrea/issues/5162
 	// for more information.
 	family := antreasyscall.AF_INET
-	filter := &util.Route{
+	filter := &winnet.Route{
 		LinkIndex:      i.nodeConfig.UplinkNetConfig.Index,
 		GatewayAddress: net.ParseIP(i.nodeConfig.UplinkNetConfig.Gateway),
 	}
-	routes, err := util.RouteListFiltered(family, filter, util.RT_FILTER_IF|util.RT_FILTER_GW)
+	routes, err := winnetUtil.RouteListFiltered(family, filter, winnet.RT_FILTER_IF|winnet.RT_FILTER_GW)
 	if err != nil {
 		return err
 	}

pkg/agent/cniserver/interface_configuration_windows.go

@@ -34,6 +34,7 @@ import (
 	"k8s.io/klog/v2"
 
 	"antrea.io/antrea/pkg/agent/util"
+	"antrea.io/antrea/pkg/agent/util/winnet"
 	cnipb "antrea.io/antrea/pkg/apis/cni/v1beta1"
 	"antrea.io/antrea/pkg/ovs/ovsconfig"
 )
@@ -45,7 +46,6 @@ const (
 var (
 	getHnsNetworkByNameFunc         = hcsshim.GetHNSNetworkByName
 	listHnsEndpointFunc             = hcsshim.HNSListEndpointRequest
-	setInterfaceMTUFunc             = util.SetInterfaceMTU
 	hostInterfaceExistsFunc         = util.HostInterfaceExists
 	getNetInterfaceAddrsFunc        = getNetInterfaceAddrs
 	createHnsEndpointFunc           = createHnsEndpoint
@@ -60,6 +60,7 @@ var (
 type ifConfigurator struct {
 	hnsNetwork *hcsshim.HNSNetwork
 	epCache    *sync.Map
+	winnet     winnet.Interface
 }
 
 // disableTXChecksumOffload is ignored on Windows.
@@ -80,6 +81,7 @@ func newInterfaceConfigurator(ovsDatapathType ovsconfig.OVSDatapathType, isOvsHa
 	return &ifConfigurator{
 		hnsNetwork: hnsNetwork,
 		epCache:    epCache,
+		winnet:     &winnet.Handle{},
 	}, nil
 }
 
@@ -177,8 +179,8 @@ func (ic *ifConfigurator) configureContainerLink(
 		// CmdAdd request is returned; 2) for Docker runtime, the interface is created after hcsshim.HotAttachEndpoint,
 		// and the hcsshim call is not synchronized from the observation.
 		return ic.addPostInterfaceCreateHook(infraContainerID, epName, containerAccess, func() error {
-			ifaceName := util.VirtualAdapterName(epName)
-			if err := setInterfaceMTUFunc(ifaceName, mtu); err != nil {
+			ifaceName := winnet.VirtualAdapterName(epName)
+			if err := ic.winnet.SetNetAdapterMTU(ifaceName, mtu); err != nil {
 				return fmt.Errorf("failed to configure MTU on container interface '%s': %v", ifaceName, err)
 			}
 			return nil
@@ -342,7 +344,7 @@ func (ic *ifConfigurator) checkContainerInterface(
 			containerIface.Sandbox, sandboxID)
 	}
 	hnsEP := strings.Split(containerIface.Name, "_")[0]
-	containerIfaceName := util.VirtualAdapterName(hnsEP)
+	containerIfaceName := winnet.VirtualAdapterName(hnsEP)
 	intf, err := getNetInterfaceByNameFunc(containerIfaceName)
 	if err != nil {
 		klog.Errorf("Failed to get container %s interface: %v", containerID, err)

pkg/agent/cniserver/server_windows_test.go

@@ -41,6 +41,7 @@ import (
 	routetest "antrea.io/antrea/pkg/agent/route/testing"
 	agenttypes "antrea.io/antrea/pkg/agent/types"
 	"antrea.io/antrea/pkg/agent/util"
+	winnettest "antrea.io/antrea/pkg/agent/util/winnet/testing"
 	cnipb "antrea.io/antrea/pkg/apis/cni/v1beta1"
 	ovsconfigtest "antrea.io/antrea/pkg/ovs/ovsconfig/testing"
 	"antrea.io/antrea/pkg/util/channel"
@@ -49,6 +50,8 @@ import (
 var (
 	containerMACStr = "23:34:56:23:22:45"
 	dnsSearches     = []string{"a.b.c.d"}
+
+	mockWinnet *winnettest.MockInterface
 )
 
 func TestUpdateResultDNSConfig(t *testing.T) {
@@ -258,6 +261,7 @@ func newMockCNIServer(t *testing.T, controller *gomock.Controller, podUpdateNoti
 	mockOVSBridgeClient = ovsconfigtest.NewMockOVSBridgeClient(controller)
 	mockOFClient = openflowtest.NewMockClient(controller)
 	mockRoute = routetest.NewMockInterface(controller)
+	mockWinnet = winnettest.NewMockInterface(controller)
 	ifaceStore = interfacestore.NewInterfaceStore()
 	cniServer := newCNIServer(t)
 	cniServer.routeClient = mockRoute
@@ -266,6 +270,7 @@ func newMockCNIServer(t *testing.T, controller *gomock.Controller, podUpdateNoti
 	gateway := &config.GatewayConfig{Name: "", IPv4: gwIPv4, MAC: gwMAC}
 	cniServer.nodeConfig = &config.NodeConfig{Name: "node1", PodIPv4CIDR: nodePodCIDRv4, GatewayConfig: gateway}
 	cniServer.podConfigurator, _ = newPodConfigurator(mockOVSBridgeClient, mockOFClient, mockRoute, ifaceStore, gwMAC, "system", false, false, podUpdateNotifier)
+	cniServer.podConfigurator.ifConfigurator.(*ifConfigurator).winnet = mockWinnet
 	return cniServer
 }
 
@@ -289,18 +294,13 @@ func prepareSetup(t *testing.T, ipamType string, name string, containerID, infra
 }
 
 func TestCmdAdd(t *testing.T) {
-	controller := gomock.NewController(t)
-	ipamType := "windows-test"
-	ipamMock := ipamtest.NewMockIPAMDriver(controller)
-	ipam.ResetIPAMDriver(ipamType, ipamMock)
 	oriIPAMResult := &ipam.IPAMResult{Result: *ipamResult}
 	ctx := context.TODO()
 
 	containerdInfraContainer := generateUUID()
 
 	defer mockHostInterfaceExists()()
 	defer mockGetHnsNetworkByName()()
-	defer mockSetInterfaceMTU(nil)()
 
 	for _, tc := range []struct {
 		name                 string
@@ -360,6 +360,11 @@ func TestCmdAdd(t *testing.T) {
 		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
+			controller := gomock.NewController(t)
+			ipamType := "windows-test"
+			ipamMock := ipamtest.NewMockIPAMDriver(controller)
+			ipam.ResetIPAMDriver(ipamType, ipamMock)
+
 			isDocker := isDockerContainer(tc.netns)
 			testUtil := newHnsTestUtil(generateUUID(), tc.existingHnsEndpoints, isDocker, tc.isAttached, tc.hnsEndpointCreateErr, tc.endpointAttachErr)
 			testUtil.setFunctions()
@@ -379,6 +384,9 @@ func TestCmdAdd(t *testing.T) {
 			if tc.ipamDel {
 				ipamMock.EXPECT().Del(gomock.Any(), gomock.Any(), gomock.Any()).Return(true, nil).Times(1)
 			}
+			if tc.endpointAttachErr == nil {
+				mockWinnet.EXPECT().SetNetAdapterMTU(gomock.Any(), gomock.Any()).Times(1)
+			}
 			ovsPortID := generateUUID()
 			if tc.connectOVS {
 				mockOVSBridgeClient.EXPECT().CreatePort(ovsPortName, ovsPortName, gomock.Any()).Return(ovsPortID, nil).Times(1)
@@ -431,18 +439,13 @@ func TestCmdAdd(t *testing.T) {
 }
 
 func TestCmdDel(t *testing.T) {
-	controller := gomock.NewController(t)
-	ipamType := "windows-test"
-	ipamMock := ipamtest.NewMockIPAMDriver(controller)
-	ipam.ResetIPAMDriver(ipamType, ipamMock)
 	ctx := context.TODO()
 
 	containerID := "261a1970-5b6c-11ed-8caf-000c294e5d03"
 	containerMAC, _ := net.ParseMAC("11:22:33:44:33:22")
 
 	defer mockHostInterfaceExists()()
 	defer mockGetHnsNetworkByName()()
-	defer mockSetInterfaceMTU(nil)()
 
 	for _, tc := range []struct {
 		name           string
@@ -476,6 +479,11 @@ func TestCmdDel(t *testing.T) {
 		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
+			controller := gomock.NewController(t)
+			ipamType := "windows-test"
+			ipamMock := ipamtest.NewMockIPAMDriver(controller)
+			ipam.ResetIPAMDriver(ipamType, ipamMock)
+
 			isDocker := isDockerContainer(tc.netns)
 			requestMsg, ovsPortName := prepareSetup(t, ipamType, testPodNameA, containerID, containerID, tc.netns, nil)
 			hnsEndpoint := getHnsEndpoint(generateUUID(), ovsPortName)
@@ -530,10 +538,6 @@ func TestCmdDel(t *testing.T) {
 }
 
 func TestCmdCheck(t *testing.T) {
-	controller := gomock.NewController(t)
-	ipamType := "windows-test"
-	ipamMock := ipamtest.NewMockIPAMDriver(controller)
-	ipam.ResetIPAMDriver(ipamType, ipamMock)
 	ctx := context.TODO()
 
 	containerNetns := generateUUID()
@@ -544,7 +548,6 @@ func TestCmdCheck(t *testing.T) {
 
 	defer mockHostInterfaceExists()()
 	defer mockGetHnsNetworkByName()()
-	defer mockSetInterfaceMTU(nil)()
 	defer mockListHnsEndpoint(nil, nil)()
 	defer mockGetNetInterfaceAddrs(containerIPNet, nil)()
 	defer mockGetHnsEndpointByName(generateUUID(), mac)()
@@ -661,6 +664,11 @@ func TestCmdCheck(t *testing.T) {
 		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
+			controller := gomock.NewController(t)
+			ipamType := "windows-test"
+			ipamMock := ipamtest.NewMockIPAMDriver(controller)
+			ipam.ResetIPAMDriver(ipamType, ipamMock)
+
 			defer mockGetNetInterfaceByName(tc.netInterface)()
 			cniserver := newMockCNIServer(t, controller, channel.NewSubscribableChannel("podUpdate", 100))
 			requestMsg, _ := prepareSetup(t, ipamType, tc.podName, tc.containerID, tc.containerID, tc.netns, tc.prevResult)
@@ -864,16 +872,3 @@ func mockListHnsEndpoint(endpoints []hcsshim.HNSEndpoint, listError error) func(
 		listHnsEndpointFunc = originalListHnsEndpoint
 	}
 }
-
-func mockSetInterfaceMTU(setMTUError error) func() {
-	originalSetInterfaceMTU := setInterfaceMTUFunc
-	setInterfaceMTUFunc = func(ifaceName string, mtu int) error {
-		if setMTUError == nil {
-			hostIfaces.Store(ifaceName, true)
-		}
-		return setMTUError
-	}
-	return func() {
-		setInterfaceMTUFunc = originalSetInterfaceMTU
-	}
-}

pkg/agent/externalnode/external_node_controller_windows.go

@@ -21,9 +21,12 @@ import (
 
 	"antrea.io/antrea/pkg/agent/config"
 	"antrea.io/antrea/pkg/agent/util"
+	"antrea.io/antrea/pkg/agent/util/winnet"
 	"antrea.io/antrea/pkg/signals"
 )
 
+var winnetUtil winnet.Interface = &winnet.Handle{}
+
 // moveIFConfigurations returns nil for single interface case, as it relies
 // on Windows New-VMSwitch command to create a host network adapter and copy
 // the uplink adapter configurations to host adapter.
@@ -45,7 +48,7 @@ func (c *ExternalNodeController) removeExternalNodeConfig() error {
 		klog.ErrorS(ovsErr, "Failed to delete OVS bridge")
 	}
 
-	if err := util.RemoveVMSwitch(); err != nil {
+	if err := winnetUtil.RemoveVMSwitch(util.LocalVMSwitch); err != nil {
 		return fmt.Errorf("failed to delete VM Switch, err: %v", err)
 	}
 	// Antrea Agent initializer creates a VM Switch corresponding to an