Skip to content

Commit b132f7b

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

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,8 +5,10 @@ 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"
11+
"github.com/stretchr/testify/require"
1012
)
1113

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

‎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)