From fd16ce2ab68673c359976868d37303c6b3c0770b Mon Sep 17 00:00:00 2001 From: Vaibhav Patel Date: Thu, 25 Jul 2024 14:29:11 -0400 Subject: [PATCH 1/2] Failure to Enforce clustercidr on nodes with exisiting cidr should not be treated as a fatal error Address code comments fix linting error go mod vendor --- pkg/nodeipam/ipam/range_allocator.go | 7 ++++- pkg/nodeipam/ipam/range_allocator_test.go | 32 +++++++++++------------ 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/pkg/nodeipam/ipam/range_allocator.go b/pkg/nodeipam/ipam/range_allocator.go index 4cfa92fc92..9f4b500b8b 100644 --- a/pkg/nodeipam/ipam/range_allocator.go +++ b/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 } } diff --git a/pkg/nodeipam/ipam/range_allocator_test.go b/pkg/nodeipam/ipam/range_allocator_test.go index c08d334bd2..ef027cf13f 100644 --- a/pkg/nodeipam/ipam/range_allocator_test.go +++ b/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, From 24529963ec46c832c12c6e9d5c783eba8c3a4eb2 Mon Sep 17 00:00:00 2001 From: Vaibhav Patel Date: Thu, 22 Aug 2024 14:48:48 -0400 Subject: [PATCH 2/2] Add azurepipelines --- azure-pipelines.yml | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 azure-pipelines.yml diff --git a/azure-pipelines.yml b/azure-pipelines.yml new file mode 100644 index 0000000000..08461065db --- /dev/null +++ b/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 \ No newline at end of file