From 12b681c0cfc55fde4f1e550f965ba10e4cc1397b Mon Sep 17 00:00:00 2001 From: aviadlevy Date: Wed, 25 May 2022 10:56:03 +0300 Subject: [PATCH 01/15] Add support for custom hpa name Signed-off-by: aviadlevy --- apis/keda/v1alpha1/scaledobject_types.go | 4 ++ controllers/keda/hpa.go | 30 +++++++++- controllers/keda/scaledobject_controller.go | 7 ++- .../keda/scaledobject_controller_test.go | 60 +++++++++++++++++++ 4 files changed, 97 insertions(+), 4 deletions(-) diff --git a/apis/keda/v1alpha1/scaledobject_types.go b/apis/keda/v1alpha1/scaledobject_types.go index abfe2f4f981..c6d41bf1d86 100644 --- a/apis/keda/v1alpha1/scaledobject_types.go +++ b/apis/keda/v1alpha1/scaledobject_types.go @@ -104,6 +104,8 @@ type AdvancedConfig struct { type HorizontalPodAutoscalerConfig struct { // +optional Behavior *autoscalingv2beta2.HorizontalPodAutoscalerBehavior `json:"behavior,omitempty"` + // +optional + Name string `json:"name,omitempty"` } // ScaleTarget holds the a reference to the scale target Object @@ -152,6 +154,8 @@ type ScaledObjectStatus struct { Health map[string]HealthStatus `json:"health,omitempty"` // +optional PausedReplicaCount *int32 `json:"pausedReplicaCount,omitempty"` + // +optional + CurrentHpaName string `json:"currentHpaName,omitempty"` } // +kubebuilder:object:root=true diff --git a/controllers/keda/hpa.go b/controllers/keda/hpa.go index 353df2c0044..815244fa092 100644 --- a/controllers/keda/hpa.go +++ b/controllers/keda/hpa.go @@ -107,6 +107,8 @@ func (r *ScaledObjectReconciler) newHPAForScaledObject(ctx context.Context, logg maxReplicas = *pausedCount } + hpaName := getHPAName(scaledObject) + hpa := &autoscalingv2beta2.HorizontalPodAutoscaler{ Spec: autoscalingv2beta2.HorizontalPodAutoscalerSpec{ MinReplicas: minReplicas, @@ -119,7 +121,7 @@ func (r *ScaledObjectReconciler) newHPAForScaledObject(ctx context.Context, logg APIVersion: gvkr.GroupVersion().String(), }}, ObjectMeta: metav1.ObjectMeta{ - Name: getHPAName(scaledObject), + Name: hpaName, Namespace: scaledObject.Namespace, Labels: labels, Annotations: scaledObject.Annotations, @@ -134,6 +136,16 @@ func (r *ScaledObjectReconciler) newHPAForScaledObject(ctx context.Context, logg return nil, err } + // store hpaName in the ScaledObject + status := scaledObject.Status.DeepCopy() + status.CurrentHpaName = hpaName + + err = kedacontrollerutil.UpdateScaledObjectStatus(ctx, r.Client, logger, scaledObject, status) + if err != nil { + logger.Error(err, "Error updating scaledObject status with used hpaName") + return nil, err + } + return hpa, nil } @@ -145,9 +157,18 @@ func (r *ScaledObjectReconciler) updateHPAIfNeeded(ctx context.Context, logger l return err } + // check if hpa name is updated, and if so we need to delete the old hpa before creating new one + if hpa.Name != foundHpa.Name { + logger.V(1).Info("Found difference in the HPA name according to ScaledObject", "currentHPA", foundHpa.ObjectMeta, "newHPA", hpa.ObjectMeta) + 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 + } + } + // DeepDerivative ignores extra entries in arrays which makes removing the last trigger not update things, so trigger and update any time the metrics count is different. if len(hpa.Spec.Metrics) != len(foundHpa.Spec.Metrics) || !equality.Semantic.DeepDerivative(hpa.Spec, foundHpa.Spec) { - logger.V(1).Info("Found difference in the HPA spec accordint to ScaledObject", "currentHPA", foundHpa.Spec, "newHPA", hpa.Spec) + logger.V(1).Info("Found difference in the HPA spec according to ScaledObject", "currentHPA", foundHpa.Spec, "newHPA", hpa.Spec) if err = r.Client.Update(ctx, hpa); err != nil { foundHpa.Spec = hpa.Spec logger.Error(err, "Failed to update HPA", "HPA.Namespace", foundHpa.Namespace, "HPA.Name", foundHpa.Name) @@ -160,7 +181,7 @@ func (r *ScaledObjectReconciler) updateHPAIfNeeded(ctx context.Context, logger l } if !equality.Semantic.DeepDerivative(hpa.ObjectMeta.Labels, foundHpa.ObjectMeta.Labels) { - logger.V(1).Info("Found difference in the HPA labels accordint to ScaledObject", "currentHPA", foundHpa.ObjectMeta.Labels, "newHPA", hpa.ObjectMeta.Labels) + logger.V(1).Info("Found difference in the HPA labels according to ScaledObject", "currentHPA", foundHpa.ObjectMeta.Labels, "newHPA", hpa.ObjectMeta.Labels) if err = r.Client.Update(ctx, hpa); err != nil { foundHpa.ObjectMeta.Labels = hpa.ObjectMeta.Labels logger.Error(err, "Failed to update HPA", "HPA.Namespace", foundHpa.Namespace, "HPA.Name", foundHpa.Name) @@ -250,6 +271,9 @@ func (r *ScaledObjectReconciler) checkMinK8sVersionforHPABehavior(logger logr.Lo // getHPAName returns generated HPA name for ScaledObject specified in the parameter func getHPAName(scaledObject *kedav1alpha1.ScaledObject) string { + if scaledObject.Spec.Advanced != nil && scaledObject.Spec.Advanced.HorizontalPodAutoscalerConfig != nil && scaledObject.Spec.Advanced.HorizontalPodAutoscalerConfig.Name != "" { + return scaledObject.Spec.Advanced.HorizontalPodAutoscalerConfig.Name + } return fmt.Sprintf("keda-hpa-%s", scaledObject.Name) } diff --git a/controllers/keda/scaledobject_controller.go b/controllers/keda/scaledobject_controller.go index ffa6f5fabb6..5f85c2d317c 100644 --- a/controllers/keda/scaledobject_controller.go +++ b/controllers/keda/scaledobject_controller.go @@ -360,7 +360,12 @@ func (r *ScaledObjectReconciler) checkReplicaCountBoundsAreValid(scaledObject *k // ensureHPAForScaledObjectExists ensures that in cluster exist up-to-date HPA for specified ScaledObject, returns true if a new HPA was created func (r *ScaledObjectReconciler) ensureHPAForScaledObjectExists(ctx context.Context, logger logr.Logger, scaledObject *kedav1alpha1.ScaledObject, gvkr *kedav1alpha1.GroupVersionKindResource) (bool, error) { - hpaName := getHPAName(scaledObject) + var hpaName string + if scaledObject.Status.CurrentHpaName != "" { + hpaName = scaledObject.Status.CurrentHpaName + } else { + hpaName = getHPAName(scaledObject) + } foundHpa := &autoscalingv2beta2.HorizontalPodAutoscaler{} // Check if HPA for this ScaledObject already exists err := r.Client.Get(ctx, types.NamespacedName{Name: hpaName, Namespace: scaledObject.Namespace}, foundHpa) diff --git a/controllers/keda/scaledobject_controller_test.go b/controllers/keda/scaledobject_controller_test.go index 394a02fe962..09a0acf07b5 100644 --- a/controllers/keda/scaledobject_controller_test.go +++ b/controllers/keda/scaledobject_controller_test.go @@ -19,6 +19,7 @@ package keda import ( "context" "fmt" + "k8s.io/apimachinery/pkg/api/errors" "time" "github.com/golang/mock/gomock" @@ -292,6 +293,65 @@ var _ = Describe("ScaledObjectController", func() { Expect(hpa.Spec.Metrics[0].External.Metric.Name).To(Equal("s0-cron-UTC-0xxxx-1xxxx")) }) + It("cleans up old hpa when hpa name is updated", func() { + // Create the scaling target. + deploymentName := "changing-name" + soName := "so-" + deploymentName + err := k8sClient.Create(context.Background(), generateDeployment(deploymentName)) + Expect(err).ToNot(HaveOccurred()) + + // Create the ScaledObject with two triggers. + so := &kedav1alpha1.ScaledObject{ + ObjectMeta: metav1.ObjectMeta{Name: soName, Namespace: "default"}, + 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()) + + // Get and confirm the HPA. + hpa := &autoscalingv2beta2.HorizontalPodAutoscaler{} + Eventually(func() error { + return k8sClient.Get(context.Background(), types.NamespacedName{Name: fmt.Sprintf("keda-hpa-%s", soName), Namespace: "default"}, hpa) + }).ShouldNot(HaveOccurred()) + Expect(hpa.Name).To(Equal(fmt.Sprintf("keda-hpa-%s", soName))) + + // Update hpa name + Eventually(func() error { + err = k8sClient.Get(context.Background(), types.NamespacedName{Name: soName, Namespace: "default"}, so) + Expect(err).ToNot(HaveOccurred()) + so.Spec.Advanced.HorizontalPodAutoscalerConfig.Name = fmt.Sprintf("new-%s", soName) + return k8sClient.Update(context.Background(), so) + }).ShouldNot(HaveOccurred()) + + // Wait until the HPA is updated. + Eventually(func() error { + return k8sClient.Get(context.Background(), types.NamespacedName{Name: fmt.Sprintf("new-%s", soName), Namespace: "default"}, hpa) + }).ShouldNot(HaveOccurred()) + + // And validate that old hpa is deleted. + err = k8sClient.Get(context.Background(), types.NamespacedName{Name: fmt.Sprintf("keda-hpa-%s", soName), Namespace: "default"}, hpa) + Expect(err).Should(HaveOccurred()) + Expect(errors.IsNotFound(err)).To(Equal(true)) + }) + //https://github.com/kedacore/keda/issues/2407 It("cache is correctly recreated if SO is deleted and created", func() { // Create the scaling target. From 6c666dc4c0bad99ea4eb0c854ae0822b7024507d Mon Sep 17 00:00:00 2001 From: aviadlevy Date: Wed, 25 May 2022 11:02:13 +0300 Subject: [PATCH 02/15] Add Changelog entry Signed-off-by: aviadlevy --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e61dbfeedd..35207681a35 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,7 @@ To learn more about our roadmap, we recommend reading [this document](ROADMAP.md ### New - **General:** Support for Azure AD Workload Identity as a pod identity provider. ([2487](https://github.com/kedacore/keda/issues/2487)) +- **General:** Add support to custom hpa name ([3057](https://github.com/kedacore/keda/issues/3057)) ### Improvements From 68623bf7300637c096027d7012a77e1f22b24012 Mon Sep 17 00:00:00 2001 From: aviadlevy Date: Wed, 25 May 2022 15:33:57 +0300 Subject: [PATCH 03/15] fix import linting Signed-off-by: aviadlevy --- controllers/keda/scaledobject_controller_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controllers/keda/scaledobject_controller_test.go b/controllers/keda/scaledobject_controller_test.go index 09a0acf07b5..4cfcc34fd28 100644 --- a/controllers/keda/scaledobject_controller_test.go +++ b/controllers/keda/scaledobject_controller_test.go @@ -19,7 +19,6 @@ package keda import ( "context" "fmt" - "k8s.io/apimachinery/pkg/api/errors" "time" "github.com/golang/mock/gomock" @@ -28,6 +27,7 @@ import ( appsv1 "k8s.io/api/apps/v1" autoscalingv2beta2 "k8s.io/api/autoscaling/v2beta2" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/log/zap" From 2007bde58b9484871d0db81569d58e336ab0fde0 Mon Sep 17 00:00:00 2001 From: aviadlevy Date: Wed, 25 May 2022 16:01:22 +0300 Subject: [PATCH 04/15] edit small comment Signed-off-by: aviadlevy --- controllers/keda/scaledobject_controller_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controllers/keda/scaledobject_controller_test.go b/controllers/keda/scaledobject_controller_test.go index 4cfcc34fd28..fb374cc2268 100644 --- a/controllers/keda/scaledobject_controller_test.go +++ b/controllers/keda/scaledobject_controller_test.go @@ -300,7 +300,7 @@ var _ = Describe("ScaledObjectController", func() { err := k8sClient.Create(context.Background(), generateDeployment(deploymentName)) Expect(err).ToNot(HaveOccurred()) - // Create the ScaledObject with two triggers. + // Create the ScaledObject without specifying name. so := &kedav1alpha1.ScaledObject{ ObjectMeta: metav1.ObjectMeta{Name: soName, Namespace: "default"}, Spec: kedav1alpha1.ScaledObjectSpec{ From fe46bc526090198a61fbf60533f3677b61bca1a7 Mon Sep 17 00:00:00 2001 From: Aviad Levy Date: Wed, 25 May 2022 16:38:37 +0300 Subject: [PATCH 05/15] fix CR comment Co-authored-by: Tom Kerkhove Signed-off-by: Aviad Levy --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8fb96d8d3a3..30a7c91f4cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,7 +37,7 @@ To learn more about our roadmap, we recommend reading [this document](ROADMAP.md ### New - **General:** Support for Azure AD Workload Identity as a pod identity provider. ([2487](https://github.com/kedacore/keda/issues/2487)) -- **General:** Add support to custom hpa name ([3057](https://github.com/kedacore/keda/issues/3057)) +- **General:** Add support to customize HPA name ([3057](https://github.com/kedacore/keda/issues/3057)) ### Improvements From cfb0c3884c5b6a4ce24f17baff8dc99eda0176b1 Mon Sep 17 00:00:00 2001 From: aviadlevy Date: Thu, 26 May 2022 12:33:44 +0300 Subject: [PATCH 06/15] Change variable name Signed-off-by: aviadlevy --- apis/keda/v1alpha1/scaledobject_types.go | 2 +- controllers/keda/hpa.go | 2 +- controllers/keda/scaledobject_controller.go | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/apis/keda/v1alpha1/scaledobject_types.go b/apis/keda/v1alpha1/scaledobject_types.go index c6d41bf1d86..628d4d67785 100644 --- a/apis/keda/v1alpha1/scaledobject_types.go +++ b/apis/keda/v1alpha1/scaledobject_types.go @@ -155,7 +155,7 @@ type ScaledObjectStatus struct { // +optional PausedReplicaCount *int32 `json:"pausedReplicaCount,omitempty"` // +optional - CurrentHpaName string `json:"currentHpaName,omitempty"` + HpaName string `json:"hpaName,omitempty"` } // +kubebuilder:object:root=true diff --git a/controllers/keda/hpa.go b/controllers/keda/hpa.go index 815244fa092..45dd46a6d4b 100644 --- a/controllers/keda/hpa.go +++ b/controllers/keda/hpa.go @@ -138,7 +138,7 @@ func (r *ScaledObjectReconciler) newHPAForScaledObject(ctx context.Context, logg // store hpaName in the ScaledObject status := scaledObject.Status.DeepCopy() - status.CurrentHpaName = hpaName + status.HpaName = hpaName err = kedacontrollerutil.UpdateScaledObjectStatus(ctx, r.Client, logger, scaledObject, status) if err != nil { diff --git a/controllers/keda/scaledobject_controller.go b/controllers/keda/scaledobject_controller.go index 5f85c2d317c..ed37c50e9a0 100644 --- a/controllers/keda/scaledobject_controller.go +++ b/controllers/keda/scaledobject_controller.go @@ -361,8 +361,8 @@ func (r *ScaledObjectReconciler) checkReplicaCountBoundsAreValid(scaledObject *k // ensureHPAForScaledObjectExists ensures that in cluster exist up-to-date HPA for specified ScaledObject, returns true if a new HPA was created func (r *ScaledObjectReconciler) ensureHPAForScaledObjectExists(ctx context.Context, logger logr.Logger, scaledObject *kedav1alpha1.ScaledObject, gvkr *kedav1alpha1.GroupVersionKindResource) (bool, error) { var hpaName string - if scaledObject.Status.CurrentHpaName != "" { - hpaName = scaledObject.Status.CurrentHpaName + if scaledObject.Status.HpaName != "" { + hpaName = scaledObject.Status.HpaName } else { hpaName = getHPAName(scaledObject) } From 3b27a0a8ce934c257f7adb5999f3d8d51efcd748 Mon Sep 17 00:00:00 2001 From: aviadlevy Date: Thu, 26 May 2022 15:49:27 +0300 Subject: [PATCH 07/15] re-organize the flow Signed-off-by: aviadlevy --- controllers/keda/hpa.go | 52 +++++++++++---------- controllers/keda/scaledobject_controller.go | 19 ++++++++ 2 files changed, 47 insertions(+), 24 deletions(-) diff --git a/controllers/keda/hpa.go b/controllers/keda/hpa.go index 45dd46a6d4b..166eee55fd9 100644 --- a/controllers/keda/hpa.go +++ b/controllers/keda/hpa.go @@ -56,6 +56,16 @@ func (r *ScaledObjectReconciler) createAndDeployNewHPA(ctx context.Context, logg return err } + // store hpaName in the ScaledObject + status := scaledObject.Status.DeepCopy() + status.HpaName = hpaName + + err = kedacontrollerutil.UpdateScaledObjectStatus(ctx, r.Client, logger, scaledObject, status) + if err != nil { + logger.Error(err, "Error updating scaledObject status with used hpaName") + return err + } + return nil } @@ -107,8 +117,6 @@ func (r *ScaledObjectReconciler) newHPAForScaledObject(ctx context.Context, logg maxReplicas = *pausedCount } - hpaName := getHPAName(scaledObject) - hpa := &autoscalingv2beta2.HorizontalPodAutoscaler{ Spec: autoscalingv2beta2.HorizontalPodAutoscalerSpec{ MinReplicas: minReplicas, @@ -121,7 +129,7 @@ func (r *ScaledObjectReconciler) newHPAForScaledObject(ctx context.Context, logg APIVersion: gvkr.GroupVersion().String(), }}, ObjectMeta: metav1.ObjectMeta{ - Name: hpaName, + Name: getHPAName(scaledObject), Namespace: scaledObject.Namespace, Labels: labels, Annotations: scaledObject.Annotations, @@ -136,16 +144,6 @@ func (r *ScaledObjectReconciler) newHPAForScaledObject(ctx context.Context, logg return nil, err } - // store hpaName in the ScaledObject - status := scaledObject.Status.DeepCopy() - status.HpaName = hpaName - - err = kedacontrollerutil.UpdateScaledObjectStatus(ctx, r.Client, logger, scaledObject, status) - if err != nil { - logger.Error(err, "Error updating scaledObject status with used hpaName") - return nil, err - } - return hpa, nil } @@ -157,18 +155,9 @@ func (r *ScaledObjectReconciler) updateHPAIfNeeded(ctx context.Context, logger l return err } - // check if hpa name is updated, and if so we need to delete the old hpa before creating new one - if hpa.Name != foundHpa.Name { - logger.V(1).Info("Found difference in the HPA name according to ScaledObject", "currentHPA", foundHpa.ObjectMeta, "newHPA", hpa.ObjectMeta) - 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 - } - } - // DeepDerivative ignores extra entries in arrays which makes removing the last trigger not update things, so trigger and update any time the metrics count is different. if len(hpa.Spec.Metrics) != len(foundHpa.Spec.Metrics) || !equality.Semantic.DeepDerivative(hpa.Spec, foundHpa.Spec) { - logger.V(1).Info("Found difference in the HPA spec according to ScaledObject", "currentHPA", foundHpa.Spec, "newHPA", hpa.Spec) + logger.V(1).Info("Found difference in the HPA spec accordint to ScaledObject", "currentHPA", foundHpa.Spec, "newHPA", hpa.Spec) if err = r.Client.Update(ctx, hpa); err != nil { foundHpa.Spec = hpa.Spec logger.Error(err, "Failed to update HPA", "HPA.Namespace", foundHpa.Namespace, "HPA.Name", foundHpa.Name) @@ -181,7 +170,7 @@ func (r *ScaledObjectReconciler) updateHPAIfNeeded(ctx context.Context, logger l } if !equality.Semantic.DeepDerivative(hpa.ObjectMeta.Labels, foundHpa.ObjectMeta.Labels) { - logger.V(1).Info("Found difference in the HPA labels according to ScaledObject", "currentHPA", foundHpa.ObjectMeta.Labels, "newHPA", hpa.ObjectMeta.Labels) + logger.V(1).Info("Found difference in the HPA labels accordint to ScaledObject", "currentHPA", foundHpa.ObjectMeta.Labels, "newHPA", hpa.ObjectMeta.Labels) if err = r.Client.Update(ctx, hpa); err != nil { foundHpa.ObjectMeta.Labels = hpa.ObjectMeta.Labels logger.Error(err, "Failed to update HPA", "HPA.Namespace", foundHpa.Namespace, "HPA.Name", foundHpa.Name) @@ -193,6 +182,17 @@ func (r *ScaledObjectReconciler) updateHPAIfNeeded(ctx context.Context, logger l return nil } +// deleteAndCreateHpa delete old HPA and create new one +func (r *ScaledObjectReconciler) renameHPA(ctx context.Context, logger logr.Logger, scaledObject *kedav1alpha1.ScaledObject, foundHpa *autoscalingv2beta2.HorizontalPodAutoscaler, gvkr *kedav1alpha1.GroupVersionKindResource) error { + logger.Info("Deleting old 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) +} + // getScaledObjectMetricSpecs returns MetricSpec for HPA, generater from Triggers defitinion in ScaledObject func (r *ScaledObjectReconciler) getScaledObjectMetricSpecs(ctx context.Context, logger logr.Logger, scaledObject *kedav1alpha1.ScaledObject) ([]autoscalingv2beta2.MetricSpec, error) { var scaledObjectMetricSpecs []autoscalingv2beta2.MetricSpec @@ -274,6 +274,10 @@ func getHPAName(scaledObject *kedav1alpha1.ScaledObject) string { if scaledObject.Spec.Advanced != nil && scaledObject.Spec.Advanced.HorizontalPodAutoscalerConfig != nil && scaledObject.Spec.Advanced.HorizontalPodAutoscalerConfig.Name != "" { return scaledObject.Spec.Advanced.HorizontalPodAutoscalerConfig.Name } + return getDefaultHpaName(scaledObject) +} + +func getDefaultHpaName(scaledObject *kedav1alpha1.ScaledObject) string { return fmt.Sprintf("keda-hpa-%s", scaledObject.Name) } diff --git a/controllers/keda/scaledobject_controller.go b/controllers/keda/scaledobject_controller.go index ed37c50e9a0..5ca876d697a 100644 --- a/controllers/keda/scaledobject_controller.go +++ b/controllers/keda/scaledobject_controller.go @@ -386,6 +386,16 @@ func (r *ScaledObjectReconciler) ensureHPAForScaledObjectExists(ctx context.Cont return false, err } + // check if hpa name is changed, and if so we need to delete the old hpa before creating new one + if isHpaRenamed(scaledObject, foundHpa) { + err = r.renameHPA(ctx, logger, scaledObject, foundHpa, gvkr) + if err != nil { + return false, err + } + // new HPA created successfully -> notify Reconcile function so it could fire a new ScaleLoop + return true, nil + } + // HPA was found -> let's check if we need to update it err = r.updateHPAIfNeeded(ctx, logger, scaledObject, foundHpa, gvkr) if err != nil { @@ -396,6 +406,15 @@ func (r *ScaledObjectReconciler) ensureHPAForScaledObjectExists(ctx context.Cont return false, nil } +func isHpaRenamed(scaledObject *kedav1alpha1.ScaledObject, foundHpa *autoscalingv2beta2.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 { + return scaledObject.Spec.Advanced.HorizontalPodAutoscalerConfig.Name != foundHpa.Name + } + // if HPA name not defined in SO -> check if the found HPA is equals to the default + return foundHpa.Name != getDefaultHpaName(scaledObject) +} + // requestScaleLoop tries to start ScaleLoop handler for the respective ScaledObject func (r *ScaledObjectReconciler) requestScaleLoop(ctx context.Context, logger logr.Logger, scaledObject *kedav1alpha1.ScaledObject) error { logger.V(1).Info("Notify scaleHandler of an update in scaledObject") From a2ef4dd8d19368fde6da2c9bab2b885660a78e30 Mon Sep 17 00:00:00 2001 From: aviadlevy Date: Mon, 30 May 2022 14:34:45 +0300 Subject: [PATCH 08/15] make generate Signed-off-by: aviadlevy --- config/crd/bases/keda.sh_scaledobjects.yaml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/config/crd/bases/keda.sh_scaledobjects.yaml b/config/crd/bases/keda.sh_scaledobjects.yaml index 02c30732757..0d14533bf6a 100644 --- a/config/crd/bases/keda.sh_scaledobjects.yaml +++ b/config/crd/bases/keda.sh_scaledobjects.yaml @@ -1,4 +1,3 @@ - --- apiVersion: apiextensions.k8s.io/v1 kind: CustomResourceDefinition @@ -196,6 +195,8 @@ spec: type: integer type: object type: object + name: + type: string type: object restoreToOriginalReplicaCount: type: boolean @@ -327,6 +328,8 @@ spec: type: string type: object type: object + hpaName: + type: string lastActiveTime: format: date-time type: string From eba4dc2a39668dbdc8cc788080572aa560132c0a Mon Sep 17 00:00:00 2001 From: aviadlevy Date: Wed, 1 Jun 2022 11:04:53 +0300 Subject: [PATCH 09/15] verify that HPA name defined in SO Signed-off-by: aviadlevy --- controllers/keda/scaledobject_controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controllers/keda/scaledobject_controller.go b/controllers/keda/scaledobject_controller.go index 5ca876d697a..0961d8a8db7 100644 --- a/controllers/keda/scaledobject_controller.go +++ b/controllers/keda/scaledobject_controller.go @@ -408,7 +408,7 @@ func (r *ScaledObjectReconciler) ensureHPAForScaledObjectExists(ctx context.Cont func isHpaRenamed(scaledObject *kedav1alpha1.ScaledObject, foundHpa *autoscalingv2beta2.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 { + if scaledObject.Spec.Advanced != nil && scaledObject.Spec.Advanced.HorizontalPodAutoscalerConfig != nil && scaledObject.Spec.Advanced.HorizontalPodAutoscalerConfig.Name != "" { return scaledObject.Spec.Advanced.HorizontalPodAutoscalerConfig.Name != foundHpa.Name } // if HPA name not defined in SO -> check if the found HPA is equals to the default From 3ef9f29ca55d54ec238420566b15970caec90ded Mon Sep 17 00:00:00 2001 From: aviadlevy Date: Thu, 16 Jun 2022 18:02:53 +0300 Subject: [PATCH 10/15] Add e2e test Signed-off-by: aviadlevy --- .../custom_hpa_name/custom_hpa_name_test.go | 178 ++++++++++++++++++ 1 file changed, 178 insertions(+) create mode 100644 tests/scalers_go/custom_hpa_name/custom_hpa_name_test.go diff --git a/tests/scalers_go/custom_hpa_name/custom_hpa_name_test.go b/tests/scalers_go/custom_hpa_name/custom_hpa_name_test.go new file mode 100644 index 00000000000..ac1aa62fe2d --- /dev/null +++ b/tests/scalers_go/custom_hpa_name/custom_hpa_name_test.go @@ -0,0 +1,178 @@ +//go:build e2e +// +build e2e + +package custom_hpa_name + +import ( + "context" + "fmt" + "github.com/joho/godotenv" + . "github.com/kedacore/keda/v2/tests" + "github.com/stretchr/testify/assert" + autoscalingv2 "k8s.io/api/autoscaling/v2" + "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" + "testing" + "time" +) + +// Load environment variables from .env file +var _ = godotenv.Load("../../.env") + +type templateData struct { + TestNamespace string + DeploymentName string + ScaledObjectName string + CustomHpaName string +} + +const ( + deploymentTemplate = ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: {{.DeploymentName}} + namespace: {{.TestNamespace}} +spec: + selector: + matchLabels: + run: {{.DeploymentName}} + replicas: 1 + template: + metadata: + labels: + run: {{.DeploymentName}} + spec: + containers: + - name: {{.DeploymentName}} + image: k8s.gcr.io/hpa-example + ports: + - containerPort: 80 + resources: + limits: + cpu: 500m + requests: + cpu: 200m + imagePullPolicy: IfNotPresent +` + + scaledObjectTemplate = ` +apiVersion: keda.sh/v1alpha1 +kind: ScaledObject +metadata: + name: {{.ScaledObjectName}} + namespace: {{.TestNamespace}} +spec: + scaleTargetRef: + name: {{.DeploymentName}} + pollingInterval: 5 + minReplicaCount: 0 + maxReplicaCount: 1 + cooldownPeriod: 10 + triggers: + - type: cpu + metadata: + type: Utilization + value: "50" +` + + scaledObjectTemplateWithCustomName = ` +apiVersion: keda.sh/v1alpha1 +kind: ScaledObject +metadata: + name: {{.ScaledObjectName}} + namespace: {{.TestNamespace}} +spec: + scaleTargetRef: + name: {{.DeploymentName}} + minReplicaCount: 1 + maxReplicaCount: 1 + cooldownPeriod: 10 + advanced: + horizontalPodAutoscalerConfig: + name: {{.CustomHpaName}} + triggers: + - type: cpu + metadata: + type: Utilization + value: "50" +` +) + +func TestCustomToDefault(t *testing.T) { + // setup + testName := "custom-to-default-hpa-name" + scaledObjectName := fmt.Sprintf("%s-so", testName) + defaultHpaName := fmt.Sprintf("keda-hpa-%s", scaledObjectName) + customHpaName := fmt.Sprintf("%s-custom", testName) + test(t, testName, customHpaName, scaledObjectTemplateWithCustomName, "custom", + defaultHpaName, scaledObjectTemplate, "default") +} + +func TestDefaultToCustom(t *testing.T) { + // setup + testName := "default-to-custom-hpa-name" + scaledObjectName := fmt.Sprintf("%s-so", testName) + defaultHpaName := fmt.Sprintf("keda-hpa-%s", scaledObjectName) + customHpaName := fmt.Sprintf("%s-custom", testName) + test(t, testName, defaultHpaName, scaledObjectTemplate, "default", + customHpaName, scaledObjectTemplateWithCustomName, "custom") +} + +func test(t *testing.T, testName string, firstHpaName string, firstSOTemplate string, firstHpaDescription string, + secondHpaName string, secondSOTemplate string, secondHpaDescription string) { + testNamespace := fmt.Sprintf("%s-ns", testName) + deploymentName := fmt.Sprintf("%s-deployment", testName) + scaledObjectName := fmt.Sprintf("%s-so", testName) + customHpaName := fmt.Sprintf("%s-custom", testName) + // Create kubernetes resources + kc := GetKubernetesClient(t) + data := getTemplateData(testNamespace, deploymentName, scaledObjectName, customHpaName) + templates := []string{deploymentTemplate, firstSOTemplate} + + CreateKubernetesResources(t, kc, testNamespace, data, templates...) + + t.Logf("--- validate hpa is with %s name ---", firstHpaDescription) + hpa, _ := WaitForHpaCreation(t, kc, firstHpaName, testNamespace, 10, 1) + assert.Equal(t, firstHpaName, hpa.Name) + + t.Log("--- change hpa name ---") + templatesCustomName := []string{secondSOTemplate} + KubectlApplyMultipleWithTemplate(t, data, templatesCustomName...) + + t.Logf("--- validate new hpa is with %s name ---", secondHpaDescription) + hpa, _ = WaitForHpaCreation(t, kc, secondHpaName, testNamespace, 10, 1) + assert.Equal(t, secondHpaName, hpa.Name) + + t.Logf("--- validate hpa with %s name does not exists ---", firstHpaDescription) + _, err := WaitForHpaCreation(t, kc, firstHpaName, testNamespace, 10, 1) + assert.True(t, errors.IsNotFound(err)) + + // cleanup + DeleteKubernetesResources(t, kc, testNamespace, data, templates...) +} + +func getTemplateData(testNamespace string, deploymentName string, scaledObjectName string, customHpaName string) templateData { + return templateData{ + TestNamespace: testNamespace, + DeploymentName: deploymentName, + ScaledObjectName: scaledObjectName, + CustomHpaName: customHpaName, + } +} + +func WaitForHpaCreation(t *testing.T, kc *kubernetes.Clientset, name, namespace string, + iterations, intervalSeconds int) (*autoscalingv2.HorizontalPodAutoscaler, error) { + hpa := &autoscalingv2.HorizontalPodAutoscaler{} + var err error + for i := 0; i < iterations; i++ { + hpa, err = kc.AutoscalingV2().HorizontalPodAutoscalers(namespace).Get(context.Background(), name, metav1.GetOptions{}) + t.Log("Waiting for hpa creation") + if err == nil { + return hpa, err + } + time.Sleep(time.Duration(intervalSeconds) * time.Second) + } + return hpa, err +} From ed77aa8ff6a59b55f05ecea0729cdec7ef932e5f Mon Sep 17 00:00:00 2001 From: aviadlevy Date: Thu, 16 Jun 2022 19:43:49 +0300 Subject: [PATCH 11/15] e2e - increase iteration waiting to hpa Signed-off-by: aviadlevy --- tests/scalers_go/custom_hpa_name/custom_hpa_name_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/scalers_go/custom_hpa_name/custom_hpa_name_test.go b/tests/scalers_go/custom_hpa_name/custom_hpa_name_test.go index ac1aa62fe2d..160186460e1 100644 --- a/tests/scalers_go/custom_hpa_name/custom_hpa_name_test.go +++ b/tests/scalers_go/custom_hpa_name/custom_hpa_name_test.go @@ -134,7 +134,7 @@ func test(t *testing.T, testName string, firstHpaName string, firstSOTemplate st CreateKubernetesResources(t, kc, testNamespace, data, templates...) t.Logf("--- validate hpa is with %s name ---", firstHpaDescription) - hpa, _ := WaitForHpaCreation(t, kc, firstHpaName, testNamespace, 10, 1) + hpa, _ := WaitForHpaCreation(t, kc, firstHpaName, testNamespace, 60, 1) assert.Equal(t, firstHpaName, hpa.Name) t.Log("--- change hpa name ---") @@ -142,11 +142,11 @@ func test(t *testing.T, testName string, firstHpaName string, firstSOTemplate st KubectlApplyMultipleWithTemplate(t, data, templatesCustomName...) t.Logf("--- validate new hpa is with %s name ---", secondHpaDescription) - hpa, _ = WaitForHpaCreation(t, kc, secondHpaName, testNamespace, 10, 1) + hpa, _ = WaitForHpaCreation(t, kc, secondHpaName, testNamespace, 60, 1) assert.Equal(t, secondHpaName, hpa.Name) t.Logf("--- validate hpa with %s name does not exists ---", firstHpaDescription) - _, err := WaitForHpaCreation(t, kc, firstHpaName, testNamespace, 10, 1) + _, err := WaitForHpaCreation(t, kc, firstHpaName, testNamespace, 15, 1) assert.True(t, errors.IsNotFound(err)) // cleanup From 372e555b9e2279dcd7047870d7ef4bea4f0a252c Mon Sep 17 00:00:00 2001 From: aviadlevy Date: Thu, 16 Jun 2022 19:44:41 +0300 Subject: [PATCH 12/15] e2e - move func to `helper.go` Signed-off-by: aviadlevy --- tests/helper.go | 16 +++++++++++++++ .../custom_hpa_name/custom_hpa_name_test.go | 20 ------------------- 2 files changed, 16 insertions(+), 20 deletions(-) diff --git a/tests/helper.go b/tests/helper.go index f5ec2bcfda6..e546a9ca282 100644 --- a/tests/helper.go +++ b/tests/helper.go @@ -13,6 +13,7 @@ import ( "github.com/joho/godotenv" "github.com/stretchr/testify/assert" + autoscalingv2 "k8s.io/api/autoscaling/v2" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" @@ -135,6 +136,21 @@ func WaitForDeploymentReplicaCount(t *testing.T, kc *kubernetes.Clientset, name, return false } +func WaitForHpaCreation(t *testing.T, kc *kubernetes.Clientset, name, namespace string, + iterations, intervalSeconds int) (*autoscalingv2.HorizontalPodAutoscaler, error) { + hpa := &autoscalingv2.HorizontalPodAutoscaler{} + var err error + for i := 0; i < iterations; i++ { + hpa, err = kc.AutoscalingV2().HorizontalPodAutoscalers(namespace).Get(context.Background(), name, metav1.GetOptions{}) + t.Log("Waiting for hpa creation") + if err == nil { + return hpa, err + } + time.Sleep(time.Duration(intervalSeconds) * time.Second) + } + return hpa, err +} + func KubectlApplyWithTemplate(t *testing.T, data interface{}, config string) { tmpl, err := template.New("kubernetes resource template").Parse(config) assert.NoErrorf(t, err, "cannot parse template - %s", err) diff --git a/tests/scalers_go/custom_hpa_name/custom_hpa_name_test.go b/tests/scalers_go/custom_hpa_name/custom_hpa_name_test.go index 160186460e1..6be30a6363f 100644 --- a/tests/scalers_go/custom_hpa_name/custom_hpa_name_test.go +++ b/tests/scalers_go/custom_hpa_name/custom_hpa_name_test.go @@ -4,17 +4,12 @@ package custom_hpa_name import ( - "context" "fmt" "github.com/joho/godotenv" . "github.com/kedacore/keda/v2/tests" "github.com/stretchr/testify/assert" - autoscalingv2 "k8s.io/api/autoscaling/v2" "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/client-go/kubernetes" "testing" - "time" ) // Load environment variables from .env file @@ -161,18 +156,3 @@ func getTemplateData(testNamespace string, deploymentName string, scaledObjectNa CustomHpaName: customHpaName, } } - -func WaitForHpaCreation(t *testing.T, kc *kubernetes.Clientset, name, namespace string, - iterations, intervalSeconds int) (*autoscalingv2.HorizontalPodAutoscaler, error) { - hpa := &autoscalingv2.HorizontalPodAutoscaler{} - var err error - for i := 0; i < iterations; i++ { - hpa, err = kc.AutoscalingV2().HorizontalPodAutoscalers(namespace).Get(context.Background(), name, metav1.GetOptions{}) - t.Log("Waiting for hpa creation") - if err == nil { - return hpa, err - } - time.Sleep(time.Duration(intervalSeconds) * time.Second) - } - return hpa, err -} From 09ade1f5a2d49ebb86b922a19836083ba7f0c1cb Mon Sep 17 00:00:00 2001 From: aviadlevy Date: Thu, 16 Jun 2022 19:56:18 +0300 Subject: [PATCH 13/15] e2e - downgrade to autoscalingv2beta2 Signed-off-by: aviadlevy --- tests/helper.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/helper.go b/tests/helper.go index e546a9ca282..4f6630afcf5 100644 --- a/tests/helper.go +++ b/tests/helper.go @@ -13,7 +13,7 @@ import ( "github.com/joho/godotenv" "github.com/stretchr/testify/assert" - autoscalingv2 "k8s.io/api/autoscaling/v2" + autoscalingv2beta2 "k8s.io/api/autoscaling/v2beta2" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" @@ -137,11 +137,11 @@ func WaitForDeploymentReplicaCount(t *testing.T, kc *kubernetes.Clientset, name, } func WaitForHpaCreation(t *testing.T, kc *kubernetes.Clientset, name, namespace string, - iterations, intervalSeconds int) (*autoscalingv2.HorizontalPodAutoscaler, error) { - hpa := &autoscalingv2.HorizontalPodAutoscaler{} + iterations, intervalSeconds int) (*autoscalingv2beta2.HorizontalPodAutoscaler, error) { + hpa := &autoscalingv2beta2.HorizontalPodAutoscaler{} var err error for i := 0; i < iterations; i++ { - hpa, err = kc.AutoscalingV2().HorizontalPodAutoscalers(namespace).Get(context.Background(), name, metav1.GetOptions{}) + hpa, err = kc.AutoscalingV2beta2().HorizontalPodAutoscalers(namespace).Get(context.Background(), name, metav1.GetOptions{}) t.Log("Waiting for hpa creation") if err == nil { return hpa, err From d4521767fdfd2e6df7d7db1d6b8aa6be1c59f737 Mon Sep 17 00:00:00 2001 From: aviadlevy Date: Fri, 17 Jun 2022 10:52:45 +0300 Subject: [PATCH 14/15] e2e - fix import blocks Signed-off-by: aviadlevy --- tests/scalers_go/custom_hpa_name/custom_hpa_name_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/scalers_go/custom_hpa_name/custom_hpa_name_test.go b/tests/scalers_go/custom_hpa_name/custom_hpa_name_test.go index 6be30a6363f..b5a7c948721 100644 --- a/tests/scalers_go/custom_hpa_name/custom_hpa_name_test.go +++ b/tests/scalers_go/custom_hpa_name/custom_hpa_name_test.go @@ -5,11 +5,13 @@ package custom_hpa_name import ( "fmt" + "testing" + "github.com/joho/godotenv" - . "github.com/kedacore/keda/v2/tests" "github.com/stretchr/testify/assert" "k8s.io/apimachinery/pkg/api/errors" - "testing" + + . "github.com/kedacore/keda/v2/tests" ) // Load environment variables from .env file From c3163d972753f89311fe61307d55e0a0ed27ffab Mon Sep 17 00:00:00 2001 From: aviadlevy Date: Mon, 20 Jun 2022 15:00:19 +0300 Subject: [PATCH 15/15] e2e - fix linting Signed-off-by: aviadlevy --- tests/scalers_go/custom_hpa_name/custom_hpa_name_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/scalers_go/custom_hpa_name/custom_hpa_name_test.go b/tests/scalers_go/custom_hpa_name/custom_hpa_name_test.go index b5a7c948721..55779b17d53 100644 --- a/tests/scalers_go/custom_hpa_name/custom_hpa_name_test.go +++ b/tests/scalers_go/custom_hpa_name/custom_hpa_name_test.go @@ -1,7 +1,7 @@ //go:build e2e // +build e2e -package custom_hpa_name +package custom_hpa_name_test import ( "fmt" @@ -11,7 +11,7 @@ import ( "github.com/stretchr/testify/assert" "k8s.io/apimachinery/pkg/api/errors" - . "github.com/kedacore/keda/v2/tests" + . "github.com/kedacore/keda/v2/tests/helper" ) // Load environment variables from .env file