Merge pull request #8412 from sbueringer/pr-fix-scale-to-0

Fix scale to 0 for Cluster API NodePool

Author: k8s-ci-robot

cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured.go

@@ -26,7 +26,6 @@ import (
 	"time"
 
 	"github.com/pkg/errors"
-	autoscalingv1 "k8s.io/api/autoscaling/v1"
 	apiv1 "k8s.io/api/core/v1"
 	corev1 "k8s.io/api/core/v1"
 	resourceapi "k8s.io/api/resource/v1beta1"
@@ -124,9 +123,9 @@ func (r unstructuredScalableResource) SetSize(nreplicas int) error {
 		return err
 	}
 
-	spec := autoscalingv1.Scale{
-		Spec: autoscalingv1.ScaleSpec{
-			Replicas: int32(nreplicas),
+	spec := autoscalingv1Scale{
+		Spec: autoscalingv1ScaleSpec{
+			Replicas: ptr.To(int32(nreplicas)),
 		},
 	}
 
@@ -144,6 +143,24 @@ func (r unstructuredScalableResource) SetSize(nreplicas int) error {
 	return updateErr
 }
 
+// scale is a version of the autoscalingv1.Scale struct that marshals correctly.
+// Specifically the Spec.Replicas field is *int32 instead of int32.
+// Accordingly Spec.Replicas = 0 is not omitted, which is important for autoscale to 0 to work.
+type autoscalingv1Scale struct {
+	// spec defines the behavior of the scale. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#spec-and-status.
+	// +optional
+	Spec autoscalingv1ScaleSpec `json:"spec,omitempty" protobuf:"bytes,2,opt,name=spec"`
+}
+
+type autoscalingv1ScaleSpec struct {
+	// replicas is the desired number of instances for the scaled object.
+	// +optional
+	// +k8s:optional
+	// +default=0
+	// +k8s:minimum=0
+	Replicas *int32 `json:"replicas,omitempty" protobuf:"varint,1,opt,name=replicas"`
+}
+
 func (r unstructuredScalableResource) UnmarkMachineForDeletion(machine *unstructured.Unstructured) error {
 	u, err := r.controller.managementClient.Resource(r.controller.machineResource).Namespace(machine.GetNamespace()).Get(context.TODO(), machine.GetName(), metav1.GetOptions{})
 	if err != nil {

cluster-autoscaler/cloudprovider/clusterapi/clusterapi_unstructured_test.go

@@ -41,6 +41,7 @@ const (
 func TestSetSize(t *testing.T) {
 	initialReplicas := 1
 	updatedReplicas := 5
+	finalReplicas := 0
 
 	test := func(t *testing.T, testConfig *testConfig) {
 		controller, stop := mustCreateTestController(t, testConfig)
@@ -61,6 +62,7 @@ func TestSetSize(t *testing.T) {
 			t.Fatal(err)
 		}
 
+		// First update to updatedReplicas
 		err = sr.SetSize(updatedReplicas)
 		if err != nil {
 			t.Fatal(err)
@@ -86,6 +88,33 @@ func TestSetSize(t *testing.T) {
 		if replicas != int64(updatedReplicas) {
 			t.Errorf("expected %v, got: %v", updatedReplicas, replicas)
 		}
+
+		// Second update to finalReplicas
+		err = sr.SetSize(finalReplicas)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		s, err = sr.controller.managementScaleClient.Scales(testResource.GetNamespace()).
+			Get(context.TODO(), gvr.GroupResource(), testResource.GetName(), metav1.GetOptions{})
+		if err != nil {
+			t.Fatalf("error getting scale subresource: %v", err)
+		}
+
+		if s.Spec.Replicas != int32(finalReplicas) {
+			t.Errorf("expected %v, got: %v", finalReplicas, s.Spec.Replicas)
+		}
+
+		replicas, found, err = unstructured.NestedInt64(sr.unstructured.Object, "spec", "replicas")
+		if err != nil {
+			t.Fatal(err)
+		}
+		if !found {
+			t.Fatal("replicas = 0")
+		}
+		if replicas != int64(finalReplicas) {
+			t.Errorf("expected %v, got: %v", finalReplicas, replicas)
+		}
 	}
 
 	t.Run("MachineSet", func(t *testing.T) {
@@ -94,7 +123,7 @@ func TestSetSize(t *testing.T) {
 			RandomString(6),
 			RandomString(6),
 			initialReplicas, map[string]string{
-				nodeGroupMinSizeAnnotationKey: "1",
+				nodeGroupMinSizeAnnotationKey: "0",
 				nodeGroupMaxSizeAnnotationKey: "10",
 			},
 			nil,
@@ -107,7 +136,7 @@ func TestSetSize(t *testing.T) {
 			RandomString(6),
 			RandomString(6),
 			initialReplicas, map[string]string{
-				nodeGroupMinSizeAnnotationKey: "1",
+				nodeGroupMinSizeAnnotationKey: "0",
 				nodeGroupMaxSizeAnnotationKey: "10",
 			},
 			nil,