From 6dbc9e2f6821b0873e557335088b65d33d39ceda Mon Sep 17 00:00:00 2001 From: rejain789 Date: Wed, 5 Nov 2025 09:42:25 -0800 Subject: [PATCH 1/3] [CNS] Overlay Expansion Subnet Update Job Bug Fix (#4103) hot fix --- cns/restserver/internalapi.go | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/cns/restserver/internalapi.go b/cns/restserver/internalapi.go index 8d477cbab8..9d614156ef 100644 --- a/cns/restserver/internalapi.go +++ b/cns/restserver/internalapi.go @@ -600,13 +600,19 @@ func (service *HTTPRestService) CreateOrUpdateNetworkContainerInternal(req *cns. if ok { existingReq := existingNCInfo.CreateNetworkContainerRequest if !reflect.DeepEqual(existingReq.IPConfiguration.IPSubnet, req.IPConfiguration.IPSubnet) { - logger.Errorf("[Azure CNS] Error. PrimaryCA is not same, NCId %s, old CA %s/%d, new CA %s/%d", - req.NetworkContainerid, - existingReq.IPConfiguration.IPSubnet.IPAddress, - existingReq.IPConfiguration.IPSubnet.PrefixLength, - req.IPConfiguration.IPSubnet.IPAddress, - req.IPConfiguration.IPSubnet.PrefixLength) - return types.PrimaryCANotSame + // check for potential overlay subnet expansion - checking if new subnet is a superset of old subnet + isCIDRSuperset := validateCIDRSuperset( + fmt.Sprintf("%s/%d", req.IPConfiguration.IPSubnet.IPAddress, req.IPConfiguration.IPSubnet.PrefixLength), + fmt.Sprintf("%s/%d", existingReq.IPConfiguration.IPSubnet.IPAddress, existingReq.IPConfiguration.IPSubnet.PrefixLength)) + if !isCIDRSuperset { + 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 + req.NetworkContainerid, + existingReq.IPConfiguration.IPSubnet.IPAddress, + existingReq.IPConfiguration.IPSubnet.PrefixLength, + req.IPConfiguration.IPSubnet.IPAddress, + req.IPConfiguration.IPSubnet.PrefixLength) + return types.PrimaryCANotSame + } } } From 1d28dda9ff2278b2a4cd5427ee8739c544dcd5c1 Mon Sep 17 00:00:00 2001 From: Riya Date: Tue, 11 Nov 2025 23:18:25 +0000 Subject: [PATCH 2/3] back porting to v1.6 --- cns/restserver/internalapi_test.go | 128 +++++++++++++++++++++++------ 1 file changed, 103 insertions(+), 25 deletions(-) diff --git a/cns/restserver/internalapi_test.go b/cns/restserver/internalapi_test.go index 10b1852c63..a3a4eddb2c 100644 --- a/cns/restserver/internalapi_test.go +++ b/cns/restserver/internalapi_test.go @@ -63,39 +63,117 @@ func TestReconcileNCStatePrimaryIPChangeShouldFail(t *testing.T) { setOrchestratorTypeInternal(cns.KubernetesCRD) svc.state.ContainerStatus = make(map[string]containerstatus) - // start with a NC in state - ncID := "555ac5c9-89f2-4b5d-b8d0-616894d6d151" - svc.state.ContainerStatus[ncID] = containerstatus{ - ID: ncID, - VMVersion: "0", - HostVersion: "0", - CreateNetworkContainerRequest: cns.CreateNetworkContainerRequest{ - NetworkContainerid: ncID, - IPConfiguration: cns.IPConfiguration{ - IPSubnet: cns.IPSubnet{ - IPAddress: "10.0.1.0", - PrefixLength: 24, + testCases := []struct { + reqIPAddress string + reqPrefixLength uint8 + existingIPAddress string + existingPrefixLength uint8 + }{ + {"10.240.1.0", 16, "10.240.0.0", 16}, + {"10.240.0.0", 64, "2001:db8::", 64}, + {"2001:db8::", 64, "10.240.0.0", 16}, + {"10.0.1.0", 24, "10.0.2.0", 22}, + {"10.0.1.0", 23, "10.0.1.0", 21}, + } + + // Run test cases + for _, tc := range testCases { + // start with a NC in state + ncID := "555ac5c9-89f2-4b5d-b8d0-616894d6d150" + svc.state.ContainerStatus[ncID] = containerstatus{ + ID: ncID, + VMVersion: "0", + HostVersion: "0", + CreateNetworkContainerRequest: cns.CreateNetworkContainerRequest{ + NetworkContainerid: ncID, + IPConfiguration: cns.IPConfiguration{ + IPSubnet: cns.IPSubnet{ + IPAddress: tc.existingIPAddress, + PrefixLength: tc.existingPrefixLength, + }, }, }, - }, - } + } - ncReqs := []*cns.CreateNetworkContainerRequest{ - { - NetworkContainerid: ncID, - IPConfiguration: cns.IPConfiguration{ - IPSubnet: cns.IPSubnet{ - IPAddress: "10.0.2.0", // note this IP has changed - PrefixLength: 24, + ncReqs := []*cns.CreateNetworkContainerRequest{ + { + NetworkContainerid: ncID, + IPConfiguration: cns.IPConfiguration{ + IPSubnet: cns.IPSubnet{ + IPAddress: tc.reqIPAddress, + PrefixLength: tc.reqPrefixLength, + }, }, }, - }, + } + + // now try to reconcile the state where the NC primary IP has changed + resp := svc.ReconcileIPAMStateForSwift(ncReqs, map[string]cns.PodInfo{}, &v1alpha.NodeNetworkConfig{}) + + assert.Equal(t, types.PrimaryCANotSame, resp) } - // now try to reconcile the state where the NC primary IP has changed - resp := svc.ReconcileIPAMStateForSwift(ncReqs, map[string]cns.PodInfo{}, &v1alpha.NodeNetworkConfig{}) +} + +// TestReconcileNCStatePrimaryIPChangeShouldNotFail tests that reconciling NC state with +// a NC whose IP has changed should not fail if new IP is superset of old IP +func TestReconcileNCStatePrimaryIPChangeShouldNotFail(t *testing.T) { + restartService() + setEnv(t) + setOrchestratorTypeInternal(cns.KubernetesCRD) + svc.state.ContainerStatus = make(map[string]containerstatus) + + testCases := []struct { + reqIPAddress string + reqPrefixLength uint8 + existingIPAddress string + existingPrefixLength uint8 + }{ + {"10.240.0.0", 20, "10.240.0.0", 24}, + + {"10.0.1.0", 22, "10.0.2.0", 24}, + {"10.0.1.0", 18, "10.0.1.0", 20}, + {"10.0.1.0", 15, "10.0.0.0", 19}, + {"10.0.1.0", 18, "10.0.1.0", 18}, + } + + // Run test cases + for _, tc := range testCases { + // start with a NC in state + ncID := "555ac5c9-89f2-4b5d-b8d0-616894d6d150" + svc.state.ContainerStatus[ncID] = containerstatus{ + ID: ncID, + VMVersion: "0", + HostVersion: "0", + CreateNetworkContainerRequest: cns.CreateNetworkContainerRequest{ + NetworkContainerid: ncID, + IPConfiguration: cns.IPConfiguration{ + IPSubnet: cns.IPSubnet{ + IPAddress: tc.existingIPAddress, + PrefixLength: tc.existingPrefixLength, + }, + }, + }, + } + + ncReqs := []*cns.CreateNetworkContainerRequest{ + { + NetworkContainerid: ncID, + IPConfiguration: cns.IPConfiguration{ + IPSubnet: cns.IPSubnet{ + IPAddress: tc.reqIPAddress, + PrefixLength: tc.reqPrefixLength, + }, + }, + NetworkContainerType: cns.Kubernetes, + }, + } - assert.Equal(t, types.PrimaryCANotSame, resp) + // now try to reconcile the state where the NC primary IP has changed + resp := svc.ReconcileIPAMStateForSwift(ncReqs, map[string]cns.PodInfo{}, &v1alpha.NodeNetworkConfig{}) + + assert.Equal(t, types.Success, resp) + } } // TestReconcileNCStateGatewayChange tests that NC state gets updated when reconciled From 164effabeaa413202a013c72b7771103527bdc22 Mon Sep 17 00:00:00 2001 From: Riya Date: Tue, 11 Nov 2025 23:42:03 +0000 Subject: [PATCH 3/3] added cidr check function --- cns/restserver/internalapi.go | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/cns/restserver/internalapi.go b/cns/restserver/internalapi.go index 9d614156ef..08004616a0 100644 --- a/cns/restserver/internalapi.go +++ b/cns/restserver/internalapi.go @@ -11,6 +11,7 @@ import ( "net" "net/http" "net/http/httptest" + "net/netip" "reflect" "strconv" "strings" @@ -640,3 +641,29 @@ func (service *HTTPRestService) CreateOrUpdateNetworkContainerInternal(req *cns. func (service *HTTPRestService) SetVFForAccelnetNICs() error { return service.setVFForAccelnetNICs() } + +// IsCIDRSuperset returns true if newCIDR is a superset of oldCIDR (i.e., all IPs in oldCIDR are contained in newCIDR). +func validateCIDRSuperset(newCIDR, oldCIDR string) bool { + // Parse newCIDR and oldCIDR into netip.Prefix + newPrefix, err := netip.ParsePrefix(newCIDR) + if err != nil { + return false + } + + oldPrefix, err := netip.ParsePrefix(oldCIDR) + if err != nil { + return false + } + + // Condition 1: Check if the new prefix length is smaller (larger range) than the old prefix length + if newPrefix.Bits() >= oldPrefix.Bits() { + return false + } + + // Condition 2: Check for Overlap - this will also ensure containment + if !newPrefix.Overlaps(oldPrefix) { + return false + } + + return true +}