Skip to content

Commit 8c87379

Browse files
rejain789rejain789
authored
[backport] v1.6 ci: [CNS] Overlay Expansion Subnet Update Job Bug Fix #4103 (#4114)
* [CNS] Overlay Expansion Subnet Update Job Bug Fix (#4103) hot fix * back porting to v1.6 * added cidr check function
1 parent b2762a3 commit 8c87379

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"
@@ -600,13 +601,19 @@ func (service *HTTPRestService) CreateOrUpdateNetworkContainerInternal(req *cns.
600601
if ok {
601602
existingReq := existingNCInfo.CreateNetworkContainerRequest
602603
if !reflect.DeepEqual(existingReq.IPConfiguration.IPSubnet, req.IPConfiguration.IPSubnet) {
603-
logger.Errorf("[Azure CNS] Error. PrimaryCA is not same, NCId %s, old CA %s/%d, new CA %s/%d",
604-
req.NetworkContainerid,
605-
existingReq.IPConfiguration.IPSubnet.IPAddress,
606-
existingReq.IPConfiguration.IPSubnet.PrefixLength,
607-
req.IPConfiguration.IPSubnet.IPAddress,
608-
req.IPConfiguration.IPSubnet.PrefixLength)
609-
return types.PrimaryCANotSame
604+
// check for potential overlay subnet expansion - checking if new subnet is a superset of old subnet
605+
isCIDRSuperset := validateCIDRSuperset(
606+
fmt.Sprintf("%s/%d", req.IPConfiguration.IPSubnet.IPAddress, req.IPConfiguration.IPSubnet.PrefixLength),
607+
fmt.Sprintf("%s/%d", existingReq.IPConfiguration.IPSubnet.IPAddress, existingReq.IPConfiguration.IPSubnet.PrefixLength))
608+
if !isCIDRSuperset {
609+
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
610+
req.NetworkContainerid,
611+
existingReq.IPConfiguration.IPSubnet.IPAddress,
612+
existingReq.IPConfiguration.IPSubnet.PrefixLength,
613+
req.IPConfiguration.IPSubnet.IPAddress,
614+
req.IPConfiguration.IPSubnet.PrefixLength)
615+
return types.PrimaryCANotSame
616+
}
610617
}
611618
}
612619

@@ -634,3 +641,29 @@ func (service *HTTPRestService) CreateOrUpdateNetworkContainerInternal(req *cns.
634641
func (service *HTTPRestService) SetVFForAccelnetNICs() error {
635642
return service.setVFForAccelnetNICs()
636643
}
644+
645+
// IsCIDRSuperset returns true if newCIDR is a superset of oldCIDR (i.e., all IPs in oldCIDR are contained in newCIDR).
646+
func validateCIDRSuperset(newCIDR, oldCIDR string) bool {
647+
// Parse newCIDR and oldCIDR into netip.Prefix
648+
newPrefix, err := netip.ParsePrefix(newCIDR)
649+
if err != nil {
650+
return false
651+
}
652+
653+
oldPrefix, err := netip.ParsePrefix(oldCIDR)
654+
if err != nil {
655+
return false
656+
}
657+
658+
// Condition 1: Check if the new prefix length is smaller (larger range) than the old prefix length
659+
if newPrefix.Bits() >= oldPrefix.Bits() {
660+
return false
661+
}
662+
663+
// Condition 2: Check for Overlap - this will also ensure containment
664+
if !newPrefix.Overlaps(oldPrefix) {
665+
return false
666+
}
667+
668+
return true
669+
}

‎cns/restserver/internalapi_test.go‎

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

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

83-
ncReqs := []*cns.CreateNetworkContainerRequest{
84-
{
85-
NetworkContainerid: ncID,
86-
IPConfiguration: cns.IPConfiguration{
87-
IPSubnet: cns.IPSubnet{
88-
IPAddress: "10.0.2.0", // note this IP has changed
89-
PrefixLength: 24,
98+
ncReqs := []*cns.CreateNetworkContainerRequest{
99+
{
100+
NetworkContainerid: ncID,
101+
IPConfiguration: cns.IPConfiguration{
102+
IPSubnet: cns.IPSubnet{
103+
IPAddress: tc.reqIPAddress,
104+
PrefixLength: tc.reqPrefixLength,
105+
},
90106
},
91107
},
92-
},
108+
}
109+
110+
// now try to reconcile the state where the NC primary IP has changed
111+
resp := svc.ReconcileIPAMStateForSwift(ncReqs, map[string]cns.PodInfo{}, &v1alpha.NodeNetworkConfig{})
112+
113+
assert.Equal(t, types.PrimaryCANotSame, resp)
93114
}
94115

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

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

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

0 commit comments

Comments
 (0)