Skip to content

Commit 26a56ab

Browse files
committed
Address review comments
Signed-off-by: Tamilmani <[email protected]>
1 parent ac7f244 commit 26a56ab

File tree

6 files changed

+357
-3
lines changed

6 files changed

+357
-3
lines changed

cni/network/invoker_cns_test.go

Lines changed: 313 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2298,3 +2298,316 @@ func TestMultipleIBNICsToResult(t *testing.T) {
22982298
})
22992299
}
23002300
}
2301+
2302+
func TestCNSIPAMInvoker_Add_ApipaNIC(t *testing.T) {
2303+
require := require.New(t)
2304+
2305+
type fields struct {
2306+
podName string
2307+
podNamespace string
2308+
cnsClient cnsclient
2309+
ipamMode util.IpamMode
2310+
}
2311+
type args struct {
2312+
nwCfg *cni.NetworkConfig
2313+
args *cniSkel.CmdArgs
2314+
options map[string]interface{}
2315+
}
2316+
2317+
tests := []struct {
2318+
name string
2319+
fields fields
2320+
args args
2321+
wantApipaResult network.InterfaceInfo
2322+
wantErr bool
2323+
wantErrMsg string
2324+
}{
2325+
{
2326+
name: "Test CNI Add with InfraNIC + ApipaNIC",
2327+
fields: fields{
2328+
podName: testPodInfo.PodName,
2329+
podNamespace: testPodInfo.PodNamespace,
2330+
cnsClient: &MockCNSClient{
2331+
require: require,
2332+
requestIPs: requestIPsHandler{
2333+
ipconfigArgument: cns.IPConfigsRequest{
2334+
PodInterfaceID: "testcont-testifname",
2335+
InfraContainerID: "testcontainerid",
2336+
OrchestratorContext: marshallPodInfo(testPodInfo),
2337+
},
2338+
result: &cns.IPConfigsResponse{
2339+
PodIPInfo: []cns.PodIpInfo{
2340+
{
2341+
PodIPConfig: cns.IPSubnet{
2342+
IPAddress: "10.0.1.10",
2343+
PrefixLength: 24,
2344+
},
2345+
NetworkContainerPrimaryIPConfig: cns.IPConfiguration{
2346+
IPSubnet: cns.IPSubnet{
2347+
IPAddress: "10.0.1.0",
2348+
PrefixLength: 24,
2349+
},
2350+
GatewayIPAddress: "10.0.1.1",
2351+
},
2352+
HostPrimaryIPInfo: cns.HostIPInfo{
2353+
Gateway: "10.0.0.1",
2354+
PrimaryIP: "10.0.0.1",
2355+
Subnet: "10.0.0.0/24",
2356+
},
2357+
NICType: cns.InfraNIC,
2358+
},
2359+
{
2360+
PodIPConfig: cns.IPSubnet{
2361+
IPAddress: "169.254.128.10",
2362+
PrefixLength: 17,
2363+
},
2364+
NetworkContainerPrimaryIPConfig: cns.IPConfiguration{
2365+
GatewayIPAddress: "169.254.128.1",
2366+
},
2367+
NICType: cns.ApipaNIC,
2368+
NetworkContainerID: "test-nc-id",
2369+
AllowHostToNCCommunication: true,
2370+
AllowNCToHostCommunication: false,
2371+
},
2372+
},
2373+
Response: cns.Response{
2374+
ReturnCode: 0,
2375+
Message: "",
2376+
},
2377+
},
2378+
err: nil,
2379+
},
2380+
},
2381+
},
2382+
args: args{
2383+
nwCfg: &cni.NetworkConfig{},
2384+
args: &cniSkel.CmdArgs{
2385+
ContainerID: "testcontainerid",
2386+
Netns: "testnetns",
2387+
IfName: "testifname",
2388+
},
2389+
options: map[string]interface{}{},
2390+
},
2391+
wantApipaResult: network.InterfaceInfo{
2392+
IPConfigs: []*network.IPConfig{
2393+
{
2394+
Address: net.IPNet{
2395+
IP: net.ParseIP("169.254.128.10"),
2396+
Mask: net.CIDRMask(17, 32),
2397+
},
2398+
Gateway: net.ParseIP("169.254.128.1"),
2399+
},
2400+
},
2401+
NICType: cns.ApipaNIC,
2402+
SkipDefaultRoutes: true,
2403+
NetworkContainerID: "test-nc-id",
2404+
AllowHostToNCCommunication: true,
2405+
AllowNCToHostCommunication: false,
2406+
},
2407+
wantErr: false,
2408+
},
2409+
{
2410+
name: "Test CNI add with Frontend Nic + ApipaNIC",
2411+
fields: fields{
2412+
podName: testPodInfo.PodName,
2413+
podNamespace: testPodInfo.PodNamespace,
2414+
cnsClient: &MockCNSClient{
2415+
require: require,
2416+
requestIPs: requestIPsHandler{
2417+
ipconfigArgument: cns.IPConfigsRequest{
2418+
PodInterfaceID: "testcont-testifname",
2419+
InfraContainerID: "testcontainerid",
2420+
OrchestratorContext: marshallPodInfo(testPodInfo),
2421+
},
2422+
result: &cns.IPConfigsResponse{
2423+
PodIPInfo: []cns.PodIpInfo{
2424+
{
2425+
PodIPConfig: cns.IPSubnet{
2426+
IPAddress: "10.0.1.10",
2427+
PrefixLength: 24,
2428+
},
2429+
NetworkContainerPrimaryIPConfig: cns.IPConfiguration{
2430+
IPSubnet: cns.IPSubnet{
2431+
IPAddress: "10.0.1.0",
2432+
PrefixLength: 24,
2433+
},
2434+
GatewayIPAddress: "10.0.1.1",
2435+
},
2436+
HostPrimaryIPInfo: cns.HostIPInfo{
2437+
Gateway: "10.0.0.1",
2438+
PrimaryIP: "10.0.0.1",
2439+
Subnet: "10.0.0.0/24",
2440+
},
2441+
MacAddress: "bc:9a:78:56:34:12",
2442+
NICType: cns.NodeNetworkInterfaceFrontendNIC,
2443+
},
2444+
{
2445+
PodIPConfig: cns.IPSubnet{
2446+
IPAddress: "169.254.5.50",
2447+
PrefixLength: 16,
2448+
},
2449+
NetworkContainerPrimaryIPConfig: cns.IPConfiguration{
2450+
GatewayIPAddress: "169.254.5.1",
2451+
},
2452+
NICType: cns.ApipaNIC,
2453+
NetworkContainerID: "mixed-nc-id",
2454+
AllowHostToNCCommunication: true,
2455+
AllowNCToHostCommunication: false,
2456+
},
2457+
},
2458+
Response: cns.Response{
2459+
ReturnCode: 0,
2460+
Message: "",
2461+
},
2462+
},
2463+
err: nil,
2464+
},
2465+
},
2466+
},
2467+
args: args{
2468+
nwCfg: &cni.NetworkConfig{},
2469+
args: &cniSkel.CmdArgs{
2470+
ContainerID: "testcontainerid",
2471+
Netns: "testnetns",
2472+
IfName: "testifname",
2473+
},
2474+
options: map[string]interface{}{},
2475+
},
2476+
wantApipaResult: network.InterfaceInfo{
2477+
IPConfigs: []*network.IPConfig{
2478+
{
2479+
Address: net.IPNet{
2480+
IP: net.ParseIP("169.254.5.50"),
2481+
Mask: net.CIDRMask(16, 32),
2482+
},
2483+
Gateway: net.ParseIP("169.254.5.1"),
2484+
},
2485+
},
2486+
NICType: cns.ApipaNIC,
2487+
SkipDefaultRoutes: true,
2488+
NetworkContainerID: "mixed-nc-id",
2489+
AllowHostToNCCommunication: true,
2490+
AllowNCToHostCommunication: false,
2491+
},
2492+
wantErr: false,
2493+
},
2494+
{
2495+
name: "Test CNI add with ApipaNIC fails when GetIPNet fails",
2496+
fields: fields{
2497+
podName: testPodInfo.PodName,
2498+
podNamespace: testPodInfo.PodNamespace,
2499+
cnsClient: &MockCNSClient{
2500+
require: require,
2501+
requestIPs: requestIPsHandler{
2502+
ipconfigArgument: cns.IPConfigsRequest{
2503+
PodInterfaceID: "testcont-testifname",
2504+
InfraContainerID: "testcontainerid",
2505+
OrchestratorContext: marshallPodInfo(testPodInfo),
2506+
},
2507+
result: &cns.IPConfigsResponse{
2508+
PodIPInfo: []cns.PodIpInfo{
2509+
{
2510+
PodIPConfig: cns.IPSubnet{
2511+
IPAddress: "invalid-ip-address",
2512+
PrefixLength: 16,
2513+
},
2514+
NetworkContainerPrimaryIPConfig: cns.IPConfiguration{
2515+
GatewayIPAddress: "169.254.1.1",
2516+
},
2517+
NICType: cns.ApipaNIC,
2518+
NetworkContainerID: "failed-nc-id",
2519+
AllowHostToNCCommunication: false,
2520+
AllowNCToHostCommunication: false,
2521+
},
2522+
},
2523+
Response: cns.Response{
2524+
ReturnCode: 0,
2525+
Message: "",
2526+
},
2527+
},
2528+
err: nil,
2529+
},
2530+
},
2531+
},
2532+
args: args{
2533+
nwCfg: &cni.NetworkConfig{},
2534+
args: &cniSkel.CmdArgs{
2535+
ContainerID: "testcontainerid",
2536+
Netns: "testnetns",
2537+
IfName: "testifname",
2538+
},
2539+
options: map[string]interface{}{},
2540+
},
2541+
wantErr: true,
2542+
},
2543+
}
2544+
2545+
for _, tt := range tests {
2546+
tt := tt
2547+
t.Run(tt.name, func(t *testing.T) {
2548+
invoker := &CNSIPAMInvoker{
2549+
podName: tt.fields.podName,
2550+
podNamespace: tt.fields.podNamespace,
2551+
cnsClient: tt.fields.cnsClient,
2552+
}
2553+
if tt.fields.ipamMode != "" {
2554+
invoker.ipamMode = tt.fields.ipamMode
2555+
}
2556+
2557+
ipamAddResult, err := invoker.Add(IPAMAddConfig{
2558+
nwCfg: tt.args.nwCfg,
2559+
args: tt.args.args,
2560+
options: tt.args.options,
2561+
})
2562+
2563+
if tt.wantErr {
2564+
require.Error(err)
2565+
return
2566+
}
2567+
2568+
require.NoError(err)
2569+
2570+
// Find the ApipaNIC interface in the result
2571+
var apipaInterfaceFound bool
2572+
var actualApipaResult network.InterfaceInfo
2573+
2574+
for _, ifInfo := range ipamAddResult.interfaceInfo {
2575+
if ifInfo.NICType == cns.ApipaNIC {
2576+
apipaInterfaceFound = true
2577+
actualApipaResult = ifInfo
2578+
break
2579+
}
2580+
}
2581+
2582+
require.True(apipaInterfaceFound, "ApipaNIC interface should be found in the result")
2583+
2584+
// Verify the ApipaNIC interface info
2585+
// Lines around 2586-2590 should be:
2586+
require.Equal(string(tt.wantApipaResult.NICType), string(actualApipaResult.NICType), "NICType should match expected value")
2587+
require.Equal(tt.wantApipaResult.SkipDefaultRoutes, actualApipaResult.SkipDefaultRoutes)
2588+
require.Equal(tt.wantApipaResult.NetworkContainerID, actualApipaResult.NetworkContainerID)
2589+
require.Equal(tt.wantApipaResult.AllowHostToNCCommunication, actualApipaResult.AllowHostToNCCommunication)
2590+
require.Equal(tt.wantApipaResult.AllowNCToHostCommunication, actualApipaResult.AllowNCToHostCommunication)
2591+
2592+
// Verify IP configs
2593+
require.Equal(1, len(actualApipaResult.IPConfigs), "Should have exactly one IP config for ApipaNIC")
2594+
actualIPConfig := actualApipaResult.IPConfigs[0]
2595+
expectedIPConfig := tt.wantApipaResult.IPConfigs[0]
2596+
2597+
require.True(actualIPConfig.Address.IP.Equal(expectedIPConfig.Address.IP),
2598+
"IP addresses should match: expected %s, got %s",
2599+
expectedIPConfig.Address.IP, actualIPConfig.Address.IP)
2600+
require.Equal(expectedIPConfig.Address.Mask, actualIPConfig.Address.Mask,
2601+
"IP masks should match")
2602+
2603+
if expectedIPConfig.Gateway != nil {
2604+
require.NotNil(actualIPConfig.Gateway, "Gateway should not be nil")
2605+
require.True(actualIPConfig.Gateway.Equal(expectedIPConfig.Gateway),
2606+
"Gateway IPs should match: expected %s, got %s",
2607+
expectedIPConfig.Gateway, actualIPConfig.Gateway)
2608+
} else {
2609+
require.Nil(actualIPConfig.Gateway, "Gateway should be nil")
2610+
}
2611+
})
2612+
}
2613+
}

