Skip to content

Commit 2239673

Browse files
rejain456Riyarejain789
authored
CNS Change for Subnet Overlay Expansion Job (#4074)
* added logic to fix cns bug for overlay subnet expansion * reverted a line change * fixed spelling * added unit test * fixing go lint * expanded on a comment * updated logic * updated test * updated validate superset logic * updated to return bool instead of error for checking cidr superset * updated logic to check for containment --------- Co-authored-by: Riya <jainriya@microsoft.com> Co-authored-by: Riya <rejain6@gmail.com>
1 parent a3f179b commit 2239673

File tree

2 files changed

+137
-32
lines changed

2 files changed

+137
-32
lines changed

‎cns/restserver/internalapi.go‎

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"net"
1212
"net/http"
1313
"net/http/httptest"
14+
"net/netip"
1415
"reflect"
1516
"strconv"
1617
"strings"
@@ -630,13 +631,17 @@ func (service *HTTPRestService) CreateOrUpdateNetworkContainerInternal(req *cns.
630631
if ok {
631632
existingReq := existingNCInfo.CreateNetworkContainerRequest
632633
if !reflect.DeepEqual(existingReq.IPConfiguration.IPSubnet, req.IPConfiguration.IPSubnet) {
633-
logger.Errorf("[Azure CNS] Error. PrimaryCA is not same, NCId %s, old CA %s/%d, new CA %s/%d",
634-
req.NetworkContainerid,
635-
existingReq.IPConfiguration.IPSubnet.IPAddress,
636-
existingReq.IPConfiguration.IPSubnet.PrefixLength,
637-
req.IPConfiguration.IPSubnet.IPAddress,
638-
req.IPConfiguration.IPSubnet.PrefixLength)
639-
return types.PrimaryCANotSame
634+
// check for potential overlay subnet expansion - checking if new subnet is a superset of old subnet
635+
isCIDRSuperset := validateCIDRSuperset(req.IPConfiguration.IPSubnet.IPAddress, existingReq.IPConfiguration.IPSubnet.IPAddress)
636+
if !isCIDRSuperset {
637+
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
638+
req.NetworkContainerid,
639+
existingReq.IPConfiguration.IPSubnet.IPAddress,
640+
existingReq.IPConfiguration.IPSubnet.PrefixLength,
641+
req.IPConfiguration.IPSubnet.IPAddress,
642+
req.IPConfiguration.IPSubnet.PrefixLength)
643+
return types.PrimaryCANotSame
644+
}
640645
}
641646
}
642647

@@ -722,3 +727,29 @@ func (service *HTTPRestService) GetIMDSNCs(ctx context.Context) (map[string]stri
722727

723728
return ncs, nil
724729
}
730+
731+
// IsCIDRSuperset returns true if newCIDR is a superset of oldCIDR (i.e., all IPs in oldCIDR are contained in newCIDR).
732+
func validateCIDRSuperset(newCIDR, oldCIDR string) bool {
733+
// Parse newCIDR and oldCIDR into netip.Prefix
734+
newPrefix, err := netip.ParsePrefix(newCIDR)
735+
if err != nil {
736+
return false
737+
}
738+
739+
oldPrefix, err := netip.ParsePrefix(oldCIDR)
740+
if err != nil {
741+
return false
742+
}
743+
744+
// Condition 1: Check if the new prefix length is smaller (larger range) than the old prefix length
745+
if newPrefix.Bits() >= oldPrefix.Bits() {
746+
return false
747+
}
748+
749+
// Condition 2: Check for Overlap - this will also ensure containment
750+
if !newPrefix.Overlaps(oldPrefix) {
751+
return false
752+
}
753+
754+
return true
755+
}

‎cns/restserver/internalapi_test.go‎

Lines changed: 99 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -66,39 +66,113 @@ 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-616894d6d150"
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-
},
111+
}
112+
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)
96117
}
97118

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{})
119+
}
120+
121+
// TestReconcileNCStatePrimaryIPChangeShouldNotFail tests that reconciling NC state with
122+
// a NC whose IP has changed should not fail if new IP is superset of old IP
123+
func TestReconcileNCStatePrimaryIPChangeShouldNotFail(t *testing.T) {
124+
restartService()
125+
setEnv(t)
126+
setOrchestratorTypeInternal(cns.KubernetesCRD)
127+
svc.state.ContainerStatus = make(map[string]containerstatus)
128+
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+
{"10.0.1.0/18", "10.0.1.0/18"},
137+
}
138+
139+
// Run test cases
140+
for _, tc := range testCases {
141+
// start with a NC in state
142+
ncID := "555ac5c9-89f2-4b5d-b8d0-616894d6d150"
143+
svc.state.ContainerStatus[ncID] = containerstatus{
144+
ID: ncID,
145+
VMVersion: "0",
146+
HostVersion: "0",
147+
CreateNetworkContainerRequest: cns.CreateNetworkContainerRequest{
148+
NetworkContainerid: ncID,
149+
IPConfiguration: cns.IPConfiguration{
150+
IPSubnet: cns.IPSubnet{
151+
IPAddress: tc.existingIPAddress,
152+
PrefixLength: 24,
153+
},
154+
},
155+
},
156+
}
157+
158+
ncReqs := []*cns.CreateNetworkContainerRequest{
159+
{
160+
NetworkContainerid: ncID,
161+
IPConfiguration: cns.IPConfiguration{
162+
IPSubnet: cns.IPSubnet{
163+
IPAddress: tc.requestIPAddress,
164+
PrefixLength: 24,
165+
},
166+
},
167+
NetworkContainerType: cns.Kubernetes,
168+
},
169+
}
100170

101-
assert.Equal(t, types.PrimaryCANotSame, resp)
171+
// now try to reconcile the state where the NC primary IP has changed
172+
resp := svc.ReconcileIPAMStateForSwift(ncReqs, map[string]cns.PodInfo{}, &v1alpha.NodeNetworkConfig{})
173+
174+
assert.Equal(t, types.Success, resp)
175+
}
102176
}
103177

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

0 commit comments

Comments
 (0)