From c6653851338cb212c876b37337768fbe0379f81a Mon Sep 17 00:00:00 2001 From: "Behzad.Mirkhanzadeh" Date: Wed, 20 May 2026 11:33:45 -0700 Subject: [PATCH 1/3] fix: sort epInfos to process InfraNIC first before EndpointCreate Move the epInfos sort from network.EndpointCreate to the CNI plugin layer (cni/network/network.go) after the createEpInfo loop. In stateless mode, ExternalInterfaces is empty on every ADD so whichever NIC is processed first determines which interface the network gets bound to. If DelegatedVMNIC wins the race, SecondaryEndpointClient moves its interface into the pod namespace, then TransparentEndpointClient fails with "no such network interface". Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cni/network/network.go | 19 +++++++++++++++ cni/network/network_linux_test.go | 39 +++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/cni/network/network.go b/cni/network/network.go index fa22e798e7..d56e80450c 100644 --- a/cni/network/network.go +++ b/cni/network/network.go @@ -9,6 +9,7 @@ import ( "fmt" "net" "regexp" + "slices" "strconv" "time" @@ -640,6 +641,24 @@ func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error { } }() + // Sort epInfos so InfraNIC is processed first. In stateless mode, ExternalInterfaces is empty + // on every ADD, so whichever NIC is processed first determines which interface the network + // gets bound to. If DelegatedVMNIC wins the race, the network gets created with eth1 as extIf. + // SecondaryEndpointClient then moves eth1 into the pod namespace, and the subsequent InfraNIC + // iteration finds the network bound to that now-gone interface, causing + // TransparentEndpointClient.AddEndpoints to fail with "no such network interface". + slices.SortStableFunc(epInfos, func(a, b *network.EndpointInfo) int { + aIsInfra := a.NICType == cns.InfraNIC || a.NICType == "" + bIsInfra := b.NICType == cns.InfraNIC || b.NICType == "" + if aIsInfra && !bIsInfra { + return -1 + } + if !aIsInfra && bIsInfra { + return 1 + } + return 0 + }) + err = plugin.nm.EndpointCreate(cnsclient, epInfos) if err != nil { return errors.Wrap(err, "failed to create endpoint") // behavior can change if you don't assign to err prior to returning diff --git a/cni/network/network_linux_test.go b/cni/network/network_linux_test.go index 3403447501..1aa4a4d7cf 100644 --- a/cni/network/network_linux_test.go +++ b/cni/network/network_linux_test.go @@ -8,6 +8,7 @@ import ( "fmt" "net" "regexp" + "slices" "testing" "github.com/Azure/azure-container-networking/cni" @@ -794,3 +795,41 @@ func (m *mockInterfaceGetter) GetNetworkInterfaces() ([]net.Interface, error) { func (m *mockInterfaceGetter) GetNetworkInterfaceAddrs(_ *net.Interface) ([]net.Addr, error) { return nil, errNotImplemented } + +func TestSortEpInfosInfraNICFirst(t *testing.T) { + // Replicate the sort logic used before EndpointCreate to verify InfraNIC + // is always processed first regardless of input order. + epInfos := []*network.EndpointInfo{ + { + NICType: cns.DelegatedVMNIC, + EndpointID: "delegated-ep", + NetworkID: network.DefaultNetworkID, + }, + { + NICType: cns.InfraNIC, + EndpointID: "infra-ep", + NetworkID: network.DefaultNetworkID, + }, + { + NICType: cns.NodeNetworkInterfaceFrontendNIC, + EndpointID: "frontend-ep", + NetworkID: "other", + }, + } + + slices.SortStableFunc(epInfos, func(a, b *network.EndpointInfo) int { + aIsInfra := a.NICType == cns.InfraNIC || a.NICType == "" + bIsInfra := b.NICType == cns.InfraNIC || b.NICType == "" + if aIsInfra && !bIsInfra { + return -1 + } + if !aIsInfra && bIsInfra { + return 1 + } + return 0 + }) + + assert.Equal(t, cns.InfraNIC, epInfos[0].NICType, "InfraNIC should be sorted first") + assert.Equal(t, cns.DelegatedVMNIC, epInfos[1].NICType, "DelegatedVMNIC should come after InfraNIC") + assert.Equal(t, cns.NodeNetworkInterfaceFrontendNIC, epInfos[2].NICType, "FrontendNIC should come last") +} From 324112bd85b2103761ffb26dc0ea5071a03606d3 Mon Sep 17 00:00:00 2001 From: "Behzad.Mirkhanzadeh" Date: Fri, 29 May 2026 12:51:46 -0700 Subject: [PATCH 2/3] refactor: extract isInfraOrLegacyNICType helper and rewrite sort test Address PR review comments: - Extract isInfraOrLegacyNICType() helper function to deduplicate the repeated NICType == InfraNIC || NICType == "" checks - Replace TestSortEpInfosInfraNICFirst with TestAddSortsInfraNICFirst that tests through the plugin.Add() flow, verifying InfraNIC is created first by recording endpoint creation order via mock callback - Use the helper in both the sort comparator and the Delete path Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cni/network/network.go | 15 ++-- cni/network/network_linux_test.go | 134 +++++++++++++++++++++++------- 2 files changed, 112 insertions(+), 37 deletions(-) diff --git a/cni/network/network.go b/cni/network/network.go index d56e80450c..0404a29f54 100644 --- a/cni/network/network.go +++ b/cni/network/network.go @@ -648,12 +648,10 @@ func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error { // iteration finds the network bound to that now-gone interface, causing // TransparentEndpointClient.AddEndpoints to fail with "no such network interface". slices.SortStableFunc(epInfos, func(a, b *network.EndpointInfo) int { - aIsInfra := a.NICType == cns.InfraNIC || a.NICType == "" - bIsInfra := b.NICType == cns.InfraNIC || b.NICType == "" - if aIsInfra && !bIsInfra { + if isInfraOrLegacyNICType(a.NICType) && !isInfraOrLegacyNICType(b.NICType) { return -1 } - if !aIsInfra && bIsInfra { + if !isInfraOrLegacyNICType(a.NICType) && isInfraOrLegacyNICType(b.NICType) { return 1 } return 0 @@ -666,6 +664,12 @@ func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error { return nil } +// isInfraOrLegacyNICType returns true if the NIC type is InfraNIC or empty (legacy). +// Empty NICType is treated as infra for backward compatibility with older CNS responses. +func isInfraOrLegacyNICType(nicType cns.NICType) bool { + return nicType == cns.InfraNIC || nicType == "" +} + func (plugin *NetPlugin) findMasterInterface(opt *createEpInfoOpt) string { switch opt.ifInfo.NICType { case cns.InfraNIC: @@ -1174,8 +1178,7 @@ func (plugin *NetPlugin) Delete(args *cniSkel.CmdArgs) error { zap.String("endpointID", epInfo.EndpointID)) telemetryClient.SendEvent("Deleting endpoint: " + epInfo.EndpointID) - isInfraOrLegacyNIC := epInfo.NICType == cns.InfraNIC || epInfo.NICType == "" - if !nwCfg.MultiTenancy && isInfraOrLegacyNIC { + if !nwCfg.MultiTenancy && isInfraOrLegacyNICType(epInfo.NICType) { // Call into IPAM plugin to release the endpoint's addresses. for i := range epInfo.IPAddresses { logger.Info("Release ip", zap.String("ip", epInfo.IPAddresses[i].IP.String())) diff --git a/cni/network/network_linux_test.go b/cni/network/network_linux_test.go index 1aa4a4d7cf..a0eec842ec 100644 --- a/cni/network/network_linux_test.go +++ b/cni/network/network_linux_test.go @@ -8,7 +8,6 @@ import ( "fmt" "net" "regexp" - "slices" "testing" "github.com/Azure/azure-container-networking/cni" @@ -796,40 +795,113 @@ func (m *mockInterfaceGetter) GetNetworkInterfaceAddrs(_ *net.Interface) ([]net. return nil, errNotImplemented } -func TestSortEpInfosInfraNICFirst(t *testing.T) { - // Replicate the sort logic used before EndpointCreate to verify InfraNIC - // is always processed first regardless of input order. - epInfos := []*network.EndpointInfo{ - { - NICType: cns.DelegatedVMNIC, - EndpointID: "delegated-ep", - NetworkID: network.DefaultNetworkID, +func TestAddSortsInfraNICFirst(t *testing.T) { + // Verify that plugin.Add() sorts InfraNIC first even when IPAM returns + // several non-infra NICs before infra. The mock records the order of + // endpoint creation to confirm InfraNIC is always processed first. + resources := GetTestResources() + mac1 := "60:45:bd:76:f6:44" + mac2 := "60:45:bd:76:f6:55" + mac3 := "60:45:bd:76:f6:66" + parsedMAC1, _ := net.ParseMAC(mac1) + parsedMAC2, _ := net.ParseMAC(mac2) + parsedMAC3, _ := net.ParseMAC(mac3) + + var creationOrder []cns.NICType + mockEpClient := network.NewMockEndpointClient(func(epInfo *network.EndpointInfo) error { + creationOrder = append(creationOrder, epInfo.NICType) + return nil + }) + + nwCfg := cni.NetworkConfig{ + CNIVersion: "0.3.0", + Name: "net", + MultiTenancy: false, + IPAM: cni.IPAM{Type: "azure-cns"}, + DNS: types.DNS{ + Nameservers: []string{"ns1", "ns2"}, + Domain: "myDomain", }, - { - NICType: cns.InfraNIC, - EndpointID: "infra-ep", - NetworkID: network.DefaultNetworkID, + } + + plugin := &NetPlugin{ + Plugin: resources.Plugin, + nm: network.NewMockNetworkmanager(mockEpClient), + ipamInvoker: &MockIpamInvoker{ + add: func(opt IPAMAddConfig) (IPAMAddResult, error) { + // Return several FrontendNICs and one InfraNIC to test sorting. + // FrontendNIC is the secondary NIC type used in Linux SwiftV2. + result := IPAMAddResult{interfaceInfo: map[string]network.InterfaceInfo{ + "frontend1": { + MacAddress: parsedMAC1, + IPConfigs: []*network.IPConfig{ + {Address: *getCIDRNotationForAddress("10.241.0.35/32")}, + }, + NICType: cns.NodeNetworkInterfaceFrontendNIC, + }, + "frontend2": { + MacAddress: parsedMAC2, + IPConfigs: []*network.IPConfig{ + {Address: *getCIDRNotationForAddress("10.241.0.36/32")}, + }, + NICType: cns.NodeNetworkInterfaceFrontendNIC, + }, + "frontend3": { + MacAddress: parsedMAC3, + IPConfigs: []*network.IPConfig{ + {Address: *getCIDRNotationForAddress("10.241.0.37/32")}, + }, + NICType: cns.NodeNetworkInterfaceFrontendNIC, + }, + string(cns.InfraNIC): { + IPConfigs: []*network.IPConfig{ + {Address: *getCIDRNotationForAddress("20.241.0.35/16")}, + }, + NICType: cns.InfraNIC, + HostSubnetPrefix: *getCIDRNotationForAddress("10.224.0.0/16"), + }, + }} + return result, nil + }, + ipMap: make(map[string]bool), }, - { - NICType: cns.NodeNetworkInterfaceFrontendNIC, - EndpointID: "frontend-ep", - NetworkID: "other", + netClient: &InterfaceGetterMock{ + interfaces: []net.Interface{ + {Name: "eth1", HardwareAddr: parsedMAC1}, + {Name: "eth2", HardwareAddr: parsedMAC2}, + {Name: "eth3", HardwareAddr: parsedMAC3}, + {Name: "primary", HardwareAddr: net.HardwareAddr{}}, + }, + interfaceAddrs: map[string][]net.Addr{ + "primary": {getCIDRNotationForAddress("10.224.0.0/16")}, + }, }, } - slices.SortStableFunc(epInfos, func(a, b *network.EndpointInfo) int { - aIsInfra := a.NICType == cns.InfraNIC || a.NICType == "" - bIsInfra := b.NICType == cns.InfraNIC || b.NICType == "" - if aIsInfra && !bIsInfra { - return -1 - } - if !aIsInfra && bIsInfra { - return 1 - } - return 0 - }) + // Stub resolveMasterInterface for all frontend NICs + original := nlClient + nlClient = &mockNetlinkClient{ + links: map[string]vishnetlink.Link{ + "eth1": &vishnetlink.Dummy{LinkAttrs: vishnetlink.LinkAttrs{Name: "eth1", MasterIndex: 0}}, + "eth2": &vishnetlink.Dummy{LinkAttrs: vishnetlink.LinkAttrs{Name: "eth2", MasterIndex: 0}}, + "eth3": &vishnetlink.Dummy{LinkAttrs: vishnetlink.LinkAttrs{Name: "eth3", MasterIndex: 0}}, + }, + } + t.Cleanup(func() { nlClient = original }) + + args := &cniSkel.CmdArgs{ + StdinData: nwCfg.Serialize(), + ContainerID: "test-container", + Netns: "bc526fae-4ba0-4e80-bc90-ad721e5850bf", + Args: fmt.Sprintf("K8S_POD_NAME=%v;K8S_POD_NAMESPACE=%v", "test-pod", "test-pod-ns"), + IfName: eth0IfName, + } - assert.Equal(t, cns.InfraNIC, epInfos[0].NICType, "InfraNIC should be sorted first") - assert.Equal(t, cns.DelegatedVMNIC, epInfos[1].NICType, "DelegatedVMNIC should come after InfraNIC") - assert.Equal(t, cns.NodeNetworkInterfaceFrontendNIC, epInfos[2].NICType, "FrontendNIC should come last") + err := plugin.Add(args) + require.NoError(t, err) + require.Len(t, creationOrder, 4, "expected 4 endpoints to be created") + assert.Equal(t, cns.InfraNIC, creationOrder[0], "InfraNIC should be created first") + for i := 1; i < len(creationOrder); i++ { + assert.Equal(t, cns.NodeNetworkInterfaceFrontendNIC, creationOrder[i], "non-infra NICs should be created after InfraNIC") + } } From 997455731be7e2e1709ca6d44a2ff4c278e8f8f4 Mon Sep 17 00:00:00 2001 From: "Behzad.Mirkhanzadeh" Date: Fri, 29 May 2026 13:46:05 -0700 Subject: [PATCH 3/3] refactor: extract sortInfraNICFirst and isInfraOrLegacyNICType helpers Address PR review comments: - Extract sortInfraNICFirst() to encapsulate the sort logic that ensures InfraNIC is always processed before other NIC types - Extract isInfraOrLegacyNICType() helper to deduplicate the repeated NICType == InfraNIC || NICType == "" checks - Replace complex integration test with simple unit test for sortInfraNICFirst covering multiple NIC type orderings - Use the helper in both the sort and the Delete path Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cni/network/network.go | 18 ++-- cni/network/network_linux_test.go | 133 +++++++----------------------- 2 files changed, 43 insertions(+), 108 deletions(-) diff --git a/cni/network/network.go b/cni/network/network.go index 0404a29f54..d209e0cc07 100644 --- a/cni/network/network.go +++ b/cni/network/network.go @@ -647,6 +647,18 @@ func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error { // SecondaryEndpointClient then moves eth1 into the pod namespace, and the subsequent InfraNIC // iteration finds the network bound to that now-gone interface, causing // TransparentEndpointClient.AddEndpoints to fail with "no such network interface". + sortInfraNICFirst(epInfos) + + err = plugin.nm.EndpointCreate(cnsclient, epInfos) + if err != nil { + return errors.Wrap(err, "failed to create endpoint") // behavior can change if you don't assign to err prior to returning + } + return nil +} + +// sortInfraNICFirst sorts endpoint infos so that InfraNIC (or legacy empty NICType) +// entries come before all other NIC types, preserving relative order among equals. +func sortInfraNICFirst(epInfos []*network.EndpointInfo) { slices.SortStableFunc(epInfos, func(a, b *network.EndpointInfo) int { if isInfraOrLegacyNICType(a.NICType) && !isInfraOrLegacyNICType(b.NICType) { return -1 @@ -656,12 +668,6 @@ func (plugin *NetPlugin) Add(args *cniSkel.CmdArgs) error { } return 0 }) - - err = plugin.nm.EndpointCreate(cnsclient, epInfos) - if err != nil { - return errors.Wrap(err, "failed to create endpoint") // behavior can change if you don't assign to err prior to returning - } - return nil } // isInfraOrLegacyNICType returns true if the NIC type is InfraNIC or empty (legacy). diff --git a/cni/network/network_linux_test.go b/cni/network/network_linux_test.go index a0eec842ec..e67d225348 100644 --- a/cni/network/network_linux_test.go +++ b/cni/network/network_linux_test.go @@ -795,113 +795,42 @@ func (m *mockInterfaceGetter) GetNetworkInterfaceAddrs(_ *net.Interface) ([]net. return nil, errNotImplemented } -func TestAddSortsInfraNICFirst(t *testing.T) { - // Verify that plugin.Add() sorts InfraNIC first even when IPAM returns - // several non-infra NICs before infra. The mock records the order of - // endpoint creation to confirm InfraNIC is always processed first. - resources := GetTestResources() - mac1 := "60:45:bd:76:f6:44" - mac2 := "60:45:bd:76:f6:55" - mac3 := "60:45:bd:76:f6:66" - parsedMAC1, _ := net.ParseMAC(mac1) - parsedMAC2, _ := net.ParseMAC(mac2) - parsedMAC3, _ := net.ParseMAC(mac3) - - var creationOrder []cns.NICType - mockEpClient := network.NewMockEndpointClient(func(epInfo *network.EndpointInfo) error { - creationOrder = append(creationOrder, epInfo.NICType) - return nil - }) - - nwCfg := cni.NetworkConfig{ - CNIVersion: "0.3.0", - Name: "net", - MultiTenancy: false, - IPAM: cni.IPAM{Type: "azure-cns"}, - DNS: types.DNS{ - Nameservers: []string{"ns1", "ns2"}, - Domain: "myDomain", +func TestSortInfraNICFirst(t *testing.T) { + tests := []struct { + name string + input []cns.NICType + wantFirst cns.NICType + }{ + { + name: "infra already first", + input: []cns.NICType{cns.InfraNIC, cns.NodeNetworkInterfaceFrontendNIC, cns.NodeNetworkInterfaceFrontendNIC}, + wantFirst: cns.InfraNIC, }, - } - - plugin := &NetPlugin{ - Plugin: resources.Plugin, - nm: network.NewMockNetworkmanager(mockEpClient), - ipamInvoker: &MockIpamInvoker{ - add: func(opt IPAMAddConfig) (IPAMAddResult, error) { - // Return several FrontendNICs and one InfraNIC to test sorting. - // FrontendNIC is the secondary NIC type used in Linux SwiftV2. - result := IPAMAddResult{interfaceInfo: map[string]network.InterfaceInfo{ - "frontend1": { - MacAddress: parsedMAC1, - IPConfigs: []*network.IPConfig{ - {Address: *getCIDRNotationForAddress("10.241.0.35/32")}, - }, - NICType: cns.NodeNetworkInterfaceFrontendNIC, - }, - "frontend2": { - MacAddress: parsedMAC2, - IPConfigs: []*network.IPConfig{ - {Address: *getCIDRNotationForAddress("10.241.0.36/32")}, - }, - NICType: cns.NodeNetworkInterfaceFrontendNIC, - }, - "frontend3": { - MacAddress: parsedMAC3, - IPConfigs: []*network.IPConfig{ - {Address: *getCIDRNotationForAddress("10.241.0.37/32")}, - }, - NICType: cns.NodeNetworkInterfaceFrontendNIC, - }, - string(cns.InfraNIC): { - IPConfigs: []*network.IPConfig{ - {Address: *getCIDRNotationForAddress("20.241.0.35/16")}, - }, - NICType: cns.InfraNIC, - HostSubnetPrefix: *getCIDRNotationForAddress("10.224.0.0/16"), - }, - }} - return result, nil - }, - ipMap: make(map[string]bool), + { + name: "infra last among several frontends", + input: []cns.NICType{cns.NodeNetworkInterfaceFrontendNIC, cns.NodeNetworkInterfaceFrontendNIC, cns.NodeNetworkInterfaceFrontendNIC, cns.InfraNIC}, + wantFirst: cns.InfraNIC, }, - netClient: &InterfaceGetterMock{ - interfaces: []net.Interface{ - {Name: "eth1", HardwareAddr: parsedMAC1}, - {Name: "eth2", HardwareAddr: parsedMAC2}, - {Name: "eth3", HardwareAddr: parsedMAC3}, - {Name: "primary", HardwareAddr: net.HardwareAddr{}}, - }, - interfaceAddrs: map[string][]net.Addr{ - "primary": {getCIDRNotationForAddress("10.224.0.0/16")}, - }, + { + name: "infra in the middle", + input: []cns.NICType{cns.DelegatedVMNIC, cns.InfraNIC, cns.NodeNetworkInterfaceFrontendNIC}, + wantFirst: cns.InfraNIC, }, - } - - // Stub resolveMasterInterface for all frontend NICs - original := nlClient - nlClient = &mockNetlinkClient{ - links: map[string]vishnetlink.Link{ - "eth1": &vishnetlink.Dummy{LinkAttrs: vishnetlink.LinkAttrs{Name: "eth1", MasterIndex: 0}}, - "eth2": &vishnetlink.Dummy{LinkAttrs: vishnetlink.LinkAttrs{Name: "eth2", MasterIndex: 0}}, - "eth3": &vishnetlink.Dummy{LinkAttrs: vishnetlink.LinkAttrs{Name: "eth3", MasterIndex: 0}}, + { + name: "empty NICType treated as infra", + input: []cns.NICType{cns.NodeNetworkInterfaceFrontendNIC, ""}, + wantFirst: "", }, } - t.Cleanup(func() { nlClient = original }) - - args := &cniSkel.CmdArgs{ - StdinData: nwCfg.Serialize(), - ContainerID: "test-container", - Netns: "bc526fae-4ba0-4e80-bc90-ad721e5850bf", - Args: fmt.Sprintf("K8S_POD_NAME=%v;K8S_POD_NAMESPACE=%v", "test-pod", "test-pod-ns"), - IfName: eth0IfName, - } - err := plugin.Add(args) - require.NoError(t, err) - require.Len(t, creationOrder, 4, "expected 4 endpoints to be created") - assert.Equal(t, cns.InfraNIC, creationOrder[0], "InfraNIC should be created first") - for i := 1; i < len(creationOrder); i++ { - assert.Equal(t, cns.NodeNetworkInterfaceFrontendNIC, creationOrder[i], "non-infra NICs should be created after InfraNIC") + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + epInfos := make([]*network.EndpointInfo, len(tt.input)) + for i, nic := range tt.input { + epInfos[i] = &network.EndpointInfo{NICType: nic} + } + sortInfraNICFirst(epInfos) + assert.Equal(t, tt.wantFirst, epInfos[0].NICType, "infra/legacy NIC should be sorted first") + }) } }