From 01e4ea0eb01486b07f391985b95fb90fa67e5815 Mon Sep 17 00:00:00 2001 From: Niha Nallappagari Date: Tue, 30 Sep 2025 15:41:52 -0500 Subject: [PATCH 1/7] Add registry keys for prefix on nic scenario --- cns/NetworkContainerContract.go | 1 + .../nodenetworkconfig/conversion.go | 1 + .../nodenetworkconfig/conversion_linux.go | 1 + cns/restserver/internalapi.go | 32 ++++++- cns/restserver/internalapi_linux.go | 10 ++ cns/restserver/internalapi_windows.go | 96 ++++++++++++++++++- 6 files changed, 139 insertions(+), 2 deletions(-) diff --git a/cns/NetworkContainerContract.go b/cns/NetworkContainerContract.go index 8f5939c28e..20d808897f 100644 --- a/cns/NetworkContainerContract.go +++ b/cns/NetworkContainerContract.go @@ -128,6 +128,7 @@ type CreateNetworkContainerRequest struct { AllowNCToHostCommunication bool EndpointPolicies []NetworkContainerRequestPolicies NCStatus v1alpha.NCStatus + SwiftV2PrefixOnNic bool // Indicates if is swiftv2 nc, PrefixOnNic scenario (isSwiftV2 && nc.Type == VNETBlock) NetworkInterfaceInfo NetworkInterfaceInfo //nolint // introducing new field for backendnic, to be used later by cni code } diff --git a/cns/kubecontroller/nodenetworkconfig/conversion.go b/cns/kubecontroller/nodenetworkconfig/conversion.go index f2f3d9e9cf..1fe66a2bb5 100644 --- a/cns/kubecontroller/nodenetworkconfig/conversion.go +++ b/cns/kubecontroller/nodenetworkconfig/conversion.go @@ -62,6 +62,7 @@ func CreateNCRequestFromDynamicNC(nc v1alpha.NetworkContainer) (*cns.CreateNetwo NetworkContainerid: nc.ID, NetworkContainerType: cns.Docker, Version: strconv.FormatInt(nc.Version, 10), //nolint:gomnd // it's decimal + SwiftV2PrefixOnNic: false, // Dynamic NCs don't use SwiftV2 PrefixOnNic IPConfiguration: cns.IPConfiguration{ IPSubnet: subnet, GatewayIPAddress: nc.DefaultGateway, diff --git a/cns/kubecontroller/nodenetworkconfig/conversion_linux.go b/cns/kubecontroller/nodenetworkconfig/conversion_linux.go index 9d425aa48f..c8e805c185 100644 --- a/cns/kubecontroller/nodenetworkconfig/conversion_linux.go +++ b/cns/kubecontroller/nodenetworkconfig/conversion_linux.go @@ -60,6 +60,7 @@ func createNCRequestFromStaticNCHelper(nc v1alpha.NetworkContainer, primaryIPPre GatewayIPv6Address: nc.DefaultGatewayV6, }, NCStatus: nc.Status, + SwiftV2PrefixOnNic: isSwiftV2 && nc.Type == v1alpha.VNETBlock, NetworkInterfaceInfo: cns.NetworkInterfaceInfo{ MACAddress: nc.MacAddress, }, diff --git a/cns/restserver/internalapi.go b/cns/restserver/internalapi.go index efefb3f2d3..0c8e9fc197 100644 --- a/cns/restserver/internalapi.go +++ b/cns/restserver/internalapi.go @@ -15,6 +15,7 @@ import ( "strconv" "strings" "time" + "runtime" "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/logger" @@ -194,6 +195,16 @@ var errNonExistentContainerStatus = errors.New("nonExistantContainerstatus") // all NCs and update the CNS state accordingly. This function returns the the total number of NCs on this VM that have been programmed to // some version, NOT the number of NCs that are up-to-date. func (service *HTTPRestService) syncHostNCVersion(ctx context.Context, channelMode string) (int, error) { + // will remove it later + // hasVNETBlockNC1 := service.isPrefixonNicSwiftV2() + // logger.Printf("winDebug: syncHostNCVersion hasVNETBlockNC1 %v", hasVNETBlockNC1) + // if runtime.GOOS == "windows" { + // logger.Printf("winDebug: before setPrefixOnNICRegistry in syncHostNCVersion") + // err := service.setPrefixOnNICRegistry(true, "aa:bb:cc:dd:ee:ff") + // if err != nil { + // logger.Debugf("failed to add PrefixOnNic keys to Windows registry: %w", err) + // } + // } outdatedNCs := map[string]struct{}{} programmedNCs := map[string]struct{}{} for idx := range service.state.ContainerStatus { @@ -219,7 +230,7 @@ func (service *HTTPRestService) syncHostNCVersion(ctx context.Context, channelMo programmedNCs[service.state.ContainerStatus[idx].ID] = struct{}{} } } - if len(outdatedNCs) == 0 { + if len(outdatedNCs) != 0 { return len(programmedNCs), nil } @@ -717,8 +728,27 @@ func (service *HTTPRestService) GetIMDSNCs(ctx context.Context) (map[string]stri if ncID != "" { ncs[ncID] = PrefixOnNicNCVersion // for prefix on nic version scenario nc version is 1 + } else if runtime.GOOS == "windows" && service.isPrefixonNicSwiftV2() { + err := service.setPrefixOnNICRegistry(true, iface.MacAddress.String()) + if err != nil { + logger.Debugf("failed to add PrefixOnNic keys to Windows registry: %w", err) + } } } return ncs, nil } + +// isPrefixonNicSwiftV2 checks if any NC in the container status should use SwiftV2 PrefixOnNic +// Uses the SwiftV2PrefixOnNic field which captures the condition: isSwiftV2 && nc.Type == VNETBlock +func (service *HTTPRestService) isPrefixonNicSwiftV2() bool { + for _, containerStatus := range service.state.ContainerStatus { + req := containerStatus.CreateNetworkContainerRequest + + // Check if this NC is SwiftV2 PrefixOnNic setting + if req.SwiftV2PrefixOnNic { + return true + } + } + return false +} \ No newline at end of file diff --git a/cns/restserver/internalapi_linux.go b/cns/restserver/internalapi_linux.go index 0abae0a72a..285f1abc80 100644 --- a/cns/restserver/internalapi_linux.go +++ b/cns/restserver/internalapi_linux.go @@ -181,3 +181,13 @@ func (service *HTTPRestService) programSNATRules(req *cns.CreateNetworkContainer func (service *HTTPRestService) setVFForAccelnetNICs() error { return nil } + +func (service *HTTPRestService) setPrefixOnNICRegistry(enabled bool, infraNicMacAddress string) error { + logger.Printf("[setPrefixOnNicEnabled winDebug] No-op on Linux platform") + return nil +} + +func (service *HTTPRestService) getPrefixOnNicEnabled() (bool, error) { + logger.Printf(" winDebug getPrefixOnNicEnabled is a no-op on non-Windows platforms") + return false, nil // Add this missing return statement +} \ No newline at end of file diff --git a/cns/restserver/internalapi_windows.go b/cns/restserver/internalapi_windows.go index 245053c8d6..8c66d1bb83 100644 --- a/cns/restserver/internalapi_windows.go +++ b/cns/restserver/internalapi_windows.go @@ -6,14 +6,20 @@ import ( "time" "github.com/Azure/azure-container-networking/cns" + "github.com/Azure/azure-container-networking/cns/logger" "github.com/Azure/azure-container-networking/cns/types" "github.com/Microsoft/hcsshim" "github.com/pkg/errors" + "golang.org/x/sys/windows/registry" ) const ( // timeout for powershell command to return the interfaces list - pwshTimeout = 120 * time.Second + pwshTimeout = 120 * time.Second + hnsRegistryPath = `SYSTEM\CurrentControlSet\Services\HNS\wcna_state\config` + prefixOnNicRegistryPath = `SYSTEM\CurrentControlSet\Services\HNS\wcna_state\config\PrefixOnNic` + infraNicIfName = "eth0" + enableSNAT = false ) var errUnsupportedAPI = errors.New("unsupported api") @@ -75,3 +81,91 @@ func (service *HTTPRestService) getPrimaryNICMACAddress() (string, error) { } return macAddress, nil } + +func (service *HTTPRestService) enablePrefixOnNic(enabled bool) error { + return service.setRegistryValue(prefixOnNicRegistryPath, "enabled", enabled) +} + +func (service *HTTPRestService) setInfraNicMacAddress(macAddress string) error { + return service.setRegistryValue(prefixOnNicRegistryPath, "infra_nic_mac_address", macAddress) +} + +func (service *HTTPRestService) setInfraNicIfName(ifName string) error { + return service.setRegistryValue(prefixOnNicRegistryPath, "infra_nic_ifname", ifName) +} + +func (service *HTTPRestService) setEnableSNAT(enabled bool) error { + return service.setRegistryValue(hnsRegistryPath, "EnableSNAT", enabled) +} + +func (service *HTTPRestService) setPrefixOnNICRegistry(enablePrefixOnNic bool, infraNicMacAddress string) error { + if err := service.enablePrefixOnNic(enablePrefixOnNic); err != nil { + return fmt.Errorf("failed to set enablePrefixOnNic key to windows registry: %w", err) + } + + if err := service.setInfraNicMacAddress(infraNicMacAddress); err != nil { + return fmt.Errorf("failed to set InfraNicMacAddress key to windows registry: %w", err) + } + + if err := service.setInfraNicIfName(infraNicIfName); err != nil { + return fmt.Errorf("failed to set InfraNicIfName key to windows registry: %w", err) + } + + if err := service.setEnableSNAT(enableSNAT); err != nil { + return fmt.Errorf("failed to set EnableSNAT key to windows registry: %w", err) + } + + return nil +} + +func (service *HTTPRestService) setRegistryValue(registryPath, keyName string, value interface{}) error { + key, _, err := registry.CreateKey(registry.LOCAL_MACHINE, registryPath, registry.SET_VALUE) + if err != nil { + return fmt.Errorf("failed to create/open registry key %s: %w", registryPath, err) + } + defer key.Close() + + switch v := value.(type) { + case string: + err = key.SetStringValue(keyName, v) + case bool: + dwordValue := uint32(0) + if v { + dwordValue = 1 + } + err = key.SetDWordValue(keyName, dwordValue) + case uint32: + err = key.SetDWordValue(keyName, v) + case int: + err = key.SetDWordValue(keyName, uint32(v)) + default: + return fmt.Errorf("unsupported value type for registry key %s: %T", keyName, value) + } + + if err != nil { + return fmt.Errorf("failed to set registry value '%s': %w", keyName, err) + } + + logger.Printf("[setRegistryValue] Set %s\\%s = %v", registryPath, keyName, value) + // have to remove this log later + // test, _ := service.getPrefixOnNicEnabled() + // logger.Printf("winDebug: setRegistryValue getPrefixOnNicEnabled %v", test) + return nil +} + +// for testing purpose, will remove it later + +// func (service *HTTPRestService) getPrefixOnNicEnabled() (bool, error) { +// key, err := registry.OpenKey(registry.LOCAL_MACHINE, prefixOnNicRegistryPath, registry.QUERY_VALUE) +// if err != nil { +// return false, nil // Key doesn't exist, default to false +// } +// defer key.Close() + +// value, _, err := key.GetIntegerValue("enabled") +// if err != nil { +// return false, nil // Value doesn't exist, default to false +// } + +// return value == 1, nil +// } From 942e8906e5691ca80ed710d630587bf746655dde Mon Sep 17 00:00:00 2001 From: Niha Nallappagari Date: Wed, 1 Oct 2025 09:35:07 -0500 Subject: [PATCH 2/7] update tests --- cns/restserver/internalapi.go | 6 +- cns/restserver/internalapi_test.go | 133 +++++++++++++++++++++++++++++ 2 files changed, 136 insertions(+), 3 deletions(-) diff --git a/cns/restserver/internalapi.go b/cns/restserver/internalapi.go index 0c8e9fc197..c7fd159662 100644 --- a/cns/restserver/internalapi.go +++ b/cns/restserver/internalapi.go @@ -230,7 +230,7 @@ func (service *HTTPRestService) syncHostNCVersion(ctx context.Context, channelMo programmedNCs[service.state.ContainerStatus[idx].ID] = struct{}{} } } - if len(outdatedNCs) != 0 { + if len(outdatedNCs) == 0 { return len(programmedNCs), nil } @@ -240,7 +240,7 @@ func (service *HTTPRestService) syncHostNCVersion(ctx context.Context, channelMo } // Get IMDS NC versions for delegated NIC scenarios - imdsNCVersions, err := service.GetIMDSNCs(ctx) + imdsNCVersions, err := service.getIMDSNCs(ctx) if err != nil { // If any of the NMA API check calls, imds calls fails assume that nma build doesn't have the latest changes and create empty map imdsNCVersions = make(map[string]string) @@ -696,7 +696,7 @@ func (service *HTTPRestService) isNCDetailsAPIExists(ctx context.Context) bool { } // GetIMDSNCs gets NC versions from IMDS and returns them as a map -func (service *HTTPRestService) GetIMDSNCs(ctx context.Context) (map[string]string, error) { +func (service *HTTPRestService) getIMDSNCs(ctx context.Context) (map[string]string, error) { imdsClient := service.imdsClient if imdsClient == nil { //nolint:staticcheck // SA1019: suppress deprecated logger.Printf usage. Todo: legacy logger usage is consistent in cns repo. Migrates when all logger usage is migrated diff --git a/cns/restserver/internalapi_test.go b/cns/restserver/internalapi_test.go index 4df797a498..6708ed55eb 100644 --- a/cns/restserver/internalapi_test.go +++ b/cns/restserver/internalapi_test.go @@ -15,6 +15,7 @@ import ( "sync" "testing" "time" + "runtime" "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/common" @@ -1680,3 +1681,135 @@ func setupIMDSMockAPIsWithCustomIDs(svc *HTTPRestService, interfaceIDs []string) // Return cleanup function return func() { svc.imdsClient = originalIMDS } } + +// TestSyncHostNCVersionWithWindowsSwiftV2 tests SyncHostNCVersion and verifies it calls Windows SwiftV2 PrefixOnNic scenario +func TestSyncHostNCVersionWithWindowsSwiftV2(t *testing.T) { + svc := getTestService(cns.Kubernetes) + + // Set up test NCs with different scenarios + regularNCID := "regular-nc-id" + swiftV2NCID := "swift-v2-vnet-block-nc" + + // Initialize ContainerStatus map if nil + if svc.state.ContainerStatus == nil { + svc.state.ContainerStatus = make(map[string]containerstatus) + } + + // Add a regular NC + svc.state.ContainerStatus[regularNCID] = containerstatus{ + ID: regularNCID, + CreateNetworkContainerRequest: cns.CreateNetworkContainerRequest{ + NetworkContainerid: regularNCID, + SwiftV2PrefixOnNic: false, + NetworkContainerType: cns.Docker, + Version: "2", + }, + HostVersion: "1", + } + + // Add a SwiftV2 VNETBlock NC that should trigger Windows registry operations + svc.state.ContainerStatus[swiftV2NCID] = containerstatus{ + ID: swiftV2NCID, + CreateNetworkContainerRequest: cns.CreateNetworkContainerRequest{ + NetworkContainerid: swiftV2NCID, + SwiftV2PrefixOnNic: true, + NetworkContainerType: cns.Docker, + Version: "2", + }, + HostVersion: "1", + } + + // Set up mock NMAgent with NC versions + mockNMA := &fakes.NMAgentClientFake{} + mockNMA.GetNCVersionListF = func(ctx context.Context) (nma.NCVersionList, error) { + return nma.NCVersionList{ + Containers: []nma.NCVersion{ + { + NetworkContainerID: regularNCID, + Version: "2", + }, + { + NetworkContainerID: swiftV2NCID, + Version: "2", + }, + }, + }, nil + } + svc.nma = mockNMA + + // Set up mock IMDS client for Windows SwiftV2 scenario + mac1, _ := net.ParseMAC("AA:BB:CC:DD:EE:FF") + mac2, _ := net.ParseMAC("11:22:33:44:55:66") + + interfaceMap := map[string]imds.NetworkInterface{ + "interface1": { + InterfaceCompartmentID: "", // Empty for Windows condition + MacAddress: imds.HardwareAddr(mac1), + }, + "interface2": { + InterfaceCompartmentID: "nc-with-compartment-id", + MacAddress: imds.HardwareAddr(mac2), + }, + } + mockIMDS := &mockIMDSAdapter{ + mock: &struct { + networkInterfaces func(_ context.Context) ([]imds.NetworkInterface, error) + imdsVersions func(_ context.Context) (*imds.APIVersionsResponse, error) + }{ + networkInterfaces: func(_ context.Context) ([]imds.NetworkInterface, error) { + var interfaces []imds.NetworkInterface + for _, iface := range interfaceMap { + interfaces = append(interfaces, iface) + } + return interfaces, nil + }, + imdsVersions: func(_ context.Context) (*imds.APIVersionsResponse, error) { + return &imds.APIVersionsResponse{ + APIVersions: []string{expectedIMDSAPIVersion}, + }, nil + }, + }, + } + + // Replace the IMDS client + originalIMDS := svc.imdsClient + svc.imdsClient = mockIMDS + defer func() { svc.imdsClient = originalIMDS }() + + // Verify preconditions + assert.True(t, svc.isPrefixonNicSwiftV2(), "isPrefixonNicSwiftV2() should return true") + + ctx := context.Background() + svc.SyncHostNCVersion(ctx, cns.CRD) + + // Verify that NC versions were updated + updatedRegularNC := svc.state.ContainerStatus[regularNCID] + updatedSwiftV2NC := svc.state.ContainerStatus[swiftV2NCID] + + assert.Equal(t, "2", updatedRegularNC.HostVersion, "Regular NC host version should be updated to 2") + assert.Equal(t, "2", updatedSwiftV2NC.HostVersion, "SwiftV2 NC host version should be updated to 2") + + imdsNCs, err := svc.getIMDSNCs(ctx) + assert.NoError(t, err, "getIMDSNCs should not return error") + + // Verify IMDS results + assert.Contains(t, imdsNCs, "nc-with-compartment-id", "NC with compartment ID should be in results") + assert.Equal(t, PrefixOnNicNCVersion, imdsNCs["nc-with-compartment-id"], "NC should have expected version") + + // Log the conditions that would trigger Windows registry operations + isWindows := runtime.GOOS == "windows" + hasSwiftV2PrefixOnNic := svc.isPrefixonNicSwiftV2() + + t.Logf("Windows SwiftV2 PrefixOnNic conditions: (runtime.GOOS == 'windows' && service.isPrefixonNicSwiftV2()): %t", + isWindows && hasSwiftV2PrefixOnNic) + + + // Test with no SwiftV2 NCs + delete(svc.state.ContainerStatus, swiftV2NCID) + assert.False(t, svc.isPrefixonNicSwiftV2(), "isPrefixonNicSwiftV2() should return false without SwiftV2 NCs") + + // Call getIMDSNCs again to verify condition is not triggered + _, err2 := svc.getIMDSNCs(ctx) + assert.NoError(t, err2, "getIMDSNCs should not return error") +} + From eed50805d30bba7802b137379c0fdb04ea49c94b Mon Sep 17 00:00:00 2001 From: Alexander <39818795+QxBytes@users.noreply.github.com> Date: Mon, 29 Sep 2025 09:16:33 -0700 Subject: [PATCH 3/7] ci: add iptables block signed image (#4049) * add iptables block to signed image * fix syntax * mariner version --- .../azure-iptables-monitor.Dockerfile | 1 + .pipelines/build/ob-prepare.steps.yaml | 4 ++ .../build/scripts/azure-iptables-monitor.sh | 55 +++++++++++++++++++ .pipelines/run-pipeline.yaml | 2 + 4 files changed, 62 insertions(+) diff --git a/.pipelines/build/dockerfiles/azure-iptables-monitor.Dockerfile b/.pipelines/build/dockerfiles/azure-iptables-monitor.Dockerfile index 0fe8fd4c1c..e256ce2838 100644 --- a/.pipelines/build/dockerfiles/azure-iptables-monitor.Dockerfile +++ b/.pipelines/build/dockerfiles/azure-iptables-monitor.Dockerfile @@ -14,5 +14,6 @@ ARG ARTIFACT_DIR COPY --from=iptables /usr/sbin/*tables* /usr/sbin/ COPY --from=iptables /usr/lib /usr/lib COPY ${ARTIFACT_DIR}/bin/azure-iptables-monitor /azure-iptables-monitor +COPY ${ARTIFACT_DIR}/bin/azure-block-iptables /azure-block-iptables ENTRYPOINT ["/azure-iptables-monitor"] diff --git a/.pipelines/build/ob-prepare.steps.yaml b/.pipelines/build/ob-prepare.steps.yaml index 863f92d246..6afc89497c 100644 --- a/.pipelines/build/ob-prepare.steps.yaml +++ b/.pipelines/build/ob-prepare.steps.yaml @@ -66,6 +66,10 @@ steps: echo "##vso[task.setvariable variable=azureIptablesMonitorVersion;isOutput=true]$AZUREIPTABLESMONITORVERSION" echo "azureIptablesMonitorVersion: $AZUREIPTABLESMONITORVERSION" + AZUREBLOCKIPTABLESVERSION=$(make azure-block-iptables-version) + echo "##vso[task.setvariable variable=azureBlockIptablesVersion;isOutput=true]$AZUREBLOCKIPTABLESVERSION" + echo "azureBlockIptablesVersion: $AZUREBLOCKIPTABLESVERSION" + CNIVERSION=$(make cni-version) echo "##vso[task.setvariable variable=cniVersion;isOutput=true]$CNIVERSION" echo "cniVersion: $CNIVERSION" diff --git a/.pipelines/build/scripts/azure-iptables-monitor.sh b/.pipelines/build/scripts/azure-iptables-monitor.sh index 5ef9daacb8..5f4eea65dd 100644 --- a/.pipelines/build/scripts/azure-iptables-monitor.sh +++ b/.pipelines/build/scripts/azure-iptables-monitor.sh @@ -5,6 +5,7 @@ set -eux FILE_EXT='' export CGO_ENABLED=0 +export C_INCLUDE_PATH=/usr/include/bpf mkdir -p "$OUT_DIR"/bin mkdir -p "$OUT_DIR"/files @@ -16,3 +17,57 @@ pushd "$REPO_ROOT"/azure-iptables-monitor -gcflags="-dwarflocationlists=true" \ . popd + +echo "Building azure-block-iptables binary..." + +# Debian/Ubuntu +if [[ -f /etc/debian_version ]]; then + + apt-get update -y + apt-get install -y --no-install-recommends llvm clang linux-libc-dev linux-headers-generic libbpf-dev libc6-dev nftables iproute2 + + if [[ $ARCH =~ amd64 ]]; then + apt-get install -y --no-install-recommends gcc-multilib + ARCH_GNU=x86_64-linux-gnu + elif [[ $ARCH =~ arm64 ]]; then + apt-get install -y --no-install-recommends gcc-aarch64-linux-gnu + ARCH_GNU=aarch64-linux-gnu + fi + + # Create symlinks for architecture-specific includes + for dir in /usr/include/"$ARCH_GNU"/*; do + if [[ -d "$dir" || -f "$dir" ]]; then + ln -sfn "$dir" /usr/include/$(basename "$dir") + fi + done + +# Mariner +else + tdnf install -y llvm clang libbpf-devel nftables gcc binutils iproute glibc + + if [[ $ARCH =~ amd64 ]]; then + ARCH_GNU=x86_64-linux-gnu + elif [[ $ARCH =~ arm64 ]]; then + ARCH_GNU=aarch64-linux-gnu + fi + + # Create symlinks for architecture-specific includes + for dir in /usr/include/"$ARCH_GNU"/*; do + if [[ -d "$dir" || -f "$dir" ]]; then + ln -sfn "$dir" /usr/include/$(basename "$dir") + fi + done +fi + +pushd "$REPO_ROOT" + # Generate BPF objects + GOOS="$OS" CGO_ENABLED=0 go generate ./bpf-prog/azure-block-iptables/... + + # Build the binary + GOOS="$OS" CGO_ENABLED=0 go build -a \ + -o "$OUT_DIR"/bin/azure-block-iptables"$FILE_EXT" \ + -trimpath \ + -ldflags "-s -w -X main.version=$AZURE_BLOCK_IPTABLES_VERSION" \ + -gcflags="-dwarflocationlists=true" \ + ./bpf-prog/azure-block-iptables/cmd/azure-block-iptables +popd diff --git a/.pipelines/run-pipeline.yaml b/.pipelines/run-pipeline.yaml index f759f11e8f..6ad478ef86 100644 --- a/.pipelines/run-pipeline.yaml +++ b/.pipelines/run-pipeline.yaml @@ -39,6 +39,7 @@ stages: AZURE_IPAM_VERSION: $[ stageDependencies.setup.env.outputs['EnvironmentalVariables.azureIpamVersion'] ] AZURE_IP_MASQ_MERGER_VERSION: $[ stageDependencies.setup.env.outputs['EnvironmentalVariables.azureIpMasqMergerVersion'] ] AZURE_IPTABLES_MONITOR_VERSION: $[ stageDependencies.setup.env.outputs['EnvironmentalVariables.azureIptablesMonitorVersion'] ] + AZURE_BLOCK_IPTABLES_VERSION: $[ stageDependencies.setup.env.outputs['EnvironmentalVariables.azureBlockIptablesVersion'] ] CNI_VERSION: $[ stageDependencies.setup.env.outputs['EnvironmentalVariables.cniVersion'] ] CNS_VERSION: $[ stageDependencies.setup.env.outputs['EnvironmentalVariables.cnsVersion'] ] IPV6_HP_BPF_VERSION: $[ stageDependencies.setup.env.outputs['EnvironmentalVariables.ipv6HpBpfVersion'] ] @@ -204,6 +205,7 @@ stages: AZURE_IPAM_VERSION: $[ stageDependencies.setup.env.outputs['EnvironmentalVariables.azureIpamVersion'] ] AZURE_IP_MASQ_MERGER_VERSION: $[ stageDependencies.setup.env.outputs['EnvironmentalVariables.azureIpMasqMergerVersion'] ] AZURE_IPTABLES_MONITOR_VERSION: $[ stageDependencies.setup.env.outputs['EnvironmentalVariables.azureIptablesMonitorVersion'] ] + AZURE_BLOCK_IPTABLES_VERSION: $[ stageDependencies.setup.env.outputs['EnvironmentalVariables.azureBlockIptablesVersion'] ] CNI_VERSION: $[ stageDependencies.setup.env.outputs['EnvironmentalVariables.cniVersion'] ] CNS_VERSION: $[ stageDependencies.setup.env.outputs['EnvironmentalVariables.cnsVersion'] ] IPV6_HP_BPF_VERSION: $[ stageDependencies.setup.env.outputs['EnvironmentalVariables.ipv6HpBpfVersion'] ] From ec4f35900c449d45d7ae3477c1bb52d38ae29a35 Mon Sep 17 00:00:00 2001 From: NihaNallappagari <69693983+NihaNallappagari@users.noreply.github.com> Date: Mon, 29 Sep 2025 14:57:09 -0500 Subject: [PATCH 4/7] Prefix on nicv6 support (#3658) * Prefix on NIC v6 support * update dedup comment in ipam * add log for a gatway error scenario * handle synchostversion sync on pon swiftv2 nic * handle gatewayip parse failure or nil scenario * remove unused code Address PR comments remove unnecessary logger update synhost skip test scenario fix linting error updated lint error fix linting error . update synhost skip test scenario fix linting error updated lint error fix linting error . * add test scenario * update test to handle gatway nil case * remove synchost skip and single ip skip . . * Add missing test scenarios * fix ipam_test conflicts * fix conflits small fixes fix lint errors lint fix . Delete examples/imds_nc_lookup.go Signed-off-by: NihaNallappagari <69693983+NihaNallappagari@users.noreply.github.com> fix lint . * fix linting * add todo and remove multiple interface scenario * Skip add primary ip's to secondary config for swiftv2 pon scenarios fix lint error . * code review comment * Update gateway ipv6 to use default value, that auto detects and adds to neigh table * remove unwanted changes * address ipfamily comment * fix test failure * Handle nilgateway scenario * remove unnedded else block * comments changes --------- Co-authored-by: Kaushik Vuligonda Co-authored-by: nn052161 --- azure-ipam/ipam.go | 26 +- azure-ipam/ipam_test.go | 151 +++- azure-ipam/ipconfig/ipconfig.go | 29 +- cns/NetworkContainerContract.go | 15 +- .../nodenetworkconfig/conversion.go | 4 +- .../nodenetworkconfig/conversion_linux.go | 21 +- .../nodenetworkconfig/conversion_test.go | 136 ++- .../nodenetworkconfig/conversion_windows.go | 15 +- .../nodenetworkconfig/reconciler.go | 6 +- .../nodenetworkconfig/reconciler_test.go | 4 +- cns/restserver/internalapi.go | 1 + cns/restserver/internalapi_linux.go | 6 + cns/restserver/ipam.go | 92 +- cns/restserver/ipam_test.go | 825 ++++++++++++------ cns/restserver/util.go | 1 + cns/service/main.go | 8 +- 16 files changed, 1012 insertions(+), 328 deletions(-) diff --git a/azure-ipam/ipam.go b/azure-ipam/ipam.go index e71782ebd8..fdb09c4669 100644 --- a/azure-ipam/ipam.go +++ b/azure-ipam/ipam.go @@ -115,7 +115,7 @@ func (p *IPAMPlugin) CmdAdd(args *cniSkel.CmdArgs) error { p.logger.Debug("Received CNS IP config response", zap.Any("response", resp)) // Get Pod IP and gateway IP from ip config response - podIPNet, err := ipconfig.ProcessIPConfigsResp(resp) + podIPNet, gatewayIP, err := ipconfig.ProcessIPConfigsResp(resp) if err != nil { p.logger.Error("Failed to interpret CNS IPConfigResponse", zap.Error(err), zap.Any("response", resp)) return cniTypes.NewError(ErrProcessIPConfigResponse, err.Error(), "failed to interpret CNS IPConfigResponse") @@ -136,9 +136,33 @@ func (p *IPAMPlugin) CmdAdd(args *cniSkel.CmdArgs) error { Mask: net.CIDRMask(ipNet.Bits(), 128), // nolint } } + ipConfig.Gateway = (*gatewayIP)[i] cniResult.IPs[i] = ipConfig } + cniResult.Interfaces = []*types100.Interface{} + seenInterfaces := map[string]bool{} + + for _, podIPInfo := range resp.PodIPInfo { + // Skip if interface already seen + // This is to avoid duplicate interfaces in the result + // Deduplication is necessary because there is one podIPInfo entry for each IP family(IPv4 and IPv6), and both may point to the same interface + if podIPInfo.MacAddress == "" || seenInterfaces[podIPInfo.MacAddress] { + continue + } + + infMac, err := net.ParseMAC(podIPInfo.MacAddress) + if err != nil { + p.logger.Error("Failed to parse interface MAC address", zap.Error(err), zap.String("macAddress", podIPInfo.MacAddress)) + return cniTypes.NewError(cniTypes.ErrUnsupportedField, err.Error(), "failed to parse interface MAC address") + } + + cniResult.Interfaces = append(cniResult.Interfaces, &types100.Interface{ + Mac: infMac.String(), + }) + seenInterfaces[podIPInfo.MacAddress] = true + } + // Get versioned result versionedCniResult, err := cniResult.GetAsVersion(nwCfg.CNIVersion) if err != nil { diff --git a/azure-ipam/ipam_test.go b/azure-ipam/ipam_test.go index ecae57606c..4fa93bd71c 100644 --- a/azure-ipam/ipam_test.go +++ b/azure-ipam/ipam_test.go @@ -58,6 +58,60 @@ func (c *MockCNSClient) RequestIPAddress(ctx context.Context, ipconfig cns.IPCon }, } return result, nil + case "nilGateway": + result := &cns.IPConfigResponse{ + PodIpInfo: cns.PodIpInfo{ + PodIPConfig: cns.IPSubnet{ + IPAddress: "10.0.1.10", + PrefixLength: 24, + }, + NetworkContainerPrimaryIPConfig: cns.IPConfiguration{ + IPSubnet: cns.IPSubnet{ + IPAddress: "10.0.1.0", + PrefixLength: 24, + }, + DNSServers: nil, + GatewayIPAddress: "", // nil/empty gateway + }, + HostPrimaryIPInfo: cns.HostIPInfo{ + Gateway: "", + PrimaryIP: "10.0.0.1", + Subnet: "10.0.0.0/24", + }, + }, + Response: cns.Response{ + ReturnCode: 0, + Message: "", + }, + } + return result, nil + case "invalidGateway": + result := &cns.IPConfigResponse{ + PodIpInfo: cns.PodIpInfo{ + PodIPConfig: cns.IPSubnet{ + IPAddress: "10.0.1.10", + PrefixLength: 24, + }, + NetworkContainerPrimaryIPConfig: cns.IPConfiguration{ + IPSubnet: cns.IPSubnet{ + IPAddress: "10.0.1.0", + PrefixLength: 24, + }, + DNSServers: nil, + GatewayIPAddress: "invalidgatewayip", + }, + HostPrimaryIPInfo: cns.HostIPInfo{ + Gateway: "invalidgatewayip", + PrimaryIP: "10.0.0.1", + Subnet: "10.0.0.0/24", + }, + }, + Response: cns.Response{ + ReturnCode: 0, + Message: "", + }, + } + return result, nil default: result := &cns.IPConfigResponse{ PodIpInfo: cns.PodIpInfo{ @@ -65,6 +119,7 @@ func (c *MockCNSClient) RequestIPAddress(ctx context.Context, ipconfig cns.IPCon IPAddress: "10.0.1.10", PrefixLength: 24, }, + MacAddress: "00:11:22:33:44:55", NetworkContainerPrimaryIPConfig: cns.IPConfiguration{ IPSubnet: cns.IPSubnet{ IPAddress: "10.0.1.0", @@ -92,11 +147,60 @@ func (c *MockCNSClient) RequestIPs(ctx context.Context, ipconfig cns.IPConfigsRe switch ipconfig.InfraContainerID { case "failRequestCNSArgs": return nil, errFoo - case "happyArgsSingle", "failProcessCNSRespSingleIP", "failRequestCNSArgsSingleIP": + case "happyArgsSingle", "failProcessCNSRespSingleIP", "failRequestCNSArgsSingleIP", "nilGateway", "invalidGateway": e := &client.CNSClientError{} e.Code = types.UnsupportedAPI e.Err = errUnsupportedAPI return nil, e + case "happyArgsDual": + result := &cns.IPConfigsResponse{ + PodIPInfo: []cns.PodIpInfo{ + { + PodIPConfig: cns.IPSubnet{ + IPAddress: "10.0.1.10", + PrefixLength: 24, + }, + MacAddress: "00:11:22:33:44:55", + NetworkContainerPrimaryIPConfig: cns.IPConfiguration{ + IPSubnet: cns.IPSubnet{ + IPAddress: "10.0.1.0", + PrefixLength: 24, + }, + DNSServers: nil, + GatewayIPAddress: "10.0.0.1", + }, + HostPrimaryIPInfo: cns.HostIPInfo{ + Gateway: "10.0.0.1", + PrimaryIP: "10.0.0.1", + Subnet: "10.0.0.0/24", + }, + }, + { + PodIPConfig: cns.IPSubnet{ + IPAddress: "fd11:1234::1", + PrefixLength: 120, + }, + MacAddress: "00:11:22:33:44:55", // Same MAC for dual-stack scenario + NetworkContainerPrimaryIPConfig: cns.IPConfiguration{ + IPSubnet: cns.IPSubnet{ + IPAddress: "fd11:1234::", + PrefixLength: 120, + }, + DNSServers: nil, + }, + HostPrimaryIPInfo: cns.HostIPInfo{ + Gateway: "fe80::1234:5678:9abc", + PrimaryIP: "fe80::1234:5678:9abc", + Subnet: "fd11:1234::/120", + }, + }, + }, + Response: cns.Response{ + ReturnCode: 0, + Message: "", + }, + } + return result, nil case "failProcessCNSResp": result := &cns.IPConfigsResponse{ PodIPInfo: []cns.PodIpInfo{ @@ -129,8 +233,7 @@ func (c *MockCNSClient) RequestIPs(ctx context.Context, ipconfig cns.IPConfigsRe IPAddress: "fd11:1234::", PrefixLength: 112, }, - DNSServers: nil, - GatewayIPAddress: "fe80::1234:5678:9abc", + DNSServers: nil, }, HostPrimaryIPInfo: cns.HostIPInfo{ Gateway: "fe80::1234:5678:9abc", @@ -153,6 +256,7 @@ func (c *MockCNSClient) RequestIPs(ctx context.Context, ipconfig cns.IPConfigsRe IPAddress: "10.0.1.10", PrefixLength: 24, }, + MacAddress: "00:11:22:33:44:55", NetworkContainerPrimaryIPConfig: cns.IPConfiguration{ IPSubnet: cns.IPSubnet{ IPAddress: "10.0.1.0", @@ -172,13 +276,13 @@ func (c *MockCNSClient) RequestIPs(ctx context.Context, ipconfig cns.IPConfigsRe IPAddress: "fd11:1234::1", PrefixLength: 120, }, + MacAddress: "00:11:22:33:44:55", // Same MAC for dual-stack scenario NetworkContainerPrimaryIPConfig: cns.IPConfiguration{ IPSubnet: cns.IPSubnet{ IPAddress: "fd11:1234::", PrefixLength: 120, }, - DNSServers: nil, - GatewayIPAddress: "fe80::1234:5678:9abc", + DNSServers: nil, }, HostPrimaryIPInfo: cns.HostIPInfo{ Gateway: "fe80::1234:5678:9abc", @@ -281,12 +385,18 @@ func TestCmdAdd(t *testing.T) { args: buildArgs("happyArgsSingle", happyPodArgs, happyNetConfByteArr), want: &types100.Result{ CNIVersion: "1.0.0", + Interfaces: []*types100.Interface{ + { + Mac: "00:11:22:33:44:55", + }, + }, IPs: []*types100.IPConfig{ { Address: net.IPNet{ IP: net.IPv4(10, 0, 1, 10), Mask: net.CIDRMask(24, 32), }, + Gateway: net.IPv4(10, 0, 0, 1), }, }, DNS: cniTypes.DNS{}, @@ -298,24 +408,55 @@ func TestCmdAdd(t *testing.T) { args: buildArgs("happyArgsDual", happyPodArgs, happyNetConfByteArr), want: &types100.Result{ CNIVersion: "1.0.0", + Interfaces: []*types100.Interface{ + { + Mac: "00:11:22:33:44:55", // Single interface for dual-stack + }, + }, IPs: []*types100.IPConfig{ { Address: net.IPNet{ IP: net.IPv4(10, 0, 1, 10), Mask: net.CIDRMask(24, 32), }, + Gateway: net.IPv4(10, 0, 0, 1), }, { Address: net.IPNet{ IP: net.ParseIP("fd11:1234::1"), Mask: net.CIDRMask(120, 128), }, + Gateway: net.ParseIP("fe80::1234:5678:9abc"), }, }, DNS: cniTypes.DNS{}, }, wantErr: false, }, + { + name: "CNI add with nil gateway IP", + args: buildArgs("nilGateway", happyPodArgs, happyNetConfByteArr), + want: &types100.Result{ + CNIVersion: "1.0.0", + Interfaces: nil, + IPs: []*types100.IPConfig{ + { + Address: net.IPNet{ + IP: net.IPv4(10, 0, 1, 10), + Mask: net.CIDRMask(24, 32), + }, + Gateway: nil, // No gateway + }, + }, + DNS: cniTypes.DNS{}, + }, + wantErr: false, + }, + { + name: "CNI add with invalid gateway IP", + args: buildArgs("invalidGateway", happyPodArgs, happyNetConfByteArr), + wantErr: true, + }, { name: "Fail request CNS ipconfig during CmdAdd", args: buildArgs("failRequestCNSArgs", happyPodArgs, happyNetConfByteArr), diff --git a/azure-ipam/ipconfig/ipconfig.go b/azure-ipam/ipconfig/ipconfig.go index 9b2ecc97f4..d090935273 100644 --- a/azure-ipam/ipconfig/ipconfig.go +++ b/azure-ipam/ipconfig/ipconfig.go @@ -3,6 +3,7 @@ package ipconfig import ( "encoding/json" "fmt" + "net" "net/netip" "github.com/Azure/azure-container-networking/cns" @@ -11,6 +12,10 @@ import ( "github.com/pkg/errors" ) +const ( + defaultV6Gateway = "fe80::1234:5678:9abc" +) + func CreateOrchestratorContext(args *cniSkel.CmdArgs) ([]byte, error) { podConf, err := parsePodConf(args.Args) if err != nil { @@ -63,10 +68,13 @@ func CreateIPConfigsReq(args *cniSkel.CmdArgs) (cns.IPConfigsRequest, error) { return req, nil } -func ProcessIPConfigsResp(resp *cns.IPConfigsResponse) (*[]netip.Prefix, error) { +func ProcessIPConfigsResp(resp *cns.IPConfigsResponse) (*[]netip.Prefix, *[]net.IP, error) { podIPNets := make([]netip.Prefix, len(resp.PodIPInfo)) + gatewaysIPs := make([]net.IP, len(resp.PodIPInfo)) for i := range resp.PodIPInfo { + var gatewayIP net.IP + podCIDR := fmt.Sprintf( "%s/%d", resp.PodIPInfo[i].PodIPConfig.IPAddress, @@ -74,12 +82,27 @@ func ProcessIPConfigsResp(resp *cns.IPConfigsResponse) (*[]netip.Prefix, error) ) podIPNet, err := netip.ParsePrefix(podCIDR) if err != nil { - return nil, errors.Wrapf(err, "cns returned invalid pod CIDR %q", podCIDR) + return nil, nil, errors.Wrapf(err, "cns returned invalid pod CIDR %q", podCIDR) } podIPNets[i] = podIPNet + + var gatewayStr string + if podIPNet.Addr().Is4() { + gatewayStr = resp.PodIPInfo[i].NetworkContainerPrimaryIPConfig.GatewayIPAddress + } else if podIPNet.Addr().Is6() { + gatewayStr = defaultV6Gateway + } + + if gatewayStr != "" { + gatewayIP = net.ParseIP(gatewayStr) + if gatewayIP == nil { + return nil, nil, errors.Errorf("failed to parse gateway IP %q for pod ip %s", gatewayStr, resp.PodIPInfo[i].PodIPConfig.IPAddress) + } + } + gatewaysIPs[i] = gatewayIP } - return &podIPNets, nil + return &podIPNets, &gatewaysIPs, nil } type k8sPodEnvArgs struct { diff --git a/cns/NetworkContainerContract.go b/cns/NetworkContainerContract.go index 406c45b554..8f5939c28e 100644 --- a/cns/NetworkContainerContract.go +++ b/cns/NetworkContainerContract.go @@ -398,9 +398,10 @@ type NetworkInterfaceInfo struct { // IPConfiguration contains details about ip config to provision in the VM. type IPConfiguration struct { - IPSubnet IPSubnet - DNSServers []string - GatewayIPAddress string + IPSubnet IPSubnet + DNSServers []string + GatewayIPAddress string + GatewayIPv6Address string } // SecondaryIPConfig contains IP info of SecondaryIP @@ -755,3 +756,11 @@ type NodeRegisterRequest struct { NumCores int NmAgentSupportedApis []string } + +// IPFamily - Enum for determining IPFamily when retrieving IPs from network containers +type IPFamily string + +const ( + IPv4 IPFamily = "ipv4" + IPv6 IPFamily = "ipv6" +) diff --git a/cns/kubecontroller/nodenetworkconfig/conversion.go b/cns/kubecontroller/nodenetworkconfig/conversion.go index 01bda6780e..f2f3d9e9cf 100644 --- a/cns/kubecontroller/nodenetworkconfig/conversion.go +++ b/cns/kubecontroller/nodenetworkconfig/conversion.go @@ -73,7 +73,7 @@ func CreateNCRequestFromDynamicNC(nc v1alpha.NetworkContainer) (*cns.CreateNetwo // CreateNCRequestFromStaticNC generates a CreateNetworkContainerRequest from a static NetworkContainer. // //nolint:gocritic //ignore hugeparam -func CreateNCRequestFromStaticNC(nc v1alpha.NetworkContainer) (*cns.CreateNetworkContainerRequest, error) { +func CreateNCRequestFromStaticNC(nc v1alpha.NetworkContainer, isSwiftV2 bool) (*cns.CreateNetworkContainerRequest, error) { if nc.Type == v1alpha.Overlay { nc.Version = 0 // fix for NMA always giving us version 0 for Overlay NCs } @@ -97,7 +97,7 @@ func CreateNCRequestFromStaticNC(nc v1alpha.NetworkContainer) (*cns.CreateNetwor subnet.IPAddress = primaryPrefix.Addr().String() } - req, err := createNCRequestFromStaticNCHelper(nc, primaryPrefix, subnet) + req, err := createNCRequestFromStaticNCHelper(nc, primaryPrefix, subnet, isSwiftV2) if err != nil { return nil, errors.Wrapf(err, "error while creating NC request from static NC") } diff --git a/cns/kubecontroller/nodenetworkconfig/conversion_linux.go b/cns/kubecontroller/nodenetworkconfig/conversion_linux.go index c89d41646b..9d425aa48f 100644 --- a/cns/kubecontroller/nodenetworkconfig/conversion_linux.go +++ b/cns/kubecontroller/nodenetworkconfig/conversion_linux.go @@ -13,15 +13,18 @@ import ( // by adding all IPs in the the block to the secondary IP configs list. It does not skip any IPs. // //nolint:gocritic //ignore hugeparam -func createNCRequestFromStaticNCHelper(nc v1alpha.NetworkContainer, primaryIPPrefix netip.Prefix, subnet cns.IPSubnet) (*cns.CreateNetworkContainerRequest, error) { +func createNCRequestFromStaticNCHelper(nc v1alpha.NetworkContainer, primaryIPPrefix netip.Prefix, subnet cns.IPSubnet, isSwiftV2 bool) (*cns.CreateNetworkContainerRequest, error) { secondaryIPConfigs := map[string]cns.SecondaryIPConfig{} // iterate through all IP addresses in the subnet described by primaryPrefix and // add them to the request as secondary IPConfigs. - for addr := primaryIPPrefix.Masked().Addr(); primaryIPPrefix.Contains(addr); addr = addr.Next() { - secondaryIPConfigs[addr.String()] = cns.SecondaryIPConfig{ - IPAddress: addr.String(), - NCVersion: int(nc.Version), + // Process primary prefix IPs in all scenarios except when nc.Type is v1alpha.VNETBlock AND SwiftV2 is enabled + if !(isSwiftV2 && nc.Type == v1alpha.VNETBlock) { + for addr := primaryIPPrefix.Masked().Addr(); primaryIPPrefix.Contains(addr); addr = addr.Next() { + secondaryIPConfigs[addr.String()] = cns.SecondaryIPConfig{ + IPAddress: addr.String(), + NCVersion: int(nc.Version), + } } } @@ -52,9 +55,13 @@ func createNCRequestFromStaticNCHelper(nc v1alpha.NetworkContainer, primaryIPPre NetworkContainerType: cns.Docker, Version: strconv.FormatInt(nc.Version, 10), //nolint:gomnd // it's decimal IPConfiguration: cns.IPConfiguration{ - IPSubnet: subnet, - GatewayIPAddress: nc.DefaultGateway, + IPSubnet: subnet, + GatewayIPAddress: nc.DefaultGateway, + GatewayIPv6Address: nc.DefaultGatewayV6, }, NCStatus: nc.Status, + NetworkInterfaceInfo: cns.NetworkInterfaceInfo{ + MACAddress: nc.MacAddress, + }, }, nil } diff --git a/cns/kubecontroller/nodenetworkconfig/conversion_test.go b/cns/kubecontroller/nodenetworkconfig/conversion_test.go index eff8059a63..b160356170 100644 --- a/cns/kubecontroller/nodenetworkconfig/conversion_test.go +++ b/cns/kubecontroller/nodenetworkconfig/conversion_test.go @@ -7,6 +7,7 @@ import ( "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) const ( @@ -339,7 +340,7 @@ func TestCreateNCRequestFromStaticNC(t *testing.T) { for _, tt := range tests { tt := tt t.Run(tt.name, func(t *testing.T) { - got, err := CreateNCRequestFromStaticNC(tt.input) + got, err := CreateNCRequestFromStaticNC(tt.input, false) if tt.wantErr { assert.Error(t, err) return @@ -349,3 +350,136 @@ func TestCreateNCRequestFromStaticNC(t *testing.T) { }) } } + +func TestCreateNCRequestFromStaticNCWithConfig(t *testing.T) { + tests := []struct { + name string + input v1alpha.NetworkContainer + isSwiftV2 bool + want *cns.CreateNetworkContainerRequest + wantErr bool + }{ + { + name: "SwiftV2 enabled with VNETBlock - should NOT process all IPs in prefix", + input: v1alpha.NetworkContainer{ + ID: ncID, + PrimaryIP: "10.0.0.0/32", + NodeIP: "10.0.0.1", + Type: v1alpha.VNETBlock, + SubnetAddressSpace: "10.0.0.0/24", + DefaultGateway: "10.0.0.1", + Version: 1, + Status: "Available", + }, + isSwiftV2: true, + want: &cns.CreateNetworkContainerRequest{ + NetworkContainerid: ncID, + NetworkContainerType: cns.Docker, + Version: "1", + HostPrimaryIP: "10.0.0.1", + IPConfiguration: cns.IPConfiguration{ + IPSubnet: cns.IPSubnet{ + IPAddress: "10.0.0.1", + PrefixLength: 24, + }, + GatewayIPAddress: "10.0.0.1", + }, + SecondaryIPConfigs: map[string]cns.SecondaryIPConfig{ + // No IPs from primary prefix + }, + NCStatus: "Available", + }, + wantErr: false, + }, + { + name: "SwiftV2 disabled with VNETBlock - should process all IP in prefix", + input: v1alpha.NetworkContainer{ + ID: ncID, + PrimaryIP: "10.0.0.0/32", + NodeIP: "10.0.0.1", + Type: v1alpha.VNETBlock, + SubnetAddressSpace: "10.0.0.0/24", + DefaultGateway: "10.0.0.1", + Version: 1, + Status: "Available", + IPAssignments: []v1alpha.IPAssignment{ + { + Name: "test-ip", + IP: "10.0.0.10/32", + }, + }, + }, + isSwiftV2: false, + want: &cns.CreateNetworkContainerRequest{ + NetworkContainerid: ncID, + NetworkContainerType: cns.Docker, + Version: "1", + HostPrimaryIP: "10.0.0.1", + IPConfiguration: cns.IPConfiguration{ + IPSubnet: cns.IPSubnet{ + IPAddress: "10.0.0.1", + PrefixLength: 24, + }, + GatewayIPAddress: "10.0.0.1", + }, + SecondaryIPConfigs: map[string]cns.SecondaryIPConfig{ + "10.0.0.0": {IPAddress: "10.0.0.0", NCVersion: 1}, + // IP assignments + "10.0.0.10": {IPAddress: "10.0.0.10", NCVersion: 1}, + }, + NCStatus: "Available", + }, + wantErr: false, + }, + { + name: "SwiftV2 disabled with non-VNETBlock type - should process IP in prefix", + input: v1alpha.NetworkContainer{ + ID: ncID, + PrimaryIP: "10.0.0.0/32", + NodeIP: "10.0.0.1", + Type: v1alpha.Overlay, + SubnetAddressSpace: "10.0.0.0/24", + DefaultGateway: "10.0.0.1", + Version: 1, + Status: "Available", + IPAssignments: []v1alpha.IPAssignment{ + { + Name: "test-ip", + IP: "10.0.0.10/32", + }, + }, + }, + isSwiftV2: false, + want: &cns.CreateNetworkContainerRequest{ + NetworkContainerid: ncID, + NetworkContainerType: cns.Docker, + Version: "0", + HostPrimaryIP: "10.0.0.1", + IPConfiguration: cns.IPConfiguration{ + IPSubnet: cns.IPSubnet{ + IPAddress: "10.0.0.0", + PrefixLength: 24, + }, + GatewayIPAddress: "10.0.0.1", + }, + SecondaryIPConfigs: map[string]cns.SecondaryIPConfig{ + "10.0.0.0": {IPAddress: "10.0.0.0", NCVersion: 0}, + }, + NCStatus: "Available", + }, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := CreateNCRequestFromStaticNC(tt.input, tt.isSwiftV2) + if tt.wantErr { + assert.Error(t, err) + return + } + require.NoError(t, err) + assert.EqualValues(t, tt.want, got) + }) + } +} diff --git a/cns/kubecontroller/nodenetworkconfig/conversion_windows.go b/cns/kubecontroller/nodenetworkconfig/conversion_windows.go index 0a41ccd41c..6af2297bb7 100644 --- a/cns/kubecontroller/nodenetworkconfig/conversion_windows.go +++ b/cns/kubecontroller/nodenetworkconfig/conversion_windows.go @@ -14,7 +14,7 @@ import ( // secondary IPs. If the gateway is not empty, it will not reserve the 2nd IP and add it as a secondary IP. // //nolint:gocritic //ignore hugeparam -func createNCRequestFromStaticNCHelper(nc v1alpha.NetworkContainer, primaryIPPrefix netip.Prefix, subnet cns.IPSubnet) (*cns.CreateNetworkContainerRequest, error) { +func createNCRequestFromStaticNCHelper(nc v1alpha.NetworkContainer, primaryIPPrefix netip.Prefix, subnet cns.IPSubnet, isSwiftV2 bool) (*cns.CreateNetworkContainerRequest, error) { secondaryIPConfigs := map[string]cns.SecondaryIPConfig{} // the masked address is the 0th IP in the subnet and startingAddr is the 2nd IP (*.1) startingAddr := primaryIPPrefix.Masked().Addr().Next() @@ -27,12 +27,15 @@ func createNCRequestFromStaticNCHelper(nc v1alpha.NetworkContainer, primaryIPPre // iterate through all IP addresses in the subnet described by primaryPrefix and // add them to the request as secondary IPConfigs. - for addr := startingAddr; primaryIPPrefix.Contains(addr); addr = addr.Next() { - secondaryIPConfigs[addr.String()] = cns.SecondaryIPConfig{ - IPAddress: addr.String(), - NCVersion: int(nc.Version), + // Process primary prefix IPs in all scenarios except when nc.Type is v1alpha.VNETBlock AND SwiftV2 is enabled + if !(isSwiftV2 && nc.Type == v1alpha.VNETBlock) { + for addr := startingAddr; primaryIPPrefix.Contains(addr); addr = addr.Next() { + secondaryIPConfigs[addr.String()] = cns.SecondaryIPConfig{ + IPAddress: addr.String(), + NCVersion: int(nc.Version), + } + lastAddr = addr } - lastAddr = addr } if nc.Type == v1alpha.VNETBlock { diff --git a/cns/kubecontroller/nodenetworkconfig/reconciler.go b/cns/kubecontroller/nodenetworkconfig/reconciler.go index 4d5eb1bcd1..81d389f73d 100644 --- a/cns/kubecontroller/nodenetworkconfig/reconciler.go +++ b/cns/kubecontroller/nodenetworkconfig/reconciler.go @@ -43,18 +43,20 @@ type Reconciler struct { once sync.Once started chan interface{} nodeIP string + isSwiftV2 bool } // NewReconciler creates a NodeNetworkConfig Reconciler which will get updates from the Kubernetes // apiserver for NNC events. // Provided nncListeners are passed the NNC after the Reconcile preprocesses it. Note: order matters! The // passed Listeners are notified in the order provided. -func NewReconciler(cnscli cnsClient, ipampoolmonitorcli nodeNetworkConfigListener, nodeIP string) *Reconciler { +func NewReconciler(cnscli cnsClient, ipampoolmonitorcli nodeNetworkConfigListener, nodeIP string, isSwiftV2 bool) *Reconciler { return &Reconciler{ cnscli: cnscli, ipampoolmonitorcli: ipampoolmonitorcli, started: make(chan interface{}), nodeIP: nodeIP, + isSwiftV2: isSwiftV2, } } @@ -104,7 +106,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco switch nnc.Status.NetworkContainers[i].AssignmentMode { //nolint:exhaustive // skipping dynamic case // For Overlay and Vnet Scale Scenarios case v1alpha.Static: - req, err = CreateNCRequestFromStaticNC(nnc.Status.NetworkContainers[i]) + req, err = CreateNCRequestFromStaticNC(nnc.Status.NetworkContainers[i], r.isSwiftV2) // For Pod Subnet scenario default: // For backward compatibility, default will be treated as Dynamic too. req, err = CreateNCRequestFromDynamicNC(nnc.Status.NetworkContainers[i]) diff --git a/cns/kubecontroller/nodenetworkconfig/reconciler_test.go b/cns/kubecontroller/nodenetworkconfig/reconciler_test.go index aed3de81b7..bcf882b2b8 100644 --- a/cns/kubecontroller/nodenetworkconfig/reconciler_test.go +++ b/cns/kubecontroller/nodenetworkconfig/reconciler_test.go @@ -192,7 +192,7 @@ func TestReconcile(t *testing.T) { } t.Run(tt.name, func(t *testing.T) { - r := NewReconciler(&tt.cnsClient, &tt.cnsClient, tt.nodeIP) + r := NewReconciler(&tt.cnsClient, &tt.cnsClient, tt.nodeIP, false) r.nnccli = &tt.ncGetter got, err := r.Reconcile(context.Background(), tt.in) if tt.wantErr { @@ -249,7 +249,7 @@ func TestReconcileStaleNCs(t *testing.T) { return &nncLog[len(nncLog)-1], nil } - r := NewReconciler(&cnsClient, &cnsClient, nodeIP) + r := NewReconciler(&cnsClient, &cnsClient, nodeIP, false) r.nnccli = &mockNCGetter{get: nncIterator} _, err := r.Reconcile(context.Background(), reconcile.Request{}) diff --git a/cns/restserver/internalapi.go b/cns/restserver/internalapi.go index 9792aed5a3..efefb3f2d3 100644 --- a/cns/restserver/internalapi.go +++ b/cns/restserver/internalapi.go @@ -222,6 +222,7 @@ func (service *HTTPRestService) syncHostNCVersion(ctx context.Context, channelMo if len(outdatedNCs) == 0 { return len(programmedNCs), nil } + ncVersionListResp, err := service.nma.GetNCVersionList(ctx) if err != nil { return len(programmedNCs), errors.Wrap(err, "failed to get nc version list from nmagent") diff --git a/cns/restserver/internalapi_linux.go b/cns/restserver/internalapi_linux.go index 81e70124fb..0abae0a72a 100644 --- a/cns/restserver/internalapi_linux.go +++ b/cns/restserver/internalapi_linux.go @@ -116,6 +116,12 @@ func (service *HTTPRestService) programSNATRules(req *cns.CreateNetworkContainer // use any secondary ip + the nnc prefix length to get an iptables rule to allow dns and imds traffic from the pods for _, v := range req.SecondaryIPConfigs { + // check if the ip address is IPv4. A check is required because DNS and IMDS do not have IPv6 addresses. Since support currently exists only for IPv4, other ip families are skipped. + if net.ParseIP(v.IPAddress).To4() == nil { + // skip if the ip address is not IPv4 + continue + } + // put the ip address in standard cidr form (where we zero out the parts that are not relevant) _, podSubnet, _ := net.ParseCIDR(v.IPAddress + "/" + fmt.Sprintf("%d", req.IPConfiguration.IPSubnet.PrefixLength)) diff --git a/cns/restserver/ipam.go b/cns/restserver/ipam.go index 883d9aaef3..7c1366149d 100644 --- a/cns/restserver/ipam.go +++ b/cns/restserver/ipam.go @@ -8,6 +8,7 @@ import ( "fmt" "net" "net/http" + "net/netip" "strconv" "strings" @@ -995,38 +996,57 @@ func (service *HTTPRestService) AssignAvailableIPConfigs(podInfo cns.PodInfo) ([ if numOfNCs == 0 { return nil, ErrNoNCs } + + // Get the number of distinct IP families (IPv4/IPv6) across all NC's and determine the number of IPs to assign based on IP families found + numberOfIPs := service.GetIPFamilyCount() + + // Get the actual IP families map for validation + ncIPFamilies := service.getIPFamiliesMap() + service.Lock() defer service.Unlock() // Creates a slice of PodIpInfo with the size as number of NCs to hold the result for assigned IP configs - podIPInfo := make([]cns.PodIpInfo, numOfNCs) + podIPInfo := make([]cns.PodIpInfo, numberOfIPs) // This map is used to store whether or not we have found an available IP from an NC when looping through the pool ipsToAssign := make(map[string]cns.IPConfigurationStatus) // Searches for available IPs in the pool for _, ipState := range service.PodIPConfigState { - // check if an IP from this NC is already set side for assignment. - if _, ncAlreadyMarkedForAssignment := ipsToAssign[ipState.NCID]; ncAlreadyMarkedForAssignment { + + // get the IPFamily of the current ipState + var ipStateFamily cns.IPFamily = cns.IPv4 + + if ipAddr, err := netip.ParseAddr(ipState.IPAddress); err == nil && ipAddr.Is6() { + ipStateFamily = cns.IPv6 + } + + key := generateAssignedIPKey(ipState.NCID, ipStateFamily) + + // check if the IP with the same family type exists already + if _, ncIPFamilyAlreadyMarkedForAssignment := ipsToAssign[key]; ncIPFamilyAlreadyMarkedForAssignment { continue } // Checks if the current IP is available if ipState.GetState() != types.Available { continue } - ipsToAssign[ipState.NCID] = ipState - // Once one IP per container is found break out of the loop and stop searching - if len(ipsToAssign) == numOfNCs { + ipsToAssign[key] = ipState + // Once numberOfIPs per container is found break out of the loop and stop searching + if len(ipsToAssign) == numberOfIPs { break } } - // Checks to make sure we found one IP for each NC - if len(ipsToAssign) != numOfNCs { + // Checks to make sure we found one IP for each NCxIPFamily + if len(ipsToAssign) != numberOfIPs { for ncID := range service.state.ContainerStatus { - if _, found := ipsToAssign[ncID]; found { - continue + for ipFamily := range ncIPFamilies { + if _, found := ipsToAssign[generateAssignedIPKey(ncID, ipFamily)]; found { + continue + } + return podIPInfo, errors.Errorf("not enough IPs available of type %s for %s, waiting on Azure CNS to allocate more with NC Status: %s", + ipFamily, ncID, string(service.state.ContainerStatus[ncID].CreateNetworkContainerRequest.NCStatus)) } - return podIPInfo, errors.Errorf("not enough IPs available for %s, waiting on Azure CNS to allocate more with NC Status: %s", - ncID, string(service.state.ContainerStatus[ncID].CreateNetworkContainerRequest.NCStatus)) } } @@ -1061,10 +1081,18 @@ func (service *HTTPRestService) AssignAvailableIPConfigs(podInfo cns.PodInfo) ([ return podIPInfo, fmt.Errorf("not enough IPs available, waiting on Azure CNS to allocate more") } - logger.Printf("[AssignDesiredIPConfigs] Successfully assigned IPs for pod %+v", podInfo) + //nolint:staticcheck // SA1019: suppress deprecated logger.Printf usage. Todo: legacy logger usage is consistent in cns repo. Migrates when all logger usage is migrated + logger.Printf( + "[AssignAvailableIPConfigs] Successfully assigned IPs for pod %+v", + podInfo, + ) return podIPInfo, nil } +func generateAssignedIPKey(ncID string, ipFamily cns.IPFamily) string { + return fmt.Sprintf("%s_%s", ncID, string(ipFamily)) +} + // If IPConfigs are already assigned to the pod, it returns that else it returns the available ipconfigs. func requestIPConfigsHelper(service *HTTPRestService, req cns.IPConfigsRequest) ([]cns.PodIpInfo, error) { // check if ipconfigs already assigned to this pod and return if exists or error @@ -1327,3 +1355,41 @@ func verifyUpdateEndpointStateRequest(req map[string]*IPInfo) error { } return nil } + +// getIPFamiliesMap returns a map of IP families present across all NC's +func (service *HTTPRestService) getIPFamiliesMap() map[cns.IPFamily]struct{} { + ncIPFamilies := map[cns.IPFamily]struct{}{} + + for ncID := range service.state.ContainerStatus { + // Exit if we already found both IPv4 and IPv6 + if len(ncIPFamilies) == 2 { + break + } + + for _, secIPConfig := range service.state.ContainerStatus[ncID].CreateNetworkContainerRequest.SecondaryIPConfigs { + // Exit if we already found both IPv4 and IPv6 + if len(ncIPFamilies) == 2 { + break + } + addr, err := netip.ParseAddr(secIPConfig.IPAddress) + if err != nil { + continue + } + if addr.Is4() { + ncIPFamilies[cns.IPv4] = struct{}{} + } else if addr.Is6() { + ncIPFamilies[cns.IPv6] = struct{}{} + } + } + } + + return ncIPFamilies +} + +// GetIPFamilyCount returns the number of distinct IP families (IPv4/IPv6) across all NC's. +// This is used to determine how many IPs to assign per pod: +// - In single-stack: 1 IP per pod +// - In dual-stack: 2 IPs per pod (one IPv4, one IPv6) +func (service *HTTPRestService) GetIPFamilyCount() int { + return len(service.getIPFamiliesMap()) +} diff --git a/cns/restserver/ipam_test.go b/cns/restserver/ipam_test.go index 52c5b6d0d6..dc582a0e8d 100644 --- a/cns/restserver/ipam_test.go +++ b/cns/restserver/ipam_test.go @@ -178,36 +178,46 @@ func updatePnpIDMacAddressState(svc *HTTPRestService) { } } -// create an endpoint with only one IP -func TestEndpointStateReadAndWriteSingleNC(t *testing.T) { - ncStates := []ncState{ +func TestEndpointStateReadAndWrite(t *testing.T) { + testNcs := [][]ncState{ + // single stack NC { - ncID: testNCID, - ips: []string{ - testIP1, + { + ncID: testNCID, + ips: []string{ + testIP1, + }, }, }, - } - endpointStateReadAndWrite(t, ncStates) -} - -// create an endpoint with one IP from each NC -func TestEndpointStateReadAndWriteMultipleNCs(t *testing.T) { - ncStates := []ncState{ + // two NCs (one V4 and one V6) { - ncID: testNCID, - ips: []string{ - testIP1, + { + ncID: testNCID, + ips: []string{ + testIP1, + }, + }, + { + ncID: testNCIDv6, + ips: []string{ + testIP1v6, + }, }, }, + // dualstack NC { - ncID: testNCIDv6, - ips: []string{ - testIP1v6, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP1v6, + }, }, }, } - endpointStateReadAndWrite(t, ncStates) + for _, ncState := range testNcs { + endpointStateReadAndWrite(t, ncState) + } } // Tests the creation of an endpoint using the NCs and IPs as input and then tests the deletion of that endpoint @@ -282,35 +292,46 @@ func endpointStateReadAndWrite(t *testing.T, ncStates []ncState) { } // assign the available IP to the new pod -func TestIPAMGetAvailableIPConfigSingleNC(t *testing.T) { - ncStates := []ncState{ +func TestIPAMGetAvailableIPConfig(t *testing.T) { + testNcs := [][]ncState{ + // single stack NC { - ncID: testNCID, - ips: []string{ - testIP1, + { + ncID: testNCID, + ips: []string{ + testIP1, + }, }, }, - } - ipamGetAvailableIPConfig(t, ncStates) -} - -// assign one IP per NC to the pod -func TestIPAMGetAvailableIPConfigMultipleNCs(t *testing.T) { - nsStates := []ncState{ + // two NCs (one V4 and one V6) { - ncID: testNCID, - ips: []string{ - testIP1, + { + ncID: testNCID, + ips: []string{ + testIP1, + }, + }, + { + ncID: testNCIDv6, + ips: []string{ + testIP1v6, + }, }, }, + // dualstack NC { - ncID: testNCIDv6, - ips: []string{ - testIP1v6, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP1v6, + }, }, }, } - ipamGetAvailableIPConfig(t, nsStates) + for _, ncState := range testNcs { + ipamGetAvailableIPConfig(t, ncState) + } } // Add one IP per NC to the pool and request those IPs @@ -359,37 +380,51 @@ func ipamGetAvailableIPConfig(t *testing.T, ncStates []ncState) { } } -func TestIPAMGetNextAvailableIPConfigSingleNC(t *testing.T) { - ncStates := []ncState{ +func TestIPAMGetNextAvailableIPConfig(t *testing.T) { + testNcs := [][]ncState{ + // single stack NC { - ncID: testNCID, - ips: []string{ - testIP1, - testIP2, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP2, + }, }, }, - } - getNextAvailableIPConfig(t, ncStates) -} - -func TestIPAMGetNextAvailableIPConfigMultipleNCs(t *testing.T) { - ncStates := []ncState{ + // two NCs (one V4 and one V6) { - ncID: testNCID, - ips: []string{ - testIP1, - testIP2, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP2, + }, + }, + { + ncID: testNCIDv6, + ips: []string{ + testIP1v6, + testIP2v6, + }, }, }, + // dualstack NC { - ncID: testNCIDv6, - ips: []string{ - testIP1v6, - testIP2v6, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP2, + testIP1v6, + testIP2v6, + }, }, }, } - getNextAvailableIPConfig(t, ncStates) + for _, ncState := range testNcs { + getNextAvailableIPConfig(t, ncState) + } } // First IP is already assigned to a pod, want second IP @@ -442,34 +477,46 @@ func getNextAvailableIPConfig(t *testing.T, ncStates []ncState) { } } -func TestIPAMGetAlreadyAssignedIPConfigForSamePodSingleNC(t *testing.T) { - ncStates := []ncState{ +func TestIPAMGetAlreadyAssignedIPConfigForSamePod(t *testing.T) { + testNcs := [][]ncState{ + // single stack NC { - ncID: testNCID, - ips: []string{ - testIP1, + { + ncID: testNCID, + ips: []string{ + testIP1, + }, }, }, - } - ipamGetAlreadyAssignedIPConfigForSamePod(t, ncStates) -} - -func TestIPAMGetAlreadyAssignedIPConfigForSamePodMultipleNCs(t *testing.T) { - ncStates := []ncState{ + // two NCs (one V4 and one V6) { - ncID: testNCID, - ips: []string{ - testIP1, + { + ncID: testNCID, + ips: []string{ + testIP1, + }, + }, + { + ncID: testNCIDv6, + ips: []string{ + testIP1v6, + }, }, }, + // dualstack NC { - ncID: testNCIDv6, - ips: []string{ - testIP1v6, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP1v6, + }, }, }, } - ipamGetAlreadyAssignedIPConfigForSamePod(t, ncStates) + for _, ncState := range testNcs { + ipamGetAlreadyAssignedIPConfigForSamePod(t, ncState) + } } func ipamGetAlreadyAssignedIPConfigForSamePod(t *testing.T, ncStates []ncState) { @@ -517,37 +564,51 @@ func ipamGetAlreadyAssignedIPConfigForSamePod(t *testing.T, ncStates []ncState) } } -func TestIPAMAttemptToRequestIPNotFoundInPoolSingleNC(t *testing.T) { - ncStates := []ncState{ +func TestIPAMAttemptToRequestIPNotFoundInPool(t *testing.T) { + testNcs := [][]ncState{ + // single stack NC { - ncID: testNCID, - ips: []string{ - testIP1, - testIP2, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP2, + }, }, }, - } - ipamAttemptToRequestIPNotFoundInPool(t, ncStates) -} - -func TestIPAMAttemptToRequestIPNotFoundInPoolMultipleNCs(t *testing.T) { - ncStates := []ncState{ + // two NCs (one V4 and one V6) { - ncID: testNCID, - ips: []string{ - testIP1, - testIP2, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP2, + }, + }, + { + ncID: testNCIDv6, + ips: []string{ + testIP1v6, + testIP2v6, + }, }, }, + // dualstack NC { - ncID: testNCIDv6, - ips: []string{ - testIP1v6, - testIP2v6, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP2, + testIP1v6, + testIP2v6, + }, }, }, } - ipamAttemptToRequestIPNotFoundInPool(t, ncStates) + for _, ncState := range testNcs { + ipamAttemptToRequestIPNotFoundInPool(t, ncState) + } } func ipamAttemptToRequestIPNotFoundInPool(t *testing.T, ncStates []ncState) { @@ -580,34 +641,46 @@ func ipamAttemptToRequestIPNotFoundInPool(t *testing.T, ncStates []ncState) { } } -func TestIPAMGetDesiredIPConfigWithSpecfiedIPSingleNC(t *testing.T) { - ncStates := []ncState{ +func TestIPAMGetDesiredIPConfigWithSpecfiedIP(t *testing.T) { + testNcs := [][]ncState{ + // single stack NC { - ncID: testNCID, - ips: []string{ - testIP1, + { + ncID: testNCID, + ips: []string{ + testIP1, + }, }, }, - } - ipamGetDesiredIPConfigWithSpecfiedIP(t, ncStates) -} - -func TestIPAMGetDesiredIPConfigWithSpecfiedIPMultipleNCs(t *testing.T) { - ncStates := []ncState{ + // two NCs (one V4 and one V6) { - ncID: testNCID, - ips: []string{ - testIP1, + { + ncID: testNCID, + ips: []string{ + testIP1, + }, + }, + { + ncID: testNCIDv6, + ips: []string{ + testIP1v6, + }, }, }, + // dualstack NC { - ncID: testNCIDv6, - ips: []string{ - testIP1v6, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP1v6, + }, }, }, } - ipamGetDesiredIPConfigWithSpecfiedIP(t, ncStates) + for _, ncState := range testNcs { + ipamGetDesiredIPConfigWithSpecfiedIP(t, ncState) + } } func ipamGetDesiredIPConfigWithSpecfiedIP(t *testing.T, ncStates []ncState) { @@ -659,34 +732,46 @@ func ipamGetDesiredIPConfigWithSpecfiedIP(t *testing.T, ncStates []ncState) { } } -func TestIPAMFailToGetDesiredIPConfigWithAlreadyAssignedSpecfiedIPSingleNC(t *testing.T) { - ncStates := []ncState{ +func TestIPAMFailToGetDesiredIPConfigWithAlreadyAssignedSpecfiedIP(t *testing.T) { + testNcs := [][]ncState{ + // single stack NC { - ncID: testNCID, - ips: []string{ - testIP1, + { + ncID: testNCID, + ips: []string{ + testIP1, + }, }, }, - } - ipamFailToGetDesiredIPConfigWithAlreadyAssignedSpecfiedIP(t, ncStates) -} - -func TestIPAMFailToGetDesiredIPConfigWithAlreadyAssignedSpecfiedIPMultipleNCs(t *testing.T) { - ncStates := []ncState{ + // two NCs (one V4 and one V6) { - ncID: testNCID, - ips: []string{ - testIP1, + { + ncID: testNCID, + ips: []string{ + testIP1, + }, + }, + { + ncID: testNCIDv6, + ips: []string{ + testIP1v6, + }, }, }, + // dualstack NC { - ncID: testNCIDv6, - ips: []string{ - testIP1v6, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP1v6, + }, }, }, } - ipamFailToGetDesiredIPConfigWithAlreadyAssignedSpecfiedIP(t, ncStates) + for _, ncState := range testNcs { + ipamFailToGetDesiredIPConfigWithAlreadyAssignedSpecfiedIP(t, ncState) + } } func ipamFailToGetDesiredIPConfigWithAlreadyAssignedSpecfiedIP(t *testing.T, ncStates []ncState) { @@ -720,37 +805,51 @@ func ipamFailToGetDesiredIPConfigWithAlreadyAssignedSpecfiedIP(t *testing.T, ncS } } -func TestIPAMFailToGetIPWhenAllIPsAreAssignedSingleNC(t *testing.T) { - ncStates := []ncState{ +func TestIPAMFailToGetIPWhenAllIPsAreAssigned(t *testing.T) { + testNcs := [][]ncState{ + // single stack NC { - ncID: testNCID, - ips: []string{ - testIP1, - testIP2, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP2, + }, }, }, - } - ipamFailToGetIPWhenAllIPsAreAssigned(t, ncStates) -} - -func TestIPAMFailToGetIPWhenAllIPsAreAssignedMultipleNCs(t *testing.T) { - ncStates := []ncState{ + // two NCs (one V4 and one V6) { - ncID: testNCID, - ips: []string{ - testIP1, - testIP2, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP2, + }, + }, + { + ncID: testNCIDv6, + ips: []string{ + testIP1v6, + testIP2v6, + }, }, }, + // dualstack NC { - ncID: testNCIDv6, - ips: []string{ - testIP1v6, - testIP2v6, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP2, + testIP1v6, + testIP2v6, + }, }, }, } - ipamFailToGetIPWhenAllIPsAreAssigned(t, ncStates) + for _, ncState := range testNcs { + ipamFailToGetIPWhenAllIPsAreAssigned(t, ncState) + } } func ipamFailToGetIPWhenAllIPsAreAssigned(t *testing.T, ncStates []ncState) { @@ -780,34 +879,46 @@ func ipamFailToGetIPWhenAllIPsAreAssigned(t *testing.T, ncStates []ncState) { } } -func TestIPAMRequestThenReleaseThenRequestAgainSingleNC(t *testing.T) { - ncStates := []ncState{ +func TestIPAMRequestThenReleaseThenRequestAgain(t *testing.T) { + testNcs := [][]ncState{ + // single stack NC { - ncID: testNCID, - ips: []string{ - testIP1, + { + ncID: testNCID, + ips: []string{ + testIP1, + }, }, }, - } - ipamRequestThenReleaseThenRequestAgain(t, ncStates) -} - -func TestIPAMRequestThenReleaseThenRequestAgainMultipleNCs(t *testing.T) { - ncStates := []ncState{ + // two NCs (one V4 and one V6) { - ncID: testNCID, - ips: []string{ - testIP1, + { + ncID: testNCID, + ips: []string{ + testIP1, + }, + }, + { + ncID: testNCIDv6, + ips: []string{ + testIP1v6, + }, }, }, + // dualstack NC { - ncID: testNCIDv6, - ips: []string{ - testIP1v6, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP1v6, + }, }, }, } - ipamRequestThenReleaseThenRequestAgain(t, ncStates) + for _, ncState := range testNcs { + ipamRequestThenReleaseThenRequestAgain(t, ncState) + } } // 10.0.0.1 = PodInfo1 @@ -889,34 +1000,46 @@ func ipamRequestThenReleaseThenRequestAgain(t *testing.T, ncStates []ncState) { } } -func TestIPAMReleaseIPIdempotencySingleNC(t *testing.T) { - ncStates := []ncState{ +func TestIPAMReleaseIPIdempotency(t *testing.T) { + testNcs := [][]ncState{ + // single stack NC { - ncID: testNCID, - ips: []string{ - testIP1, + { + ncID: testNCID, + ips: []string{ + testIP1, + }, }, }, - } - ipamReleaseIPIdempotency(t, ncStates) -} - -func TestIPAMReleaseIPIdempotencyMultipleNCs(t *testing.T) { - ncStates := []ncState{ + // two NCs (one V4 and one V6) { - ncID: testNCID, - ips: []string{ - testIP1, + { + ncID: testNCID, + ips: []string{ + testIP1, + }, + }, + { + ncID: testNCIDv6, + ips: []string{ + testIP1v6, + }, }, }, + // dualstack NC { - ncID: testNCIDv6, - ips: []string{ - testIP1v6, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP1v6, + }, }, }, } - ipamReleaseIPIdempotency(t, ncStates) + for _, ncState := range testNcs { + ipamReleaseIPIdempotency(t, ncState) + } } func ipamReleaseIPIdempotency(t *testing.T, ncStates []ncState) { @@ -945,34 +1068,46 @@ func ipamReleaseIPIdempotency(t *testing.T, ncStates []ncState) { } } -func TestIPAMAllocateIPIdempotencySingleNC(t *testing.T) { - ncStates := []ncState{ +func TestIPAMAllocateIPIdempotency(t *testing.T) { + testNcs := [][]ncState{ + // single stack NC { - ncID: testNCID, - ips: []string{ - testIP1, + { + ncID: testNCID, + ips: []string{ + testIP1, + }, }, }, - } - ipamAllocateIPIdempotency(t, ncStates) -} - -func TestIPAMAllocateIPIdempotencyMultipleNCs(t *testing.T) { - ncStates := []ncState{ + // two NCs (one V4 and one V6) { - ncID: testNCID, - ips: []string{ - testIP1, + { + ncID: testNCID, + ips: []string{ + testIP1, + }, + }, + { + ncID: testNCIDv6, + ips: []string{ + testIP1v6, + }, }, }, + // dualstack NC { - ncID: testNCIDv6, - ips: []string{ - testIP1v6, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP1v6, + }, }, }, } - ipamAllocateIPIdempotency(t, ncStates) + for _, ncState := range testNcs { + ipamAllocateIPIdempotency(t, ncState) + } } func ipamAllocateIPIdempotency(t *testing.T, ncStates []ncState) { @@ -994,40 +1129,56 @@ func ipamAllocateIPIdempotency(t *testing.T, ncStates []ncState) { } } -func TestAvailableIPConfigsSingleNC(t *testing.T) { - ncStates := []ncState{ +func TestAvailableIPConfigs(t *testing.T) { + testNcs := [][]ncState{ + // single stack NC { - ncID: testNCID, - ips: []string{ - testIP1, - testIP2, - testIP3, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP2, + testIP3, + }, }, }, - } - availableIPConfigs(t, ncStates) -} - -func TestAvailableIPConfigsMultipleNCs(t *testing.T) { - ncStates := []ncState{ + // two NCs (one V4 and one V6) { - ncID: testNCID, - ips: []string{ - testIP1, - testIP2, - testIP3, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP2, + testIP3, + }, + }, + { + ncID: testNCIDv6, + ips: []string{ + testIP1v6, + testIP2v6, + testIP3v6, + }, }, }, + // dualstack NC { - ncID: testNCIDv6, - ips: []string{ - testIP1v6, - testIP2v6, - testIP3v6, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP2, + testIP3, + testIP1v6, + testIP2v6, + testIP3v6, + }, }, }, } - availableIPConfigs(t, ncStates) + for _, ncState := range testNcs { + availableIPConfigs(t, ncState) + } } func availableIPConfigs(t *testing.T, ncStates []ncState) { @@ -1113,34 +1264,46 @@ func validateIpState(t *testing.T, actualIps []cns.IPConfigurationStatus, expect } } -func TestIPAMMarkIPCountAsPendingSingleNC(t *testing.T) { - ncStates := []ncState{ +func TestIPAMMarkIPCountAsPending(t *testing.T) { + testNcs := [][]ncState{ + // single stack NC { - ncID: testNCID, - ips: []string{ - testIP1, + { + ncID: testNCID, + ips: []string{ + testIP1, + }, }, }, - } - ipamMarkIPCountAsPending(t, ncStates) -} - -func TestIPAMMarkIPCountAsPendingMultipleNCs(t *testing.T) { - ncStates := []ncState{ + // two NCs (one V4 and one V6) { - ncID: testNCID, - ips: []string{ - testIP1, + { + ncID: testNCID, + ips: []string{ + testIP1, + }, + }, + { + ncID: testNCIDv6, + ips: []string{ + testIP1v6, + }, }, }, + // dualstack NC { - ncID: testNCIDv6, - ips: []string{ - testIP1v6, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP1v6, + }, }, }, } - ipamMarkIPCountAsPending(t, ncStates) + for _, ncState := range testNcs { + ipamMarkIPCountAsPending(t, ncState) + } } func ipamMarkIPCountAsPending(t *testing.T, ncStates []ncState) { @@ -1272,37 +1435,51 @@ func constructSecondaryIPConfigs(ipAddress, uuid string, ncVersion int, secondar secondaryIPConfigs[uuid] = secIPConfig } -func TestIPAMMarkExistingIPConfigAsPendingSingleNC(t *testing.T) { - ncStates := []ncState{ +func TestIPAMMarkExistingIPConfigAsPending(t *testing.T) { + testNcs := [][]ncState{ + // single stack NC { - ncID: testNCID, - ips: []string{ - testIP1, - testIP2, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP2, + }, }, }, - } - ipamMarkExistingIPConfigAsPending(t, ncStates) -} - -func TestIPAMMarkExistingIPConfigAsPendingMultipleNCs(t *testing.T) { - ncStates := []ncState{ + // two NCs (one V4 and one V6) { - ncID: testNCID, - ips: []string{ - testIP1, - testIP2, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP2, + }, + }, + { + ncID: testNCIDv6, + ips: []string{ + testIP1v6, + testIP2v6, + }, }, }, + // dualstack NC { - ncID: testNCIDv6, - ips: []string{ - testIP1v6, - testIP2v6, + { + ncID: testNCID, + ips: []string{ + testIP1, + testIP2, + testIP1v6, + testIP2v6, + }, }, }, } - ipamMarkExistingIPConfigAsPending(t, ncStates) + for _, ncState := range testNcs { + ipamMarkExistingIPConfigAsPending(t, ncState) + } } func ipamMarkExistingIPConfigAsPending(t *testing.T, ncStates []ncState) { @@ -1433,27 +1610,32 @@ func TestIPAMReleaseOneIPWhenExpectedToHaveTwo(t *testing.T) { func TestIPAMFailToRequestOneIPWhenExpectedToHaveTwo(t *testing.T) { svc := getTestService(cns.KubernetesCRD) - // set state as already assigned - testState := newPodState(testIP1, ipIDs[0][0], testNCID, types.Available, 0) + testState1, _ := newPodStateWithOrchestratorContext(testIP1, testIPID1, testNCID, types.Assigned, 24, 0, testPod1Info) + testState2 := newPodState(testIP2, testIPID2, testNCID, types.Available, 0) ipconfigs := map[string]cns.IPConfigurationStatus{ - testState.ID: testState, + testState1.ID: testState1, + testState2.ID: testState2, + } + testState1v6, _ := newPodStateWithOrchestratorContext(testIP1v6, testIPID1v6, testNCIDv6, types.Assigned, 120, 0, testPod1Info) + // setting v6 IPs to Assigned so there are none available + ipconfigsV6 := map[string]cns.IPConfigurationStatus{ + testState1v6.ID: testState1v6, } - emptyIpconfigs := map[string]cns.IPConfigurationStatus{} err := updatePodIPConfigState(t, svc, ipconfigs, testNCID) if err != nil { t.Fatalf("Expected to not fail adding IPs to state: %+v", err) } - err = updatePodIPConfigState(t, svc, emptyIpconfigs, testNCIDv6) + err = updatePodIPConfigState(t, svc, ipconfigsV6, testNCIDv6) if err != nil { t.Fatalf("Expected to not fail adding empty NC to state: %+v", err) } // request should expect 2 IPs but there is only 1 in the pool req := cns.IPConfigsRequest{} - b, _ := testPod1Info.OrchestratorContext() + b, _ := testPod2Info.OrchestratorContext() req.OrchestratorContext = b _, err = requestIPAddressAndGetState(t, req) @@ -2128,6 +2310,91 @@ func setupMockNMAgent(t *testing.T, svc *HTTPRestService, mockNMAgent *fakes.NMA }) } +func TestIPAMRequestIPConfigsTwoFamiliesSameNC(t *testing.T) { + svc := getTestService(cns.KubernetesCRD) + + // Add single NC with both IPv4 and IPv6 IPs + mixedConfigs := map[string]cns.IPConfigurationStatus{ + "ipv4-1": newPodState("10.0.0.1", "ipv4-1", testNCID, types.Available, 0), + "ipv4-2": newPodState("10.0.0.2", "ipv4-2", testNCID, types.Available, 0), + "ipv6-1": newPodState("2001:db8::1", "ipv6-1", testNCID, types.Available, 0), + "ipv6-2": newPodState("2001:db8::2", "ipv6-2", testNCID, types.Available, 0), + } + + err := updatePodIPConfigState(t, svc, mixedConfigs, testNCID) + require.NoError(t, err) + + req := cns.IPConfigsRequest{ + PodInterfaceID: testPod1Info.InterfaceID(), + InfraContainerID: testPod1Info.InfraContainerID(), + } + b, _ := testPod1Info.OrchestratorContext() + req.OrchestratorContext = b + + podIPs, err := requestIPConfigsHelper(svc, req) + require.NoError(t, err) + + // Should assign 2 IPs (one of each family) of same NC + assert.Len(t, podIPs, 2) + + // Verify we got one of each family + hasIPv4 := false + hasIPv6 := false + for _, podIP := range podIPs { + ip := net.ParseIP(podIP.PodIPConfig.IPAddress) + if ip.To4() != nil { + hasIPv4 = true + } else { + hasIPv6 = true + } + } + assert.True(t, hasIPv4, "Should have IPv4 address") + assert.True(t, hasIPv6, "Should have IPv6 address") +} + +func TestIPAMRequestIPConfigsProgrammingPendingFailure(t *testing.T) { + svc := getTestService(cns.KubernetesCRD) + + // Add IPv4 NC with available IP + ipv4configs := map[string]cns.IPConfigurationStatus{ + "ipv4-1": newPodState("10.0.0.1", "ipv4-1", testNCID, types.Available, 0), + } + + // Add IPv6 NC with IP initially available + ipv6configs := map[string]cns.IPConfigurationStatus{ + "ipv6-1": newPodState("2001:db8::1", "ipv6-1", testNCIDv6, types.Available, 0), + } + + err := updatePodIPConfigState(t, svc, ipv4configs, testNCID) + require.NoError(t, err) + err = updatePodIPConfigState(t, svc, ipv6configs, testNCIDv6) + require.NoError(t, err) + + // Manually set IPv6 IP to PendingProgramming state after NC creation + ipv6State := svc.PodIPConfigState["ipv6-1"] + ipv6State.SetState(types.PendingProgramming) + svc.PodIPConfigState["ipv6-1"] = ipv6State + + req := cns.IPConfigsRequest{ + PodInterfaceID: testPod1Info.InterfaceID(), + InfraContainerID: testPod1Info.InfraContainerID(), + } + b, _ := testPod1Info.OrchestratorContext() + req.OrchestratorContext = b + + _, err = requestIPConfigsHelper(svc, req) + require.Error(t, err) + assert.Contains(t, err.Error(), "not enough IPs available") + + // Verify no IPs were assigned + assignedIPs := svc.GetAssignedIPConfigs() + assert.Empty(t, assignedIPs) + + // Verify IPv4 IP is still available and ipv6 is not available as the programming failed + availableIPs := svc.GetAvailableIPConfigs() + assert.Len(t, availableIPs, 1) +} + func createAndSaveMockNCRequest(t *testing.T, svc *HTTPRestService, ncID string, orchestratorContext json.RawMessage, desiredIP, mockGatewayIP, mockMACAddress string) { t.Helper() diff --git a/cns/restserver/util.go b/cns/restserver/util.go index 43d1e1aef9..a84eb8cef0 100644 --- a/cns/restserver/util.go +++ b/cns/restserver/util.go @@ -845,6 +845,7 @@ func (service *HTTPRestService) populateIPConfigInfoUntransacted(ipConfigStatus podIPInfo.HostPrimaryIPInfo.PrimaryIP = primaryHostInterface.PrimaryIP podIPInfo.HostPrimaryIPInfo.Subnet = primaryHostInterface.Subnet podIPInfo.HostPrimaryIPInfo.Gateway = primaryHostInterface.Gateway + podIPInfo.MacAddress = ncStatus.CreateNetworkContainerRequest.NetworkInterfaceInfo.MACAddress podIPInfo.NICType = cns.InfraNIC return nil diff --git a/cns/service/main.go b/cns/service/main.go index ff79090d8b..67f7872f44 100644 --- a/cns/service/main.go +++ b/cns/service/main.go @@ -1312,7 +1312,7 @@ type ipamStateReconciler interface { // TODO(rbtr) where should this live?? // reconcileInitialCNSState initializes cns by passing pods and a CreateNetworkContainerRequest -func reconcileInitialCNSState(ctx context.Context, cli nodeNetworkConfigGetter, ipamReconciler ipamStateReconciler, podInfoByIPProvider cns.PodInfoByIPProvider) error { +func reconcileInitialCNSState(ctx context.Context, cli nodeNetworkConfigGetter, ipamReconciler ipamStateReconciler, podInfoByIPProvider cns.PodInfoByIPProvider, isSwiftV2 bool) error { // Get nnc using direct client nnc, err := cli.Get(ctx) if err != nil { @@ -1351,7 +1351,7 @@ func reconcileInitialCNSState(ctx context.Context, cli nodeNetworkConfigGetter, ) switch nnc.Status.NetworkContainers[i].AssignmentMode { //nolint:exhaustive // skipping dynamic case case v1alpha.Static: - ncRequest, err = nncctrl.CreateNCRequestFromStaticNC(nnc.Status.NetworkContainers[i]) + ncRequest, err = nncctrl.CreateNCRequestFromStaticNC(nnc.Status.NetworkContainers[i], isSwiftV2) default: // For backward compatibility, default will be treated as Dynamic too. ncRequest, err = nncctrl.CreateNCRequestFromDynamicNC(nnc.Status.NetworkContainers[i]) } @@ -1459,7 +1459,7 @@ func InitializeCRDState(ctx context.Context, z *zap.Logger, httpRestService cns. _ = retry.Do(func() error { attempt++ logger.Printf("reconciling initial CNS state attempt: %d", attempt) - err = reconcileInitialCNSState(ctx, directscopedcli, httpRestServiceImplementation, podInfoByIPProvider) + err = reconcileInitialCNSState(ctx, directscopedcli, httpRestServiceImplementation, podInfoByIPProvider, cnsconfig.EnableSwiftV2) if err != nil { logger.Errorf("failed to reconcile initial CNS state, attempt: %d err: %v", attempt, err) nncInitFailure.Inc() @@ -1556,7 +1556,7 @@ func InitializeCRDState(ctx context.Context, z *zap.Logger, httpRestService cns. // get CNS Node IP to compare NC Node IP with this Node IP to ensure NCs were created for this node nodeIP := configuration.NodeIP() - nncReconciler := nncctrl.NewReconciler(httpRestServiceImplementation, poolMonitor, nodeIP) + nncReconciler := nncctrl.NewReconciler(httpRestServiceImplementation, poolMonitor, nodeIP, cnsconfig.EnableSwiftV2) // pass Node to the Reconciler for Controller xref // IPAMv1 - reconcile only status changes (where generation doesn't change). // IPAMv2 - reconcile all updates. From 5ca999b3518d54b4f60bd02b7e5b449968699948 Mon Sep 17 00:00:00 2001 From: Niha Nallappagari Date: Tue, 30 Sep 2025 15:41:52 -0500 Subject: [PATCH 5/7] Add registry keys for prefix on nic scenario --- cns/NetworkContainerContract.go | 1 + .../nodenetworkconfig/conversion.go | 1 + .../nodenetworkconfig/conversion_linux.go | 1 + cns/restserver/internalapi.go | 32 ++++++- cns/restserver/internalapi_linux.go | 10 ++ cns/restserver/internalapi_windows.go | 96 ++++++++++++++++++- 6 files changed, 139 insertions(+), 2 deletions(-) diff --git a/cns/NetworkContainerContract.go b/cns/NetworkContainerContract.go index 8f5939c28e..20d808897f 100644 --- a/cns/NetworkContainerContract.go +++ b/cns/NetworkContainerContract.go @@ -128,6 +128,7 @@ type CreateNetworkContainerRequest struct { AllowNCToHostCommunication bool EndpointPolicies []NetworkContainerRequestPolicies NCStatus v1alpha.NCStatus + SwiftV2PrefixOnNic bool // Indicates if is swiftv2 nc, PrefixOnNic scenario (isSwiftV2 && nc.Type == VNETBlock) NetworkInterfaceInfo NetworkInterfaceInfo //nolint // introducing new field for backendnic, to be used later by cni code } diff --git a/cns/kubecontroller/nodenetworkconfig/conversion.go b/cns/kubecontroller/nodenetworkconfig/conversion.go index f2f3d9e9cf..1fe66a2bb5 100644 --- a/cns/kubecontroller/nodenetworkconfig/conversion.go +++ b/cns/kubecontroller/nodenetworkconfig/conversion.go @@ -62,6 +62,7 @@ func CreateNCRequestFromDynamicNC(nc v1alpha.NetworkContainer) (*cns.CreateNetwo NetworkContainerid: nc.ID, NetworkContainerType: cns.Docker, Version: strconv.FormatInt(nc.Version, 10), //nolint:gomnd // it's decimal + SwiftV2PrefixOnNic: false, // Dynamic NCs don't use SwiftV2 PrefixOnNic IPConfiguration: cns.IPConfiguration{ IPSubnet: subnet, GatewayIPAddress: nc.DefaultGateway, diff --git a/cns/kubecontroller/nodenetworkconfig/conversion_linux.go b/cns/kubecontroller/nodenetworkconfig/conversion_linux.go index 9d425aa48f..c8e805c185 100644 --- a/cns/kubecontroller/nodenetworkconfig/conversion_linux.go +++ b/cns/kubecontroller/nodenetworkconfig/conversion_linux.go @@ -60,6 +60,7 @@ func createNCRequestFromStaticNCHelper(nc v1alpha.NetworkContainer, primaryIPPre GatewayIPv6Address: nc.DefaultGatewayV6, }, NCStatus: nc.Status, + SwiftV2PrefixOnNic: isSwiftV2 && nc.Type == v1alpha.VNETBlock, NetworkInterfaceInfo: cns.NetworkInterfaceInfo{ MACAddress: nc.MacAddress, }, diff --git a/cns/restserver/internalapi.go b/cns/restserver/internalapi.go index efefb3f2d3..0c8e9fc197 100644 --- a/cns/restserver/internalapi.go +++ b/cns/restserver/internalapi.go @@ -15,6 +15,7 @@ import ( "strconv" "strings" "time" + "runtime" "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/logger" @@ -194,6 +195,16 @@ var errNonExistentContainerStatus = errors.New("nonExistantContainerstatus") // all NCs and update the CNS state accordingly. This function returns the the total number of NCs on this VM that have been programmed to // some version, NOT the number of NCs that are up-to-date. func (service *HTTPRestService) syncHostNCVersion(ctx context.Context, channelMode string) (int, error) { + // will remove it later + // hasVNETBlockNC1 := service.isPrefixonNicSwiftV2() + // logger.Printf("winDebug: syncHostNCVersion hasVNETBlockNC1 %v", hasVNETBlockNC1) + // if runtime.GOOS == "windows" { + // logger.Printf("winDebug: before setPrefixOnNICRegistry in syncHostNCVersion") + // err := service.setPrefixOnNICRegistry(true, "aa:bb:cc:dd:ee:ff") + // if err != nil { + // logger.Debugf("failed to add PrefixOnNic keys to Windows registry: %w", err) + // } + // } outdatedNCs := map[string]struct{}{} programmedNCs := map[string]struct{}{} for idx := range service.state.ContainerStatus { @@ -219,7 +230,7 @@ func (service *HTTPRestService) syncHostNCVersion(ctx context.Context, channelMo programmedNCs[service.state.ContainerStatus[idx].ID] = struct{}{} } } - if len(outdatedNCs) == 0 { + if len(outdatedNCs) != 0 { return len(programmedNCs), nil } @@ -717,8 +728,27 @@ func (service *HTTPRestService) GetIMDSNCs(ctx context.Context) (map[string]stri if ncID != "" { ncs[ncID] = PrefixOnNicNCVersion // for prefix on nic version scenario nc version is 1 + } else if runtime.GOOS == "windows" && service.isPrefixonNicSwiftV2() { + err := service.setPrefixOnNICRegistry(true, iface.MacAddress.String()) + if err != nil { + logger.Debugf("failed to add PrefixOnNic keys to Windows registry: %w", err) + } } } return ncs, nil } + +// isPrefixonNicSwiftV2 checks if any NC in the container status should use SwiftV2 PrefixOnNic +// Uses the SwiftV2PrefixOnNic field which captures the condition: isSwiftV2 && nc.Type == VNETBlock +func (service *HTTPRestService) isPrefixonNicSwiftV2() bool { + for _, containerStatus := range service.state.ContainerStatus { + req := containerStatus.CreateNetworkContainerRequest + + // Check if this NC is SwiftV2 PrefixOnNic setting + if req.SwiftV2PrefixOnNic { + return true + } + } + return false +} \ No newline at end of file diff --git a/cns/restserver/internalapi_linux.go b/cns/restserver/internalapi_linux.go index 0abae0a72a..285f1abc80 100644 --- a/cns/restserver/internalapi_linux.go +++ b/cns/restserver/internalapi_linux.go @@ -181,3 +181,13 @@ func (service *HTTPRestService) programSNATRules(req *cns.CreateNetworkContainer func (service *HTTPRestService) setVFForAccelnetNICs() error { return nil } + +func (service *HTTPRestService) setPrefixOnNICRegistry(enabled bool, infraNicMacAddress string) error { + logger.Printf("[setPrefixOnNicEnabled winDebug] No-op on Linux platform") + return nil +} + +func (service *HTTPRestService) getPrefixOnNicEnabled() (bool, error) { + logger.Printf(" winDebug getPrefixOnNicEnabled is a no-op on non-Windows platforms") + return false, nil // Add this missing return statement +} \ No newline at end of file diff --git a/cns/restserver/internalapi_windows.go b/cns/restserver/internalapi_windows.go index 245053c8d6..8c66d1bb83 100644 --- a/cns/restserver/internalapi_windows.go +++ b/cns/restserver/internalapi_windows.go @@ -6,14 +6,20 @@ import ( "time" "github.com/Azure/azure-container-networking/cns" + "github.com/Azure/azure-container-networking/cns/logger" "github.com/Azure/azure-container-networking/cns/types" "github.com/Microsoft/hcsshim" "github.com/pkg/errors" + "golang.org/x/sys/windows/registry" ) const ( // timeout for powershell command to return the interfaces list - pwshTimeout = 120 * time.Second + pwshTimeout = 120 * time.Second + hnsRegistryPath = `SYSTEM\CurrentControlSet\Services\HNS\wcna_state\config` + prefixOnNicRegistryPath = `SYSTEM\CurrentControlSet\Services\HNS\wcna_state\config\PrefixOnNic` + infraNicIfName = "eth0" + enableSNAT = false ) var errUnsupportedAPI = errors.New("unsupported api") @@ -75,3 +81,91 @@ func (service *HTTPRestService) getPrimaryNICMACAddress() (string, error) { } return macAddress, nil } + +func (service *HTTPRestService) enablePrefixOnNic(enabled bool) error { + return service.setRegistryValue(prefixOnNicRegistryPath, "enabled", enabled) +} + +func (service *HTTPRestService) setInfraNicMacAddress(macAddress string) error { + return service.setRegistryValue(prefixOnNicRegistryPath, "infra_nic_mac_address", macAddress) +} + +func (service *HTTPRestService) setInfraNicIfName(ifName string) error { + return service.setRegistryValue(prefixOnNicRegistryPath, "infra_nic_ifname", ifName) +} + +func (service *HTTPRestService) setEnableSNAT(enabled bool) error { + return service.setRegistryValue(hnsRegistryPath, "EnableSNAT", enabled) +} + +func (service *HTTPRestService) setPrefixOnNICRegistry(enablePrefixOnNic bool, infraNicMacAddress string) error { + if err := service.enablePrefixOnNic(enablePrefixOnNic); err != nil { + return fmt.Errorf("failed to set enablePrefixOnNic key to windows registry: %w", err) + } + + if err := service.setInfraNicMacAddress(infraNicMacAddress); err != nil { + return fmt.Errorf("failed to set InfraNicMacAddress key to windows registry: %w", err) + } + + if err := service.setInfraNicIfName(infraNicIfName); err != nil { + return fmt.Errorf("failed to set InfraNicIfName key to windows registry: %w", err) + } + + if err := service.setEnableSNAT(enableSNAT); err != nil { + return fmt.Errorf("failed to set EnableSNAT key to windows registry: %w", err) + } + + return nil +} + +func (service *HTTPRestService) setRegistryValue(registryPath, keyName string, value interface{}) error { + key, _, err := registry.CreateKey(registry.LOCAL_MACHINE, registryPath, registry.SET_VALUE) + if err != nil { + return fmt.Errorf("failed to create/open registry key %s: %w", registryPath, err) + } + defer key.Close() + + switch v := value.(type) { + case string: + err = key.SetStringValue(keyName, v) + case bool: + dwordValue := uint32(0) + if v { + dwordValue = 1 + } + err = key.SetDWordValue(keyName, dwordValue) + case uint32: + err = key.SetDWordValue(keyName, v) + case int: + err = key.SetDWordValue(keyName, uint32(v)) + default: + return fmt.Errorf("unsupported value type for registry key %s: %T", keyName, value) + } + + if err != nil { + return fmt.Errorf("failed to set registry value '%s': %w", keyName, err) + } + + logger.Printf("[setRegistryValue] Set %s\\%s = %v", registryPath, keyName, value) + // have to remove this log later + // test, _ := service.getPrefixOnNicEnabled() + // logger.Printf("winDebug: setRegistryValue getPrefixOnNicEnabled %v", test) + return nil +} + +// for testing purpose, will remove it later + +// func (service *HTTPRestService) getPrefixOnNicEnabled() (bool, error) { +// key, err := registry.OpenKey(registry.LOCAL_MACHINE, prefixOnNicRegistryPath, registry.QUERY_VALUE) +// if err != nil { +// return false, nil // Key doesn't exist, default to false +// } +// defer key.Close() + +// value, _, err := key.GetIntegerValue("enabled") +// if err != nil { +// return false, nil // Value doesn't exist, default to false +// } + +// return value == 1, nil +// } From 23a756b8c0c97313d4c8add5172c3b6a138dcb6d Mon Sep 17 00:00:00 2001 From: Niha Nallappagari Date: Wed, 1 Oct 2025 09:35:07 -0500 Subject: [PATCH 6/7] update tests --- cns/restserver/internalapi.go | 6 +- cns/restserver/internalapi_test.go | 133 +++++++++++++++++++++++++++++ 2 files changed, 136 insertions(+), 3 deletions(-) diff --git a/cns/restserver/internalapi.go b/cns/restserver/internalapi.go index 0c8e9fc197..c7fd159662 100644 --- a/cns/restserver/internalapi.go +++ b/cns/restserver/internalapi.go @@ -230,7 +230,7 @@ func (service *HTTPRestService) syncHostNCVersion(ctx context.Context, channelMo programmedNCs[service.state.ContainerStatus[idx].ID] = struct{}{} } } - if len(outdatedNCs) != 0 { + if len(outdatedNCs) == 0 { return len(programmedNCs), nil } @@ -240,7 +240,7 @@ func (service *HTTPRestService) syncHostNCVersion(ctx context.Context, channelMo } // Get IMDS NC versions for delegated NIC scenarios - imdsNCVersions, err := service.GetIMDSNCs(ctx) + imdsNCVersions, err := service.getIMDSNCs(ctx) if err != nil { // If any of the NMA API check calls, imds calls fails assume that nma build doesn't have the latest changes and create empty map imdsNCVersions = make(map[string]string) @@ -696,7 +696,7 @@ func (service *HTTPRestService) isNCDetailsAPIExists(ctx context.Context) bool { } // GetIMDSNCs gets NC versions from IMDS and returns them as a map -func (service *HTTPRestService) GetIMDSNCs(ctx context.Context) (map[string]string, error) { +func (service *HTTPRestService) getIMDSNCs(ctx context.Context) (map[string]string, error) { imdsClient := service.imdsClient if imdsClient == nil { //nolint:staticcheck // SA1019: suppress deprecated logger.Printf usage. Todo: legacy logger usage is consistent in cns repo. Migrates when all logger usage is migrated diff --git a/cns/restserver/internalapi_test.go b/cns/restserver/internalapi_test.go index 4df797a498..6708ed55eb 100644 --- a/cns/restserver/internalapi_test.go +++ b/cns/restserver/internalapi_test.go @@ -15,6 +15,7 @@ import ( "sync" "testing" "time" + "runtime" "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/common" @@ -1680,3 +1681,135 @@ func setupIMDSMockAPIsWithCustomIDs(svc *HTTPRestService, interfaceIDs []string) // Return cleanup function return func() { svc.imdsClient = originalIMDS } } + +// TestSyncHostNCVersionWithWindowsSwiftV2 tests SyncHostNCVersion and verifies it calls Windows SwiftV2 PrefixOnNic scenario +func TestSyncHostNCVersionWithWindowsSwiftV2(t *testing.T) { + svc := getTestService(cns.Kubernetes) + + // Set up test NCs with different scenarios + regularNCID := "regular-nc-id" + swiftV2NCID := "swift-v2-vnet-block-nc" + + // Initialize ContainerStatus map if nil + if svc.state.ContainerStatus == nil { + svc.state.ContainerStatus = make(map[string]containerstatus) + } + + // Add a regular NC + svc.state.ContainerStatus[regularNCID] = containerstatus{ + ID: regularNCID, + CreateNetworkContainerRequest: cns.CreateNetworkContainerRequest{ + NetworkContainerid: regularNCID, + SwiftV2PrefixOnNic: false, + NetworkContainerType: cns.Docker, + Version: "2", + }, + HostVersion: "1", + } + + // Add a SwiftV2 VNETBlock NC that should trigger Windows registry operations + svc.state.ContainerStatus[swiftV2NCID] = containerstatus{ + ID: swiftV2NCID, + CreateNetworkContainerRequest: cns.CreateNetworkContainerRequest{ + NetworkContainerid: swiftV2NCID, + SwiftV2PrefixOnNic: true, + NetworkContainerType: cns.Docker, + Version: "2", + }, + HostVersion: "1", + } + + // Set up mock NMAgent with NC versions + mockNMA := &fakes.NMAgentClientFake{} + mockNMA.GetNCVersionListF = func(ctx context.Context) (nma.NCVersionList, error) { + return nma.NCVersionList{ + Containers: []nma.NCVersion{ + { + NetworkContainerID: regularNCID, + Version: "2", + }, + { + NetworkContainerID: swiftV2NCID, + Version: "2", + }, + }, + }, nil + } + svc.nma = mockNMA + + // Set up mock IMDS client for Windows SwiftV2 scenario + mac1, _ := net.ParseMAC("AA:BB:CC:DD:EE:FF") + mac2, _ := net.ParseMAC("11:22:33:44:55:66") + + interfaceMap := map[string]imds.NetworkInterface{ + "interface1": { + InterfaceCompartmentID: "", // Empty for Windows condition + MacAddress: imds.HardwareAddr(mac1), + }, + "interface2": { + InterfaceCompartmentID: "nc-with-compartment-id", + MacAddress: imds.HardwareAddr(mac2), + }, + } + mockIMDS := &mockIMDSAdapter{ + mock: &struct { + networkInterfaces func(_ context.Context) ([]imds.NetworkInterface, error) + imdsVersions func(_ context.Context) (*imds.APIVersionsResponse, error) + }{ + networkInterfaces: func(_ context.Context) ([]imds.NetworkInterface, error) { + var interfaces []imds.NetworkInterface + for _, iface := range interfaceMap { + interfaces = append(interfaces, iface) + } + return interfaces, nil + }, + imdsVersions: func(_ context.Context) (*imds.APIVersionsResponse, error) { + return &imds.APIVersionsResponse{ + APIVersions: []string{expectedIMDSAPIVersion}, + }, nil + }, + }, + } + + // Replace the IMDS client + originalIMDS := svc.imdsClient + svc.imdsClient = mockIMDS + defer func() { svc.imdsClient = originalIMDS }() + + // Verify preconditions + assert.True(t, svc.isPrefixonNicSwiftV2(), "isPrefixonNicSwiftV2() should return true") + + ctx := context.Background() + svc.SyncHostNCVersion(ctx, cns.CRD) + + // Verify that NC versions were updated + updatedRegularNC := svc.state.ContainerStatus[regularNCID] + updatedSwiftV2NC := svc.state.ContainerStatus[swiftV2NCID] + + assert.Equal(t, "2", updatedRegularNC.HostVersion, "Regular NC host version should be updated to 2") + assert.Equal(t, "2", updatedSwiftV2NC.HostVersion, "SwiftV2 NC host version should be updated to 2") + + imdsNCs, err := svc.getIMDSNCs(ctx) + assert.NoError(t, err, "getIMDSNCs should not return error") + + // Verify IMDS results + assert.Contains(t, imdsNCs, "nc-with-compartment-id", "NC with compartment ID should be in results") + assert.Equal(t, PrefixOnNicNCVersion, imdsNCs["nc-with-compartment-id"], "NC should have expected version") + + // Log the conditions that would trigger Windows registry operations + isWindows := runtime.GOOS == "windows" + hasSwiftV2PrefixOnNic := svc.isPrefixonNicSwiftV2() + + t.Logf("Windows SwiftV2 PrefixOnNic conditions: (runtime.GOOS == 'windows' && service.isPrefixonNicSwiftV2()): %t", + isWindows && hasSwiftV2PrefixOnNic) + + + // Test with no SwiftV2 NCs + delete(svc.state.ContainerStatus, swiftV2NCID) + assert.False(t, svc.isPrefixonNicSwiftV2(), "isPrefixonNicSwiftV2() should return false without SwiftV2 NCs") + + // Call getIMDSNCs again to verify condition is not triggered + _, err2 := svc.getIMDSNCs(ctx) + assert.NoError(t, err2, "getIMDSNCs should not return error") +} + From 4395f1723d63739fdb56ae8dc956ff893e8f3740 Mon Sep 17 00:00:00 2001 From: Niha Nallappagari Date: Mon, 13 Oct 2025 01:29:01 -0500 Subject: [PATCH 7/7] remove unused code --- cns/NetworkContainerContract.go | 9 +- .../nodenetworkconfig/conversion.go | 5 +- cns/restserver/internalapi.go | 40 +++---- cns/restserver/internalapi_linux.go | 15 ++- cns/restserver/internalapi_test.go | 100 ++++++++---------- cns/restserver/internalapi_windows.go | 36 ++----- 6 files changed, 87 insertions(+), 118 deletions(-) diff --git a/cns/NetworkContainerContract.go b/cns/NetworkContainerContract.go index 20d808897f..0a9e9cb2e6 100644 --- a/cns/NetworkContainerContract.go +++ b/cns/NetworkContainerContract.go @@ -7,12 +7,13 @@ import ( "strconv" "strings" - "github.com/Azure/azure-container-networking/cns/types" - "github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha" - "github.com/Azure/azure-container-networking/network/policy" "github.com/google/uuid" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" + + "github.com/Azure/azure-container-networking/cns/types" + "github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha" + "github.com/Azure/azure-container-networking/network/policy" ) // Container Network Service DNC Contract @@ -128,7 +129,7 @@ type CreateNetworkContainerRequest struct { AllowNCToHostCommunication bool EndpointPolicies []NetworkContainerRequestPolicies NCStatus v1alpha.NCStatus - SwiftV2PrefixOnNic bool // Indicates if is swiftv2 nc, PrefixOnNic scenario (isSwiftV2 && nc.Type == VNETBlock) + SwiftV2PrefixOnNic bool // Indicates if is swiftv2 nc, PrefixOnNic scenario (isSwiftV2 && nc.Type == VNETBlock) NetworkInterfaceInfo NetworkInterfaceInfo //nolint // introducing new field for backendnic, to be used later by cni code } diff --git a/cns/kubecontroller/nodenetworkconfig/conversion.go b/cns/kubecontroller/nodenetworkconfig/conversion.go index 1fe66a2bb5..38cf9bb067 100644 --- a/cns/kubecontroller/nodenetworkconfig/conversion.go +++ b/cns/kubecontroller/nodenetworkconfig/conversion.go @@ -6,9 +6,10 @@ import ( "strconv" "strings" + "github.com/pkg/errors" + "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha" - "github.com/pkg/errors" ) var ( @@ -62,7 +63,7 @@ func CreateNCRequestFromDynamicNC(nc v1alpha.NetworkContainer) (*cns.CreateNetwo NetworkContainerid: nc.ID, NetworkContainerType: cns.Docker, Version: strconv.FormatInt(nc.Version, 10), //nolint:gomnd // it's decimal - SwiftV2PrefixOnNic: false, // Dynamic NCs don't use SwiftV2 PrefixOnNic + SwiftV2PrefixOnNic: false, // Dynamic NCs don't use SwiftV2 PrefixOnNic IPConfiguration: cns.IPConfiguration{ IPSubnet: subnet, GatewayIPAddress: nc.DefaultGateway, diff --git a/cns/restserver/internalapi.go b/cns/restserver/internalapi.go index c7fd159662..0fbe299a12 100644 --- a/cns/restserver/internalapi.go +++ b/cns/restserver/internalapi.go @@ -12,10 +12,12 @@ import ( "net/http" "net/http/httptest" "reflect" + "runtime" "strconv" "strings" "time" - "runtime" + + "github.com/pkg/errors" "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/logger" @@ -23,7 +25,6 @@ import ( "github.com/Azure/azure-container-networking/cns/types" "github.com/Azure/azure-container-networking/common" "github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha" - "github.com/pkg/errors" ) const ( @@ -195,16 +196,6 @@ var errNonExistentContainerStatus = errors.New("nonExistantContainerstatus") // all NCs and update the CNS state accordingly. This function returns the the total number of NCs on this VM that have been programmed to // some version, NOT the number of NCs that are up-to-date. func (service *HTTPRestService) syncHostNCVersion(ctx context.Context, channelMode string) (int, error) { - // will remove it later - // hasVNETBlockNC1 := service.isPrefixonNicSwiftV2() - // logger.Printf("winDebug: syncHostNCVersion hasVNETBlockNC1 %v", hasVNETBlockNC1) - // if runtime.GOOS == "windows" { - // logger.Printf("winDebug: before setPrefixOnNICRegistry in syncHostNCVersion") - // err := service.setPrefixOnNICRegistry(true, "aa:bb:cc:dd:ee:ff") - // if err != nil { - // logger.Debugf("failed to add PrefixOnNic keys to Windows registry: %w", err) - // } - // } outdatedNCs := map[string]struct{}{} programmedNCs := map[string]struct{}{} for idx := range service.state.ContainerStatus { @@ -239,8 +230,8 @@ func (service *HTTPRestService) syncHostNCVersion(ctx context.Context, channelMo return len(programmedNCs), errors.Wrap(err, "failed to get nc version list from nmagent") } - // Get IMDS NC versions for delegated NIC scenarios - imdsNCVersions, err := service.getIMDSNCs(ctx) + // Get IMDS NC versions for delegated NIC scenarios. If any of the NMA API check calls, imds calls fails assume that nma build doesn't have the latest changes and create empty map + imdsNCVersions := service.getIMDSNCs(ctx) if err != nil { // If any of the NMA API check calls, imds calls fails assume that nma build doesn't have the latest changes and create empty map imdsNCVersions = make(map[string]string) @@ -696,18 +687,18 @@ func (service *HTTPRestService) isNCDetailsAPIExists(ctx context.Context) bool { } // GetIMDSNCs gets NC versions from IMDS and returns them as a map -func (service *HTTPRestService) getIMDSNCs(ctx context.Context) (map[string]string, error) { +func (service *HTTPRestService) getIMDSNCs(ctx context.Context) map[string]string { imdsClient := service.imdsClient if imdsClient == nil { //nolint:staticcheck // SA1019: suppress deprecated logger.Printf usage. Todo: legacy logger usage is consistent in cns repo. Migrates when all logger usage is migrated logger.Errorf("IMDS client is not available") - return make(map[string]string), nil + return make(map[string]string) } // Check NC version support if !service.isNCDetailsAPIExists(ctx) { //nolint:staticcheck // SA1019: suppress deprecated logger.Printf usage. Todo: legacy logger usage is consistent in cns repo. Migrates when all logger usage is migrated logger.Errorf("IMDS does not support NC details API") - return make(map[string]string), nil + return make(map[string]string) } // Get all network interfaces from IMDS @@ -715,7 +706,7 @@ func (service *HTTPRestService) getIMDSNCs(ctx context.Context) (map[string]stri if err != nil { //nolint:staticcheck // SA1019: suppress deprecated logger.Printf usage. Todo: legacy logger usage is consistent in cns repo. Migrates when all logger usage is migrated logger.Errorf("Failed to get network interfaces from IMDS: %v", err) - return make(map[string]string), nil + return make(map[string]string) } // Build ncs map from the network interfaces @@ -731,24 +722,23 @@ func (service *HTTPRestService) getIMDSNCs(ctx context.Context) (map[string]stri } else if runtime.GOOS == "windows" && service.isPrefixonNicSwiftV2() { err := service.setPrefixOnNICRegistry(true, iface.MacAddress.String()) if err != nil { + //nolint:staticcheck // SA1019: suppress deprecated logger.Debugf usage. Todo: legacy logger usage is consistent in cns repo. Migrates when all logger usage is migrated logger.Debugf("failed to add PrefixOnNic keys to Windows registry: %w", err) } } } - return ncs, nil + return ncs } -// isPrefixonNicSwiftV2 checks if any NC in the container status should use SwiftV2 PrefixOnNic -// Uses the SwiftV2PrefixOnNic field which captures the condition: isSwiftV2 && nc.Type == VNETBlock +// Check whether NC is SwiftV2 NIC associated NC and prefix on nic is enabled func (service *HTTPRestService) isPrefixonNicSwiftV2() bool { - for _, containerStatus := range service.state.ContainerStatus { - req := containerStatus.CreateNetworkContainerRequest + for i := range service.state.ContainerStatus { + req := service.state.ContainerStatus[i].CreateNetworkContainerRequest - // Check if this NC is SwiftV2 PrefixOnNic setting if req.SwiftV2PrefixOnNic { return true } } return false -} \ No newline at end of file +} diff --git a/cns/restserver/internalapi_linux.go b/cns/restserver/internalapi_linux.go index 285f1abc80..0020ada454 100644 --- a/cns/restserver/internalapi_linux.go +++ b/cns/restserver/internalapi_linux.go @@ -6,13 +6,14 @@ import ( "os/exec" "strconv" + goiptables "github.com/coreos/go-iptables/iptables" + "github.com/pkg/errors" + "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/logger" "github.com/Azure/azure-container-networking/cns/types" "github.com/Azure/azure-container-networking/iptables" "github.com/Azure/azure-container-networking/network/networkutils" - goiptables "github.com/coreos/go-iptables/iptables" - "github.com/pkg/errors" ) const SWIFTPOSTROUTING = "SWIFT-POSTROUTING" @@ -183,11 +184,9 @@ func (service *HTTPRestService) setVFForAccelnetNICs() error { } func (service *HTTPRestService) setPrefixOnNICRegistry(enabled bool, infraNicMacAddress string) error { - logger.Printf("[setPrefixOnNicEnabled winDebug] No-op on Linux platform") + // Assigning parameters to '_' to avoid unused parameter linting errors. + // These parameters are only used in the Windows implementation. + _ = enabled + _ = infraNicMacAddress return nil } - -func (service *HTTPRestService) getPrefixOnNicEnabled() (bool, error) { - logger.Printf(" winDebug getPrefixOnNicEnabled is a no-op on non-Windows platforms") - return false, nil // Add this missing return statement -} \ No newline at end of file diff --git a/cns/restserver/internalapi_test.go b/cns/restserver/internalapi_test.go index 6708ed55eb..ec9295e358 100644 --- a/cns/restserver/internalapi_test.go +++ b/cns/restserver/internalapi_test.go @@ -10,12 +10,18 @@ import ( "net" "os" "reflect" + "runtime" "strconv" "strings" "sync" "testing" "time" - "runtime" + + "github.com/google/uuid" + "github.com/pkg/errors" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.org/x/exp/maps" "github.com/Azure/azure-container-networking/cns" "github.com/Azure/azure-container-networking/cns/common" @@ -26,11 +32,6 @@ import ( "github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha" nma "github.com/Azure/azure-container-networking/nmagent" "github.com/Azure/azure-container-networking/store" - "github.com/google/uuid" - "github.com/pkg/errors" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - "golang.org/x/exp/maps" ) const ( @@ -1684,19 +1685,19 @@ func setupIMDSMockAPIsWithCustomIDs(svc *HTTPRestService, interfaceIDs []string) // TestSyncHostNCVersionWithWindowsSwiftV2 tests SyncHostNCVersion and verifies it calls Windows SwiftV2 PrefixOnNic scenario func TestSyncHostNCVersionWithWindowsSwiftV2(t *testing.T) { - svc := getTestService(cns.Kubernetes) - + testSvc := getTestService(cns.Kubernetes) + // Set up test NCs with different scenarios regularNCID := "regular-nc-id" swiftV2NCID := "swift-v2-vnet-block-nc" - + // Initialize ContainerStatus map if nil - if svc.state.ContainerStatus == nil { - svc.state.ContainerStatus = make(map[string]containerstatus) + if testSvc.state.ContainerStatus == nil { + testSvc.state.ContainerStatus = make(map[string]containerstatus) } - + // Add a regular NC - svc.state.ContainerStatus[regularNCID] = containerstatus{ + testSvc.state.ContainerStatus[regularNCID] = containerstatus{ ID: regularNCID, CreateNetworkContainerRequest: cns.CreateNetworkContainerRequest{ NetworkContainerid: regularNCID, @@ -1706,49 +1707,49 @@ func TestSyncHostNCVersionWithWindowsSwiftV2(t *testing.T) { }, HostVersion: "1", } - + // Add a SwiftV2 VNETBlock NC that should trigger Windows registry operations - svc.state.ContainerStatus[swiftV2NCID] = containerstatus{ + testSvc.state.ContainerStatus[swiftV2NCID] = containerstatus{ ID: swiftV2NCID, CreateNetworkContainerRequest: cns.CreateNetworkContainerRequest{ NetworkContainerid: swiftV2NCID, - SwiftV2PrefixOnNic: true, + SwiftV2PrefixOnNic: true, NetworkContainerType: cns.Docker, Version: "2", }, HostVersion: "1", } - + // Set up mock NMAgent with NC versions mockNMA := &fakes.NMAgentClientFake{} - mockNMA.GetNCVersionListF = func(ctx context.Context) (nma.NCVersionList, error) { + mockNMA.GetNCVersionListF = func(_ context.Context) (nma.NCVersionList, error) { return nma.NCVersionList{ Containers: []nma.NCVersion{ { NetworkContainerID: regularNCID, - Version: "2", + Version: "2", }, { NetworkContainerID: swiftV2NCID, - Version: "2", + Version: "2", }, }, }, nil } - svc.nma = mockNMA - + testSvc.nma = mockNMA + // Set up mock IMDS client for Windows SwiftV2 scenario mac1, _ := net.ParseMAC("AA:BB:CC:DD:EE:FF") mac2, _ := net.ParseMAC("11:22:33:44:55:66") - + interfaceMap := map[string]imds.NetworkInterface{ "interface1": { InterfaceCompartmentID: "", // Empty for Windows condition - MacAddress: imds.HardwareAddr(mac1), + MacAddress: imds.HardwareAddr(mac1), }, "interface2": { InterfaceCompartmentID: "nc-with-compartment-id", - MacAddress: imds.HardwareAddr(mac2), + MacAddress: imds.HardwareAddr(mac2), }, } mockIMDS := &mockIMDSAdapter{ @@ -1770,46 +1771,39 @@ func TestSyncHostNCVersionWithWindowsSwiftV2(t *testing.T) { }, }, } - + // Replace the IMDS client - originalIMDS := svc.imdsClient - svc.imdsClient = mockIMDS - defer func() { svc.imdsClient = originalIMDS }() - + originalIMDS := testSvc.imdsClient + testSvc.imdsClient = mockIMDS + defer func() { testSvc.imdsClient = originalIMDS }() + // Verify preconditions - assert.True(t, svc.isPrefixonNicSwiftV2(), "isPrefixonNicSwiftV2() should return true") + assert.True(t, testSvc.isPrefixonNicSwiftV2(), "isPrefixonNicSwiftV2() should return true") ctx := context.Background() - svc.SyncHostNCVersion(ctx, cns.CRD) - + testSvc.SyncHostNCVersion(ctx, cns.CRD) + // Verify that NC versions were updated - updatedRegularNC := svc.state.ContainerStatus[regularNCID] - updatedSwiftV2NC := svc.state.ContainerStatus[swiftV2NCID] - + updatedRegularNC := testSvc.state.ContainerStatus[regularNCID] + updatedSwiftV2NC := testSvc.state.ContainerStatus[swiftV2NCID] + assert.Equal(t, "2", updatedRegularNC.HostVersion, "Regular NC host version should be updated to 2") assert.Equal(t, "2", updatedSwiftV2NC.HostVersion, "SwiftV2 NC host version should be updated to 2") - - imdsNCs, err := svc.getIMDSNCs(ctx) - assert.NoError(t, err, "getIMDSNCs should not return error") - + + imdsNCs := testSvc.getIMDSNCs(ctx) + // Verify IMDS results assert.Contains(t, imdsNCs, "nc-with-compartment-id", "NC with compartment ID should be in results") assert.Equal(t, PrefixOnNicNCVersion, imdsNCs["nc-with-compartment-id"], "NC should have expected version") - + // Log the conditions that would trigger Windows registry operations isWindows := runtime.GOOS == "windows" - hasSwiftV2PrefixOnNic := svc.isPrefixonNicSwiftV2() - - t.Logf("Windows SwiftV2 PrefixOnNic conditions: (runtime.GOOS == 'windows' && service.isPrefixonNicSwiftV2()): %t", + hasSwiftV2PrefixOnNic := testSvc.isPrefixonNicSwiftV2() + + t.Logf("Windows SwiftV2 PrefixOnNic conditions: (runtime.GOOS == 'windows' && service.isPrefixonNicSwiftV2()): %t", isWindows && hasSwiftV2PrefixOnNic) - - + // Test with no SwiftV2 NCs - delete(svc.state.ContainerStatus, swiftV2NCID) - assert.False(t, svc.isPrefixonNicSwiftV2(), "isPrefixonNicSwiftV2() should return false without SwiftV2 NCs") - - // Call getIMDSNCs again to verify condition is not triggered - _, err2 := svc.getIMDSNCs(ctx) - assert.NoError(t, err2, "getIMDSNCs should not return error") + delete(testSvc.state.ContainerStatus, swiftV2NCID) + assert.False(t, testSvc.isPrefixonNicSwiftV2(), "isPrefixonNicSwiftV2() should return false without SwiftV2 NCs") } - diff --git a/cns/restserver/internalapi_windows.go b/cns/restserver/internalapi_windows.go index 8c66d1bb83..446be42d6e 100644 --- a/cns/restserver/internalapi_windows.go +++ b/cns/restserver/internalapi_windows.go @@ -5,17 +5,18 @@ import ( "fmt" "time" - "github.com/Azure/azure-container-networking/cns" - "github.com/Azure/azure-container-networking/cns/logger" - "github.com/Azure/azure-container-networking/cns/types" "github.com/Microsoft/hcsshim" "github.com/pkg/errors" "golang.org/x/sys/windows/registry" + + "github.com/Azure/azure-container-networking/cns" + "github.com/Azure/azure-container-networking/cns/logger" + "github.com/Azure/azure-container-networking/cns/types" ) const ( // timeout for powershell command to return the interfaces list - pwshTimeout = 120 * time.Second + pwshTimeout = 120 * time.Second hnsRegistryPath = `SYSTEM\CurrentControlSet\Services\HNS\wcna_state\config` prefixOnNicRegistryPath = `SYSTEM\CurrentControlSet\Services\HNS\wcna_state\config\PrefixOnNic` infraNicIfName = "eth0" @@ -137,35 +138,18 @@ func (service *HTTPRestService) setRegistryValue(registryPath, keyName string, v case uint32: err = key.SetDWordValue(keyName, v) case int: + case int: + if v < 0 || v > int(^uint32(0)) { + return fmt.Errorf("int value %d overflows uint32 for registry key %s", v, keyName) + } err = key.SetDWordValue(keyName, uint32(v)) default: return fmt.Errorf("unsupported value type for registry key %s: %T", keyName, value) } - if err != nil { return fmt.Errorf("failed to set registry value '%s': %w", keyName, err) } - logger.Printf("[setRegistryValue] Set %s\\%s = %v", registryPath, keyName, value) - // have to remove this log later - // test, _ := service.getPrefixOnNicEnabled() - // logger.Printf("winDebug: setRegistryValue getPrefixOnNicEnabled %v", test) + logger.Printf("[setRegistryValue] Set %s\\%s = %v", registryPath, keyName, value) return nil } - -// for testing purpose, will remove it later - -// func (service *HTTPRestService) getPrefixOnNicEnabled() (bool, error) { -// key, err := registry.OpenKey(registry.LOCAL_MACHINE, prefixOnNicRegistryPath, registry.QUERY_VALUE) -// if err != nil { -// return false, nil // Key doesn't exist, default to false -// } -// defer key.Close() - -// value, _, err := key.GetIntegerValue("enabled") -// if err != nil { -// return false, nil // Value doesn't exist, default to false -// } - -// return value == 1, nil -// }