Skip to content

Commit b414cf5

Browse files
rejain789rejain789
committed
updated logic
1 parent d452d9f commit b414cf5

File tree

5 files changed

+128
-67
lines changed

5 files changed

+128
-67
lines changed

‎cns/NetworkContainerContract.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,6 @@ type CreateNetworkContainerRequest struct {
129129
EndpointPolicies []NetworkContainerRequestPolicies
130130
NCStatus v1alpha.NCStatus
131131
NetworkInterfaceInfo NetworkInterfaceInfo //nolint // introducing new field for backendnic, to be used later by cni code
132-
Scenario v1alpha.NCType //nolint // introducing new field for nnc reconciler
133132
}
134133

135134
func (req *CreateNetworkContainerRequest) Validate() error {

‎cns/kubecontroller/nodenetworkconfig/conversion.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,6 @@ func CreateNCRequestFromDynamicNC(nc v1alpha.NetworkContainer) (*cns.CreateNetwo
6767
GatewayIPAddress: nc.DefaultGateway,
6868
},
6969
NCStatus: nc.Status,
70-
Scenario: nc.Type,
7170
}, nil
7271
}
7372

‎cns/kubecontroller/nodenetworkconfig/conversion_linux.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,5 @@ func createNCRequestFromStaticNCHelper(nc v1alpha.NetworkContainer, primaryIPPre
6363
NetworkInterfaceInfo: cns.NetworkInterfaceInfo{
6464
MACAddress: nc.MacAddress,
6565
},
66-
Scenario: nc.Type,
6766
}, nil
6867
}

‎cns/restserver/internalapi.go‎

Lines changed: 45 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ import (
2323
"github.com/Azure/azure-container-networking/common"
2424
"github.com/Azure/azure-container-networking/crd/nodenetworkconfig/api/v1alpha"
2525
"github.com/pkg/errors"
26-
"go.uber.org/zap"
2726
)
2827

