diff --git a/cns/restserver/api.go b/cns/restserver/api.go index ac5ba5994c..2744cb150c 100644 --- a/cns/restserver/api.go +++ b/cns/restserver/api.go @@ -501,6 +501,25 @@ func (service *HTTPRestService) getHomeAz(w http.ResponseWriter, r *http.Request } } +// cleanupStaleHNSForDelegatedNIC runs stale-HNS-resource cleanup for a delegated-NIC NC create +// when the feature is enabled. It is a no-op unless stale-HNS cleanup and endpoint-state +// management are both enabled and the request carries a delegated NIC (MAC + delegated NICType). +// On success (including the no-op path) it returns types.Success; on cleanup failure it returns +// UnexpectedError and a message so the caller can fail the NC create closed. +func (service *HTTPRestService) cleanupStaleHNSForDelegatedNIC(req cns.CreateNetworkContainerRequest) (types.ResponseCode, string) { + cleanupEnabled := service.Options[common.OptEnableStaleHNSCleanupOnNCCreate] == true && + service.Options[common.OptManageEndpointState] == true + hasDelegatedNIC := req.NetworkInterfaceInfo.MACAddress != "" && + (req.NetworkInterfaceInfo.NICType == cns.DelegatedVMNIC || req.NetworkInterfaceInfo.NICType == cns.NodeNetworkInterfaceFrontendNIC) + if !cleanupEnabled || !hasDelegatedNIC { + return types.Success, "" + } + if err := service.cleanupStaleHNSResources(req.NetworkContainerid, req.NetworkInterfaceInfo.MACAddress, req.LocalIPConfiguration.IPSubnet.IPAddress); err != nil { + return types.UnexpectedError, fmt.Sprintf("[Azure CNS] stale HNS cleanup failed for MAC %s: %v", req.NetworkInterfaceInfo.MACAddress, err) + } + return types.Success, "" +} + func (service *HTTPRestService) createOrUpdateNetworkContainer(w http.ResponseWriter, r *http.Request) { var req cns.CreateNetworkContainerRequest if err := common.Decode(w, r, &req); err != nil { @@ -535,16 +554,10 @@ func (service *HTTPRestService) createOrUpdateNetworkContainer(w http.ResponseWr } } else if req.NetworkContainerType == cns.AzureContainerInstance { // Clean up stale HNS resources from a previous NC that used the same delegated NIC. - cleanupEnabled := service.Options[common.OptEnableStaleHNSCleanupOnNCCreate] == true && - service.Options[common.OptManageEndpointState] == true - hasDelegatedNIC := req.NetworkInterfaceInfo.MACAddress != "" && - (req.NetworkInterfaceInfo.NICType == cns.DelegatedVMNIC || req.NetworkInterfaceInfo.NICType == cns.NodeNetworkInterfaceFrontendNIC) - if cleanupEnabled && hasDelegatedNIC { - if cleanupErr := service.cleanupStaleHNSResources(req.NetworkContainerid, req.NetworkInterfaceInfo.MACAddress, req.LocalIPConfiguration.IPSubnet.IPAddress); cleanupErr != nil { - returnMessage = fmt.Sprintf("[Azure CNS] stale HNS cleanup failed for MAC %s: %v", req.NetworkInterfaceInfo.MACAddress, cleanupErr) - returnCode = types.UnexpectedError - break - } + if code, msg := service.cleanupStaleHNSForDelegatedNIC(req); code != types.Success { + returnCode = code + returnMessage = msg + break } // try to get the saved nc state if it exists @@ -560,6 +573,14 @@ func (service *HTTPRestService) createOrUpdateNetworkContainer(w http.ResponseWr break } } + } else if req.NetworkContainerType == cns.Docker { + // Clean up stale HNS resources left by a previous NC that used the same delegated NIC, + // then proceed as a normal Docker NC (saveNetworkContainerGoalState below). + if code, msg := service.cleanupStaleHNSForDelegatedNIC(req); code != types.Success { + returnCode = code + returnMessage = msg + break + } } returnCode, returnMessage = service.saveNetworkContainerGoalState(req) diff --git a/cns/restserver/ipam_windows_test.go b/cns/restserver/ipam_windows_test.go index 025f4fad3c..d3a6f82f02 100644 --- a/cns/restserver/ipam_windows_test.go +++ b/cns/restserver/ipam_windows_test.go @@ -5,6 +5,8 @@ import ( "testing" "github.com/Azure/azure-container-networking/cns" + "github.com/Azure/azure-container-networking/cns/types" + "github.com/Azure/azure-container-networking/common" "github.com/Azure/azure-container-networking/store" "github.com/pkg/errors" "github.com/stretchr/testify/assert" @@ -520,6 +522,149 @@ func TestCleanupStaleHNSResources(t *testing.T) { } } +// TestCleanupStaleHNSForDelegatedNIC covers the api.go gating helper that fronts +// cleanupStaleHNSResources for delegated-NIC NC creates (both AzureContainerInstance and Docker). +func TestCleanupStaleHNSForDelegatedNIC(t *testing.T) { + staleDelegatedState := func() map[string]*EndpointInfo { + return map[string]*EndpointInfo{ + "stale-container": { + PodName: "pod1", PodNamespace: "ns1", + IfnameToIPMap: map[string]*IPInfo{ + "eth0": {NICType: cns.DelegatedVMNIC, MacAddress: "00:11:22:33:44:55", HnsEndpointID: "ep-1", HnsNetworkID: "net-1"}, + }, + }, + } + } + + tests := []struct { + name string + cleanupEnabled bool + ncType string + nicType cns.NICType + mac string + localIP string + endpointState map[string]*EndpointInfo + hnsErr error + wantCode types.ResponseCode + wantDeletedEndpoints []string + wantRemaining int + }{ + { + name: "docker delegated NIC with cleanup enabled deletes stale HNS", + cleanupEnabled: true, + ncType: cns.Docker, + nicType: cns.DelegatedVMNIC, + mac: "00:11:22:33:44:55", + endpointState: staleDelegatedState(), + wantCode: types.Success, + wantDeletedEndpoints: []string{"ep-1"}, + wantRemaining: 0, + }, + { + name: "cleanup disabled is a no-op", + cleanupEnabled: false, + ncType: cns.Docker, + nicType: cns.DelegatedVMNIC, + mac: "00:11:22:33:44:55", + endpointState: staleDelegatedState(), + wantCode: types.Success, + wantRemaining: 1, + }, + { + name: "non-delegated NIC is a no-op even when enabled", + cleanupEnabled: true, + ncType: cns.Docker, + nicType: cns.InfraNIC, + mac: "00:11:22:33:44:55", + endpointState: staleDelegatedState(), + wantCode: types.Success, + wantRemaining: 1, + }, + { + name: "empty MAC is a no-op even when enabled", + cleanupEnabled: true, + ncType: cns.Docker, + nicType: cns.DelegatedVMNIC, + mac: "", + endpointState: staleDelegatedState(), + wantCode: types.Success, + wantRemaining: 1, + }, + { + // AKS never has ApipaNIC endpoints; passing the NC local IP as apipaIP must be a safe + // no-op (findStaleContainerByApipaIP finds no match) and only the delegated NIC is cleaned. + name: "local IP passed with no APIPA endpoint present is a safe no-op", + cleanupEnabled: true, + ncType: cns.Docker, + nicType: cns.DelegatedVMNIC, + mac: "00:11:22:33:44:55", + localIP: "10.0.0.4", + endpointState: staleDelegatedState(), + wantCode: types.Success, + wantDeletedEndpoints: []string{"ep-1"}, + wantRemaining: 0, + }, + { + name: "cleanup failure fails closed with UnexpectedError", + cleanupEnabled: true, + ncType: cns.Docker, + nicType: cns.DelegatedVMNIC, + mac: "00:11:22:33:44:55", + endpointState: staleDelegatedState(), + hnsErr: errors.New("HNS access denied"), + wantCode: types.UnexpectedError, + wantRemaining: 1, + }, + { + name: "AzureContainerInstance path still cleans up (regression guard)", + cleanupEnabled: true, + ncType: cns.AzureContainerInstance, + nicType: cns.DelegatedVMNIC, + mac: "00:11:22:33:44:55", + endpointState: staleDelegatedState(), + wantCode: types.Success, + wantDeletedEndpoints: []string{"ep-1"}, + wantRemaining: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + svc := getTestService(cns.AzureContainerInstance) + svc.EndpointStateStore = store.NewMockStore("") + svc.EndpointState = tt.endpointState + require.NoError(t, svc.EndpointStateStore.Write(EndpointStoreKey, svc.EndpointState)) + svc.Options = map[string]interface{}{ + common.OptEnableStaleHNSCleanupOnNCCreate: tt.cleanupEnabled, + common.OptManageEndpointState: tt.cleanupEnabled, + } + + mockClient := &mockHNSClient{err: tt.hnsErr} + orig := defaultHNSClient + t.Cleanup(func() { defaultHNSClient = orig }) + defaultHNSClient = mockClient + + req := cns.CreateNetworkContainerRequest{ + NetworkContainerid: "Swift_new-nc", + NetworkContainerType: tt.ncType, + NetworkInterfaceInfo: cns.NetworkInterfaceInfo{NICType: tt.nicType, MACAddress: tt.mac}, + LocalIPConfiguration: cns.IPConfiguration{IPSubnet: cns.IPSubnet{IPAddress: tt.localIP}}, + } + + code, msg := svc.cleanupStaleHNSForDelegatedNIC(req) + + assert.Equal(t, tt.wantCode, code) + if tt.wantCode != types.Success { + assert.NotEmpty(t, msg) + } + assert.Len(t, svc.EndpointState, tt.wantRemaining) + if tt.wantDeletedEndpoints != nil { + assert.ElementsMatch(t, tt.wantDeletedEndpoints, mockClient.deletedEndpointIDs) + } + }) + } +} + type mockHNSClient struct { err error deletedEndpointIDs []string