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 16 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 @@ -38,6 +38,7 @@ To learn more about our roadmap, we recommend reading [this document](ROADMAP.md

- **General:** Support for Azure AD Workload Identity as a pod identity provider. ([#2487](https://github.com/kedacore/keda/issues/2487))
- **General:** Basic setup for migrating e2e tests to Go. ([#2737](https://github.com/kedacore/keda/issues/2737))
- **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 {
JorTurFer marked this conversation as resolved.
Show resolved Hide resolved
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
aviadlevy marked this conversation as resolved.
Show resolved Hide resolved
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
178 changes: 178 additions & 0 deletions tests/scalers_go/custom_hpa_name/custom_hpa_name_test.go
Original file line number Diff line number Diff line change
@@ -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,
aviadlevy marked this conversation as resolved.
Show resolved Hide resolved
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
}