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

fix(scaledobject): reconcile hpa on label or annotations change #5482

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
15 changes: 8 additions & 7 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ Here is an overview of all new **experimental** features:
### Fixes

- **General**: Check for missing CRD references and sample CRs ([#5920](https://github.com/kedacore/keda/issues/5920))
- **General**: Fix reconcile HPA when ScaledObject annotations or labels is updated ([#5468](https://github.com/kedacore/keda/issues/5468))
- **General**: Scalers are properly closed after being refreshed ([#5806](https://github.com/kedacore/keda/issues/5806))
- **MongoDB Scaler**: MongoDB url parses correctly `+srv` scheme ([#5760](https://github.com/kedacore/keda/issues/5760))

Expand Down Expand Up @@ -239,7 +240,6 @@ Here is an overview of all new **experimental** features:

### Deprecations


You can find all deprecations in [this overview](https://github.com/kedacore/keda/issues?q=is%3Aissue+is%3Aopen+sort%3Aupdated-desc+label%3Abreaking-change) and [join the discussion here](https://github.com/kedacore/keda/discussions/categories/deprecations).

New deprecation(s):
Expand Down Expand Up @@ -948,7 +948,7 @@ None.
- Refactor AWS related scalers to reuse the AWS clients instead of creating a new one for every `GetMetrics` call ([#2255](https://github.com/kedacore/keda/pull/2255))
- Improve context handling in appropriate functionality in which we instantiate scalers ([#2267](https://github.com/kedacore/keda/pull/2267))
- Migrate to Kubebuilder v3 ([#2082](https://github.com/kedacore/keda/pull/2082))
- API path has been changed: `github.com/kedacore/keda/v2/api/v1alpha1` -> `github.com/kedacore/keda/v2/apis/keda/v1alpha1`
- API path has been changed: `github.com/kedacore/keda/v2/api/v1alpha1` -> `github.com/kedacore/keda/v2/apis/keda/v1alpha1`
- Use Patch to set FallbackCondition on ScaledObject.Status ([#2037](https://github.com/kedacore/keda/pull/2037))
- Bump Golang to 1.17.3 ([#2329](https://github.com/kedacore/keda/pull/2329))
- Add Makefile mockgen targets ([#2090](https://github.com/kedacore/keda/issues/2090)|[#2184](https://github.com/kedacore/keda/pull/2184))
Expand Down Expand Up @@ -1141,6 +1141,7 @@ None.
- CRDs are using `apiextensions.k8s.io/v1` apiVersion ([#1202](https://github.com/kedacore/keda/pull/1202))

### Other

- Change API optional structs to pointers to conform with k8s guide ([#1170](https://github.com/kedacore/keda/issues/1170))
- Update Operator SDK and k8s deps ([#1007](https://github.com/kedacore/keda/pull/1007),[#870](https://github.com/kedacore/keda/issues/870),[#1180](https://github.com/kedacore/keda/pull/1180))
- Change Metrics Server image name from `keda-metrics-adapter` to `keda-metrics-apiserver` ([#1105](https://github.com/kedacore/keda/issues/1105))
Expand All @@ -1153,13 +1154,13 @@ Learn more about our release in [our milestone](https://github.com/kedacore/keda
### New

- **Scalers**
- Introduce Active MQ Artemis scaler ([Docs](https://keda.sh/docs/1.5/scalers/artemis/))
- Introduce Redis Streams scaler ([Docs](https://keda.sh/docs/1.5/scalers/redis-streams/) | [Details](https://github.com/kedacore/keda/issues/746))
- Introduce Cron scaler ([Docs](https://keda.sh/docs/1.5/scalers/cron/) | [Details](https://github.com/kedacore/keda/issues/812))
- Introduce Active MQ Artemis scaler ([Docs](https://keda.sh/docs/1.5/scalers/artemis/))
- Introduce Redis Streams scaler ([Docs](https://keda.sh/docs/1.5/scalers/redis-streams/) | [Details](https://github.com/kedacore/keda/issues/746))
- Introduce Cron scaler ([Docs](https://keda.sh/docs/1.5/scalers/cron/) | [Details](https://github.com/kedacore/keda/issues/812))
- **Secret Providers**
- Introduce HashiCorp Vault secret provider ([Docs](https://keda.sh/docs/1.5/concepts/authentication/#hashicorp-vault-secrets) | [Details](https://github.com/kedacore/keda/issues/673))
- Introduce HashiCorp Vault secret provider ([Docs](https://keda.sh/docs/1.5/concepts/authentication/#hashicorp-vault-secrets) | [Details](https://github.com/kedacore/keda/issues/673))
- **Other**
- Introduction of `nodeSelector` in raw YAML deployment specifications ([Details](https://github.com/kedacore/keda/pull/856))
- Introduction of `nodeSelector` in raw YAML deployment specifications ([Details](https://github.com/kedacore/keda/pull/856))

### Improvements

Expand Down
8 changes: 8 additions & 0 deletions apis/keda/v1alpha1/scaledobject_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,14 @@ type HorizontalPodAutoscalerConfig struct {
Behavior *autoscalingv2.HorizontalPodAutoscalerBehavior `json:"behavior,omitempty"`
// +optional
Name string `json:"name,omitempty"`
// Those labels are added to the HorizontalPodAutoscaler created by KEDA
// and reconciled by KEDA on change
// +optional
Labels map[string]string `json:"labels,omitempty"`
// Those annotations are added to the HorizontalPodAutoscaler created by KEDA
// and reconciled by KEDA on change
// +optional
Annotations map[string]string `json:"annotations,omitempty"`
}

// ScaleTarget holds the reference to the scale target Object
Expand Down
14 changes: 14 additions & 0 deletions config/crd/bases/keda.sh_scaledobjects.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,13 @@ spec:
description: HorizontalPodAutoscalerConfig specifies horizontal
scale config
properties:
annotations:
additionalProperties:
type: string
description: |-
Those annotations are added to the HorizontalPodAutoscaler created by KEDA
and reconciled by KEDA on change
type: object
behavior:
description: |-
HorizontalPodAutoscalerBehavior configures the scaling behavior of the target
Expand Down Expand Up @@ -197,6 +204,13 @@ spec:
type: integer
type: object
type: object
labels:
additionalProperties:
type: string
description: |-
Those labels are added to the HorizontalPodAutoscaler created by KEDA
and reconciled by KEDA on change
type: object
name:
type: string
type: object
Expand Down
23 changes: 20 additions & 3 deletions controllers/keda/hpa.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import (
kedacontrollerutil "github.com/kedacore/keda/v2/controllers/keda/util"
"github.com/kedacore/keda/v2/pkg/scaling/executor"
kedastatus "github.com/kedacore/keda/v2/pkg/status"
version "github.com/kedacore/keda/v2/version"
"github.com/kedacore/keda/v2/version"
)

// createAndDeployNewHPA creates and deploy HPA in the cluster for specified ScaledObject
Expand Down Expand Up @@ -99,6 +99,23 @@ func (r *ScaledObjectReconciler) newHPAForScaledObject(ctx context.Context, logg
labels[key] = value
}

if scaledObject.Spec.Advanced != nil && scaledObject.Spec.Advanced.HorizontalPodAutoscalerConfig != nil && scaledObject.Spec.Advanced.HorizontalPodAutoscalerConfig.Labels != nil {
for key, value := range scaledObject.Spec.Advanced.HorizontalPodAutoscalerConfig.Labels {
labels[key] = value
}
}

annotations := map[string]string{}
for key, value := range scaledObject.ObjectMeta.Annotations {
annotations[key] = value
}

if scaledObject.Spec.Advanced != nil && scaledObject.Spec.Advanced.HorizontalPodAutoscalerConfig != nil && scaledObject.Spec.Advanced.HorizontalPodAutoscalerConfig.Annotations != nil {
for key, value := range scaledObject.Spec.Advanced.HorizontalPodAutoscalerConfig.Annotations {
annotations[key] = value
}
}

minReplicas := scaledObject.GetHPAMinReplicas()
maxReplicas := scaledObject.GetHPAMaxReplicas()

Expand Down Expand Up @@ -130,7 +147,7 @@ func (r *ScaledObjectReconciler) newHPAForScaledObject(ctx context.Context, logg
Name: getHPAName(scaledObject),
Namespace: scaledObject.Namespace,
Labels: labels,
Annotations: scaledObject.Annotations,
Annotations: annotations,
},
TypeMeta: metav1.TypeMeta{
APIVersion: "v2",
Expand Down Expand Up @@ -176,7 +193,7 @@ func (r *ScaledObjectReconciler) updateHPAIfNeeded(ctx context.Context, logger l
}

if (hpa.ObjectMeta.Annotations == nil && foundHpa.ObjectMeta.Annotations != nil) ||
!equality.Semantic.DeepDerivative(hpa.ObjectMeta.Annotations, foundHpa.ObjectMeta.Annotations) {
!equality.Semantic.DeepDerivative(hpa.ObjectMeta.Annotations, foundHpa.ObjectMeta.Annotations) || !equality.Semantic.DeepDerivative(foundHpa.ObjectMeta.Annotations, hpa.ObjectMeta.Annotations) {
logger.V(1).Info("Found difference in the HPA annotations according to ScaledObject", "currentHPA", foundHpa.ObjectMeta.Annotations, "newHPA", hpa.ObjectMeta.Annotations)
if err = r.Client.Update(ctx, hpa); err != nil {
foundHpa.ObjectMeta.Annotations = hpa.ObjectMeta.Annotations
Expand Down
149 changes: 149 additions & 0 deletions controllers/keda/scaledobject_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1441,6 +1441,155 @@ var _ = Describe("ScaledObjectController", func() {
}).Should(Equal(metav1.ConditionFalse))
})

// Fix issue 5468
It("reconciles hpa when scaledobject annotation is changed", func() {
var (
deploymentName = "scaledobject-annotation-change"
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{},
},
Spec: kedav1alpha1.ScaledObjectSpec{
ScaleTargetRef: &kedav1alpha1.ScaleTarget{
Name: deploymentName,
},
Advanced: &kedav1alpha1.AdvancedConfig{
HorizontalPodAutoscalerConfig: &kedav1alpha1.HorizontalPodAutoscalerConfig{
Annotations: map[string]string{
"annotation-email": "email@example.com",
"annotation-url": "https://example.com",
},
},
},
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())

// And validate that hpa is created.
hpa := &autoscalingv2.HorizontalPodAutoscaler{}
Eventually(func() error {
return k8sClient.Get(context.Background(), types.NamespacedName{Name: fmt.Sprintf("keda-hpa-%s", soName), Namespace: "default"}, hpa)
}).ShouldNot(HaveOccurred())

Eventually(func() error {
err = k8sClient.Get(context.Background(), types.NamespacedName{Name: soName, Namespace: "default"}, so)
Expect(err).ToNot(HaveOccurred())

so.Spec.Advanced.HorizontalPodAutoscalerConfig.Annotations = map[string]string{"new-annotation": "new-annotation-value"}
return k8sClient.Update(context.Background(), so)
}).WithTimeout(1 * time.Minute).WithPolling(10 * time.Second).ShouldNot(HaveOccurred())
testLogger.Info("annotation is set")

// hpa should be reconciled and should contain this new annotation
Eventually(func() bool {
err = k8sClient.Get(context.Background(), types.NamespacedName{Name: fmt.Sprintf("keda-hpa-%s", soName), Namespace: "default"}, hpa)
if err != nil {
return false
}
// Check if the annotation is present
if _, ok := hpa.ObjectMeta.Annotations["new-annotation"]; ok {
return true
}
return false
}, 5*time.Second).Should(BeTrue())
})

It("reconciles hpa when scaledobject labels is changed", func() {
var (
deploymentName = "scaledobject-label-change"
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",
Labels: map[string]string{
"awesome-label": "my-label",
},
},
Spec: kedav1alpha1.ScaledObjectSpec{
ScaleTargetRef: &kedav1alpha1.ScaleTarget{
Name: deploymentName,
},
Advanced: &kedav1alpha1.AdvancedConfig{
HorizontalPodAutoscalerConfig: &kedav1alpha1.HorizontalPodAutoscalerConfig{
Labels: map[string]string{
"label-key": "label-value",
},
},
},
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())

// And validate that hpa is created.
hpa := &autoscalingv2.HorizontalPodAutoscaler{}
Eventually(func() error {
return k8sClient.Get(context.Background(), types.NamespacedName{Name: fmt.Sprintf("keda-hpa-%s", soName), Namespace: "default"}, hpa)
}).ShouldNot(HaveOccurred())

Eventually(func() error {
err = k8sClient.Get(context.Background(), types.NamespacedName{Name: soName, Namespace: "default"}, so)
Expect(err).ToNot(HaveOccurred())

so.Spec.Advanced.HorizontalPodAutoscalerConfig.Labels = map[string]string{"new-label": "new-label-value"}

return k8sClient.Update(context.Background(), so)
}).WithTimeout(1 * time.Minute).WithPolling(10 * time.Second).ShouldNot(HaveOccurred())
testLogger.Info("label is set")

// hpa should be reconciled and should contain this new label
Eventually(func() bool {
err = k8sClient.Get(context.Background(), types.NamespacedName{Name: fmt.Sprintf("keda-hpa-%s", soName), Namespace: "default"}, hpa)
if err != nil {
return false
}
// Check if the label is present
if _, ok := hpa.ObjectMeta.Labels["new-label"]; ok {
return true
}
return false
}, 5*time.Second).Should(BeTrue())
})

})

func generateDeployment(name string) *appsv1.Deployment {
Expand Down
Loading