fix: stop scale loop for pause Scaledbject (kedacore#4253)

Signed-off-by: Tobo Atchou <[email protected]>

CHANGELOG.md

@@ -99,6 +99,7 @@ New deprecation(s):
 - **General**: Use default metrics provider from sigs.k8s.io/custom-metrics-apiserver ([#4473](https://github.com/kedacore/keda/pull/4473))
 - **General**: Refactor several functions for Status & Conditions handling into pkg util functions ([#2906](https://github.com/kedacore/keda/pull/2906))
 - **General**: Bump `kubernetes-sigs/controller-runtime` to v0.15.0 and code alignment ([#4582](https://github.com/kedacore/keda/pull/4582))
+- **General**: Stop logging errors for paused ScaledObject (with "autoscaling.keda.sh/paused-replicas" annotation) by skipping reconciliation loop for the object (stop the scale loop and delete the HPA) ([#4253](https://github.com/kedacore/keda/pull/4253))
 
 ## v2.10.1
 

controllers/keda/hpa.go

@@ -183,13 +183,21 @@ func (r *ScaledObjectReconciler) updateHPAIfNeeded(ctx context.Context, logger l
 
 // deleteAndCreateHpa delete old HPA and create new one
 func (r *ScaledObjectReconciler) renameHPA(ctx context.Context, logger logr.Logger, scaledObject *kedav1alpha1.ScaledObject, foundHpa *autoscalingv2.HorizontalPodAutoscaler, gvkr *kedav1alpha1.GroupVersionKindResource) error {
-	logger.Info("Deleting old HPA", "HPA.Namespace", scaledObject.Namespace, "HPA.Name", foundHpa.Name)
+	if err := r.deleteHPA(ctx, logger, scaledObject, foundHpa); err != nil {
+		return err
+	}
+	return r.createAndDeployNewHPA(ctx, logger, scaledObject, gvkr)
+}
+
+// deleteHpa delete existing HPA
+func (r *ScaledObjectReconciler) deleteHPA(ctx context.Context, logger logr.Logger, scaledObject *kedav1alpha1.ScaledObject, foundHpa *autoscalingv2.HorizontalPodAutoscaler) error {
+	logger.Info("Deleting existing HPA", "HPA.Namespace", scaledObject.Namespace, "HPA.Name", foundHpa.Name)
 	if err := r.Client.Delete(ctx, foundHpa); err != nil {
 		logger.Error(err, "Failed to delete old HPA", "HPA.Namespace", foundHpa.Namespace, "HPA.Name", foundHpa.Name)
 		return err
 	}
 
-	return r.createAndDeployNewHPA(ctx, logger, scaledObject, gvkr)
+	return nil
 }
 
 // getScaledObjectMetricSpecs returns MetricSpec for HPA, generater from Triggers defitinion in ScaledObject

controllers/keda/scaledobject_controller.go

@@ -198,6 +198,20 @@ func (r *ScaledObjectReconciler) Reconcile(ctx context.Context, req ctrl.Request
 
 // reconcileScaledObject implements reconciler logic for ScaledObject
 func (r *ScaledObjectReconciler) reconcileScaledObject(ctx context.Context, logger logr.Logger, scaledObject *kedav1alpha1.ScaledObject) (string, error) {
+	// Check the presence of "autoscaling.keda.sh/paused-replicas" annotation on the scaledObject (since the presence of this annotation will pause
+	// autoscaling no matter what number of replicas is provided), and if so, stop the scale loop and delete the HPA on the scaled object.
+	_, paused := scaledObject.GetAnnotations()[kedacontrollerutil.PausedReplicasAnnotation]
+	if paused {
+		logger.Info("ScaledObject is paused, so skipping the request.")
+		if err := r.stopScaleLoop(ctx, logger, scaledObject); err != nil {
+			return "Failed to stop the scale loop for paused ScaledObject", err
+		}
+		if deleted, err := r.ensureHPAForScaledObjectIsDeleted(ctx, logger, scaledObject); !deleted {
+			return "Failed to delete HPA for paused ScaledObject", err
+		}
+		return kedav1alpha1.ScaledObjectConditionReadySuccessMessage, nil
+	}
+
 	// Check scale target Name is specified
 	if scaledObject.Spec.ScaleTargetRef.Name == "" {
 		err := fmt.Errorf("ScaledObject.spec.scaleTargetRef.name is missing")
@@ -438,6 +452,31 @@ func (r *ScaledObjectReconciler) ensureHPAForScaledObjectExists(ctx context.Cont
 	return false, nil
 }
 
+// ensureHPAForScaledObjectIsDeleted ensures that in cluster any HPA for specified ScaledObject is deleted, returns true if no HPA exists
+func (r *ScaledObjectReconciler) ensureHPAForScaledObjectIsDeleted(ctx context.Context, logger logr.Logger, scaledObject *kedav1alpha1.ScaledObject) (bool, error) {
+	var hpaName string
+	if scaledObject.Status.HpaName != "" {
+		hpaName = scaledObject.Status.HpaName
+	} else {
+		hpaName = getHPAName(scaledObject)
+	}
+	foundHpa := &autoscalingv2.HorizontalPodAutoscaler{}
+	// Check if HPA for this ScaledObject already exists
+	err := r.Client.Get(ctx, types.NamespacedName{Name: hpaName, Namespace: scaledObject.Namespace}, foundHpa)
+	if err != nil && errors.IsNotFound(err) {
+		return true, nil
+	} else if err != nil {
+		logger.Error(err, "Failed to get HPA from cluster")
+		return false, err
+	}
+
+	if err := r.deleteHPA(ctx, logger, scaledObject, foundHpa); err != nil {
+		logger.Error(err, "Failed to delete HPA from cluster")
+		return false, err
+	}
+	return true, nil
+}
+
 func isHpaRenamed(scaledObject *kedav1alpha1.ScaledObject, foundHpa *autoscalingv2.HorizontalPodAutoscaler) bool {
 	// if HPA name defined in SO -> check if equals to the found HPA
 	if scaledObject.Spec.Advanced != nil && scaledObject.Spec.Advanced.HorizontalPodAutoscalerConfig != nil && scaledObject.Spec.Advanced.HorizontalPodAutoscalerConfig.Name != "" {

controllers/keda/scaledobject_controller_test.go

@@ -35,6 +35,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 
 	kedav1alpha1 "github.com/kedacore/keda/v2/apis/keda/v1alpha1"
+	kedacontrollerutil "github.com/kedacore/keda/v2/controllers/keda/util"
 	"github.com/kedacore/keda/v2/pkg/mock/mock_client"
 	"github.com/kedacore/keda/v2/pkg/mock/mock_scaling"
 	"github.com/kedacore/keda/v2/pkg/scalers"
@@ -839,7 +840,69 @@ var _ = Describe("ScaledObjectController", func() {
 				return -1
 			}
 			return len(hpa.Spec.Metrics)
-		}, 5*time.Second).Should(Equal(2))
+		}, 1*time.Minute).Should(Equal(2))
+	})
+
+	// Fix issue 4253
+	It("deletes hpa when scaledobject has pause annotation", func() {
+		// Create the scaling target.
+		deploymentName := "to-be-paused-name"
+		soName := "so-" + deploymentName
+		err := k8sClient.Create(context.Background(), generateDeployment(deploymentName))
+		Expect(err).ToNot(HaveOccurred())
+
+		// Create the ScaledObject without specifying name.
+		so := &kedav1alpha1.ScaledObject{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      soName,
+				Namespace: "default",
+				Annotations: map[string]string{
+					kedacontrollerutil.PausedReplicasAnnotation: "0",
+				},
+			},
+			Spec: kedav1alpha1.ScaledObjectSpec{
+				ScaleTargetRef: &kedav1alpha1.ScaleTarget{
+					Name: deploymentName,
+				},
+				Advanced: &kedav1alpha1.AdvancedConfig{
+					HorizontalPodAutoscalerConfig: &kedav1alpha1.HorizontalPodAutoscalerConfig{},
+				},
+				Triggers: []kedav1alpha1.ScaleTriggers{
+					{
+						Type: "cron",
+						Metadata: map[string]string{
+							"timezone":        "UTC",
+							"start":           "0 * * * *",
+							"end":             "1 * * * *",
+							"desiredReplicas": "1",
+						},
+					},
+				},
+			},
+		}
+		err = k8sClient.Create(context.Background(), so)
+		Expect(err).ToNot(HaveOccurred())
+
+		// wait so's ready condition Ready
+		Eventually(func() metav1.ConditionStatus {
+			err := k8sClient.Get(context.Background(), types.NamespacedName{Name: soName, Namespace: "default"}, so)
+			if err != nil {
+				return metav1.ConditionUnknown
+			}
+			return so.Status.Conditions.GetReadyCondition().Status
+		}).Should(Equal(metav1.ConditionTrue))
+
+		// validate annotation is set correctly
+		err = k8sClient.Get(context.Background(), types.NamespacedName{Name: soName, Namespace: "default"}, so)
+		Expect(err).ToNot(HaveOccurred())
+		_, paused := so.GetAnnotations()[kedacontrollerutil.PausedReplicasAnnotation]
+		Expect(paused).To(Equal(true))
+
+		// And validate that hpa is deleted.
+		hpa := &autoscalingv2.HorizontalPodAutoscaler{}
+		Eventually(func() error {
+			return k8sClient.Get(context.Background(), types.NamespacedName{Name: fmt.Sprintf("keda-hpa-%s", soName), Namespace: "default"}, hpa)
+		}).Should(HaveOccurred())
 	})
 })
 

tests/internals/pause_scaling/pause_scaling_test.go

@@ -26,6 +26,9 @@ var (
 	scaledObjectName        = fmt.Sprintf("%s-so", testName)
 	maxReplicaCount         = 1
 	minReplicaCount         = 0
+	testScaleOutWaitMin     = 3
+	testPauseAtNWaitMin     = 3
+	testScaleInWaitMin      = 3
 )
 
 type templateData struct {
@@ -175,8 +178,8 @@ func testScaleOut(t *testing.T, kc *kubernetes.Clientset, data templateData) {
 	t.Log("--- testing scale out ---")
 	KubectlApplyWithTemplate(t, data, "scaledObjectTemplate", scaledObjectTemplate)
 
-	assert.True(t, WaitForDeploymentReplicaReadyCount(t, kc, deploymentName, testNamespace, maxReplicaCount, 60, 1),
-		"replica count should be 1 after 1 minute")
+	assert.Truef(t, WaitForDeploymentReplicaReadyCount(t, kc, deploymentName, testNamespace, maxReplicaCount, 60, testScaleOutWaitMin),
+		"replica count should be 1 after %d minute(s)", testScaleOutWaitMin)
 }
 
 func testPauseAtN(t *testing.T, kc *kubernetes.Clientset, data templateData, n int) {
@@ -186,14 +189,14 @@ func testPauseAtN(t *testing.T, kc *kubernetes.Clientset, data templateData, n i
 
 	KubernetesScaleDeployment(t, kc, monitoredDeploymentName, 0, testNamespace)
 
-	assert.Truef(t, WaitForDeploymentReplicaReadyCount(t, kc, deploymentName, testNamespace, n, 60, 1),
-		"replica count should be %d after 1 minute", n)
+	assert.Truef(t, WaitForDeploymentReplicaReadyCount(t, kc, deploymentName, testNamespace, n, 60, testPauseAtNWaitMin),
+		"replica count should be %d after %d minute(s)", n, testPauseAtNWaitMin)
 }
 
 func testScaleIn(t *testing.T, kc *kubernetes.Clientset, data templateData) {
 	t.Log("--- testing scale in ---")
 	KubectlApplyWithTemplate(t, data, "scaledObjectTemplate", scaledObjectTemplate)
 
-	assert.True(t, WaitForDeploymentReplicaReadyCount(t, kc, deploymentName, testNamespace, minReplicaCount, 60, 2),
-		"replica count should be 0 after 2 minutes")
+	assert.Truef(t, WaitForDeploymentReplicaReadyCount(t, kc, deploymentName, testNamespace, minReplicaCount, 60, testScaleInWaitMin),
+		"replica count should be 0 after %d minutes", testScaleInWaitMin)
 }