Skip to content

Commit a208275

Browse files
rejain789rejain789
authored
[backport] v1.5 ci: [CNS] Overlay Expansion Subnet Update Job Bug Fix #4103 (#4115)
* backporting to 1.5 * added cidr check function * backporting functions outside of overlay expansion * reverted unneeded code changse
1 parent ed94b0f commit a208275

File tree

2 files changed

+143
-32
lines changed

2 files changed

+143
-32
lines changed

‎cns/restserver/internalapi.go‎

Lines changed: 40 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"
@@ -547,13 +548,19 @@ func (service *HTTPRestService) CreateOrUpdateNetworkContainerInternal(req *cns.
547548
if ok {
548549
existingReq := existingNCInfo.CreateNetworkContainerRequest
549550
if !reflect.DeepEqual(existingReq.IPConfiguration.IPSubnet, req.IPConfiguration.IPSubnet) {
550-
logger.Errorf("[Azure CNS] Error. PrimaryCA is not same, NCId %s, old CA %s/%d, new CA %s/%d",
551-
req.NetworkContainerid,
552-
existingReq.IPConfiguration.IPSubnet.IPAddress,
553-
existingReq.IPConfiguration.IPSubnet.PrefixLength,
554-
req.IPConfiguration.IPSubnet.IPAddress,
555-
req.IPConfiguration.IPSubnet.PrefixLength)
556-
return types.PrimaryCANotSame
551+
// check for potential overlay subnet expansion - checking if new subnet is a superset of old subnet
552+
isCIDRSuperset := validateCIDRSuperset(
553+
fmt.Sprintf("%s/%d", req.IPConfiguration.IPSubnet.IPAddress, req.IPConfiguration.IPSubnet.PrefixLength),
554+
fmt.Sprintf("%s/%d", existingReq.IPConfiguration.IPSubnet.IPAddress, existingReq.IPConfiguration.IPSubnet.PrefixLength))
555+
if !isCIDRSuperset {
556+
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
557+
req.NetworkContainerid,
558+
existingReq.IPConfiguration.IPSubnet.IPAddress,
559+
existingReq.IPConfiguration.IPSubnet.PrefixLength,
560+
req.IPConfiguration.IPSubnet.IPAddress,
561+
req.IPConfiguration.IPSubnet.PrefixLength)
562+
return types.PrimaryCANotSame
563+
}
557564
}
558565
}
559566

@@ -578,3 +585,29 @@ func (service *HTTPRestService) CreateOrUpdateNetworkContainerInternal(req *cns.
578585

579586
return returnCode
580587
}
588+
589+
// IsCIDRSuperset returns true if newCIDR is a superset of oldCIDR (i.e., all IPs in oldCIDR are contained in newCIDR).
590+
func validateCIDRSuperset(newCIDR, oldCIDR string) bool {
591+
// Parse newCIDR and oldCIDR into netip.Prefix
592+
newPrefix, err := netip.ParsePrefix(newCIDR)
593+
if err != nil {
594+
return false
595+
}
596+
597+
oldPrefix, err := netip.ParsePrefix(oldCIDR)
598+
if err != nil {
599+
return false
600+
}
601+
602+
// Condition 1: Check if the new prefix length is smaller (larger range) than the old prefix length
603+
if newPrefix.Bits() >= oldPrefix.Bits() {
604+
return false
605+
}
606+
607+
// Condition 2: Check for Overlap - this will also ensure containment
608+
if !newPrefix.Overlaps(oldPrefix) {
609+
return false
610+
}
611+
612+
return true
613+
}

‎cns/restserver/internalapi_test.go‎

