fix: fixing Stateless CNI delete in SwiftV2 scenario #3967

behzad-mir · 2025-08-27T07:19:37Z

A number of changes is made to stateless CNI to fully support SWiftV2 in Linux:

For Stateless CNI, the EndpointID should include ifName when the NicType is Delegated. This ensures it can be distinguished from the InfraNIC endpoint. The reason for this behavior is that Stateless CNI uses only the ContainerID as the endpoint ID when saving the state file in CNS, to maintain consistency with Cilium. However, within CNI’s in-memory representation, the EndpointID must uniquely identify each NIC. As a result:
- We always pass the ContainerID to CNS as the endpoint state key.
- Internally, CNI uses EndpointID (which includes ifName for delegated NICs) to differentiate endpoints.
Delete flow has been revised and NetNSPath has been added to the statefile since it is needed by the TransparentClient for Frontend NIC.
The Transparent mode used by statefull CNI for SWiftV1 and V2 and stateless CNI should follow the same. TransparentVlan which is the original value seems to be a mistake.
Async delete file creation in Stateless CNI has been moved to CNS_invoker to be consistent with Stateful CNI.
Removing secondary interface from pod namespace in SwiftV2 Linux in case of asynchronous delete. This is needed to be covered for stateless CNI since if CNS is down, CNI does not have the secondary interface name. A dedicated function has been added to list the interface in pod namespace and remove the secondary.

For validating the scenario ADD/Delete calls have been issues on SwiftV2 cluster and and logs and satefile has been analyzed to make sure it is consistent with Stateful CNI and also nothing gets leaked.

Requirements:

uses conventional commit messages
includes documentation
adds unit tests
relevant PR labels added

Notes:

Copilot

Pull Request Overview

This PR fixes issues with Stateless CNI delete operations in SwiftV2 scenarios by modifying endpoint ID generation and improving the delete flow. The changes ensure proper distinction between different NIC types and provide necessary context for transparent client operations.

Modifies GetEndpointID to accept a NICType parameter and append interface name for delegated NICs
Updates delete flow to use proper network manager clients and adds NetNsPath to state information
Adds NetworkNameSpace field to CNS REST server structures for frontend NIC support

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
network/manager.go	Core logic changes for endpoint ID generation and delete flow improvements
network/manager_mock.go	Mock implementation updated to match new GetEndpointID signature
cns/restserver/restserver.go	Added NetworkNameSpace field to IPInfo struct
cns/restserver/ipam.go	Updated validation and state management for NetworkNameSpace field
cni/network/network.go	Updated callers to pass NICType parameter to GetEndpointID

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

network/manager.go

cns/restserver/ipam.go

network/manager.go

santhoshmprabhu

I don't have a lot of context on the need for these changes. The changes look fine, but we should improve the PR a bit -

Update the description to cover why the opMode change
Address other copilot comments (one about NICType in particular seems serious)
Add a description of what validation steps have been carried out. Include screenshots/logs if necessary.
Add tests.

cns/restserver/ipam.go

network/manager.go

cni/network/network.go

cns/restserver/restserver.go

network/manager.go

Copilot

Pull Request Overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

cni/network/network.go

network/manager.go

QxBytes

preliminary suggestions-- could you also add how you tested and what happens without each of these changes?

network/secondary_endpoint_client_linux.go

network/manager.go

cni/network/invoker_cns.go

cni/network/network.go

QxBytes

As I'm reading this it seems like a lot of the code is geared around accessing FetchInterfacesFromNetnsPath in secondary_endpoint_client_linux.go, requiring us to drill down through cni/network/network.go > network/manager.go > network/endpoint_linux.go > secondary_endpoint_client_linux.go, adding methods to interfaces which are only specific to swiftv2/secondary endpoint client.

What is it from secondary endpoint client that we actually need? At GetEndpoint() > nm.generateEndpointLocally, there is no guarantee we are even in swiftv2 right? Why can't we just make a helper function (doesn't even need to be part of a client) where generateEndpointLocally is called that does the following:

