From ee8222f69b6f1eadfa916c2ab8ef63781b8705bd Mon Sep 17 00:00:00 2001 From: Shuyang Xin Date: Thu, 21 Dec 2023 11:53:21 +0800 Subject: [PATCH] Add DHCP IP Retries in PrepareHNSNetwork To address the potential race condition issue where acquiring a DHCP IP address may fail after CreateHNSNetwork, we added a retry mechanism to wait for an available IP. If the DHCP IP cannot be acquired within six seconds, a warning message will be returned. Signed-off-by: Shuyang Xin --- ci/jenkins/test.sh | 21 +++++ pkg/agent/util/net_windows.go | 41 ++++++++- pkg/agent/util/net_windows_test.go | 129 ++++++++++++++++------------- 3 files changed, 131 insertions(+), 60 deletions(-) diff --git a/ci/jenkins/test.sh b/ci/jenkins/test.sh index 84606cb0767..10b128b8a05 100755 --- a/ci/jenkins/test.sh +++ b/ci/jenkins/test.sh @@ -38,6 +38,7 @@ IP_MODE="" K8S_VERSION="1.28.2-00" WINDOWS_YAML_SUFFIX="windows" WIN_IMAGE_NODE="" +echo "" > WIN_DHCP GOLANG_RELEASE_DIR=${WORKDIR}/golang-releases WINDOWS_CONFORMANCE_FOCUS="\[sig-network\].+\[Conformance\]|\[sig-windows\]" @@ -256,6 +257,19 @@ function collect_windows_network_info_and_logs { tar zcf debug_logs.tar.gz "${DEBUG_LOG_PATH}" } +function check_dhcp_status { + echo "===== Check Interface DHCP Status =====" + WINIP=$(kubectl get nodes -o wide --no-headers=true | awk '$1 ~ /win-0/ {print $6}') + WIN_BRINT_DHCP=$(ssh -o StrictHostKeyChecking=no -n administrator@${WINIP} 'powershell.exe "(Get-NetIPInterface -InterfaceAlias br-int -AddressFamily IPv4).Dhcp"') + WIN_DHCP=$( WIN_DHCP rm -f antrea-windows.tar echo "==== Finish building and delivering Windows containerd images ====" } diff --git a/pkg/agent/util/net_windows.go b/pkg/agent/util/net_windows.go index c97e5399761..4180083ba48 100644 --- a/pkg/agent/util/net_windows.go +++ b/pkg/agent/util/net_windows.go @@ -66,6 +66,10 @@ const ( RT_FILTER_METRIC RT_FILTER_DST RT_FILTER_GW + + // IP_ADAPTER_DHCP_ENABLED is defined in the Win32 API document. + // https://learn.microsoft.com/en-us/windows/win32/api/iptypes/ns-iptypes-ip_adapter_addresses_lh + IP_ADAPTER_DHCP_ENABLED = 0x00000004 ) var ( @@ -460,10 +464,29 @@ func PrepareHNSNetwork(subnetCIDR *net.IPNet, nodeIPNet *net.IPNet, uplinkAdapte hnsNetworkDelete(hnsNet) } }() - - adapter, ipFound, err := adapterIPExists(nodeIPNet.IP, uplinkAdapter.HardwareAddr, ContainerVNICPrefix) + var adapter *net.Interface + var ipFound bool + // On the current Windows testbed, it takes a maximum of 1.8 seconds to obtain a valid IP. + // Therefore, we set the timeout limit to triple of that value, allowing a maximum wait of 6 seconds here. + err = wait.PollImmediate(1*time.Second, 6*time.Second, func() (bool, error) { + var checkErr error + adapter, ipFound, checkErr = adapterIPExists(nodeIPNet.IP, uplinkAdapter.HardwareAddr, ContainerVNICPrefix) + if checkErr != nil { + return false, checkErr + } + return ipFound, nil + }) if err != nil { - return err + if err == wait.ErrWaitTimeout { + dhcpStatus, err := InterfaceIPv4DhcpEnabled(uplinkAdapter.Name) + if err != nil { + klog.ErrorS(err, "Failed to get Ipv4 DHCP status on the network adapter", "adapter", uplinkAdapter.Name) + } else { + klog.ErrorS(err, "Timeout acquiring IP for the adapter", "dhcpStatus", dhcpStatus) + } + } else { + return err + } } vNicName, index := adapter.Name, adapter.Index // By default, "ipFound" should be true after Windows creates the HNSNetwork. The following check is for some corner @@ -647,6 +670,16 @@ func HostInterfaceExists(ifaceName string) bool { return true } +// InterfaceIPv4DhcpEnabled returns the Ipv4 DHCP status on the specified interface. +func InterfaceIPv4DhcpEnabled(ifaceName string) (bool, error) { + adapter, err := getAdapterInAllCompartmentsByName(ifaceName) + if err != nil { + return false, err + } + ipv4Dhcp := (adapter.flags&IP_ADAPTER_DHCP_ENABLED != 0) + return ipv4Dhcp, nil +} + // SetInterfaceMTU configures interface MTU on host for Pods. MTU change cannot be realized with HNSEndpoint because // there's no MTU field in HNSEndpoint: // https://github.com/Microsoft/hcsshim/blob/4a468a6f7ae547974bc32911395c51fb1862b7df/internal/hns/hnsendpoint.go#L12 @@ -1085,6 +1118,7 @@ type updateIPInterfaceFunc func(entry *antreasyscall.MibIPInterfaceRow) *antreas type adapter struct { net.Interface compartmentID uint32 + flags uint32 } func (a *adapter) setMTU(mtu int, family uint16) error { @@ -1250,6 +1284,7 @@ func getAdaptersByName(name string) ([]adapter, error) { adapter := adapter{ Interface: ifi, compartmentID: aa.CompartmentId, + flags: aa.Flags, } adapters = append(adapters, adapter) } diff --git a/pkg/agent/util/net_windows_test.go b/pkg/agent/util/net_windows_test.go index 743b478a0c3..4b7e2fcc934 100644 --- a/pkg/agent/util/net_windows_test.go +++ b/pkg/agent/util/net_windows_test.go @@ -307,6 +307,7 @@ func TestPrepareHNSNetwork(t *testing.T) { setVMCmd := fmt.Sprintf("Set-VMSwitch -ComputerName $(hostname) -Name %s -EnableSoftwareRsc $True", LocalHNSNetwork) tests := []struct { name string + testAdapterAddresses *windows.IpAdapterAddresses nodeIPNet *net.IPNet dnsServers string newName string @@ -320,42 +321,47 @@ func TestPrepareHNSNetwork(t *testing.T) { wantErr error }{ { - name: "Prepare Success", - nodeIPNet: &ipv4PublicIPNet, - dnsServers: testDNSServer, - newName: testNewName, - ipFound: true, + name: "Prepare Success", + testAdapterAddresses: createTestAdapterAddresses(testAdapterName), + nodeIPNet: &ipv4PublicIPNet, + dnsServers: testDNSServer, + newName: testNewName, + ipFound: true, wantCmds: []string{getAdapterCmd, renameAdapterCmd, renameNetCmd, getVMCmd, setVMCmd}, }, { - name: "Create Error", - nodeIPNet: &ipv4PublicIPNet, - dnsServers: testDNSServer, - ipFound: true, - hnsNetworkCreateErr: testInvalidErr, - wantErr: fmt.Errorf("error creating HNSNetwork: invalid"), + name: "Create Error", + testAdapterAddresses: createTestAdapterAddresses(testAdapterName), + nodeIPNet: &ipv4PublicIPNet, + dnsServers: testDNSServer, + ipFound: true, + hnsNetworkCreateErr: testInvalidErr, + wantErr: fmt.Errorf("error creating HNSNetwork: invalid"), }, { - name: "adapter Err", - nodeIPNet: &ipv4PublicIPNet, - dnsServers: testDNSServer, - ipFound: true, - testNetInterfaceErr: testInvalidErr, - wantErr: testInvalidErr, + name: "adapter Err", + testAdapterAddresses: createTestAdapterAddresses(testAdapterName), + nodeIPNet: &ipv4PublicIPNet, + dnsServers: testDNSServer, + ipFound: true, + testNetInterfaceErr: testInvalidErr, + wantErr: testInvalidErr, }, { - name: "Rename Err", - nodeIPNet: &ipv4PublicIPNet, - dnsServers: testDNSServer, - newName: testNewName, - ipFound: true, - commandErr: testInvalidErr, - wantCmds: []string{getAdapterCmd}, - wantErr: testInvalidErr, + name: "Rename Err", + testAdapterAddresses: createTestAdapterAddresses(testAdapterName), + nodeIPNet: &ipv4PublicIPNet, + dnsServers: testDNSServer, + newName: testNewName, + ipFound: true, + commandErr: testInvalidErr, + wantCmds: []string{getAdapterCmd}, + wantErr: testInvalidErr, }, { name: "Enable HNS Err", + testAdapterAddresses: createTestAdapterAddresses(testAdapterName), nodeIPNet: &ipv4PublicIPNet, dnsServers: testDNSServer, ipFound: true, @@ -363,46 +369,51 @@ func TestPrepareHNSNetwork(t *testing.T) { wantErr: testInvalidErr, }, { - name: "Enable RSC Err", - nodeIPNet: &ipv4PublicIPNet, - dnsServers: testDNSServer, - ipFound: true, - commandErr: testInvalidErr, - wantCmds: []string{getVMCmd}, - wantErr: testInvalidErr, + name: "Enable RSC Err", + testAdapterAddresses: createTestAdapterAddresses(testAdapterName), + nodeIPNet: &ipv4PublicIPNet, + dnsServers: testDNSServer, + ipFound: true, + commandErr: testInvalidErr, + wantCmds: []string{getVMCmd}, + wantErr: testInvalidErr, }, { - name: "IP Not Found", - nodeIPNet: &ipv4ZeroIPNet, - dnsServers: testDNSServer, - ipFound: false, - wantCmds: []string{newIPCmd, setServerCmd, getVMCmd, setVMCmd}, + name: "IP Not Found", + testAdapterAddresses: createTestAdapterAddresses(testAdapterName), + nodeIPNet: &ipv4ZeroIPNet, + dnsServers: testDNSServer, + ipFound: false, + wantCmds: []string{newIPCmd, setServerCmd, getVMCmd, setVMCmd}, }, { - name: "IP Not Found Configure Default Err", - nodeIPNet: &ipv4ZeroIPNet, - dnsServers: testDNSServer, - ipFound: false, - commandErr: testInvalidErr, - wantCmds: []string{newIPCmd}, - wantErr: testInvalidErr, + name: "IP Not Found Configure Default Err", + testAdapterAddresses: createTestAdapterAddresses(testAdapterName), + nodeIPNet: &ipv4ZeroIPNet, + dnsServers: testDNSServer, + ipFound: false, + commandErr: testInvalidErr, + wantCmds: []string{newIPCmd}, + wantErr: testInvalidErr, }, { - name: "IP Not Found Set adapter Err", - nodeIPNet: &ipv4ZeroIPNet, - dnsServers: testDNSServer, - ipFound: false, - commandErr: alreadyExistsErr, - wantCmds: []string{newIPCmd, setServerCmd}, - wantErr: alreadyExistsErr, + name: "IP Not Found Set adapter Err", + testAdapterAddresses: createTestAdapterAddresses(testAdapterName), + nodeIPNet: &ipv4ZeroIPNet, + dnsServers: testDNSServer, + ipFound: false, + commandErr: alreadyExistsErr, + wantCmds: []string{newIPCmd, setServerCmd}, + wantErr: alreadyExistsErr, }, { - name: "IP Not Found New Net Route Err", - nodeIPNet: &ipv4ZeroIPNet, - ipFound: false, - createRowErr: fmt.Errorf("ip route not found"), - wantCmds: []string{newIPCmd}, - wantErr: fmt.Errorf("failed to create new IPForward row: ip route not found"), + name: "IP Not Found New Net Route Err", + testAdapterAddresses: createTestAdapterAddresses(testAdapterName), + nodeIPNet: &ipv4ZeroIPNet, + ipFound: false, + createRowErr: fmt.Errorf("ip route not found"), + wantCmds: []string{newIPCmd}, + wantErr: fmt.Errorf("failed to create new IPForward row: ip route not found"), }, } @@ -415,6 +426,7 @@ func TestPrepareHNSNetwork(t *testing.T) { defer mockHNSNetworkCreate(tc.hnsNetworkCreateErr)() defer mockHNSNetworkDelete(nil)() defer mockAntreaNetIO(&antreasyscalltest.MockNetIO{CreateIPForwardEntryErr: tc.createRowErr})() + defer mockGetAdaptersAddresses(tc.testAdapterAddresses, nil)() gotErr := PrepareHNSNetwork(testSubnetCIDR, tc.nodeIPNet, testUplinkAdapter, "testGateway", tc.dnsServers, testRoutes, tc.newName) assert.Equal(t, tc.wantErr, gotErr) }) @@ -1076,6 +1088,7 @@ func TestGetAdapterInAllCompartmentsByName(t *testing.T) { HardwareAddr: testMACAddr, }, compartmentID: 1, + flags: IP_ADAPTER_DHCP_ENABLED, } tests := []struct { name string @@ -1139,6 +1152,7 @@ func createTestAdapterAddresses(name string) *windows.IpAdapterAddresses { PhysicalAddressLength: 6, PhysicalAddress: testPhysicalAddress, CompartmentId: 1, + Flags: IP_ADAPTER_DHCP_ENABLED, } } @@ -1171,6 +1185,7 @@ func mockGetAdaptersAddresses(testAdaptersAddresses *windows.IpAdapterAddresses, adapterAddresses.PhysicalAddressLength = testAdaptersAddresses.PhysicalAddressLength adapterAddresses.PhysicalAddress = testAdaptersAddresses.PhysicalAddress adapterAddresses.CompartmentId = testAdaptersAddresses.CompartmentId + adapterAddresses.Flags = testAdaptersAddresses.Flags } return err }