Lines changed: 103 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -61,39 +61,117 @@ func TestReconcileNCStatePrimaryIPChangeShouldFail(t *testing.T) {
6161
setOrchestratorTypeInternal(cns.KubernetesCRD)
6262
svc.state.ContainerStatus = make(map[string]containerstatus)
6363

64-
// start with a NC in state
65-
ncID := "555ac5c9-89f2-4b5d-b8d0-616894d6d151"
66-
svc.state.ContainerStatus[ncID] = containerstatus{
67-
ID: ncID,
68-
VMVersion: "0",
69-
HostVersion: "0",
70-
CreateNetworkContainerRequest: cns.CreateNetworkContainerRequest{
71-
NetworkContainerid: ncID,
72-
IPConfiguration: cns.IPConfiguration{
73-
IPSubnet: cns.IPSubnet{
74-
IPAddress: "10.0.1.0",
75-
PrefixLength: 24,
64+
testCases := []struct {
65+
reqIPAddress string
66+
reqPrefixLength uint8
67+
existingIPAddress string
68+
existingPrefixLength uint8
69+
}{
70+
{"10.240.1.0", 16, "10.240.0.0", 16},
71+
{"10.240.0.0", 64, "2001:db8::", 64},
72+
{"2001:db8::", 64, "10.240.0.0", 16},
73+
{"10.0.1.0", 24, "10.0.2.0", 22},
74+
{"10.0.1.0", 23, "10.0.1.0", 21},
75+
}
76+
77+
// Run test cases
78+
for _, tc := range testCases {
79+
// start with a NC in state
80+
ncID := "555ac5c9-89f2-4b5d-b8d0-616894d6d150"
81+
svc.state.ContainerStatus[ncID] = containerstatus{
82+
ID: ncID,
83+
VMVersion: "0",
84+
HostVersion: "0",
85+
CreateNetworkContainerRequest: cns.CreateNetworkContainerRequest{
86+
NetworkContainerid: ncID,
87+
IPConfiguration: cns.IPConfiguration{
88+
IPSubnet: cns.IPSubnet{
89+
IPAddress: tc.existingIPAddress,
90+
PrefixLength: tc.existingPrefixLength,
91+
},
7692
},
7793
},
78-
},
79-
}
94+
}
8095

81-
ncReqs := []*cns.CreateNetworkContainerRequest{
82-
{
83-
NetworkContainerid: ncID,
84-
IPConfiguration: cns.IPConfiguration{
85-
IPSubnet: cns.IPSubnet{
86-
IPAddress: "10.0.2.0", // note this IP has changed
87-
PrefixLength: 24,
96+
ncReqs := []*cns.CreateNetworkContainerRequest{
97+
{
98+
NetworkContainerid: ncID,
99+
IPConfiguration: cns.IPConfiguration{
100+
IPSubnet: cns.IPSubnet{
101+
IPAddress: tc.reqIPAddress,
102+
PrefixLength: tc.reqPrefixLength,
103+
},
88104
},
89105
},
90-
},
106+
}
107+
108+
// now try to reconcile the state where the NC primary IP has changed
109+
resp := svc.ReconcileIPAMState(ncReqs, map[string]cns.PodInfo{}, &v1alpha.NodeNetworkConfig{})
110+
111+
assert.Equal(t, types.PrimaryCANotSame, resp)
91112
}
92113

93-
// now try to reconcile the state where the NC primary IP has changed
94-
resp := svc.ReconcileIPAMState(ncReqs, map[string]cns.PodInfo{}, &v1alpha.NodeNetworkConfig{})
114+
}
115+
116+
// TestReconcileNCStatePrimaryIPChangeShouldNotFail tests that reconciling NC state with
117+
// a NC whose IP has changed should not fail if new IP is superset of old IP
118+
func TestReconcileNCStatePrimaryIPChangeShouldNotFail(t *testing.T) {
119+
restartService()
120+
setEnv(t)
121+
setOrchestratorTypeInternal(cns.KubernetesCRD)
122+
svc.state.ContainerStatus = make(map[string]containerstatus)
123+
124+
testCases := []struct {
125+
reqIPAddress string
126+
reqPrefixLength uint8
127+
existingIPAddress string
128+
existingPrefixLength uint8
129+
}{
130+
{"10.240.0.0", 20, "10.240.0.0", 24},
131+
132+
{"10.0.1.0", 22, "10.0.2.0", 24},
133+
{"10.0.1.0", 18, "10.0.1.0", 20},
134+
{"10.0.1.0", 15, "10.0.0.0", 19},
135+
{"10.0.1.0", 18, "10.0.1.0", 18},
136+
}
137+
138+
// Run test cases
139+
for _, tc := range testCases {
140+
// start with a NC in state
141+
ncID := "555ac5c9-89f2-4b5d-b8d0-616894d6d150"
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: tc.existingPrefixLength,
152+
},
153+
},
154+
},
155+
}
156+
157+
ncReqs := []*cns.CreateNetworkContainerRequest{
158+
{
159+
NetworkContainerid: ncID,
160+
IPConfiguration: cns.IPConfiguration{
161+
IPSubnet: cns.IPSubnet{
162+
IPAddress: tc.reqIPAddress,
163+
PrefixLength: tc.reqPrefixLength,
164+
},
165+
},
166+
NetworkContainerType: cns.Kubernetes,
167+
},
168+
}
95169

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

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

0 commit comments

Comments
 (0)