2928
const (
@@ -36,9 +35,6 @@ const (
3635
// internal APIs (definde in internalapi.go).
3736
// This will be used internally (say by RequestController in case of AKS)
3837

39-
// Initialize a zap logger instance
40-
var zapLogger, _ = zap.NewProduction()
41-
4238
// GetPartitionKey - Get dnc/service partition key
4339
func (service *HTTPRestService) GetPartitionKey() (dncPartitionKey string) {
4440
service.RLock()
@@ -634,11 +630,15 @@ func (service *HTTPRestService) CreateOrUpdateNetworkContainerInternal(req *cns.
634630
if ok {
635631
existingReq := existingNCInfo.CreateNetworkContainerRequest
636632
if !reflect.DeepEqual(existingReq.IPConfiguration.IPSubnet, req.IPConfiguration.IPSubnet) {
637-
if req.Scenario != v1alpha.Overlay { // if overlay -> potentially an overlay subnet expansion is occurring, skip this check
638-
zapLogger.Error("[Azure CNS] Error. PrimaryCA is not same",
639-
zap.String("NCId", req.NetworkContainerid),
640-
zap.String("oldCA", fmt.Sprintf("%s/%d", existingReq.IPConfiguration.IPSubnet.IPAddress, existingReq.IPConfiguration.IPSubnet.PrefixLength)),
641-
zap.String("newCA", fmt.Sprintf("%s/%d", req.IPConfiguration.IPSubnet.IPAddress, req.IPConfiguration.IPSubnet.PrefixLength)))
633+
// check for potential overlay subnet expansion - checking if new subnet is a superset of old subnet
634+
err := validateCIDRSuperset(req.IPConfiguration.IPSubnet.IPAddress, existingReq.IPConfiguration.IPSubnet.IPAddress)
635+
if err != nil {
636+
logger.Errorf("[Azure CNS] Error. PrimaryCA is not same, NCId %s, old CA %s/%d, new CA %s/%d", //nolint:staticcheck // Suppress SA1019: logger.Errorf is deprecated
637+
req.NetworkContainerid,
638+
existingReq.IPConfiguration.IPSubnet.IPAddress,
639+
existingReq.IPConfiguration.IPSubnet.PrefixLength,
640+
req.IPConfiguration.IPSubnet.IPAddress,
641+
req.IPConfiguration.IPSubnet.PrefixLength)
642642
return types.PrimaryCANotSame
643643
}
644644
}
@@ -726,3 +726,39 @@ func (service *HTTPRestService) GetIMDSNCs(ctx context.Context) (map[string]stri
726726

727727
return ncs, nil
728728
}
729+
730+
// IsCIDRSuperset returns true if newCIDR is a superset of oldCIDR (i.e., all IPs in oldCIDR are contained in newCIDR).
731+
func validateCIDRSuperset(newCIDR, oldCIDR string) error {
732+
_, newNet, err := net.ParseCIDR(newCIDR)
733+
if err != nil {
734+
return errors.Wrapf(err, "parsing newCIDR %q", newCIDR)
735+
}
736+
_, oldNet, err := net.ParseCIDR(oldCIDR)
737+
if err != nil {
738+
return errors.Wrapf(err, "parsing oldCIDR %q", oldCIDR)
739+
}
740+
741+
// Check that the network family matches (both IPv4 or both IPv6)
742+
if len(newNet.IP) != len(oldNet.IP) {
743+
return errors.New("CIDRs belong to different IP families")
744+
}
745+
746+
// Check that the old network's base IP is contained in the new network
747+
if !newNet.Contains(oldNet.IP) {
748+
return errors.New("old network's base IP is not contained in the new network")
749+
}
750+
751+
// Calculate the last IP in oldNet
752+
oldLastIP := make(net.IP, len(oldNet.IP))
753+
for i := range oldNet.IP {
754+
oldLastIP[i] = oldNet.IP[i] | ^oldNet.Mask[i]
755+
}
756+
757+
// Check that the last IP in oldNet is also contained in newNet
758+
if !newNet.Contains(oldLastIP) {
759+
return errors.New("last IP of old network is not contained in new network")
760+
}
761+
762+
// If both the first and last IPs of oldNet are in newNet, oldNet is fully contained in newNet
763+
return nil
764+
}

‎cns/restserver/internalapi_test.go‎

Lines changed: 83 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -66,84 +66,112 @@ func TestReconcileNCStatePrimaryIPChangeShouldFail(t *testing.T) {
6666
setOrchestratorTypeInternal(cns.KubernetesCRD)
6767
svc.state.ContainerStatus = make(map[string]containerstatus)
6868

69-
// start with a NC in state
70-
ncID := "555ac5c9-89f2-4b5d-b8d0-616894d6d151"
71-
svc.state.ContainerStatus[ncID] = containerstatus{
72-
ID: ncID,
73-
VMVersion: "0",
74-
HostVersion: "0",
75-
CreateNetworkContainerRequest: cns.CreateNetworkContainerRequest{
76-
NetworkContainerid: ncID,
77-
IPConfiguration: cns.IPConfiguration{
78-
IPSubnet: cns.IPSubnet{
79-
IPAddress: "10.0.1.0",
80-
PrefixLength: 24,
69+
testCases := []struct {
70+
existingIPAddress string
71+
requestIPAddress string
72+
}{
73+
{"", "10.240.0.0/16"},
74+
{"10.240.0.0", "2001:db8::/64"},
75+
{"2001:db8::/64", "10.240.0.0/16"},
76+
{"10.0.1.0/22", "10.0.2.0/24"},
77+
{"10.0.1.0/21", "10.0.1.0/23"},
78+
{"10.0.1.0", "10.0.0.0/15"},
79+
{"10.0.1.0/15", "10.0.0.0"},
80+
}
81+
82+
// Run test cases
83+
for _, tc := range testCases {
84+
// start with a NC in state
85+
ncID := "555ac5c9-89f2-4b5d-b8d0-616894d6d151"
86+
svc.state.ContainerStatus[ncID] = containerstatus{
87+
ID: ncID,
88+
VMVersion: "0",
89+
HostVersion: "0",
90+
CreateNetworkContainerRequest: cns.CreateNetworkContainerRequest{
91+
NetworkContainerid: ncID,
92+
IPConfiguration: cns.IPConfiguration{
93+
IPSubnet: cns.IPSubnet{
94+
IPAddress: tc.existingIPAddress,
95+
PrefixLength: 24,
96+
},
8197
},
8298
},
83-
},
84-
}
99+
}
85100

86-
ncReqs := []*cns.CreateNetworkContainerRequest{
87-
{
88-
NetworkContainerid: ncID,
89-
IPConfiguration: cns.IPConfiguration{
90-
IPSubnet: cns.IPSubnet{
91-
IPAddress: "10.0.2.0", // note this IP has changed
92-
PrefixLength: 24,
101+
ncReqs := []*cns.CreateNetworkContainerRequest{
102+
{
103+
NetworkContainerid: ncID,
104+
IPConfiguration: cns.IPConfiguration{
105+
IPSubnet: cns.IPSubnet{
106+
IPAddress: tc.requestIPAddress,
107+
PrefixLength: 24,
108+
},
93109
},
94110
},
95-
},
96-
}
111+
}
97112

98-
// now try to reconcile the state where the NC primary IP has changed
99-
resp := svc.ReconcileIPAMStateForSwift(ncReqs, map[string]cns.PodInfo{}, &v1alpha.NodeNetworkConfig{})
113+
// now try to reconcile the state where the NC primary IP has changed
114+
resp := svc.ReconcileIPAMStateForSwift(ncReqs, map[string]cns.PodInfo{}, &v1alpha.NodeNetworkConfig{})
115+
116+
assert.Equal(t, types.PrimaryCANotSame, resp)
117+
}
100118

101-
assert.Equal(t, types.PrimaryCANotSame, resp)
102119
}
103120

104121
// TestReconcileNCStatePrimaryIPChangeShouldNotFail tests that reconciling NC state with
105-
// a NC whose IP has changed should not fail for overlay clusters
122+
// a NC whose IP has changed should not fail if new IP is superset of old IP
106123
func TestReconcileNCStatePrimaryIPChangeShouldNotFail(t *testing.T) {
107124
restartService()
108125
setEnv(t)
109126
setOrchestratorTypeInternal(cns.KubernetesCRD)
110127
svc.state.ContainerStatus = make(map[string]containerstatus)
111128

112-
// start with a NC in state
113-
ncID := "555ac5c9-89f2-4b5d-b8d0-616894d6d151"
114-
svc.state.ContainerStatus[ncID] = containerstatus{
115-
ID: ncID,
116-
VMVersion: "0",
117-
HostVersion: "0",
118-
CreateNetworkContainerRequest: cns.CreateNetworkContainerRequest{
119-
NetworkContainerid: ncID,
120-
IPConfiguration: cns.IPConfiguration{
121-
IPSubnet: cns.IPSubnet{
122-
IPAddress: "10.0.1.0",
123-
PrefixLength: 24,
129+
testCases := []struct {
130+
existingIPAddress string
131+
requestIPAddress string
132+
}{
133+
{"10.0.1.0/24", "10.0.2.0/22"},
134+
{"10.0.1.0/20", "10.0.1.0/18"},
135+
{"10.0.1.0/19", "10.0.0.0/15"},
136+
}
137+
138+
// Run test cases
139+
for _, tc := range testCases {
140+
// start with a NC in state
141+
ncID := "555ac5c9-89f2-4b5d-b8d0-616894d6d151"
142+
svc.state.ContainerStatus[ncID] = containerstatus{
143+
ID: ncID,
144+
VMVersion: "0",
145+
HostVersion: "0",
146+
CreateNetworkContainerRequest: cns.CreateNetworkContainerRequest{
147+
NetworkContainerid: ncID,
148+
IPConfiguration: cns.IPConfiguration{
149+
IPSubnet: cns.IPSubnet{
150+
IPAddress: tc.existingIPAddress,
151+
PrefixLength: 24,
152+
},
124153
},
125154
},
126-
},
127-
}
155+
}
128156

129-
ncReqs := []*cns.CreateNetworkContainerRequest{
130-
{
131-
NetworkContainerid: ncID,
132-
IPConfiguration: cns.IPConfiguration{
133-
IPSubnet: cns.IPSubnet{
134-
IPAddress: "10.0.2.0", // note this IP has changed
135-
PrefixLength: 24,
157+
ncReqs := []*cns.CreateNetworkContainerRequest{
158+
{
159+
NetworkContainerid: ncID,
160+
IPConfiguration: cns.IPConfiguration{
161+
IPSubnet: cns.IPSubnet{
162+
IPAddress: tc.requestIPAddress,
163+
PrefixLength: 24,
164+
},
136165
},
166+
NetworkContainerType: cns.Kubernetes,
137167
},
138-
Scenario: v1alpha.Overlay, // overlay cluster - skip primary CA check
139-
NetworkContainerType: cns.Kubernetes,
140-
},
141-
}
168+
}
142169

143-
// now try to reconcile the state where the NC primary IP has changed
144-
resp := svc.ReconcileIPAMStateForSwift(ncReqs, map[string]cns.PodInfo{}, &v1alpha.NodeNetworkConfig{})
170+
// now try to reconcile the state where the NC primary IP has changed
171+
resp := svc.ReconcileIPAMStateForSwift(ncReqs, map[string]cns.PodInfo{}, &v1alpha.NodeNetworkConfig{})
145172

146-
assert.Equal(t, types.Success, resp)
173+
assert.Equal(t, types.Success, resp)
174+
}
147175
}
148176

149177
// TestReconcileNCStateGatewayChange tests that NC state gets updated when reconciled

0 commit comments

Comments
 (0)