Skip to content

Commit

Permalink
Add support to customize HPA name (kedacore#3068)
Browse files Browse the repository at this point in the history
Co-authored-by: Tom Kerkhove <[email protected]>
  • Loading branch information
aviadlevy and tomkerkhove authored Jun 20, 2022
1 parent 268a2ab commit 40cd0a9
Show file tree
Hide file tree
Showing 8 changed files with 298 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ To learn more about our roadmap, we recommend reading [this document](ROADMAP.md

- **General:** Basic setup for migrating e2e tests to Go. ([#2737](https://github.com/kedacore/keda/issues/2737))
- **General:** Support for Azure AD Workload Identity as a pod identity provider. ([#2487](https://github.com/kedacore/keda/issues/2487))
- **General:** Add support to customize HPA name ([3057](https://github.com/kedacore/keda/issues/3057))

### Improvements

Expand Down
4 changes: 4 additions & 0 deletions apis/keda/v1alpha1/scaledobject_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -152,6 +154,8 @@ type ScaledObjectStatus struct {
Health map[string]HealthStatus `json:"health,omitempty"`
// +optional
PausedReplicaCount *int32 `json:"pausedReplicaCount,omitempty"`
// +optional
HpaName string `json:"hpaName,omitempty"`
}

// +kubebuilder:object:root=true
Expand Down
5 changes: 4 additions & 1 deletion config/crd/bases/keda.sh_scaledobjects.yaml
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

---
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
Expand Down Expand Up @@ -196,6 +195,8 @@ spec:
type: integer
type: object
type: object
name:
type: string
type: object
restoreToOriginalReplicaCount:
type: boolean
Expand Down Expand Up @@ -327,6 +328,8 @@ spec:
type: string
type: object
type: object
hpaName:
type: string
lastActiveTime:
format: date-time
type: string
Expand Down
28 changes: 28 additions & 0 deletions controllers/keda/hpa.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -172,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
Expand Down Expand Up @@ -250,6 +271,13 @@ 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 getDefaultHpaName(scaledObject)
}

func getDefaultHpaName(scaledObject *kedav1alpha1.ScaledObject) string {
return fmt.Sprintf("keda-hpa-%s", scaledObject.Name)
}

Expand Down
26 changes: 25 additions & 1 deletion controllers/keda/scaledobject_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,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.HpaName != "" {
hpaName = scaledObject.Status.HpaName
} 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)
Expand All @@ -385,6 +390,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 {
Expand All @@ -395,6 +410,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 && 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
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")
Expand Down
60 changes: 60 additions & 0 deletions controllers/keda/scaledobject_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,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"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
Expand Down Expand Up @@ -294,6 +295,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 without specifying name.
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.
Expand Down
16 changes: 16 additions & 0 deletions tests/helper/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (

"github.com/joho/godotenv"
"github.com/stretchr/testify/assert"
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"
Expand Down Expand Up @@ -138,6 +139,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) (*autoscalingv2beta2.HorizontalPodAutoscaler, error) {
hpa := &autoscalingv2beta2.HorizontalPodAutoscaler{}
var err error
for i := 0; i < iterations; i++ {
hpa, err = kc.AutoscalingV2beta2().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)
Expand Down
Loading

0 comments on commit 40cd0a9

Please sign in to comment.