Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support to customize HPA name #3068

Merged
merged 23 commits into from
Jun 20, 2022
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 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
CurrentHpaName string `json:"currentHpaName,omitempty"`
aviadlevy marked this conversation as resolved.
Show resolved Hide resolved
}

// +kubebuilder:object:root=true
Expand Down
30 changes: 27 additions & 3 deletions controllers/keda/hpa.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -134,6 +136,16 @@ func (r *ScaledObjectReconciler) newHPAForScaledObject(ctx context.Context, logg
return nil, err
}

// store hpaName in the ScaledObject
aviadlevy marked this conversation as resolved.
Show resolved Hide resolved
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
}

Expand All @@ -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
aviadlevy marked this conversation as resolved.
Show resolved Hide resolved
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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
}

Expand Down
7 changes: 6 additions & 1 deletion controllers/keda/scaledobject_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
aviadlevy marked this conversation as resolved.
Show resolved Hide resolved
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)
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 @@ -27,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"
Expand Down Expand Up @@ -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 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