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

Prevent primary hpa collision for keda scaled objects when migrating from an hpa #1677

Open
wants to merge 2 commits 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
2 changes: 2 additions & 0 deletions pkg/apis/keda/v1alpha1/scaledobject.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ type AdvancedConfig struct {

// HorizontalPodAutoscalerConfig specifies horizontal scale config
type HorizontalPodAutoscalerConfig struct {
// +optional
Name string `json:"name,omitempty"`
// +optional
Behavior *autoscalingv2beta2.HorizontalPodAutoscalerBehavior `json:"behavior,omitempty"`
}
Expand Down
36 changes: 35 additions & 1 deletion pkg/canary/scaled_object_reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/google/go-cmp/cmp"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
)

// ScaledObjectReconciler is a ScalerReconciler that reconciles KEDA ScaledObjects.
Expand Down Expand Up @@ -47,6 +48,8 @@ func (sor *ScaledObjectReconciler) reconcilePrimaryScaler(cd *flaggerv1.Canary,

setPrimaryScaledObjectQueries(cd, targetSoClone.Spec.Triggers)

setPrimaryScaledObjectHPA(targetSoClone)

soSpec := keda.ScaledObjectSpec{
ScaleTargetRef: &keda.ScaleTarget{
Name: primaryName,
Expand Down Expand Up @@ -77,7 +80,8 @@ func (sor *ScaledObjectReconciler) reconcilePrimaryScaler(cd *flaggerv1.Canary,
primarySo, err := sor.flaggerClient.KedaV1alpha1().ScaledObjects(cd.Namespace).Get(context.TODO(), primarySoName, metav1.GetOptions{})
if errors.IsNotFound(err) {
primarySo = &keda.ScaledObject{
ObjectMeta: makeObjectMeta(primarySoName, targetSoClone.Labels, cd),
// Passing in the annotations from the targetSo so that they are carried over to the primarySo. This is required so that the transfer ownership annotation can be added.
ObjectMeta: makeObjectMetaSo(primarySoName, targetSoClone.Labels, targetSoClone.Annotations, cd),
Spec: soSpec,
}
_, err = sor.flaggerClient.KedaV1alpha1().ScaledObjects(cd.Namespace).Create(context.TODO(), primarySo, metav1.CreateOptions{})
Expand Down Expand Up @@ -206,3 +210,33 @@ func setPrimaryScaledObjectQueries(cd *flaggerv1.Canary, triggers []keda.ScaleTr
}
}
}

func makeObjectMetaSo(name string, labels map[string]string, annotations map[string]string, cd *flaggerv1.Canary) metav1.ObjectMeta {
return metav1.ObjectMeta{
Name: name,
Namespace: cd.Namespace,
Labels: filterMetadata(labels),
Annotations: filterMetadata(annotations),
OwnerReferences: []metav1.OwnerReference{
*metav1.NewControllerRef(cd, schema.GroupVersionKind{
Group: flaggerv1.SchemeGroupVersion.Group,
Version: flaggerv1.SchemeGroupVersion.Version,
Kind: flaggerv1.CanaryKind,
}),
},
}
}

func setPrimaryScaledObjectHPA(targetSoClone *keda.ScaledObject) {
if targetSoClone.Spec.Advanced == nil {
targetSoClone.Spec.Advanced = &keda.AdvancedConfig{}
}
if targetSoClone.Spec.Advanced.HorizontalPodAutoscalerConfig == nil {
targetSoClone.Spec.Advanced.HorizontalPodAutoscalerConfig = &keda.HorizontalPodAutoscalerConfig{}
}
if targetSoClone.Spec.Advanced.HorizontalPodAutoscalerConfig.Name != "" {
// if the target scaled object has the hpa name set, then append "-primary" to the primary scaled object hpa name
// if the target scaled object does not have the hpa name set, then it will use the default set by keda
targetSoClone.Spec.Advanced.HorizontalPodAutoscalerConfig.Name = fmt.Sprintf("%s-primary", targetSoClone.Spec.Advanced.HorizontalPodAutoscalerConfig.Name)
}
}
4 changes: 4 additions & 0 deletions pkg/canary/scaled_object_reconciler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ func Test_reconcilePrimaryScaledObject(t *testing.T) {

primarySO, err := mocks.flaggerClient.KedaV1alpha1().ScaledObjects("default").Get(context.TODO(), "podinfo-primary", metav1.GetOptions{})
require.NoError(t, err)
// test that the hpa ownership annotation is added to the primarySO
assert.Equal(t, primarySO.ObjectMeta.Annotations["scaledobject.keda.sh/transfer-hpa-ownership"], "true")
// test that the horizontalpodautoscalerconfig is set to 'podinfo-primary', so that it takes over ownership of the HPA
assert.Equal(t, primarySO.Spec.Advanced.HorizontalPodAutoscalerConfig.Name, "podinfo-primary")
assert.Equal(t, primarySO.Spec.ScaleTargetRef.Name, fmt.Sprintf("%s-primary", mocks.canary.Spec.TargetRef.Name))
assert.Equal(t, int(*primarySO.Spec.PollingInterval), 10)
assert.Equal(t, int(*primarySO.Spec.MinReplicaCount), 1)
Expand Down
8 changes: 8 additions & 0 deletions pkg/canary/scaler_reconciler_fixture_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,11 +154,19 @@ func newScaledObject() *keda.ScaledObject {
ObjectMeta: metav1.ObjectMeta{
Namespace: "default",
Name: "podinfo",
Annotations: map[string]string{
"scaledobject.keda.sh/transfer-hpa-ownership": "true",
},
},
Spec: keda.ScaledObjectSpec{
ScaleTargetRef: &keda.ScaleTarget{
Name: "podinfo",
},
Advanced: &keda.AdvancedConfig{
HorizontalPodAutoscalerConfig: &keda.HorizontalPodAutoscalerConfig{
Name: "podinfo",
},
},
PollingInterval: int32p(10),
MinReplicaCount: int32p(1),
MaxReplicaCount: int32p(4),
Expand Down