cns: run stale-HNS cleanup for delegated-NIC Docker NCs

cns/restserver/api.go

@@ -501,6 +501,25 @@
 	}
 }
 
+// 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 @@
 			}
 		} 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 @@
 					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)

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