From 30ebf5c7aca5cb7a9d1f9dab18021484638d6c61 Mon Sep 17 00:00:00 2001 From: Max Fedotov Date: Wed, 13 Mar 2024 16:02:47 +0100 Subject: [PATCH 1/2] [clusterapi] Do not skip nodegroups with minSize=maxSize --- .../cloudprovider/clusterapi/clusterapi_nodegroup.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup.go index d28dc0a14f2..9a1d5ab2c91 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup.go @@ -18,6 +18,7 @@ package clusterapi import ( "fmt" + "k8s.io/klog/v2" "math/rand" "github.com/pkg/errors" @@ -355,7 +356,12 @@ func newNodeGroupFromScalableResource(controller *machineController, unstructure } // Ensure the node group would have the capacity to scale - if scalableResource.MaxSize()-scalableResource.MinSize() < 1 { + // allow MinSize = 0 + // allow MaxSize = MinSize + // don't allow MaxSize < MinSize + // don't allow MaxSize = MinSize = 0 + if scalableResource.MaxSize()-scalableResource.MinSize() < 0 || scalableResource.MaxSize() == 0 { + klog.V(4).Infof("nodegroup %s has no scaling capacity, skipping", scalableResource.Name()) return nil, nil } From 6c65baa1c682cea3dab3998dfea3a939b73c01ef Mon Sep 17 00:00:00 2001 From: Max Fedotov Date: Wed, 13 Mar 2024 18:38:12 +0100 Subject: [PATCH 2/2] [clusterapi] Update tests for nodegroups with minSize=maxSize --- .../clusterapi/clusterapi_controller_test.go | 13 ++++++------- .../clusterapi/clusterapi_nodegroup_test.go | 12 ++++++++++++ 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go index 359111be6bb..fc89fdae577 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go @@ -1043,9 +1043,8 @@ func TestControllerNodeGroupForNodeWithPositiveScalingBounds(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - // We don't scale from 0 so nodes must belong to a - // nodegroup that has a scale size of at least 1. - if ng != nil { + // We allow scaling if minSize=maxSize + if ng == nil { t.Fatalf("unexpected nodegroup: %v", ng) } } @@ -1124,19 +1123,19 @@ func TestControllerNodeGroups(t *testing.T) { nodeGroupMaxSizeAnnotationKey: "1", } - // Test #5: machineset with no scaling bounds results in no nodegroups + // Test #5: 5 machineset with minSize=maxSize results in a five nodegroup machineSetConfigs = createMachineSetTestConfigs(namespace, clusterName, RandomString(6), 5, 1, annotations, nil) if err := addTestConfigs(t, controller, machineSetConfigs...); err != nil { t.Fatalf("unexpected error: %v", err) } - assertNodegroupLen(t, controller, 0) + assertNodegroupLen(t, controller, 5) - // Test #6: machinedeployment with no scaling bounds results in no nodegroups + // Test #6: add 2 machinedeployment with minSize=maxSize machineDeploymentConfigs = createMachineDeploymentTestConfigs(namespace, clusterName, RandomString(6), 2, 1, annotations, nil) if err := addTestConfigs(t, controller, machineDeploymentConfigs...); err != nil { t.Fatalf("unexpected error: %v", err) } - assertNodegroupLen(t, controller, 0) + assertNodegroupLen(t, controller, 7) annotations = map[string]string{ nodeGroupMinSizeAnnotationKey: "-1", diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup_test.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup_test.go index 16077e8366b..07929155224 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup_test.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup_test.go @@ -109,6 +109,18 @@ func TestNodeGroupNewNodeGroupConstructor(t *testing.T) { nodeCount: 5, errors: false, expectNil: true, + }, { + description: "no error and expect notNil: min=max=2", + annotations: map[string]string{ + nodeGroupMinSizeAnnotationKey: "2", + nodeGroupMaxSizeAnnotationKey: "2", + }, + nodeCount: 1, + minSize: 2, + maxSize: 2, + replicas: 1, + errors: false, + expectNil: false, }} newNodeGroup := func(controller *machineController, testConfig *testConfig) (*nodegroup, error) {