cni/network/network.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1078,6 +1078,7 @@ func (plugin *NetPlugin) Delete(args *cniSkel.CmdArgs) error {
10781078
epInfos, err = plugin.nm.GetEndpointState(networkID, args.ContainerID)
10791079
// if stateless CNI fail to get the endpoint from CNS for any reason other than Endpoint Not found
10801080
if err != nil {
1081+
// async delete should be disabled for standalone scenario
10811082
if errors.Is(err, network.ErrConnectionFailure) && !nwCfg.DisableAsyncDelete {
10821083
logger.Info("failed to connect to CNS", zap.String("containerID", args.ContainerID), zap.Error(err))
10831084
addErr := fsnotify.AddFile(args.ContainerID, args.ContainerID, watcherPath)

cni/network/network_windows_test.go

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -661,6 +661,45 @@ func TestGetNetworkNameSwiftv2FromCNS(t *testing.T) {
661661
want: "",
662662
wantErr: false,
663663
},
664+
{
665+
name: "Get Network Name from CNS for swiftv2 ApipaNIC",
666+
plugin: &NetPlugin{
667+
Plugin: plugin,
668+
nm: network.NewMockNetworkmanager(network.NewMockEndpointClient(nil)),
669+
ipamInvoker: NewMockIpamInvoker(false, false, false, false, false),
670+
},
671+
netNs: "azure",
672+
nwCfg: &cni.NetworkConfig{
673+
CNIVersion: "0.3.0",
674+
MultiTenancy: false,
675+
},
676+
interfaceInfo: &network.InterfaceInfo{
677+
Name: "apipa-interface",
678+
MacAddress: parsedMacAddress,
679+
NICType: cns.ApipaNIC,
680+
},
681+
want: swiftv2NetworkNamePrefix + "apipa", // "azure-apipa"
682+
wantErr: false,
683+
},
684+
{
685+
name: "Get Network Name from CNS for swiftv2 ApipaNIC with empty MacAddress",
686+
plugin: &NetPlugin{
687+
Plugin: plugin,
688+
nm: network.NewMockNetworkmanager(network.NewMockEndpointClient(nil)),
689+
ipamInvoker: NewMockIpamInvoker(false, false, false, false, false),
690+
},
691+
netNs: "azure",
692+
nwCfg: &cni.NetworkConfig{
693+
CNIVersion: "0.3.0",
694+
MultiTenancy: false,
695+
},
696+
interfaceInfo: &network.InterfaceInfo{
697+
Name: "apipa-test-interface",
698+
NICType: cns.ApipaNIC,
699+
},
700+
want: swiftv2NetworkNamePrefix + "apipa", // "azure-apipa"
701+
wantErr: false,
702+
},
664703
}
665704

666705
for _, tt := range tests {

cns/client/client.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1101,6 +1101,8 @@ func (c *Client) UpdateEndpoint(ctx context.Context, endpointID string, ipInfo m
11011101
return &response, nil
11021102
}
11031103

1104+
// DeleteEndpointState calls the DeleteEndpointHandler API in CNS to delete the state of a given EndpointID(containerID)
1105+
// This api is called for swiftv2 standalone scenario to cleanup state in CNS
11041106
func (c *Client) DeleteEndpointState(ctx context.Context, endpointID string) (*cns.Response, error) {
11051107
// build the request
11061108
u := c.routes[cns.EndpointAPI]

cns/endpointmanager/endpointmanager_windows.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ func (em *EndpointManager) ReleaseIPs(ctx context.Context, ipconfigreq cns.IPCon
1616
if err := em.deleteEndpoint(ctx, ipconfigreq.InfraContainerID); err != nil {
1717
logger.Errorf("failed to remove HNS endpoint %s", err.Error())
1818
}
19-
2019
return errors.Wrap(em.cli.ReleaseIPs(ctx, ipconfigreq), "failed to release IP from CNS")
2120
}
2221

0 commit comments

Comments
 (0)