Skip to content

Commit e50f68c

Browse files
NihaNallappagariNihaNallappagari
committed
Skip add primary ip's to secondary config for swiftv2 pon scenarios
1 parent 68ffe58 commit e50f68c

File tree

7 files changed

+175
-24
lines changed

7 files changed

+175
-24
lines changed

‎cns/kubecontroller/nodenetworkconfig/conversion.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"strings"
88

99
"github.com/Azure/azure-container-networking/cns"
10+
"github.com/Azure/azure-container-networking/cns/configuration"
1011
"github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha"
1112
"github.com/pkg/errors"
1213
)
@@ -73,7 +74,7 @@ func CreateNCRequestFromDynamicNC(nc v1alpha.NetworkContainer) (*cns.CreateNetwo
7374
// CreateNCRequestFromStaticNC generates a CreateNetworkContainerRequest from a static NetworkContainer.
7475
//
7576
//nolint:gocritic //ignore hugeparam
76-
func CreateNCRequestFromStaticNC(nc v1alpha.NetworkContainer) (*cns.CreateNetworkContainerRequest, error) {
77+
func CreateNCRequestFromStaticNC(nc v1alpha.NetworkContainer, config *configuration.CNSConfig) (*cns.CreateNetworkContainerRequest, error) {
7778
if nc.Type == v1alpha.Overlay {
7879
nc.Version = 0 // fix for NMA always giving us version 0 for Overlay NCs
7980
}
@@ -97,7 +98,7 @@ func CreateNCRequestFromStaticNC(nc v1alpha.NetworkContainer) (*cns.CreateNetwor
9798
subnet.IPAddress = primaryPrefix.Addr().String()
9899
}
99100

100-
req, err := createNCRequestFromStaticNCHelper(nc, primaryPrefix, subnet)
101+
req, err := createNCRequestFromStaticNCHelper(nc, primaryPrefix, subnet, config)
101102
if err != nil {
102103
return nil, errors.Wrapf(err, "error while creating NC request from static NC")
103104
}

‎cns/kubecontroller/nodenetworkconfig/conversion_linux.go‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"strconv"
66

77
"github.com/Azure/azure-container-networking/cns"
8+
"github.com/Azure/azure-container-networking/cns/configuration"
89
"github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha"
910
"github.com/pkg/errors"
1011
)
@@ -13,17 +14,18 @@ import (
1314
// by adding all IPs in the the block to the secondary IP configs list. It does not skip any IPs.
1415
//
1516
//nolint:gocritic //ignore hugeparam
16-
func createNCRequestFromStaticNCHelper(nc v1alpha.NetworkContainer, primaryIPPrefix netip.Prefix, subnet cns.IPSubnet) (*cns.CreateNetworkContainerRequest, error) {
17+
func createNCRequestFromStaticNCHelper(nc v1alpha.NetworkContainer, primaryIPPrefix netip.Prefix, subnet cns.IPSubnet, config *configuration.CNSConfig) (*cns.CreateNetworkContainerRequest, error) {
1718
secondaryIPConfigs := map[string]cns.SecondaryIPConfig{}
1819

19-
// Todo: Segregate the NIC’s primary IP from the list of secondary IPs to ensure the primary IP is not assigned to pods
20-
// WorkItem: https://msazure.visualstudio.com/One/_workitems/edit/33460135
2120
// iterate through all IP addresses in the subnet described by primaryPrefix and
2221
// add them to the request as secondary IPConfigs.
23-
for addr := primaryIPPrefix.Masked().Addr(); primaryIPPrefix.Contains(addr); addr = addr.Next() {
24-
secondaryIPConfigs[addr.String()] = cns.SecondaryIPConfig{
25-
IPAddress: addr.String(),
26-
NCVersion: int(nc.Version),
22+
// Process primary prefix IPs in all scenarios except when nc.Type is v1alpha.VNETBlock AND SwiftV2 is enabled
23+
if !(config.EnableSwiftV2 && nc.Type == v1alpha.VNETBlock) {
24+
for addr := primaryIPPrefix.Masked().Addr(); primaryIPPrefix.Contains(addr); addr = addr.Next() {
25+
secondaryIPConfigs[addr.String()] = cns.SecondaryIPConfig{
26+
IPAddress: addr.String(),
27+
NCVersion: int(nc.Version),
28+
}
2729
}
2830
}
2931

‎cns/kubecontroller/nodenetworkconfig/conversion_test.go‎

Lines changed: 139 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"testing"
66

77
"github.com/Azure/azure-container-networking/cns"
8+
"github.com/Azure/azure-container-networking/cns/configuration"
89
"github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha"
910
"github.com/stretchr/testify/assert"
1011
)
@@ -339,7 +340,144 @@ func TestCreateNCRequestFromStaticNC(t *testing.T) {
339340
for _, tt := range tests {
340341
tt := tt
341342
t.Run(tt.name, func(t *testing.T) {
342-
got, err := CreateNCRequestFromStaticNC(tt.input)
343+
config := &configuration.CNSConfig{}
344+
got, err := CreateNCRequestFromStaticNC(tt.input, config)
345+
if tt.wantErr {
346+
assert.Error(t, err)
347+
return
348+
}
349+
assert.NoError(t, err)
350+
assert.EqualValues(t, tt.want, got)
351+
})
352+
}
353+
}
354+
355+
func TestCreateNCRequestFromStaticNCWithConfig(t *testing.T) {
356+
tests := []struct {
357+
name string
358+
input v1alpha.NetworkContainer
359+
config *configuration.CNSConfig
360+
want *cns.CreateNetworkContainerRequest
361+
wantErr bool
362+
}{
363+
{
364+
name: "SwiftV2 enabled with VNETBlock - should NOT process all IPs in prefix",
365+
input: v1alpha.NetworkContainer{
366+
ID: ncID,
367+
PrimaryIP: "10.0.0.0/32",
368+
NodeIP: "10.0.0.1",
369+
Type: v1alpha.VNETBlock,
370+
SubnetAddressSpace: "10.0.0.0/24",
371+
DefaultGateway: "10.0.0.1",
372+
Version: 1,
373+
Status: "Available",
374+
},
375+
config: &configuration.CNSConfig{
376+
EnableSwiftV2: true,
377+
},
378+
want: &cns.CreateNetworkContainerRequest{
379+
NetworkContainerid: ncID,
380+
NetworkContainerType: cns.Docker,
381+
Version: "1",
382+
HostPrimaryIP: "10.0.0.1",
383+
IPConfiguration: cns.IPConfiguration{
384+
IPSubnet: cns.IPSubnet{
385+
IPAddress: "10.0.0.1",
386+
PrefixLength: 24,
387+
},
388+
GatewayIPAddress: "10.0.0.1",
389+
},
390+
SecondaryIPConfigs: map[string]cns.SecondaryIPConfig{
391+
// No IPs from primary prefix
392+
},
393+
NCStatus: "Available",
394+
},
395+
wantErr: false,
396+
},
397+
{
398+
name: "SwiftV2 disabled with VNETBlock - should process all IP in prefix",
399+
input: v1alpha.NetworkContainer{
400+
ID: ncID,
401+
PrimaryIP: "10.0.0.0/32",
402+
NodeIP: "10.0.0.1",
403+
Type: v1alpha.VNETBlock,
404+
SubnetAddressSpace: "10.0.0.0/24",
405+
DefaultGateway: "10.0.0.1",
406+
Version: 1,
407+
Status: "Available",
408+
IPAssignments: []v1alpha.IPAssignment{
409+
{
410+
Name: "test-ip",
411+
IP: "10.0.0.10/32",
412+
},
413+
},
414+
},
415+
config: &configuration.CNSConfig{},
416+
want: &cns.CreateNetworkContainerRequest{
417+
NetworkContainerid: ncID,
418+
NetworkContainerType: cns.Docker,
419+
Version: "1",
420+
HostPrimaryIP: "10.0.0.1",
421+
IPConfiguration: cns.IPConfiguration{
422+
IPSubnet: cns.IPSubnet{
423+
IPAddress: "10.0.0.1",
424+
PrefixLength: 24,
425+
},
426+
GatewayIPAddress: "10.0.0.1",
427+
},
428+
SecondaryIPConfigs: map[string]cns.SecondaryIPConfig{
429+
"10.0.0.0": {IPAddress: "10.0.0.0", NCVersion: 1},
430+
// IP assignments
431+
"10.0.0.10": {IPAddress: "10.0.0.10", NCVersion: 1},
432+
},
433+
NCStatus: "Available",
434+
},
435+
wantErr: false,
436+
},
437+
{
438+
name: "SwiftV2 disabled with non-VNETBlock type - should process IP in prefix",
439+
input: v1alpha.NetworkContainer{
440+
ID: ncID,
441+
PrimaryIP: "10.0.0.0/32",
442+
NodeIP: "10.0.0.1",
443+
Type: v1alpha.Overlay,
444+
SubnetAddressSpace: "10.0.0.0/24",
445+
DefaultGateway: "10.0.0.1",
446+
Version: 1,
447+
Status: "Available",
448+
IPAssignments: []v1alpha.IPAssignment{
449+
{
450+
Name: "test-ip",
451+
IP: "10.0.0.10/32",
452+
},
453+
},
454+
},
455+
config: &configuration.CNSConfig{},
456+
want: &cns.CreateNetworkContainerRequest{
457+
NetworkContainerid: ncID,
458+
NetworkContainerType: cns.Docker,
459+
Version: "0",
460+
HostPrimaryIP: "10.0.0.1",
461+
IPConfiguration: cns.IPConfiguration{
462+
IPSubnet: cns.IPSubnet{
463+
IPAddress: "10.0.0.0",
464+
PrefixLength: 24,
465+
},
466+
GatewayIPAddress: "10.0.0.1",
467+
},
468+
SecondaryIPConfigs: map[string]cns.SecondaryIPConfig{
469+
"10.0.0.0": {IPAddress: "10.0.0.0", NCVersion: 0},
470+
},
471+
NCStatus: "Available",
472+
},
473+
wantErr: false,
474+
},
475+
}
476+
477+
for _, tt := range tests {
478+
tt := tt
479+
t.Run(tt.name, func(t *testing.T) {
480+
got, err := CreateNCRequestFromStaticNC(tt.input, tt.config)
343481
if tt.wantErr {
344482
assert.Error(t, err)
345483
return

‎cns/kubecontroller/nodenetworkconfig/conversion_windows.go‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"strconv"
66

77
"github.com/Azure/azure-container-networking/cns"
8+
"github.com/Azure/azure-container-networking/cns/configuration"
89
"github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha"
910
"github.com/pkg/errors"
1011
)
@@ -14,7 +15,7 @@ import (
1415
// secondary IPs. If the gateway is not empty, it will not reserve the 2nd IP and add it as a secondary IP.
1516
//
1617
//nolint:gocritic //ignore hugeparam
17-
func createNCRequestFromStaticNCHelper(nc v1alpha.NetworkContainer, primaryIPPrefix netip.Prefix, subnet cns.IPSubnet) (*cns.CreateNetworkContainerRequest, error) {
18+
func createNCRequestFromStaticNCHelper(nc v1alpha.NetworkContainer, primaryIPPrefix netip.Prefix, subnet cns.IPSubnet, config *configuration.CNSConfig) (*cns.CreateNetworkContainerRequest, error) {
1819
secondaryIPConfigs := map[string]cns.SecondaryIPConfig{}
1920
// the masked address is the 0th IP in the subnet and startingAddr is the 2nd IP (*.1)
2021
startingAddr := primaryIPPrefix.Masked().Addr().Next()
@@ -27,12 +28,15 @@ func createNCRequestFromStaticNCHelper(nc v1alpha.NetworkContainer, primaryIPPre
2728

2829
// iterate through all IP addresses in the subnet described by primaryPrefix and
2930
// add them to the request as secondary IPConfigs.
30-
for addr := startingAddr; primaryIPPrefix.Contains(addr); addr = addr.Next() {
31-
secondaryIPConfigs[addr.String()] = cns.SecondaryIPConfig{
32-
IPAddress: addr.String(),
33-
NCVersion: int(nc.Version),
31+
// Process primary prefix IPs in all scenarios except when nc.Type is v1alpha.VNETBlock AND SwiftV2 is enabled
32+
if !(config.EnableSwiftV2 && nc.Type == v1alpha.VNETBlock) {
33+
for addr := startingAddr; primaryIPPrefix.Contains(addr); addr = addr.Next() {
34+
secondaryIPConfigs[addr.String()] = cns.SecondaryIPConfig{
35+
IPAddress: addr.String(),
36+
NCVersion: int(nc.Version),
37+
}
38+
lastAddr = addr
3439
}
35-
lastAddr = addr
3640
}
3741

3842
if nc.Type == v1alpha.VNETBlock {

‎cns/kubecontroller/nodenetworkconfig/reconciler.go‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"sync"
66

77
"github.com/Azure/azure-container-networking/cns"
8+
"github.com/Azure/azure-container-networking/cns/configuration"
89
"github.com/Azure/azure-container-networking/cns/logger"
910
"github.com/Azure/azure-container-networking/cns/restserver"
1011
cnstypes "github.com/Azure/azure-container-networking/cns/types"
@@ -43,18 +44,20 @@ type Reconciler struct {
4344
once sync.Once
4445
started chan interface{}
4546
nodeIP string
47+
config *configuration.CNSConfig
4648
}
4749

4850
// NewReconciler creates a NodeNetworkConfig Reconciler which will get updates from the Kubernetes
4951
// apiserver for NNC events.
5052
// Provided nncListeners are passed the NNC after the Reconcile preprocesses it. Note: order matters! The
5153
// passed Listeners are notified in the order provided.
52-
func NewReconciler(cnscli cnsClient, ipampoolmonitorcli nodeNetworkConfigListener, nodeIP string) *Reconciler {
54+
func NewReconciler(cnscli cnsClient, ipampoolmonitorcli nodeNetworkConfigListener, nodeIP string, config *configuration.CNSConfig) *Reconciler {
5355
return &Reconciler{
5456
cnscli: cnscli,
5557
ipampoolmonitorcli: ipampoolmonitorcli,
5658
started: make(chan interface{}),
5759
nodeIP: nodeIP,
60+
config: config,
5861
}
5962
}
6063

@@ -104,7 +107,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco
104107
switch nnc.Status.NetworkContainers[i].AssignmentMode { //nolint:exhaustive // skipping dynamic case
105108
// For Overlay and Vnet Scale Scenarios
106109
case v1alpha.Static:
107-
req, err = CreateNCRequestFromStaticNC(nnc.Status.NetworkContainers[i])
110+
req, err = CreateNCRequestFromStaticNC(nnc.Status.NetworkContainers[i], r.config)
108111
// For Pod Subnet scenario
109112
default: // For backward compatibility, default will be treated as Dynamic too.
110113
req, err = CreateNCRequestFromDynamicNC(nnc.Status.NetworkContainers[i])

‎cns/kubecontroller/nodenetworkconfig/reconciler_test.go‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"testing"
66

77
"github.com/Azure/azure-container-networking/cns"
8+
"github.com/Azure/azure-container-networking/cns/configuration"
89
"github.com/Azure/azure-container-networking/cns/logger"
910
cnstypes "github.com/Azure/azure-container-networking/cns/types"
1011
"github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha"
@@ -192,7 +193,8 @@ func TestReconcile(t *testing.T) {
192193
}
193194

194195
t.Run(tt.name, func(t *testing.T) {
195-
r := NewReconciler(&tt.cnsClient, &tt.cnsClient, tt.nodeIP)
196+
config := &configuration.CNSConfig{} // Use default config for tests
197+
r := NewReconciler(&tt.cnsClient, &tt.cnsClient, tt.nodeIP, config)
196198
r.nnccli = &tt.ncGetter
197199
got, err := r.Reconcile(context.Background(), tt.in)
198200
if tt.wantErr {
@@ -249,7 +251,8 @@ func TestReconcileStaleNCs(t *testing.T) {
249251
return &nncLog[len(nncLog)-1], nil
250252
}
251253

252-
r := NewReconciler(&cnsClient, &cnsClient, nodeIP)
254+
config := &configuration.CNSConfig{} // Use default config for tests
255+
r := NewReconciler(&cnsClient, &cnsClient, nodeIP, config)
253256
r.nnccli = &mockNCGetter{get: nncIterator}
254257

255258
_, err := r.Reconcile(context.Background(), reconcile.Request{})

‎cns/service/main.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1311,7 +1311,7 @@ type ipamStateReconciler interface {
13111311

13121312
// TODO(rbtr) where should this live??
13131313
// reconcileInitialCNSState initializes cns by passing pods and a CreateNetworkContainerRequest
1314-
func reconcileInitialCNSState(ctx context.Context, cli nodeNetworkConfigGetter, ipamReconciler ipamStateReconciler, podInfoByIPProvider cns.PodInfoByIPProvider) error {
1314+
func reconcileInitialCNSState(ctx context.Context, cli nodeNetworkConfigGetter, ipamReconciler ipamStateReconciler, podInfoByIPProvider cns.PodInfoByIPProvider, config *configuration.CNSConfig) error {
13151315
// Get nnc using direct client
13161316
nnc, err := cli.Get(ctx)
13171317
if err != nil {
@@ -1350,7 +1350,7 @@ func reconcileInitialCNSState(ctx context.Context, cli nodeNetworkConfigGetter,
13501350
)
13511351
switch nnc.Status.NetworkContainers[i].AssignmentMode { //nolint:exhaustive // skipping dynamic case
13521352
case v1alpha.Static:
1353-
ncRequest, err = nncctrl.CreateNCRequestFromStaticNC(nnc.Status.NetworkContainers[i])
1353+
ncRequest, err = nncctrl.CreateNCRequestFromStaticNC(nnc.Status.NetworkContainers[i], config)
13541354
default: // For backward compatibility, default will be treated as Dynamic too.
13551355
ncRequest, err = nncctrl.CreateNCRequestFromDynamicNC(nnc.Status.NetworkContainers[i])
13561356
}
@@ -1458,7 +1458,7 @@ func InitializeCRDState(ctx context.Context, z *zap.Logger, httpRestService cns.
14581458
_ = retry.Do(func() error {
14591459
attempt++
14601460
logger.Printf("reconciling initial CNS state attempt: %d", attempt)
1461-
err = reconcileInitialCNSState(ctx, directscopedcli, httpRestServiceImplementation, podInfoByIPProvider)
1461+
err = reconcileInitialCNSState(ctx, directscopedcli, httpRestServiceImplementation, podInfoByIPProvider, cnsconfig)
14621462
if err != nil {
14631463
logger.Errorf("failed to reconcile initial CNS state, attempt: %d err: %v", attempt, err)
14641464
nncInitFailure.Inc()
@@ -1555,7 +1555,7 @@ func InitializeCRDState(ctx context.Context, z *zap.Logger, httpRestService cns.
15551555

15561556
// get CNS Node IP to compare NC Node IP with this Node IP to ensure NCs were created for this node
15571557
nodeIP := configuration.NodeIP()
1558-
nncReconciler := nncctrl.NewReconciler(httpRestServiceImplementation, poolMonitor, nodeIP)
1558+
nncReconciler := nncctrl.NewReconciler(httpRestServiceImplementation, poolMonitor, nodeIP, cnsconfig)
15591559
// pass Node to the Reconciler for Controller xref
15601560
// IPAMv1 - reconcile only status changes (where generation doesn't change).
15611561
// IPAMv2 - reconcile all updates.

0 commit comments

Comments
 (0)