From 2200647a14e3ab46bad573a03382cf9f35193070 Mon Sep 17 00:00:00 2001 From: Paul Johnston Date: Wed, 31 Mar 2021 10:48:40 -0700 Subject: [PATCH 01/10] feature: don't let CNS ask for more IPS than available --- cns/azure-cns.yaml | 2 +- cns/fakes/requestcontrollerfake.go | 19 +++- cns/ipampoolmonitor/ipampoolmonitor.go | 61 ++++++++++- cns/ipampoolmonitor/ipampoolmonitor_test.go | 102 ++++++++++++++++-- .../kubecontroller/crdrequestcontroller.go | 28 +++++ .../requestcontrollerintreface.go | 1 + .../manifests/cns/clusterrole.yaml | 2 +- 7 files changed, 201 insertions(+), 14 deletions(-) diff --git a/cns/azure-cns.yaml b/cns/azure-cns.yaml index a009556480..6167fc6a37 100644 --- a/cns/azure-cns.yaml +++ b/cns/azure-cns.yaml @@ -20,7 +20,7 @@ metadata: name: pod-reader-all-namespaces rules: - apiGroups: [""] - resources: ["pods"] + resources: ["pods", "nodes"] verbs: ["get", "watch", "list"] --- apiVersion: rbac.authorization.k8s.io/v1 diff --git a/cns/fakes/requestcontrollerfake.go b/cns/fakes/requestcontrollerfake.go index 9060f88bcc..c0800ef101 100644 --- a/cns/fakes/requestcontrollerfake.go +++ b/cns/fakes/requestcontrollerfake.go @@ -3,19 +3,25 @@ package fakes import ( "context" "net" + "strconv" "github.com/Azure/azure-container-networking/cns" nnc "github.com/Azure/azure-container-networking/nodenetworkconfig/api/v1alpha" "github.com/google/uuid" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" ) type RequestControllerFake struct { fakecns *HTTPServiceFake cachedCRD nnc.NodeNetworkConfig ip net.IP + node corev1.Node } -func NewRequestControllerFake(cnsService *HTTPServiceFake, scalar nnc.Scaler, subnetAddressSpace string, numberOfIPConfigs int) *RequestControllerFake { +func NewRequestControllerFake(cnsService *HTTPServiceFake, scalar nnc.Scaler, subnetAddressSpace string, numberOfIPConfigs int, maxPodIPCount int64) *RequestControllerFake { + quantity, _ := resource.ParseQuantity(strconv.FormatInt(maxPodIPCount, 10)) + rc := &RequestControllerFake{ fakecns: cnsService, cachedCRD: nnc.NodeNetworkConfig{ @@ -27,6 +33,13 @@ func NewRequestControllerFake(cnsService *HTTPServiceFake, scalar nnc.Scaler, su }}, }, }, + node: corev1.Node{ + Status: corev1.NodeStatus{ + Capacity: corev1.ResourceList{ + corev1.ResourcePods: quantity, + }, + }, + }, } rc.ip, _, _ = net.ParseCIDR(subnetAddressSpace) @@ -80,6 +93,10 @@ func (rc *RequestControllerFake) UpdateCRDSpec(cntxt context.Context, desiredSpe return nil } +func (rc *RequestControllerFake) GetMaxIPCountOfNode(ctx context.Context) (int64, error) { + return rc.node.Status.Capacity.Pods().Value(), nil +} + func remove(slice []nnc.IPAssignment, s int) []nnc.IPAssignment { return append(slice[:s], slice[s+1:]...) } diff --git a/cns/ipampoolmonitor/ipampoolmonitor.go b/cns/ipampoolmonitor/ipampoolmonitor.go index b4fcd2b94d..0224ce880c 100644 --- a/cns/ipampoolmonitor/ipampoolmonitor.go +++ b/cns/ipampoolmonitor/ipampoolmonitor.go @@ -78,6 +78,17 @@ func (pm *CNSIPAMPoolMonitor) Reconcile() error { switch { // pod count is increasing case freeIPConfigCount < pm.MinimumFreeIps: + maxIpCount, err := pm.rc.GetMaxIPCountOfNode(context.Background()) + if err != nil { + logger.Printf("[ipam-pool-monitor] Error when getting max ip count in Reconcile: %v", err) + return err + } + + if pm.cachedNNC.Spec.RequestedIPCount == maxIpCount { + // If we're already at the maxIpCount, don't try to increase + return nil + } + logger.Printf("[ipam-pool-monitor] Increasing pool size...%s ", msg) return pm.increasePoolSize() @@ -112,7 +123,28 @@ func (pm *CNSIPAMPoolMonitor) increasePoolSize() error { return err } + // Query the max pod count for this node + maxIpCount, err := pm.rc.GetMaxIPCountOfNode(context.Background()) + if err != nil { + logger.Printf("[ipam-pool-monitor] Error when getting max ip count in increasePoolSize: %v", err) + return err + } + + previouslyRequestedIPCount := tempNNCSpec.RequestedIPCount + tempNNCSpec.RequestedIPCount += pm.scalarUnits.BatchSize + if tempNNCSpec.RequestedIPCount > maxIpCount { + // We don't want to ask for more ips than the node limit + logger.Printf("[ipam-pool-monitor] Requested IP count (%v) is over the node limit (%v), requesting node limit instead.", tempNNCSpec.RequestedIPCount, maxIpCount) + tempNNCSpec.RequestedIPCount = maxIpCount + } + + // If the requested IP count is same as before, then don't do anything + if tempNNCSpec.RequestedIPCount == previouslyRequestedIPCount { + logger.Printf("[ipam-pool-monitor] Previously requested IP count %v is same as updated IP count %v, doing nothing", previouslyRequestedIPCount, tempNNCSpec.RequestedIPCount) + return nil + } + logger.Printf("[ipam-pool-monitor] Increasing pool size, Current Pool Size: %v, Updated Requested IP Count: %v, Pods with IP's:%v, ToBeDeleted Count: %v", len(pm.httpService.GetPodIPConfigState()), tempNNCSpec.RequestedIPCount, len(pm.httpService.GetAllocatedIPConfigs()), len(tempNNCSpec.IPsNotInUse)) err = pm.rc.UpdateCRDSpec(context.Background(), tempNNCSpec) @@ -135,10 +167,34 @@ func (pm *CNSIPAMPoolMonitor) decreasePoolSize(existingPendingReleaseIPCount int var err error var newIpsMarkedAsPending bool var pendingIpAddresses map[string]cns.IPConfigurationStatus + var updatedRequestedIPCount int64 + var decreaseIPCountBy int64 + + // Ensure the updated requested IP count is a multiple of the batch size + previouslyRequestedIPCount := pm.cachedNNC.Spec.RequestedIPCount + modResult := previouslyRequestedIPCount % pm.scalarUnits.BatchSize + + logger.Printf("[ipam-pool-monitor] Previously RequestedIP Count %v", previouslyRequestedIPCount) + logger.Printf("[ipam-pool-monitor] Batch size : %v", pm.scalarUnits.BatchSize) + logger.Printf("[ipam-pool-monitor] modResult of (previously requested IP count mod batch size) = %v", modResult) + + if modResult != 0 { + // Example: previouscount = 25, batchsize = 10, 25 - 10 = 15, NOT a multiple of batchsize (10) + // Don't want that, so make requestedIPCount 20 (25 - (25 % 10)) so that it is a multiple of the batchsize (10) + updatedRequestedIPCount = previouslyRequestedIPCount - modResult + } else { + // Example: previouscount = 30, batchsize = 10, 30 - 10 = 20 which is multiple of batchsize (10) so all good + updatedRequestedIPCount = previouslyRequestedIPCount - pm.scalarUnits.BatchSize + } + + decreaseIPCountBy = previouslyRequestedIPCount - updatedRequestedIPCount + + logger.Printf("[ipam-pool-monitor] updatedRequestedIPCount %v", updatedRequestedIPCount) + if pm.updatingIpsNotInUseCount == 0 || pm.updatingIpsNotInUseCount < existingPendingReleaseIPCount { - logger.Printf("[ipam-pool-monitor] Marking IPs as PendingRelease, ipsToBeReleasedCount %d", int(pm.scalarUnits.BatchSize)) - pendingIpAddresses, err = pm.httpService.MarkIPAsPendingRelease(int(pm.scalarUnits.BatchSize)) + logger.Printf("[ipam-pool-monitor] Marking IPs as PendingRelease, ipsToBeReleasedCount %d", int(decreaseIPCountBy)) + pendingIpAddresses, err = pm.httpService.MarkIPAsPendingRelease(int(decreaseIPCountBy)) if err != nil { return err } @@ -202,7 +258,6 @@ func (pm *CNSIPAMPoolMonitor) cleanPendingRelease() error { logger.Printf("[ipam-pool-monitor] cleanPendingRelease: UpdateCRDSpec succeeded for spec %+v", tempNNCSpec) - // save the updated state to cachedSpec pm.cachedNNC.Spec = tempNNCSpec pm.pendingRelease = false diff --git a/cns/ipampoolmonitor/ipampoolmonitor_test.go b/cns/ipampoolmonitor/ipampoolmonitor_test.go index b536221568..eda939aba4 100644 --- a/cns/ipampoolmonitor/ipampoolmonitor_test.go +++ b/cns/ipampoolmonitor/ipampoolmonitor_test.go @@ -9,7 +9,7 @@ import ( nnc "github.com/Azure/azure-container-networking/nodenetworkconfig/api/v1alpha" ) -func initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent int) (*fakes.HTTPServiceFake, *fakes.RequestControllerFake, *CNSIPAMPoolMonitor) { +func initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent int, maxPodIPCount int64) (*fakes.HTTPServiceFake, *fakes.RequestControllerFake, *CNSIPAMPoolMonitor) { logger.InitLogger("testlogs", 0, 0, "./") scalarUnits := nnc.Scaler{ @@ -20,7 +20,7 @@ func initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, release subnetaddresspace := "10.0.0.0/8" fakecns := fakes.NewHTTPServiceFake() - fakerc := fakes.NewRequestControllerFake(fakecns, scalarUnits, subnetaddresspace, initialIPConfigCount) + fakerc := fakes.NewRequestControllerFake(fakecns, scalarUnits, subnetaddresspace, initialIPConfigCount, maxPodIPCount) poolmonitor := NewCNSIPAMPoolMonitor(fakecns, fakerc) @@ -37,9 +37,10 @@ func TestPoolSizeIncrease(t *testing.T) { initialIPConfigCount = 10 requestThresholdPercent = 30 releaseThresholdPercent = 150 + maxPodIPCount = int64(30) ) - fakecns, fakerc, poolmonitor := initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent) + fakecns, fakerc, poolmonitor := initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) // increase number of allocated IP's in CNS err := fakecns.SetNumberOfAllocatedIPs(8) @@ -90,9 +91,10 @@ func TestPoolIncreaseDoesntChangeWhenIncreaseIsAlreadyInProgress(t *testing.T) { initialIPConfigCount = 10 requestThresholdPercent = 30 releaseThresholdPercent = 150 + maxPodIPCount = int64(30) ) - fakecns, fakerc, poolmonitor := initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent) + fakecns, fakerc, poolmonitor := initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) // increase number of allocated IP's in CNS err := fakecns.SetNumberOfAllocatedIPs(8) @@ -155,9 +157,10 @@ func TestPoolSizeIncreaseIdempotency(t *testing.T) { initialIPConfigCount = 10 requestThresholdPercent = 30 releaseThresholdPercent = 150 + maxPodIPCount = int64(30) ) - fakecns, _, poolmonitor := initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent) + fakecns, _, poolmonitor := initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) t.Logf("Minimum free IPs to request: %v", poolmonitor.MinimumFreeIps) t.Logf("Maximum free IPs to release: %v", poolmonitor.MaximumFreeIps) @@ -191,15 +194,48 @@ func TestPoolSizeIncreaseIdempotency(t *testing.T) { } } +func TestPoolIncreasePastNodeLimit(t *testing.T) { + var ( + batchSize = 16 + initialIPConfigCount = 16 + requestThresholdPercent = 50 + releaseThresholdPercent = 150 + maxPodIPCount = int64(30) + ) + + fakecns, _, poolmonitor := initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) + + t.Logf("Minimum free IPs to request: %v", poolmonitor.MinimumFreeIps) + t.Logf("Maximum free IPs to release: %v", poolmonitor.MaximumFreeIps) + + // increase number of allocated IP's in CNS + err := fakecns.SetNumberOfAllocatedIPs(9) + if err != nil { + t.Fatalf("Failed to allocate test ipconfigs with err: %v", err) + } + + // When poolmonitor reconcile is called, trigger increase and cache goal state + err = poolmonitor.Reconcile() + if err != nil { + t.Fatalf("Failed to allocate test ipconfigs with err: %v", err) + } + + // ensure pool monitor has only requested the max pod ip count + if poolmonitor.cachedNNC.Spec.RequestedIPCount != maxPodIPCount { + t.Fatalf("Pool monitor target IP count (%v) should be the node limit (%v) when the max has been reached", poolmonitor.cachedNNC.Spec.RequestedIPCount, maxPodIPCount) + } +} + func TestPoolDecrease(t *testing.T) { var ( batchSize = 10 initialIPConfigCount = 20 requestThresholdPercent = 30 releaseThresholdPercent = 150 + maxPodIPCount = int64(30) ) - fakecns, fakerc, poolmonitor := initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent) + fakecns, fakerc, poolmonitor := initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) log.Printf("Min free IP's %v", poolmonitor.MinimumFreeIps) log.Printf("Max free IP %v", poolmonitor.MaximumFreeIps) @@ -253,9 +289,10 @@ func TestPoolSizeDecreaseWhenDecreaseHasAlreadyBeenRequested(t *testing.T) { initialIPConfigCount = 20 requestThresholdPercent = 30 releaseThresholdPercent = 100 + maxPodIPCount = int64(30) ) - fakecns, fakerc, poolmonitor := initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent) + fakecns, fakerc, poolmonitor := initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) log.Printf("Min free IP's %v", poolmonitor.MinimumFreeIps) log.Printf("Max free IP %v", poolmonitor.MaximumFreeIps) @@ -320,9 +357,10 @@ func TestPoolSizeDecreaseToReallyLow(t *testing.T) { initialIPConfigCount = 30 requestThresholdPercent = 30 releaseThresholdPercent = 100 + maxPodIPCount = int64(30) ) - fakecns, fakerc, poolmonitor := initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent) + fakecns, fakerc, poolmonitor := initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) log.Printf("Min free IP's %v", poolmonitor.MinimumFreeIps) log.Printf("Max free IP %v", poolmonitor.MaximumFreeIps) @@ -395,3 +433,51 @@ func TestPoolSizeDecreaseToReallyLow(t *testing.T) { t.Fatalf("Expected IP's not in use to be 0 after reconcile, expected %v, actual %v", (initialIPConfigCount - batchSize), len(poolmonitor.cachedNNC.Spec.IPsNotInUse)) } } + +func TestDecreaseAfterNodeLimitReached(t *testing.T) { + var ( + batchSize = 16 + initialIPConfigCount = 30 + requestThresholdPercent = 50 + releaseThresholdPercent = 150 + maxPodIPCount = int64(30) + expectedRequestedIP = 16 + expectedDecreaseIP = int(maxPodIPCount) % batchSize + ) + + fakecns, _, poolmonitor := initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) + + t.Logf("Minimum free IPs to request: %v", poolmonitor.MinimumFreeIps) + t.Logf("Maximum free IPs to release: %v", poolmonitor.MaximumFreeIps) + + err := fakecns.SetNumberOfAllocatedIPs(20) + if err != nil { + t.Error(err) + } + + err = poolmonitor.Reconcile() + if err != nil { + t.Errorf("Expected pool monitor to not fail after CNS set number of allocated IP's %v", err) + } + + // Trigger a batch release + err = fakecns.SetNumberOfAllocatedIPs(5) + if err != nil { + t.Error(err) + } + + err = poolmonitor.Reconcile() + if err != nil { + t.Errorf("Expected pool monitor to not fail after CNS set number of allocated IP's %v", err) + } + + // Ensure poolmonitor asked for a multiple of batch size + if poolmonitor.cachedNNC.Spec.RequestedIPCount != int64(expectedRequestedIP) { + t.Fatalf("Expected requested ips to be %v when scaling by 1 batch size down from %v (max pod limit) but got %v", expectedRequestedIP, maxPodIPCount, poolmonitor.cachedNNC.Spec.RequestedIPCount) + } + + // Ensure we minused by the mod result + if len(poolmonitor.cachedNNC.Spec.IPsNotInUse) != expectedDecreaseIP { + t.Fatalf("Expected to decrease requested IPs by %v (max pod count mod batchsize) to make the requested ip count a multiple of the batch size in the case of hitting the max before scale down, but got %v", expectedDecreaseIP, len(poolmonitor.cachedNNC.Spec.IPsNotInUse)) + } +} \ No newline at end of file diff --git a/cns/requestcontroller/kubecontroller/crdrequestcontroller.go b/cns/requestcontroller/kubecontroller/crdrequestcontroller.go index 59d7709009..7c7b0642ac 100644 --- a/cns/requestcontroller/kubecontroller/crdrequestcontroller.go +++ b/cns/requestcontroller/kubecontroller/crdrequestcontroller.go @@ -294,6 +294,34 @@ func (crdRC *crdRequestController) UpdateCRDSpec(cntxt context.Context, crdSpec return nil } +// GetMaxIPCountOfNode gets the max ip count for this node +func (crdRC *crdRequestController) GetMaxIPCountOfNode(cntxt context.Context) (int64, error) { + var ( + node *corev1.Node + err error + count int64 + ) + + if node, err = crdRC.getNode(cntxt, crdRC.nodeName); err != nil { + logger.Printf("[cns-rc] Error getting node %v: %v", crdRC.nodeName, err) + return -1, err + } + count = node.Status.Capacity.Pods().Value() + + return count, nil +} + +func (crdRC *crdRequestController) getNode(ctx context.Context, nodeName string) (*corev1.Node, error) { + node := &corev1.Node{} + + err := crdRC.KubeClient.Get(ctx, client.ObjectKey{ + Namespace: "", + Name: nodeName, + }, node) + + return node, err +} + // getNodeNetConfig gets the nodeNetworkConfig CRD given the name and namespace of the CRD object func (crdRC *crdRequestController) getNodeNetConfig(cntxt context.Context, name, namespace string) (*nnc.NodeNetworkConfig, error) { nodeNetworkConfig := &nnc.NodeNetworkConfig{} diff --git a/cns/requestcontroller/requestcontrollerintreface.go b/cns/requestcontroller/requestcontrollerintreface.go index 19cfa9cc4a..898aa4aa40 100644 --- a/cns/requestcontroller/requestcontrollerintreface.go +++ b/cns/requestcontroller/requestcontrollerintreface.go @@ -12,4 +12,5 @@ type RequestController interface { StartRequestController(exitChan <-chan struct{}) error UpdateCRDSpec(cntxt context.Context, crdSpec nnc.NodeNetworkConfigSpec) error IsStarted() bool + GetMaxIPCountOfNode(context.Context) (int64, error) } diff --git a/test/integration/manifests/cns/clusterrole.yaml b/test/integration/manifests/cns/clusterrole.yaml index a2f397c4d0..fde2e49e07 100644 --- a/test/integration/manifests/cns/clusterrole.yaml +++ b/test/integration/manifests/cns/clusterrole.yaml @@ -5,5 +5,5 @@ metadata: namespace: kube-system rules: - apiGroups: [""] - resources: ["pods"] + resources: ["pods", "nodes"] verbs: ["get", "watch", "list"] \ No newline at end of file From 06376756a0eb138b45e9eece07b60c2e311ad2f8 Mon Sep 17 00:00:00 2001 From: Paul Johnston Date: Thu, 1 Apr 2021 16:06:03 -0700 Subject: [PATCH 02/10] fixed bad spacing --- cns/fakes/requestcontrollerfake.go | 20 ++--- cns/ipampoolmonitor/ipampoolmonitor.go | 71 +++++++-------- cns/ipampoolmonitor/ipampoolmonitor_test.go | 86 +++++++++---------- .../kubecontroller/crdrequestcontroller.go | 2 +- .../requestcontrollerintreface.go | 2 +- 5 files changed, 91 insertions(+), 90 deletions(-) diff --git a/cns/fakes/requestcontrollerfake.go b/cns/fakes/requestcontrollerfake.go index c0800ef101..a7e7fd7c82 100644 --- a/cns/fakes/requestcontrollerfake.go +++ b/cns/fakes/requestcontrollerfake.go @@ -16,11 +16,11 @@ type RequestControllerFake struct { fakecns *HTTPServiceFake cachedCRD nnc.NodeNetworkConfig ip net.IP - node corev1.Node + node corev1.Node } func NewRequestControllerFake(cnsService *HTTPServiceFake, scalar nnc.Scaler, subnetAddressSpace string, numberOfIPConfigs int, maxPodIPCount int64) *RequestControllerFake { - quantity, _ := resource.ParseQuantity(strconv.FormatInt(maxPodIPCount, 10)) + quantity, _ := resource.ParseQuantity(strconv.FormatInt(maxPodIPCount, 10)) rc := &RequestControllerFake{ fakecns: cnsService, @@ -33,13 +33,13 @@ func NewRequestControllerFake(cnsService *HTTPServiceFake, scalar nnc.Scaler, su }}, }, }, - node: corev1.Node{ - Status: corev1.NodeStatus{ - Capacity: corev1.ResourceList{ - corev1.ResourcePods: quantity, - }, - }, - }, + node: corev1.Node{ + Status: corev1.NodeStatus{ + Capacity: corev1.ResourceList{ + corev1.ResourcePods: quantity, + }, + }, + }, } rc.ip, _, _ = net.ParseCIDR(subnetAddressSpace) @@ -94,7 +94,7 @@ func (rc *RequestControllerFake) UpdateCRDSpec(cntxt context.Context, desiredSpe } func (rc *RequestControllerFake) GetMaxIPCountOfNode(ctx context.Context) (int64, error) { - return rc.node.Status.Capacity.Pods().Value(), nil + return rc.node.Status.Capacity.Pods().Value(), nil } func remove(slice []nnc.IPAssignment, s int) []nnc.IPAssignment { diff --git a/cns/ipampoolmonitor/ipampoolmonitor.go b/cns/ipampoolmonitor/ipampoolmonitor.go index 0224ce880c..ba5fd99318 100644 --- a/cns/ipampoolmonitor/ipampoolmonitor.go +++ b/cns/ipampoolmonitor/ipampoolmonitor.go @@ -78,16 +78,16 @@ func (pm *CNSIPAMPoolMonitor) Reconcile() error { switch { // pod count is increasing case freeIPConfigCount < pm.MinimumFreeIps: - maxIpCount, err := pm.rc.GetMaxIPCountOfNode(context.Background()) - if err != nil { - logger.Printf("[ipam-pool-monitor] Error when getting max ip count in Reconcile: %v", err) - return err - } - - if pm.cachedNNC.Spec.RequestedIPCount == maxIpCount { - // If we're already at the maxIpCount, don't try to increase - return nil - } + maxIpCount, err := pm.rc.GetMaxIPCountOfNode(context.Background()) + if err != nil { + logger.Printf("[ipam-pool-monitor] Error when getting max ip count in Reconcile: %v", err) + return err + } + + if pm.cachedNNC.Spec.RequestedIPCount == maxIpCount { + // If we're already at the maxIpCount, don't try to increase + return nil + } logger.Printf("[ipam-pool-monitor] Increasing pool size...%s ", msg) return pm.increasePoolSize() @@ -125,23 +125,23 @@ func (pm *CNSIPAMPoolMonitor) increasePoolSize() error { // Query the max pod count for this node maxIpCount, err := pm.rc.GetMaxIPCountOfNode(context.Background()) - if err != nil { - logger.Printf("[ipam-pool-monitor] Error when getting max ip count in increasePoolSize: %v", err) - return err - } + if err != nil { + logger.Printf("[ipam-pool-monitor] Error when getting max ip count in increasePoolSize: %v", err) + return err + } previouslyRequestedIPCount := tempNNCSpec.RequestedIPCount tempNNCSpec.RequestedIPCount += pm.scalarUnits.BatchSize if tempNNCSpec.RequestedIPCount > maxIpCount { // We don't want to ask for more ips than the node limit - logger.Printf("[ipam-pool-monitor] Requested IP count (%v) is over the node limit (%v), requesting node limit instead.", tempNNCSpec.RequestedIPCount, maxIpCount) + logger.Printf("[ipam-pool-monitor] Requested IP count (%v) is over the node limit (%v), requesting node limit instead.", tempNNCSpec.RequestedIPCount, maxIpCount) tempNNCSpec.RequestedIPCount = maxIpCount } // If the requested IP count is same as before, then don't do anything if tempNNCSpec.RequestedIPCount == previouslyRequestedIPCount { - logger.Printf("[ipam-pool-monitor] Previously requested IP count %v is same as updated IP count %v, doing nothing", previouslyRequestedIPCount, tempNNCSpec.RequestedIPCount) + logger.Printf("[ipam-pool-monitor] Previously requested IP count %v is same as updated IP count %v, doing nothing", previouslyRequestedIPCount, tempNNCSpec.RequestedIPCount) return nil } @@ -167,25 +167,25 @@ func (pm *CNSIPAMPoolMonitor) decreasePoolSize(existingPendingReleaseIPCount int var err error var newIpsMarkedAsPending bool var pendingIpAddresses map[string]cns.IPConfigurationStatus - var updatedRequestedIPCount int64 - var decreaseIPCountBy int64 - - // Ensure the updated requested IP count is a multiple of the batch size - previouslyRequestedIPCount := pm.cachedNNC.Spec.RequestedIPCount - modResult := previouslyRequestedIPCount % pm.scalarUnits.BatchSize - - logger.Printf("[ipam-pool-monitor] Previously RequestedIP Count %v", previouslyRequestedIPCount) - logger.Printf("[ipam-pool-monitor] Batch size : %v", pm.scalarUnits.BatchSize) - logger.Printf("[ipam-pool-monitor] modResult of (previously requested IP count mod batch size) = %v", modResult) - - if modResult != 0 { - // Example: previouscount = 25, batchsize = 10, 25 - 10 = 15, NOT a multiple of batchsize (10) - // Don't want that, so make requestedIPCount 20 (25 - (25 % 10)) so that it is a multiple of the batchsize (10) - updatedRequestedIPCount = previouslyRequestedIPCount - modResult - } else { - // Example: previouscount = 30, batchsize = 10, 30 - 10 = 20 which is multiple of batchsize (10) so all good - updatedRequestedIPCount = previouslyRequestedIPCount - pm.scalarUnits.BatchSize - } + var updatedRequestedIPCount int64 + var decreaseIPCountBy int64 + + // Ensure the updated requested IP count is a multiple of the batch size + previouslyRequestedIPCount := pm.cachedNNC.Spec.RequestedIPCount + modResult := previouslyRequestedIPCount % pm.scalarUnits.BatchSize + + logger.Printf("[ipam-pool-monitor] Previously RequestedIP Count %v", previouslyRequestedIPCount) + logger.Printf("[ipam-pool-monitor] Batch size : %v", pm.scalarUnits.BatchSize) + logger.Printf("[ipam-pool-monitor] modResult of (previously requested IP count mod batch size) = %v", modResult) + + if modResult != 0 { + // Example: previouscount = 25, batchsize = 10, 25 - 10 = 15, NOT a multiple of batchsize (10) + // Don't want that, so make requestedIPCount 20 (25 - (25 % 10)) so that it is a multiple of the batchsize (10) + updatedRequestedIPCount = previouslyRequestedIPCount - modResult + } else { + // Example: previouscount = 30, batchsize = 10, 30 - 10 = 20 which is multiple of batchsize (10) so all good + updatedRequestedIPCount = previouslyRequestedIPCount - pm.scalarUnits.BatchSize + } decreaseIPCountBy = previouslyRequestedIPCount - updatedRequestedIPCount @@ -258,6 +258,7 @@ func (pm *CNSIPAMPoolMonitor) cleanPendingRelease() error { logger.Printf("[ipam-pool-monitor] cleanPendingRelease: UpdateCRDSpec succeeded for spec %+v", tempNNCSpec) + // save the updated state to cachedSpec pm.cachedNNC.Spec = tempNNCSpec pm.pendingRelease = false diff --git a/cns/ipampoolmonitor/ipampoolmonitor_test.go b/cns/ipampoolmonitor/ipampoolmonitor_test.go index eda939aba4..c30b5450f6 100644 --- a/cns/ipampoolmonitor/ipampoolmonitor_test.go +++ b/cns/ipampoolmonitor/ipampoolmonitor_test.go @@ -37,7 +37,7 @@ func TestPoolSizeIncrease(t *testing.T) { initialIPConfigCount = 10 requestThresholdPercent = 30 releaseThresholdPercent = 150 - maxPodIPCount = int64(30) + maxPodIPCount = int64(30) ) fakecns, fakerc, poolmonitor := initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) @@ -91,7 +91,7 @@ func TestPoolIncreaseDoesntChangeWhenIncreaseIsAlreadyInProgress(t *testing.T) { initialIPConfigCount = 10 requestThresholdPercent = 30 releaseThresholdPercent = 150 - maxPodIPCount = int64(30) + maxPodIPCount = int64(30) ) fakecns, fakerc, poolmonitor := initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) @@ -157,7 +157,7 @@ func TestPoolSizeIncreaseIdempotency(t *testing.T) { initialIPConfigCount = 10 requestThresholdPercent = 30 releaseThresholdPercent = 150 - maxPodIPCount = int64(30) + maxPodIPCount = int64(30) ) fakecns, _, poolmonitor := initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) @@ -195,32 +195,32 @@ func TestPoolSizeIncreaseIdempotency(t *testing.T) { } func TestPoolIncreasePastNodeLimit(t *testing.T) { - var ( - batchSize = 16 - initialIPConfigCount = 16 - requestThresholdPercent = 50 - releaseThresholdPercent = 150 - maxPodIPCount = int64(30) - ) + var ( + batchSize = 16 + initialIPConfigCount = 16 + requestThresholdPercent = 50 + releaseThresholdPercent = 150 + maxPodIPCount = int64(30) + ) - fakecns, _, poolmonitor := initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) + fakecns, _, poolmonitor := initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) - t.Logf("Minimum free IPs to request: %v", poolmonitor.MinimumFreeIps) + t.Logf("Minimum free IPs to request: %v", poolmonitor.MinimumFreeIps) t.Logf("Maximum free IPs to release: %v", poolmonitor.MaximumFreeIps) - // increase number of allocated IP's in CNS + // increase number of allocated IP's in CNS err := fakecns.SetNumberOfAllocatedIPs(9) if err != nil { t.Fatalf("Failed to allocate test ipconfigs with err: %v", err) } - // When poolmonitor reconcile is called, trigger increase and cache goal state + // When poolmonitor reconcile is called, trigger increase and cache goal state err = poolmonitor.Reconcile() if err != nil { t.Fatalf("Failed to allocate test ipconfigs with err: %v", err) } - // ensure pool monitor has only requested the max pod ip count + // ensure pool monitor has only requested the max pod ip count if poolmonitor.cachedNNC.Spec.RequestedIPCount != maxPodIPCount { t.Fatalf("Pool monitor target IP count (%v) should be the node limit (%v) when the max has been reached", poolmonitor.cachedNNC.Spec.RequestedIPCount, maxPodIPCount) } @@ -232,7 +232,7 @@ func TestPoolDecrease(t *testing.T) { initialIPConfigCount = 20 requestThresholdPercent = 30 releaseThresholdPercent = 150 - maxPodIPCount = int64(30) + maxPodIPCount = int64(30) ) fakecns, fakerc, poolmonitor := initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) @@ -435,49 +435,49 @@ func TestPoolSizeDecreaseToReallyLow(t *testing.T) { } func TestDecreaseAfterNodeLimitReached(t *testing.T) { - var ( - batchSize = 16 - initialIPConfigCount = 30 - requestThresholdPercent = 50 - releaseThresholdPercent = 150 - maxPodIPCount = int64(30) - expectedRequestedIP = 16 - expectedDecreaseIP = int(maxPodIPCount) % batchSize - ) + var ( + batchSize = 16 + initialIPConfigCount = 30 + requestThresholdPercent = 50 + releaseThresholdPercent = 150 + maxPodIPCount = int64(30) + expectedRequestedIP = 16 + expectedDecreaseIP = int(maxPodIPCount) % batchSize + ) - fakecns, _, poolmonitor := initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) + fakecns, _, poolmonitor := initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) - t.Logf("Minimum free IPs to request: %v", poolmonitor.MinimumFreeIps) + t.Logf("Minimum free IPs to request: %v", poolmonitor.MinimumFreeIps) t.Logf("Maximum free IPs to release: %v", poolmonitor.MaximumFreeIps) - err := fakecns.SetNumberOfAllocatedIPs(20) - if err != nil { + err := fakecns.SetNumberOfAllocatedIPs(20) + if err != nil { t.Error(err) } - err = poolmonitor.Reconcile() + err = poolmonitor.Reconcile() if err != nil { t.Errorf("Expected pool monitor to not fail after CNS set number of allocated IP's %v", err) } - // Trigger a batch release + // Trigger a batch release err = fakecns.SetNumberOfAllocatedIPs(5) if err != nil { t.Error(err) } - err = poolmonitor.Reconcile() + err = poolmonitor.Reconcile() if err != nil { t.Errorf("Expected pool monitor to not fail after CNS set number of allocated IP's %v", err) } - - // Ensure poolmonitor asked for a multiple of batch size - if poolmonitor.cachedNNC.Spec.RequestedIPCount != int64(expectedRequestedIP) { - t.Fatalf("Expected requested ips to be %v when scaling by 1 batch size down from %v (max pod limit) but got %v", expectedRequestedIP, maxPodIPCount, poolmonitor.cachedNNC.Spec.RequestedIPCount) - } - - // Ensure we minused by the mod result - if len(poolmonitor.cachedNNC.Spec.IPsNotInUse) != expectedDecreaseIP { - t.Fatalf("Expected to decrease requested IPs by %v (max pod count mod batchsize) to make the requested ip count a multiple of the batch size in the case of hitting the max before scale down, but got %v", expectedDecreaseIP, len(poolmonitor.cachedNNC.Spec.IPsNotInUse)) - } -} \ No newline at end of file + + // Ensure poolmonitor asked for a multiple of batch size + if poolmonitor.cachedNNC.Spec.RequestedIPCount != int64(expectedRequestedIP) { + t.Fatalf("Expected requested ips to be %v when scaling by 1 batch size down from %v (max pod limit) but got %v", expectedRequestedIP, maxPodIPCount, poolmonitor.cachedNNC.Spec.RequestedIPCount) + } + + // Ensure we minused by the mod result + if len(poolmonitor.cachedNNC.Spec.IPsNotInUse) != expectedDecreaseIP { + t.Fatalf("Expected to decrease requested IPs by %v (max pod count mod batchsize) to make the requested ip count a multiple of the batch size in the case of hitting the max before scale down, but got %v", expectedDecreaseIP, len(poolmonitor.cachedNNC.Spec.IPsNotInUse)) + } +} diff --git a/cns/requestcontroller/kubecontroller/crdrequestcontroller.go b/cns/requestcontroller/kubecontroller/crdrequestcontroller.go index 7c7b0642ac..8ef2507f2e 100644 --- a/cns/requestcontroller/kubecontroller/crdrequestcontroller.go +++ b/cns/requestcontroller/kubecontroller/crdrequestcontroller.go @@ -299,7 +299,7 @@ func (crdRC *crdRequestController) GetMaxIPCountOfNode(cntxt context.Context) (i var ( node *corev1.Node err error - count int64 + count int64 ) if node, err = crdRC.getNode(cntxt, crdRC.nodeName); err != nil { diff --git a/cns/requestcontroller/requestcontrollerintreface.go b/cns/requestcontroller/requestcontrollerintreface.go index 898aa4aa40..c37cfe6d0f 100644 --- a/cns/requestcontroller/requestcontrollerintreface.go +++ b/cns/requestcontroller/requestcontrollerintreface.go @@ -12,5 +12,5 @@ type RequestController interface { StartRequestController(exitChan <-chan struct{}) error UpdateCRDSpec(cntxt context.Context, crdSpec nnc.NodeNetworkConfigSpec) error IsStarted() bool - GetMaxIPCountOfNode(context.Context) (int64, error) + GetMaxIPCountOfNode(context.Context) (int64, error) } From 6a210cdf14f65745279de87f9d55048427db4e58 Mon Sep 17 00:00:00 2001 From: Paul Johnston Date: Thu, 1 Apr 2021 16:09:13 -0700 Subject: [PATCH 03/10] no node pointer --- .../kubecontroller/crdrequestcontroller.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cns/requestcontroller/kubecontroller/crdrequestcontroller.go b/cns/requestcontroller/kubecontroller/crdrequestcontroller.go index 8ef2507f2e..0bfa4ada8a 100644 --- a/cns/requestcontroller/kubecontroller/crdrequestcontroller.go +++ b/cns/requestcontroller/kubecontroller/crdrequestcontroller.go @@ -297,7 +297,7 @@ func (crdRC *crdRequestController) UpdateCRDSpec(cntxt context.Context, crdSpec // GetMaxIPCountOfNode gets the max ip count for this node func (crdRC *crdRequestController) GetMaxIPCountOfNode(cntxt context.Context) (int64, error) { var ( - node *corev1.Node + node corev1.Node err error count int64 ) @@ -311,13 +311,13 @@ func (crdRC *crdRequestController) GetMaxIPCountOfNode(cntxt context.Context) (i return count, nil } -func (crdRC *crdRequestController) getNode(ctx context.Context, nodeName string) (*corev1.Node, error) { - node := &corev1.Node{} +func (crdRC *crdRequestController) getNode(ctx context.Context, nodeName string) (corev1.Node, error) { + node := corev1.Node{} err := crdRC.KubeClient.Get(ctx, client.ObjectKey{ Namespace: "", Name: nodeName, - }, node) + }, &node) return node, err } From 1f78ffd4a1022dfd4226f18cce7c3baa5b395b95 Mon Sep 17 00:00:00 2001 From: Paul Johnston Date: Fri, 2 Apr 2021 09:43:39 -0700 Subject: [PATCH 04/10] remove node implementation, wait for CRD to show max ip count --- cns/azure-cns.yaml | 2 +- cns/fakes/requestcontrollerfake.go | 17 +++-------- cns/ipampoolmonitor/ipampoolmonitor.go | 10 +++---- .../kubecontroller/crdrequestcontroller.go | 30 +++---------------- .../requestcontrollerintreface.go | 2 +- .../manifests/cns/clusterrole.yaml | 2 +- 6 files changed, 16 insertions(+), 47 deletions(-) diff --git a/cns/azure-cns.yaml b/cns/azure-cns.yaml index 6167fc6a37..a009556480 100644 --- a/cns/azure-cns.yaml +++ b/cns/azure-cns.yaml @@ -20,7 +20,7 @@ metadata: name: pod-reader-all-namespaces rules: - apiGroups: [""] - resources: ["pods", "nodes"] + resources: ["pods"] verbs: ["get", "watch", "list"] --- apiVersion: rbac.authorization.k8s.io/v1 diff --git a/cns/fakes/requestcontrollerfake.go b/cns/fakes/requestcontrollerfake.go index a7e7fd7c82..fe4894af6f 100644 --- a/cns/fakes/requestcontrollerfake.go +++ b/cns/fakes/requestcontrollerfake.go @@ -3,13 +3,11 @@ package fakes import ( "context" "net" - "strconv" "github.com/Azure/azure-container-networking/cns" nnc "github.com/Azure/azure-container-networking/nodenetworkconfig/api/v1alpha" "github.com/google/uuid" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/resource" ) type RequestControllerFake struct { @@ -20,8 +18,7 @@ type RequestControllerFake struct { } func NewRequestControllerFake(cnsService *HTTPServiceFake, scalar nnc.Scaler, subnetAddressSpace string, numberOfIPConfigs int, maxPodIPCount int64) *RequestControllerFake { - quantity, _ := resource.ParseQuantity(strconv.FormatInt(maxPodIPCount, 10)) - + // TODO: use maxPodIPCount to populate CRD max ip count rc := &RequestControllerFake{ fakecns: cnsService, cachedCRD: nnc.NodeNetworkConfig{ @@ -33,13 +30,6 @@ func NewRequestControllerFake(cnsService *HTTPServiceFake, scalar nnc.Scaler, su }}, }, }, - node: corev1.Node{ - Status: corev1.NodeStatus{ - Capacity: corev1.ResourceList{ - corev1.ResourcePods: quantity, - }, - }, - }, } rc.ip, _, _ = net.ParseCIDR(subnetAddressSpace) @@ -93,8 +83,9 @@ func (rc *RequestControllerFake) UpdateCRDSpec(cntxt context.Context, desiredSpe return nil } -func (rc *RequestControllerFake) GetMaxIPCountOfNode(ctx context.Context) (int64, error) { - return rc.node.Status.Capacity.Pods().Value(), nil +func (rc *RequestControllerFake) GetMaxIPCount(ctx context.Context) (int64, error) { + maxIPCount := 250 // TODO: Get this from NNC + return int64(maxIPCount), nil } func remove(slice []nnc.IPAssignment, s int) []nnc.IPAssignment { diff --git a/cns/ipampoolmonitor/ipampoolmonitor.go b/cns/ipampoolmonitor/ipampoolmonitor.go index ba5fd99318..ed869811b5 100644 --- a/cns/ipampoolmonitor/ipampoolmonitor.go +++ b/cns/ipampoolmonitor/ipampoolmonitor.go @@ -78,7 +78,7 @@ func (pm *CNSIPAMPoolMonitor) Reconcile() error { switch { // pod count is increasing case freeIPConfigCount < pm.MinimumFreeIps: - maxIpCount, err := pm.rc.GetMaxIPCountOfNode(context.Background()) + maxIpCount, err := pm.rc.GetMaxIPCount(context.Background()) if err != nil { logger.Printf("[ipam-pool-monitor] Error when getting max ip count in Reconcile: %v", err) return err @@ -123,8 +123,8 @@ func (pm *CNSIPAMPoolMonitor) increasePoolSize() error { return err } - // Query the max pod count for this node - maxIpCount, err := pm.rc.GetMaxIPCountOfNode(context.Background()) + // Query the max ip count + maxIpCount, err := pm.rc.GetMaxIPCount(context.Background()) if err != nil { logger.Printf("[ipam-pool-monitor] Error when getting max ip count in increasePoolSize: %v", err) return err @@ -134,8 +134,8 @@ func (pm *CNSIPAMPoolMonitor) increasePoolSize() error { tempNNCSpec.RequestedIPCount += pm.scalarUnits.BatchSize if tempNNCSpec.RequestedIPCount > maxIpCount { - // We don't want to ask for more ips than the node limit - logger.Printf("[ipam-pool-monitor] Requested IP count (%v) is over the node limit (%v), requesting node limit instead.", tempNNCSpec.RequestedIPCount, maxIpCount) + // We don't want to ask for more ips than the max + logger.Printf("[ipam-pool-monitor] Requested IP count (%v) is over max limit (%v), requesting max limit instead.", tempNNCSpec.RequestedIPCount, maxIpCount) tempNNCSpec.RequestedIPCount = maxIpCount } diff --git a/cns/requestcontroller/kubecontroller/crdrequestcontroller.go b/cns/requestcontroller/kubecontroller/crdrequestcontroller.go index 0bfa4ada8a..acf1bfa8ef 100644 --- a/cns/requestcontroller/kubecontroller/crdrequestcontroller.go +++ b/cns/requestcontroller/kubecontroller/crdrequestcontroller.go @@ -294,32 +294,10 @@ func (crdRC *crdRequestController) UpdateCRDSpec(cntxt context.Context, crdSpec return nil } -// GetMaxIPCountOfNode gets the max ip count for this node -func (crdRC *crdRequestController) GetMaxIPCountOfNode(cntxt context.Context) (int64, error) { - var ( - node corev1.Node - err error - count int64 - ) - - if node, err = crdRC.getNode(cntxt, crdRC.nodeName); err != nil { - logger.Printf("[cns-rc] Error getting node %v: %v", crdRC.nodeName, err) - return -1, err - } - count = node.Status.Capacity.Pods().Value() - - return count, nil -} - -func (crdRC *crdRequestController) getNode(ctx context.Context, nodeName string) (corev1.Node, error) { - node := corev1.Node{} - - err := crdRC.KubeClient.Get(ctx, client.ObjectKey{ - Namespace: "", - Name: nodeName, - }, &node) - - return node, err +// GetMaxIPCount gets the max ip count for this node +func (crdRC *crdRequestController) GetMaxIPCount(cntxt context.Context) (int64, error) { + maxIPCount := 250 // TODO: Get this from NNC + return int64(maxIPCount), nil } // getNodeNetConfig gets the nodeNetworkConfig CRD given the name and namespace of the CRD object diff --git a/cns/requestcontroller/requestcontrollerintreface.go b/cns/requestcontroller/requestcontrollerintreface.go index c37cfe6d0f..1a5ce89d97 100644 --- a/cns/requestcontroller/requestcontrollerintreface.go +++ b/cns/requestcontroller/requestcontrollerintreface.go @@ -12,5 +12,5 @@ type RequestController interface { StartRequestController(exitChan <-chan struct{}) error UpdateCRDSpec(cntxt context.Context, crdSpec nnc.NodeNetworkConfigSpec) error IsStarted() bool - GetMaxIPCountOfNode(context.Context) (int64, error) + GetMaxIPCount(context.Context) (int64, error) } diff --git a/test/integration/manifests/cns/clusterrole.yaml b/test/integration/manifests/cns/clusterrole.yaml index fde2e49e07..a2f397c4d0 100644 --- a/test/integration/manifests/cns/clusterrole.yaml +++ b/test/integration/manifests/cns/clusterrole.yaml @@ -5,5 +5,5 @@ metadata: namespace: kube-system rules: - apiGroups: [""] - resources: ["pods", "nodes"] + resources: ["pods"] verbs: ["get", "watch", "list"] \ No newline at end of file From 725ea18d0a3fa6919c43e2b561428df80839de9a Mon Sep 17 00:00:00 2001 From: Paul Johnston Date: Fri, 2 Apr 2021 15:09:13 -0700 Subject: [PATCH 05/10] use new MaxIPCount field of scaler --- cns/fakes/requestcontrollerfake.go | 7 +------ cns/ipampoolmonitor/ipampoolmonitor.go | 14 ++------------ cns/ipampoolmonitor/ipampoolmonitor_test.go | 3 ++- .../kubecontroller/crdrequestcontroller.go | 6 ------ .../requestcontrollerintreface.go | 1 - 5 files changed, 5 insertions(+), 26 deletions(-) diff --git a/cns/fakes/requestcontrollerfake.go b/cns/fakes/requestcontrollerfake.go index fe4894af6f..3b772309d8 100644 --- a/cns/fakes/requestcontrollerfake.go +++ b/cns/fakes/requestcontrollerfake.go @@ -17,7 +17,7 @@ type RequestControllerFake struct { node corev1.Node } -func NewRequestControllerFake(cnsService *HTTPServiceFake, scalar nnc.Scaler, subnetAddressSpace string, numberOfIPConfigs int, maxPodIPCount int64) *RequestControllerFake { +func NewRequestControllerFake(cnsService *HTTPServiceFake, scalar nnc.Scaler, subnetAddressSpace string, numberOfIPConfigs int) *RequestControllerFake { // TODO: use maxPodIPCount to populate CRD max ip count rc := &RequestControllerFake{ fakecns: cnsService, @@ -83,11 +83,6 @@ func (rc *RequestControllerFake) UpdateCRDSpec(cntxt context.Context, desiredSpe return nil } -func (rc *RequestControllerFake) GetMaxIPCount(ctx context.Context) (int64, error) { - maxIPCount := 250 // TODO: Get this from NNC - return int64(maxIPCount), nil -} - func remove(slice []nnc.IPAssignment, s int) []nnc.IPAssignment { return append(slice[:s], slice[s+1:]...) } diff --git a/cns/ipampoolmonitor/ipampoolmonitor.go b/cns/ipampoolmonitor/ipampoolmonitor.go index ed869811b5..78217ab457 100644 --- a/cns/ipampoolmonitor/ipampoolmonitor.go +++ b/cns/ipampoolmonitor/ipampoolmonitor.go @@ -78,13 +78,7 @@ func (pm *CNSIPAMPoolMonitor) Reconcile() error { switch { // pod count is increasing case freeIPConfigCount < pm.MinimumFreeIps: - maxIpCount, err := pm.rc.GetMaxIPCount(context.Background()) - if err != nil { - logger.Printf("[ipam-pool-monitor] Error when getting max ip count in Reconcile: %v", err) - return err - } - - if pm.cachedNNC.Spec.RequestedIPCount == maxIpCount { + if pm.cachedNNC.Spec.RequestedIPCount == pm.cachedNNC.Status.Scaler.MaxIPCount { // If we're already at the maxIpCount, don't try to increase return nil } @@ -124,11 +118,7 @@ func (pm *CNSIPAMPoolMonitor) increasePoolSize() error { } // Query the max ip count - maxIpCount, err := pm.rc.GetMaxIPCount(context.Background()) - if err != nil { - logger.Printf("[ipam-pool-monitor] Error when getting max ip count in increasePoolSize: %v", err) - return err - } + maxIpCount, err := pm.cachedNNC.Status.MaxIPCount previouslyRequestedIPCount := tempNNCSpec.RequestedIPCount diff --git a/cns/ipampoolmonitor/ipampoolmonitor_test.go b/cns/ipampoolmonitor/ipampoolmonitor_test.go index c30b5450f6..cfb6319930 100644 --- a/cns/ipampoolmonitor/ipampoolmonitor_test.go +++ b/cns/ipampoolmonitor/ipampoolmonitor_test.go @@ -16,11 +16,12 @@ func initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, release BatchSize: int64(batchSize), RequestThresholdPercent: int64(requestThresholdPercent), ReleaseThresholdPercent: int64(releaseThresholdPercent), + MaxIPCount: int64(maxPodIPCount), } subnetaddresspace := "10.0.0.0/8" fakecns := fakes.NewHTTPServiceFake() - fakerc := fakes.NewRequestControllerFake(fakecns, scalarUnits, subnetaddresspace, initialIPConfigCount, maxPodIPCount) + fakerc := fakes.NewRequestControllerFake(fakecns, scalarUnits, subnetaddresspace, initialIPConfigCount) poolmonitor := NewCNSIPAMPoolMonitor(fakecns, fakerc) diff --git a/cns/requestcontroller/kubecontroller/crdrequestcontroller.go b/cns/requestcontroller/kubecontroller/crdrequestcontroller.go index acf1bfa8ef..59d7709009 100644 --- a/cns/requestcontroller/kubecontroller/crdrequestcontroller.go +++ b/cns/requestcontroller/kubecontroller/crdrequestcontroller.go @@ -294,12 +294,6 @@ func (crdRC *crdRequestController) UpdateCRDSpec(cntxt context.Context, crdSpec return nil } -// GetMaxIPCount gets the max ip count for this node -func (crdRC *crdRequestController) GetMaxIPCount(cntxt context.Context) (int64, error) { - maxIPCount := 250 // TODO: Get this from NNC - return int64(maxIPCount), nil -} - // getNodeNetConfig gets the nodeNetworkConfig CRD given the name and namespace of the CRD object func (crdRC *crdRequestController) getNodeNetConfig(cntxt context.Context, name, namespace string) (*nnc.NodeNetworkConfig, error) { nodeNetworkConfig := &nnc.NodeNetworkConfig{} diff --git a/cns/requestcontroller/requestcontrollerintreface.go b/cns/requestcontroller/requestcontrollerintreface.go index 1a5ce89d97..19cfa9cc4a 100644 --- a/cns/requestcontroller/requestcontrollerintreface.go +++ b/cns/requestcontroller/requestcontrollerintreface.go @@ -12,5 +12,4 @@ type RequestController interface { StartRequestController(exitChan <-chan struct{}) error UpdateCRDSpec(cntxt context.Context, crdSpec nnc.NodeNetworkConfigSpec) error IsStarted() bool - GetMaxIPCount(context.Context) (int64, error) } From ddaa6223682abaa043ac0a2be09fb329d8b5103a Mon Sep 17 00:00:00 2001 From: Paul Johnston Date: Mon, 5 Apr 2021 16:21:22 -0700 Subject: [PATCH 06/10] maxIPCount to scalar units instead of cached nnc --- cns/ipampoolmonitor/ipampoolmonitor.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/cns/ipampoolmonitor/ipampoolmonitor.go b/cns/ipampoolmonitor/ipampoolmonitor.go index 78217ab457..81acb91a03 100644 --- a/cns/ipampoolmonitor/ipampoolmonitor.go +++ b/cns/ipampoolmonitor/ipampoolmonitor.go @@ -72,13 +72,13 @@ func (pm *CNSIPAMPoolMonitor) Reconcile() error { availableIPConfigCount := len(pm.httpService.GetAvailableIPConfigs()) // TODO: add pending allocation count to real cns freeIPConfigCount := pm.cachedNNC.Spec.RequestedIPCount - int64(allocatedPodIPCount) - msg := fmt.Sprintf("[ipam-pool-monitor] Pool Size: %v, Goal Size: %v, BatchSize: %v, MinFree: %v, MaxFree:%v, Allocated: %v, Available: %v, Pending Release: %v, Free: %v, Pending Program: %v", - cnsPodIPConfigCount, pm.cachedNNC.Spec.RequestedIPCount, pm.scalarUnits.BatchSize, pm.MinimumFreeIps, pm.MaximumFreeIps, allocatedPodIPCount, availableIPConfigCount, pendingReleaseIPCount, freeIPConfigCount, pendingProgramCount) + msg := fmt.Sprintf("[ipam-pool-monitor] Pool Size: %v, Goal Size: %v, BatchSize: %v, MaxIPCount: %v, MinFree: %v, MaxFree:%v, Allocated: %v, Available: %v, Pending Release: %v, Free: %v, Pending Program: %v", + cnsPodIPConfigCount, pm.cachedNNC.Spec.RequestedIPCount, pm.scalarUnits.BatchSize, pm.scalarUnits.MaxIPCount, pm.MinimumFreeIps, pm.MaximumFreeIps, allocatedPodIPCount, availableIPConfigCount, pendingReleaseIPCount, freeIPConfigCount, pendingProgramCount) switch { // pod count is increasing case freeIPConfigCount < pm.MinimumFreeIps: - if pm.cachedNNC.Spec.RequestedIPCount == pm.cachedNNC.Status.Scaler.MaxIPCount { + if pm.cachedNNC.Spec.RequestedIPCount == pm.scalarUnits.MaxIPCount { // If we're already at the maxIpCount, don't try to increase return nil } @@ -118,8 +118,7 @@ func (pm *CNSIPAMPoolMonitor) increasePoolSize() error { } // Query the max ip count - maxIpCount, err := pm.cachedNNC.Status.MaxIPCount - + maxIpCount:= pm.scalarUnits.MaxIPCount previouslyRequestedIPCount := tempNNCSpec.RequestedIPCount tempNNCSpec.RequestedIPCount += pm.scalarUnits.BatchSize From 4fbd340ca94c8290ede7da71c90e60da96be9023 Mon Sep 17 00:00:00 2001 From: Paul Johnston Date: Wed, 7 Apr 2021 10:23:10 -0700 Subject: [PATCH 07/10] Addressed comments --- cns/fakes/requestcontrollerfake.go | 1 - cns/ipampoolmonitor/ipampoolmonitor.go | 7 +++++ cns/ipampoolmonitor/ipampoolmonitor_test.go | 32 +++++++++++++++++++++ 3 files changed, 39 insertions(+), 1 deletion(-) diff --git a/cns/fakes/requestcontrollerfake.go b/cns/fakes/requestcontrollerfake.go index 3b772309d8..901c47ce1c 100644 --- a/cns/fakes/requestcontrollerfake.go +++ b/cns/fakes/requestcontrollerfake.go @@ -18,7 +18,6 @@ type RequestControllerFake struct { } func NewRequestControllerFake(cnsService *HTTPServiceFake, scalar nnc.Scaler, subnetAddressSpace string, numberOfIPConfigs int) *RequestControllerFake { - // TODO: use maxPodIPCount to populate CRD max ip count rc := &RequestControllerFake{ fakecns: cnsService, cachedCRD: nnc.NodeNetworkConfig{ diff --git a/cns/ipampoolmonitor/ipampoolmonitor.go b/cns/ipampoolmonitor/ipampoolmonitor.go index 81acb91a03..197c8784a3 100644 --- a/cns/ipampoolmonitor/ipampoolmonitor.go +++ b/cns/ipampoolmonitor/ipampoolmonitor.go @@ -12,6 +12,10 @@ import ( nnc "github.com/Azure/azure-container-networking/nodenetworkconfig/api/v1alpha" ) +const ( + defaultMaxIPCount = int64(250) +) + type CNSIPAMPoolMonitor struct { pendingRelease bool @@ -119,6 +123,9 @@ func (pm *CNSIPAMPoolMonitor) increasePoolSize() error { // Query the max ip count maxIpCount:= pm.scalarUnits.MaxIPCount + if maxIpCount == 0 { + maxIpCount = defaultMaxIPCount + } previouslyRequestedIPCount := tempNNCSpec.RequestedIPCount tempNNCSpec.RequestedIPCount += pm.scalarUnits.BatchSize diff --git a/cns/ipampoolmonitor/ipampoolmonitor_test.go b/cns/ipampoolmonitor/ipampoolmonitor_test.go index cfb6319930..0e1c1c5e16 100644 --- a/cns/ipampoolmonitor/ipampoolmonitor_test.go +++ b/cns/ipampoolmonitor/ipampoolmonitor_test.go @@ -227,6 +227,38 @@ func TestPoolIncreasePastNodeLimit(t *testing.T) { } } +func TestPoolIncreaseBatchSizeGreaterThanMaxPodIPCount(t *testing.T) { + var ( + batchSize = 50 + initialIPConfigCount = 16 + requestThresholdPercent = 50 + releaseThresholdPercent = 150 + maxPodIPCount = int64(30) + ) + + fakecns, _, poolmonitor := initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) + + t.Logf("Minimum free IPs to request: %v", poolmonitor.MinimumFreeIps) + t.Logf("Maximum free IPs to release: %v", poolmonitor.MaximumFreeIps) + + // increase number of allocated IP's in CNS + err := fakecns.SetNumberOfAllocatedIPs(16) + if err != nil { + t.Fatalf("Failed to allocate test ipconfigs with err: %v", err) + } + + // When poolmonitor reconcile is called, trigger increase and cache goal state + err = poolmonitor.Reconcile() + if err != nil { + t.Fatalf("Failed to allocate test ipconfigs with err: %v", err) + } + + // ensure pool monitor has only requested the max pod ip count + if poolmonitor.cachedNNC.Spec.RequestedIPCount != maxPodIPCount { + t.Fatalf("Pool monitor target IP count (%v) should be the node limit (%v) when the max has been reached", poolmonitor.cachedNNC.Spec.RequestedIPCount, maxPodIPCount) + } +} + func TestPoolDecrease(t *testing.T) { var ( batchSize = 10 From dd45fbd0d5feb473ac55f22efe212d0d3a25be6d Mon Sep 17 00:00:00 2001 From: Paul Johnston Date: Thu, 8 Apr 2021 09:42:08 -0700 Subject: [PATCH 08/10] Added getMaxIPCount method that does ==0 check --- cns/ipampoolmonitor/ipampoolmonitor.go | 24 ++++++++++++--------- cns/ipampoolmonitor/ipampoolmonitor_test.go | 21 ++++++++++++++++-- 2 files changed, 33 insertions(+), 12 deletions(-) diff --git a/cns/ipampoolmonitor/ipampoolmonitor.go b/cns/ipampoolmonitor/ipampoolmonitor.go index 197c8784a3..def15891ab 100644 --- a/cns/ipampoolmonitor/ipampoolmonitor.go +++ b/cns/ipampoolmonitor/ipampoolmonitor.go @@ -82,8 +82,8 @@ func (pm *CNSIPAMPoolMonitor) Reconcile() error { switch { // pod count is increasing case freeIPConfigCount < pm.MinimumFreeIps: - if pm.cachedNNC.Spec.RequestedIPCount == pm.scalarUnits.MaxIPCount { - // If we're already at the maxIpCount, don't try to increase + if pm.cachedNNC.Spec.RequestedIPCount == pm.getMaxIPCount() { + // If we're already at the maxIPCount, don't try to increase return nil } @@ -121,18 +121,15 @@ func (pm *CNSIPAMPoolMonitor) increasePoolSize() error { return err } - // Query the max ip count - maxIpCount:= pm.scalarUnits.MaxIPCount - if maxIpCount == 0 { - maxIpCount = defaultMaxIPCount - } + // Query the max IP count + maxIPCount:= pm.getMaxIPCount() previouslyRequestedIPCount := tempNNCSpec.RequestedIPCount tempNNCSpec.RequestedIPCount += pm.scalarUnits.BatchSize - if tempNNCSpec.RequestedIPCount > maxIpCount { + if tempNNCSpec.RequestedIPCount > maxIPCount { // We don't want to ask for more ips than the max - logger.Printf("[ipam-pool-monitor] Requested IP count (%v) is over max limit (%v), requesting max limit instead.", tempNNCSpec.RequestedIPCount, maxIpCount) - tempNNCSpec.RequestedIPCount = maxIpCount + logger.Printf("[ipam-pool-monitor] Requested IP count (%v) is over max limit (%v), requesting max limit instead.", tempNNCSpec.RequestedIPCount, maxIPCount) + tempNNCSpec.RequestedIPCount = maxIPCount } // If the requested IP count is same as before, then don't do anything @@ -301,3 +298,10 @@ func (pm *CNSIPAMPoolMonitor) Update(scalar nnc.Scaler, spec nnc.NodeNetworkConf return nil } + +func (pm *CNSIPAMPoolMonitor) getMaxIPCount() int64 { + if pm.scalarUnits.MaxIPCount == 0 { + pm.scalarUnits.MaxIPCount = defaultMaxIPCount + } + return pm.scalarUnits.MaxIPCount +} diff --git a/cns/ipampoolmonitor/ipampoolmonitor_test.go b/cns/ipampoolmonitor/ipampoolmonitor_test.go index 0e1c1c5e16..697975047c 100644 --- a/cns/ipampoolmonitor/ipampoolmonitor_test.go +++ b/cns/ipampoolmonitor/ipampoolmonitor_test.go @@ -259,6 +259,23 @@ func TestPoolIncreaseBatchSizeGreaterThanMaxPodIPCount(t *testing.T) { } } +func TestPoolIncreaseMaxIPCountSetToZero(t *testing.T) { + var ( + batchSize = 16 + initialIPConfigCount = 16 + requestThresholdPercent = 50 + releaseThresholdPercent = 150 + initialMaxPodIPCount = int64(0) + expectedMaxPodIPCount = defaultMaxIPCount + ) + + _, _, poolmonitor := initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, initialMaxPodIPCount) + + if poolmonitor.getMaxIPCount() != expectedMaxPodIPCount { + t.Fatalf("Pool monitor target IP count (%v) should be the node limit (%v) when the MaxIPCount field in the CRD is zero", poolmonitor.getMaxIPCount(), expectedMaxPodIPCount) + } +} + func TestPoolDecrease(t *testing.T) { var ( batchSize = 10 @@ -322,7 +339,7 @@ func TestPoolSizeDecreaseWhenDecreaseHasAlreadyBeenRequested(t *testing.T) { initialIPConfigCount = 20 requestThresholdPercent = 30 releaseThresholdPercent = 100 - maxPodIPCount = int64(30) + maxPodIPCount = int64(30) ) fakecns, fakerc, poolmonitor := initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) @@ -390,7 +407,7 @@ func TestPoolSizeDecreaseToReallyLow(t *testing.T) { initialIPConfigCount = 30 requestThresholdPercent = 30 releaseThresholdPercent = 100 - maxPodIPCount = int64(30) + maxPodIPCount = int64(30) ) fakecns, fakerc, poolmonitor := initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) From 2ab4c0180028f45bc41c493cd0923b2fc5937581 Mon Sep 17 00:00:00 2001 From: Paul Johnston Date: Thu, 8 Apr 2021 09:46:28 -0700 Subject: [PATCH 09/10] removed node since CRD will be maxIPCount holder --- cns/fakes/requestcontrollerfake.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/cns/fakes/requestcontrollerfake.go b/cns/fakes/requestcontrollerfake.go index 901c47ce1c..9060f88bcc 100644 --- a/cns/fakes/requestcontrollerfake.go +++ b/cns/fakes/requestcontrollerfake.go @@ -7,14 +7,12 @@ import ( "github.com/Azure/azure-container-networking/cns" nnc "github.com/Azure/azure-container-networking/nodenetworkconfig/api/v1alpha" "github.com/google/uuid" - corev1 "k8s.io/api/core/v1" ) type RequestControllerFake struct { fakecns *HTTPServiceFake cachedCRD nnc.NodeNetworkConfig ip net.IP - node corev1.Node } func NewRequestControllerFake(cnsService *HTTPServiceFake, scalar nnc.Scaler, subnetAddressSpace string, numberOfIPConfigs int) *RequestControllerFake { From a7e06efb9388a613bb071bce1183b2721ebc1311 Mon Sep 17 00:00:00 2001 From: Paul Johnston Date: Thu, 8 Apr 2021 10:21:23 -0700 Subject: [PATCH 10/10] getters for batch size in case customer edits it manually --- cns/ipampoolmonitor/ipampoolmonitor.go | 31 ++++++++++----- cns/ipampoolmonitor/ipampoolmonitor_test.go | 43 +++++++++++++++++++++ 2 files changed, 65 insertions(+), 9 deletions(-) diff --git a/cns/ipampoolmonitor/ipampoolmonitor.go b/cns/ipampoolmonitor/ipampoolmonitor.go index ea2f8b1385..0e2441acae 100644 --- a/cns/ipampoolmonitor/ipampoolmonitor.go +++ b/cns/ipampoolmonitor/ipampoolmonitor.go @@ -75,14 +75,17 @@ func (pm *CNSIPAMPoolMonitor) Reconcile() error { pendingReleaseIPCount := len(pm.httpService.GetPendingReleaseIPConfigs()) availableIPConfigCount := len(pm.httpService.GetAvailableIPConfigs()) // TODO: add pending allocation count to real cns freeIPConfigCount := pm.cachedNNC.Spec.RequestedIPCount - int64(allocatedPodIPCount) + batchSize := pm.getBatchSize() //Use getters in case customer changes batchsize manually + maxIPCount := pm.getMaxIPCount() + msg := fmt.Sprintf("[ipam-pool-monitor] Pool Size: %v, Goal Size: %v, BatchSize: %v, MaxIPCount: %v, MinFree: %v, MaxFree:%v, Allocated: %v, Available: %v, Pending Release: %v, Free: %v, Pending Program: %v", - cnsPodIPConfigCount, pm.cachedNNC.Spec.RequestedIPCount, pm.scalarUnits.BatchSize, pm.scalarUnits.MaxIPCount, pm.MinimumFreeIps, pm.MaximumFreeIps, allocatedPodIPCount, availableIPConfigCount, pendingReleaseIPCount, freeIPConfigCount, pendingProgramCount) + cnsPodIPConfigCount, pm.cachedNNC.Spec.RequestedIPCount, batchSize, maxIPCount, pm.MinimumFreeIps, pm.MaximumFreeIps, allocatedPodIPCount, availableIPConfigCount, pendingReleaseIPCount, freeIPConfigCount, pendingProgramCount) switch { // pod count is increasing case freeIPConfigCount < pm.MinimumFreeIps: - if pm.cachedNNC.Spec.RequestedIPCount == pm.getMaxIPCount() { + if pm.cachedNNC.Spec.RequestedIPCount == maxIPCount { // If we're already at the maxIPCount, don't try to increase return nil } @@ -122,10 +125,11 @@ func (pm *CNSIPAMPoolMonitor) increasePoolSize() error { } // Query the max IP count - maxIPCount:= pm.getMaxIPCount() + maxIPCount := pm.getMaxIPCount() previouslyRequestedIPCount := tempNNCSpec.RequestedIPCount + batchSize := pm.getBatchSize() - tempNNCSpec.RequestedIPCount += pm.scalarUnits.BatchSize + tempNNCSpec.RequestedIPCount += batchSize if tempNNCSpec.RequestedIPCount > maxIPCount { // We don't want to ask for more ips than the max logger.Printf("[ipam-pool-monitor] Requested IP count (%v) is over max limit (%v), requesting max limit instead.", tempNNCSpec.RequestedIPCount, maxIPCount) @@ -165,10 +169,11 @@ func (pm *CNSIPAMPoolMonitor) decreasePoolSize(existingPendingReleaseIPCount int // Ensure the updated requested IP count is a multiple of the batch size previouslyRequestedIPCount := pm.cachedNNC.Spec.RequestedIPCount - modResult := previouslyRequestedIPCount % pm.scalarUnits.BatchSize + batchSize := pm.getBatchSize() + modResult := previouslyRequestedIPCount % batchSize logger.Printf("[ipam-pool-monitor] Previously RequestedIP Count %v", previouslyRequestedIPCount) - logger.Printf("[ipam-pool-monitor] Batch size : %v", pm.scalarUnits.BatchSize) + logger.Printf("[ipam-pool-monitor] Batch size : %v", batchSize) logger.Printf("[ipam-pool-monitor] modResult of (previously requested IP count mod batch size) = %v", modResult) if modResult != 0 { @@ -177,7 +182,7 @@ func (pm *CNSIPAMPoolMonitor) decreasePoolSize(existingPendingReleaseIPCount int updatedRequestedIPCount = previouslyRequestedIPCount - modResult } else { // Example: previouscount = 30, batchsize = 10, 30 - 10 = 20 which is multiple of batchsize (10) so all good - updatedRequestedIPCount = previouslyRequestedIPCount - pm.scalarUnits.BatchSize + updatedRequestedIPCount = previouslyRequestedIPCount - batchSize } decreaseIPCountBy = previouslyRequestedIPCount - updatedRequestedIPCount @@ -287,8 +292,8 @@ func (pm *CNSIPAMPoolMonitor) Update(scalar nnc.Scaler, spec nnc.NodeNetworkConf pm.scalarUnits = scalar - pm.MinimumFreeIps = int64(float64(pm.scalarUnits.BatchSize) * (float64(pm.scalarUnits.RequestThresholdPercent) / 100)) - pm.MaximumFreeIps = int64(float64(pm.scalarUnits.BatchSize) * (float64(pm.scalarUnits.ReleaseThresholdPercent) / 100)) + pm.MinimumFreeIps = int64(float64(pm.getBatchSize()) * (float64(pm.scalarUnits.RequestThresholdPercent) / 100)) + pm.MaximumFreeIps = int64(float64(pm.getBatchSize()) * (float64(pm.scalarUnits.ReleaseThresholdPercent) / 100)) pm.cachedNNC.Spec = spec @@ -305,6 +310,14 @@ func (pm *CNSIPAMPoolMonitor) getMaxIPCount() int64 { return pm.scalarUnits.MaxIPCount } +func (pm *CNSIPAMPoolMonitor) getBatchSize() int64 { + maxIPCount := pm.getMaxIPCount() + if pm.scalarUnits.BatchSize > maxIPCount { + pm.scalarUnits.BatchSize = maxIPCount + } + return pm.scalarUnits.BatchSize +} + //this function sets the values for state in IPAMPoolMonitor Struct func (pm *CNSIPAMPoolMonitor) GetStateSnapshot() cns.IpamPoolMonitorStateSnapshot { pm.mu.Lock() diff --git a/cns/ipampoolmonitor/ipampoolmonitor_test.go b/cns/ipampoolmonitor/ipampoolmonitor_test.go index 697975047c..285bd3766e 100644 --- a/cns/ipampoolmonitor/ipampoolmonitor_test.go +++ b/cns/ipampoolmonitor/ipampoolmonitor_test.go @@ -531,3 +531,46 @@ func TestDecreaseAfterNodeLimitReached(t *testing.T) { t.Fatalf("Expected to decrease requested IPs by %v (max pod count mod batchsize) to make the requested ip count a multiple of the batch size in the case of hitting the max before scale down, but got %v", expectedDecreaseIP, len(poolmonitor.cachedNNC.Spec.IPsNotInUse)) } } + +func TestPoolDecreaseBatchSizeGreaterThanMaxPodIPCount(t *testing.T) { + var ( + batchSize = 31 + initialIPConfigCount = 30 + requestThresholdPercent = 50 + releaseThresholdPercent = 150 + maxPodIPCount = int64(30) + ) + + fakecns, _, poolmonitor := initFakes(batchSize, initialIPConfigCount, requestThresholdPercent, releaseThresholdPercent, maxPodIPCount) + + t.Logf("Minimum free IPs to request: %v", poolmonitor.MinimumFreeIps) + t.Logf("Maximum free IPs to release: %v", poolmonitor.MaximumFreeIps) + + // increase number of allocated IP's in CNS + err := fakecns.SetNumberOfAllocatedIPs(30) + if err != nil { + t.Fatalf("Failed to allocate test ipconfigs with err: %v", err) + } + + // When poolmonitor reconcile is called, trigger increase and cache goal state + err = poolmonitor.Reconcile() + if err != nil { + t.Fatalf("Failed to allocate test ipconfigs with err: %v", err) + } + + // Trigger a batch release + err = fakecns.SetNumberOfAllocatedIPs(1) + if err != nil { + t.Error(err) + } + + err = poolmonitor.Reconcile() + if err != nil { + t.Errorf("Expected pool monitor to not fail after CNS set number of allocated IP's %v", err) + } + + // ensure pool monitor has only requested the max pod ip count + if poolmonitor.cachedNNC.Spec.RequestedIPCount != maxPodIPCount { + t.Fatalf("Pool monitor target IP count (%v) should be the node limit (%v) when the max has been reached", poolmonitor.cachedNNC.Spec.RequestedIPCount, maxPodIPCount) + } +} \ No newline at end of file