MachinePool annotation for externally managed autoscaler

api/v1beta1/common_types.go

@@ -126,6 +126,12 @@ const (
 	// any changes to the actual object because it is a dry run) and the topology controller
 	// will receive the resulting object.
 	TopologyDryRunAnnotation = "topology.cluster.x-k8s.io/dry-run"
+
+	// ReplicasManagedByExternalAutoscalerAnnotation is an annotation that indicates an external autoscaler manages infra scaling.
+	// The practical effect of this is that the capi "replica" count should be passively derived from the number of observed infra machines,
+	// instead of being a source of truth for eventual consistency.
+	// This annotation can be used to inform MachinePool status during in-progress scaling scenarios.
+	ReplicasManagedByExternalAutoscalerAnnotation = "cluster.x-k8s.io/replicas-managed-by-external-autoscaler"
 )
 
 const (

cmd/clusterctl/client/tree/annotations.go

@@ -58,7 +58,7 @@ const (
 
 // GetMetaName returns the object meta name that should be used for the object in the presentation layer, if defined.
 func GetMetaName(obj client.Object) string {
-	if val, ok := getAnnotation(obj, ObjectMetaNameAnnotation); ok {
+	if val, ok := getAnnotationValue(obj, ObjectMetaNameAnnotation); ok {
 		return val
 	}
 	return ""
@@ -85,15 +85,15 @@ func IsGroupObject(obj client.Object) bool {
 
 // GetGroupItems returns the list of names for the objects included in a group object.
 func GetGroupItems(obj client.Object) string {
-	if val, ok := getAnnotation(obj, GroupItemsAnnotation); ok {
+	if val, ok := getAnnotationValue(obj, GroupItemsAnnotation); ok {
 		return val
 	}
 	return ""
 }
 
 // GetZOrder return the zOrder of the object. Objects with no zOrder have a default zOrder of 0.
 func GetZOrder(obj client.Object) int {
-	if val, ok := getAnnotation(obj, ObjectZOrderAnnotation); ok {
+	if val, ok := getAnnotationValue(obj, ObjectZOrderAnnotation); ok {
 		if zOrder, err := strconv.ParseInt(val, 10, 0); err == nil {
 			return int(zOrder)
 		}
@@ -118,7 +118,19 @@ func IsShowConditionsObject(obj client.Object) bool {
 	return false
 }
 
-func getAnnotation(obj client.Object, annotation string) (string, bool) {
+// HasTruthyAnnotation evaluates truthiness permissively, only return false if
+// 1) the annotation isn't present at all
+// 2) the annotation is explicitly set to "false".
+// All other value permutations are ignored and considered valid.
+// tl;dr We're looking for boolean-type annotations but respecting that some folks might want to explicitly indicate "false".
+func HasTruthyAnnotation(obj client.Object, annotation string) bool {
+	if val, ok := getAnnotationValue(obj, annotation); ok {
+		return val != "false"
+	}
+	return false
+}
+
+func getAnnotationValue(obj client.Object, annotation string) (string, bool) {
 	if obj == nil {
 		return "", false
 	}
@@ -127,7 +139,7 @@ func getAnnotation(obj client.Object, annotation string) (string, bool) {
 }
 
 func getBoolAnnotation(obj client.Object, annotation string) (bool, bool) {
-	val, ok := getAnnotation(obj, annotation)
+	val, ok := getAnnotationValue(obj, annotation)
 	if ok {
 		if boolVal, err := strconv.ParseBool(val); err == nil {
 			return boolVal, true

cmd/clusterctl/client/tree/annotations_test.go

@@ -0,0 +1,87 @@
+/*
+Copyright 2022 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package tree
+
+import (
+	"testing"
+
+	. "github.com/onsi/gomega"
+)
+
+type testObj struct {
+}
+
+func TestHasTruthyAnnotation(t *testing.T) {
+	parent := fakeCluster("parent")
+	obj := fakeMachine("my-machine")
+	type args struct {
+		treeOptions ObjectTreeOptions
+	}
+	tests := []struct {
+		name          string
+		annotationKey string
+		annotationVal string
+		expected      bool
+	}{
+		{
+			name:          "no val",
+			annotationKey: "cluster.x-k8s.io/replicas-managed-by-autoscaler",
+			annotationVal: "",
+			expected:      true,
+		},
+		{
+			name:          "no val",
+			annotationKey: "cluster.x-k8s.io/replicas-managed-by-autoscaler",
+			annotationVal: "true",
+			expected:      true,
+		},
+		{
+			name:          "no val",
+			annotationKey: "cluster.x-k8s.io/replicas-managed-by-autoscaler",
+			annotationVal: "foo",
+			expected:      true,
+		},
+		{
+			name:          "no val",
+			annotationKey: "cluster.x-k8s.io/replicas-managed-by-autoscaler",
+			annotationVal: "false",
+			expected:      false,
+		},
+	}
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			root := parent.DeepCopy()
+			tree := NewObjectTree(root, ObjectTreeOptions{ShowOtherConditions: "all"})
+			g := NewWithT(t)
+			getAdded, gotVisible := tree.Add(root, obj.DeepCopy())
+			g.Expect(getAdded).To(BeTrue())
+			g.Expect(gotVisible).To(BeTrue())
+			gotObj := tree.GetObject("my-machine")
+			g.Expect(gotObj).ToNot(BeNil())
+			gotObj.SetAnnotations(map[string]string{
+				tt.annotationKey: tt.annotationVal,
+			})
+			ret := HasTruthyAnnotation(gotObj, tt.annotationKey)
+			if tt.expected {
+				g.Expect(ret).To(BeTrue())
+			} else {
+				g.Expect(ret).To(BeFalse())
+			}
+		})
+	}
+}

exp/internal/controllers/machinepool_controller_phases.go

@@ -33,6 +33,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/source"
 
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
+	"sigs.k8s.io/cluster-api/cmd/clusterctl/client/tree"
 	"sigs.k8s.io/cluster-api/controllers/external"
 	capierrors "sigs.k8s.io/cluster-api/errors"
 	expv1 "sigs.k8s.io/cluster-api/exp/api/v1beta1"
@@ -67,14 +68,26 @@ func (r *MachinePoolReconciler) reconcilePhase(mp *expv1.MachinePool) {
 		mp.Status.SetTypedPhase(expv1.MachinePoolPhaseRunning)
 	}
 
-	// Set the phase to "scalingUp" if the infrastructure is scaling up.
+	// Set the appropriate phase in response to the MachinePool replica count being greater than the observed infrastructure replicas.
 	if mp.Status.InfrastructureReady && *mp.Spec.Replicas > mp.Status.ReadyReplicas {
-		mp.Status.SetTypedPhase(expv1.MachinePoolPhaseScalingUp)
+		// If we are being managed by an external autoscaler and can't predict scaling direction, set to "Pending".
+		if tree.HasTruthyAnnotation(mp, clusterv1.ReplicasManagedByExternalAutoscalerAnnotation) {
+			mp.Status.SetTypedPhase(expv1.MachinePoolPhasePending)
+		} else {
+			// Set the phase to "ScalingUp" if we are actively scaling the infrastructure out.
+			mp.Status.SetTypedPhase(expv1.MachinePoolPhaseScalingUp)
+		}
 	}
 
-	// Set the phase to "scalingDown" if the infrastructure is scaling down.
+	// Set the appropriate phase in response to the MachinePool replica count being less than the observed infrastructure replicas.
 	if mp.Status.InfrastructureReady && *mp.Spec.Replicas < mp.Status.ReadyReplicas {
-		mp.Status.SetTypedPhase(expv1.MachinePoolPhaseScalingDown)
+		// If we are being managed by an external autoscaler and can't predict scaling direction, set to "Pending".
+		if tree.HasTruthyAnnotation(mp, clusterv1.ReplicasManagedByExternalAutoscalerAnnotation) {
+			mp.Status.SetTypedPhase(expv1.MachinePoolPhasePending)
+		} else {
+			// Set the phase to "ScalingDown" if we are actively scaling the infrastructure in.
+			mp.Status.SetTypedPhase(expv1.MachinePoolPhaseScalingDown)
+		}
 	}
 
 	// Set the phase to "failed" if any of Status.FailureReason or Status.FailureMessage is not-nil.