Merge pull request #6857 from vpatelsj/release-1.30

Fix: Failure to Enforce clustercidr on nodes with exisiting cidr should not be treated as a fatal error

azure-pipelines.yml

@@ -0,0 +1,26 @@
+# Starter pipeline
+# Start with a minimal pipeline that you can customize to build and deploy your code.
+# Add steps that build, run tests, deploy, and more:
+# https://aka.ms/yaml
+
+trigger:
+- master
+
+pool:
+  vmImage: ubuntu-latest
+
+steps:
+  - task: AzureCLI@2
+    displayName: Build component binaries, containers and packages
+    inputs:
+      azureSubscription: aks deploy msi - dev
+      scriptType: bash
+      scriptLocation: inlineScript
+      inlineScript: |
+        set -euo pipefail
+        export DOCKER_BUILDKIT=1
+        az acr login --name acsdevdeployment      
+        IMAGE_REGISTRY=acsdevdeployment.azurecr.io ARCH=amd64 OUTPUT_TYPE=docker make build-ccm-image
+        IMAGE_TAG=$(git rev-parse --short=7 HEAD)
+        docker tag acsdevdeployment.azurecr.io/azure-cloud-controller-manager:$IMAGE_TAG acsdevdeployment.azurecr.io/azure-cloud-controller-manager:v1.30.vapa1.$IMAGE_TAG
+        docker push acsdevdeployment.azurecr.io/azure-cloud-controller-manager:v1.30.vapa1.$IMAGE_TAG

pkg/nodeipam/ipam/range_allocator.go

@@ -19,6 +19,7 @@ package ipam
 import (
 	"fmt"
 	"net"
+	"strings"
 	"sync"
 
 	v1 "k8s.io/api/core/v1"
@@ -127,7 +128,11 @@ func NewCIDRRangeAllocator(client clientset.Interface, nodeInformer informers.No
 				// This will happen if:
 				// 1. We find garbage in the podCIDRs field. Retrying is useless.
 				// 2. CIDR out of range: This means a node CIDR has changed.
-				// This error will keep crashing controller-manager.
+				// This error should not keep crashing controller-manager as there may be a podcidr migration happening.
+				if strings.Contains(err.Error(), "is out the range of cluster cidr") {
+					klog.Warningf("Error occupying podCIDR %s on node %s: %v", node.Spec.PodCIDR, node.Name, err)
+					continue
+				}
 				return nil, err
 			}
 		}

pkg/nodeipam/ipam/range_allocator_test.go

@@ -182,9 +182,8 @@ func TestOccupyPreExistingCIDR(t *testing.T) {
 			expectedAllocatedCIDR: nil,
 			ctrlCreateFail:        false,
 		},
-		// failure cases
 		{
-			description: "fail, single stack incorrect node allocation",
+			description: "success, single stack incorrect node allocation",
 			fakeNodeHandler: &testutil.FakeNodeHandler{
 				Existing: []*v1.Node{
 					{
@@ -209,10 +208,10 @@ func TestOccupyPreExistingCIDR(t *testing.T) {
 			},
 			allocatedCIDRs:        nil,
 			expectedAllocatedCIDR: nil,
-			ctrlCreateFail:        true,
+			ctrlCreateFail:        false,
 		},
 		{
-			description: "fail, dualstack node allocating from non existing cidr",
+			description: "success, dualstack node allocating bad v4",
 
 			fakeNodeHandler: &testutil.FakeNodeHandler{
 				Existing: []*v1.Node{
@@ -221,7 +220,7 @@ func TestOccupyPreExistingCIDR(t *testing.T) {
 							Name: "node0",
 						},
 						Spec: v1.NodeSpec{
-							PodCIDRs: []string{"10.10.0.1/24", "a00::/86"},
+							PodCIDRs: []string{"172.10.0.1/24", "a00::/86"},
 						},
 					},
 				},
@@ -230,18 +229,19 @@ func TestOccupyPreExistingCIDR(t *testing.T) {
 			allocatorParams: CIDRAllocatorParams{
 				ClusterCIDRs: func() []*net.IPNet {
 					_, clusterCIDRv4, _ := net.ParseCIDR("10.10.0.0/16")
-					return []*net.IPNet{clusterCIDRv4}
+					_, clusterCIDRv6, _ := net.ParseCIDR("ace:cab:deca::/8")
+					return []*net.IPNet{clusterCIDRv4, clusterCIDRv6}
 				}(),
 				ServiceCIDR:          nil,
 				SecondaryServiceCIDR: nil,
-				NodeCIDRMaskSizes:    []int{24},
+				NodeCIDRMaskSizes:    []int{24, 24},
 			},
 			allocatedCIDRs:        nil,
 			expectedAllocatedCIDR: nil,
-			ctrlCreateFail:        true,
+			ctrlCreateFail:        false,
 		},
 		{
-			description: "fail, dualstack node allocating bad v4",
+			description: "success, dualstack node allocating bad v6",
 
 			fakeNodeHandler: &testutil.FakeNodeHandler{
 				Existing: []*v1.Node{
@@ -250,7 +250,7 @@ func TestOccupyPreExistingCIDR(t *testing.T) {
 							Name: "node0",
 						},
 						Spec: v1.NodeSpec{
-							PodCIDRs: []string{"172.10.0.1/24", "a00::/86"},
+							PodCIDRs: []string{"10.10.0.1/24", "cdd::/86"},
 						},
 					},
 				},
@@ -268,10 +268,11 @@ func TestOccupyPreExistingCIDR(t *testing.T) {
 			},
 			allocatedCIDRs:        nil,
 			expectedAllocatedCIDR: nil,
-			ctrlCreateFail:        true,
+			ctrlCreateFail:        false,
 		},
+		// failure cases
 		{
-			description: "fail, dualstack node allocating bad v6",
+			description: "fail, dualstack node allocating from non existing cidr",
 
 			fakeNodeHandler: &testutil.FakeNodeHandler{
 				Existing: []*v1.Node{
@@ -280,7 +281,7 @@ func TestOccupyPreExistingCIDR(t *testing.T) {
 							Name: "node0",
 						},
 						Spec: v1.NodeSpec{
-							PodCIDRs: []string{"10.10.0.1/24", "cdd::/86"},
+							PodCIDRs: []string{"10.10.0.1/24", "a00::/86"},
 						},
 					},
 				},
@@ -289,12 +290,11 @@ func TestOccupyPreExistingCIDR(t *testing.T) {
 			allocatorParams: CIDRAllocatorParams{
 				ClusterCIDRs: func() []*net.IPNet {
 					_, clusterCIDRv4, _ := net.ParseCIDR("10.10.0.0/16")
-					_, clusterCIDRv6, _ := net.ParseCIDR("ace:cab:deca::/8")
-					return []*net.IPNet{clusterCIDRv4, clusterCIDRv6}
+					return []*net.IPNet{clusterCIDRv4}
 				}(),
 				ServiceCIDR:          nil,
 				SecondaryServiceCIDR: nil,
-				NodeCIDRMaskSizes:    []int{24, 24},
+				NodeCIDRMaskSizes:    []int{24},
 			},
 			allocatedCIDRs:        nil,
 			expectedAllocatedCIDR: nil,