Enters the pod's netns
Searches for any swiftv2/secondary interfaces if they exist
Creates endpoint infos and returns them if found
Instead of going through the trouble of creating a secondary endpoint client etc.

cni/network/network.go

network/secondary_endpoint_client_linux.go

cni/network/network.go

network/manager.go

network/endpoint_linux.go

network/secondary_endpoint_client_linux.go

network/errors.go

tamilmani1989 · 2025-10-07T02:58:56Z

cni/network/network.go

+			// This is an special case for stateless CNI when Asychronous DEL to CNS will take place
+			// At this point the endpoint is already deleted in previous block and CNS will release the IP whenever it is up
+			if epInfo.IPAddresses == nil && plugin.nm.IsStatelessCNIMode() {
+				logger.Warn("Release ip Asynchronously by CNS",


why releaseip only for stateless cni and not for stateful cni case? current code calling release for both cases

When we generate the ednpoint locally for SwiftV2 stateless when CNS is not up, then we will get to this block at the end to release the IP. The datapath already has been cleand up and we just do a Asynch delete via IPAMInvoker.Delete.

tamilmani1989 · 2025-10-07T03:03:15Z

network/manager.go

+	GetEndpoint(networkID string, args *cniSkel.CmdArgs) ([]*EndpointInfo, error)
 	GetEndpointInfosFromContainerID(containerID string) []*EndpointInfo
-	GetEndpointState(networkID, containerID string) ([]*EndpointInfo, error)
+	GetEndpointState(networkID, containerID, netns string) ([]*EndpointInfo, error)


why do we need 2 apis? can we merge to 1 api, otherwise this is confusing and not sure about purpose of 2 apis

We previous;y doscussed that, and you suggested to have two APIs.

GetEndpointInfos will be a genrall API for both Statefull and Stateless and then GetEndpointState is just for stateless CNI

tamilmani1989 · 2025-10-07T03:04:35Z

network/manager.go

+// GetEndpointIDByNicType returns a unique endpoint ID based on the CNI mode and NIC type.
+func (nm *networkManager) GetEndpointIDByNicType(containerID, ifName string, nicType cns.NICType) string {
+	// For stateless CNI, secondary NICs use containerID-ifName as endpointID.
+	if nm.IsStatelessCNIMode() && nicType != cns.InfraNIC {


for stateful cni, this is not an issue? what's the impact if we remove statelesscnimode check here?

I wanted to make sure we only add - ifname for stateless. Stateful already has this.

tamilmani1989 · 2025-10-07T03:14:57Z

network/manager.go

+		}
+		return epInfos, nil
+	}
+	return nm.GetEndpointInfosFromContainerID(args.ContainerID), nil


if this api is called only for stateful cni, can we keep in else to avoid any error in future

It will cause a linter error, so I added a comment instead.

github-actions · 2025-10-22T00:00:59Z

This pull request is stale because it has been open for 2 weeks with no activity. Remove stale label or comment or this will be closed in 7 days

github-actions · 2025-10-29T00:01:09Z

Pull request closed due to inactivity.

network/manager.go

network/errors.go

network/manager.go

cni/network/network.go

network/endpoint_linux.go

network/manager.go

Copilot AI review requested due to automatic review settings August 27, 2025 07:19

behzad-mir requested review from a team as code owners August 27, 2025 07:19

behzad-mir requested a review from tamilmani1989 August 27, 2025 07:19

Copilot AI reviewed Aug 27, 2025

View reviewed changes

network/manager.go Show resolved Hide resolved

cns/restserver/ipam.go Outdated Show resolved Hide resolved

MikeZappa87 reviewed Aug 27, 2025

View reviewed changes

cns/restserver/ipam.go Outdated Show resolved Hide resolved

MikeZappa87 reviewed Aug 27, 2025

View reviewed changes

cns/restserver/ipam.go Outdated Show resolved Hide resolved

MikeZappa87 reviewed Aug 27, 2025

View reviewed changes

network/manager.go Outdated Show resolved Hide resolved

santhoshmprabhu requested changes Aug 27, 2025

View reviewed changes

tamilmani1989 reviewed Aug 27, 2025

View reviewed changes

cns/restserver/ipam.go Outdated Show resolved Hide resolved

cns/restserver/ipam.go Outdated Show resolved Hide resolved

cns/restserver/ipam.go Outdated Show resolved Hide resolved

network/manager.go Outdated Show resolved Hide resolved

network/manager.go Show resolved Hide resolved

QxBytes reviewed Aug 27, 2025

View reviewed changes

cni/network/network.go Outdated Show resolved Hide resolved

cns/restserver/restserver.go Outdated Show resolved Hide resolved

network/manager.go Outdated Show resolved Hide resolved

network/manager.go Show resolved Hide resolved

behzad-mir force-pushed the swiftv2-stateless branch 6 times, most recently from 52b1ed1 to a6b378e Compare September 18, 2025 16:46

behzad-mir requested review from MikeZappa87, QxBytes, Copilot, santhoshmprabhu and tamilmani1989 September 18, 2025 16:46

Copilot AI reviewed Sep 18, 2025

View reviewed changes

cni/network/network.go Outdated Show resolved Hide resolved

network/manager.go Show resolved Hide resolved

QxBytes reviewed Sep 19, 2025

View reviewed changes

behzad-mir force-pushed the swiftv2-stateless branch 5 times, most recently from 4b7c752 to 5c01db3 Compare September 25, 2025 17:30

behzad-mir requested a review from tamilmani1989 October 3, 2025 01:53

behzad-mir force-pushed the swiftv2-stateless branch 4 times, most recently from 6b96a7c to 8584493 Compare October 6, 2025 18:22

QxBytes requested changes Oct 6, 2025

View reviewed changes

tamilmani1989 reviewed Oct 7, 2025

View reviewed changes

github-actions bot added the stale Stale due to inactivity. label Oct 22, 2025

github-actions bot closed this Oct 29, 2025

github-actions bot deleted the swiftv2-stateless branch October 29, 2025 00:01

behzad-mir restored the swiftv2-stateless branch November 14, 2025 18:10

behzad-mir reopened this Nov 14, 2025

Fix: adding support for SWiftV2 Linux stateless CNI mode

9600774

behzad-mir force-pushed the swiftv2-stateless branch 2 times, most recently from 89c39e4 to 38ada84 Compare November 14, 2025 23:35

github-actions bot removed the stale Stale due to inactivity. label Nov 15, 2025

QxBytes reviewed Nov 15, 2025

View reviewed changes

behzad-mir force-pushed the swiftv2-stateless branch 2 times, most recently from e0bea0b to ece1a70 Compare November 17, 2025 06:02

behzad-mir requested review from QxBytes and tamilmani1989 November 17, 2025 16:54

behzad-mir force-pushed the swiftv2-stateless branch 5 times, most recently from f9af850 to 3ca88d5 Compare November 18, 2025 18:16

fix: generating endpoint locally when CNS is not reachabe.

65b5530

behzad-mir force-pushed the swiftv2-stateless branch from 3ca88d5 to 65b5530 Compare November 18, 2025 19:32

fix: fixing Stateless CNI delete in SwiftV2 scenario #3967

Are you sure you want to change the base?

fix: fixing Stateless CNI delete in SwiftV2 scenario #3967

Uh oh!

Conversation

behzad-mir commented Aug 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

santhoshmprabhu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

QxBytes left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

QxBytes left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Oct 22, 2025

Uh oh!

github-actions bot commented Oct 29, 2025

Uh oh!

behzad-mir commented Aug 27, 2025 •

edited

Loading