Fix CAPI provider scale-down when current replicas exceed maxSize

cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured.go

@@ -116,13 +116,23 @@ func (r *unstructuredScalableResource) Replicas() (int, error) {
 }
 
 func (r *unstructuredScalableResource) SetSize(nreplicas int) error {
-	switch {
-	case nreplicas > r.maxSize:
-		return fmt.Errorf("size increase too large - desired:%d max:%d", nreplicas, r.maxSize)
-	case nreplicas < r.minSize:
+	if nreplicas < r.minSize {
 		return fmt.Errorf("size decrease too large - desired:%d min:%d", nreplicas, r.minSize)
 	}
 
+	currentSize, err := r.Replicas()
+	if err != nil {
+		return err
+	}
+
+	// Only enforce maxSize when scaling up. When scaling down (e.g. current
+	// replicas already exceed maxSize because the user lowered the annotation),
+	// we must allow the decrease so the autoscaler can converge toward the
+	// desired range.
+	if nreplicas > r.maxSize && nreplicas > currentSize {
+		return fmt.Errorf("size increase too large - desired:%d max:%d", nreplicas, r.maxSize)
+	}
+
 	gvr, err := r.GroupVersionResource()
 	if err != nil {
 		return err

cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured_test.go

@@ -145,6 +145,69 @@ func TestSetSize(t *testing.T) {
 			Build()
 		test(t, testConfig)
 	})
+
+	// Simulates the scenario where a user lowers the maxSize annotation
+	// below the current replica count. SetSize must allow decreasing
+	// replicas even though the target is still above maxSize.
+	scaleDownAboveMaxTest := func(t *testing.T, testConfig *TestConfig) {
+		controller := NewTestMachineController(t)
+		defer controller.Stop()
+		controller.AddTestConfigs(testConfig)
+
+		testResource := testConfig.machineSet
+		if testConfig.machineDeployment != nil {
+			testResource = testConfig.machineDeployment
+		}
+
+		sr, err := newUnstructuredScalableResource(controller.machineController, testResource)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		// Lower maxSize below current replicas
+		sr.maxSize = 2
+
+		// Scale down by 1: desired (3) is still above maxSize (2) but
+		// below current (4), so this must succeed.
+		err = sr.SetSize(3)
+		if err != nil {
+			t.Fatalf("expected SetSize to succeed when scaling down toward maxSize, got: %v", err)
+		}
+
+		// Scale down to maxSize must also succeed.
+		err = sr.SetSize(2)
+		if err != nil {
+			t.Fatalf("expected SetSize to succeed when scaling down to maxSize, got: %v", err)
+		}
+
+		// Scale up above maxSize must still fail.
+		err = sr.SetSize(3)
+		if err == nil {
+			t.Fatal("expected SetSize to fail when scaling up above maxSize")
+		}
+	}
+
+	scaleDownAboveMaxAnnotations := map[string]string{
+		nodeGroupMinSizeAnnotationKey: "0",
+		nodeGroupMaxSizeAnnotationKey: "10",
+	}
+	t.Run("ScaleDownWhenAboveMax/MachineSet", func(t *testing.T) {
+		testConfig := NewTestConfigBuilder().
+			ForMachineSet().
+			WithNodeCount(4).
+			WithAnnotations(scaleDownAboveMaxAnnotations).
+			Build()
+		scaleDownAboveMaxTest(t, testConfig)
+	})
+
+	t.Run("ScaleDownWhenAboveMax/MachineDeployment", func(t *testing.T) {
+		testConfig := NewTestConfigBuilder().
+			ForMachineDeployment().
+			WithNodeCount(4).
+			WithAnnotations(scaleDownAboveMaxAnnotations).
+			Build()
+		scaleDownAboveMaxTest(t, testConfig)
+	})
 }
 
 func TestReplicas(t *testing.T) {