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 multiple ScaledObjects managing one HPA #6198

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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ Here is an overview of all new **experimental** features:

### Improvements

- **General**: Prevent multiple ScaledObjects managing one HPA ([#6130](https://github.com/kedacore/keda/issues/6130))
- **AWS CloudWatch Scaler**: Add support for ignoreNullValues ([#5352](https://github.com/kedacore/keda/issues/5352))
- **GCP Scalers**: Added custom time horizon in GCP scalers ([#5778](https://github.com/kedacore/keda/issues/5778))
- **GitHub Scaler**: Fixed pagination, fetching repository list ([#5738](https://github.com/kedacore/keda/issues/5738))
Expand Down
16 changes: 16 additions & 0 deletions apis/keda/v1alpha1/scaledobject_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,7 @@ func verifyScaledObjects(incomingSo *ScaledObject, action string, _ bool) error
return err
}

incomingSoHpaName := getHpaName(*incomingSo)
for _, so := range soList.Items {
if so.Name == incomingSo.Name {
continue
Expand All @@ -297,6 +298,13 @@ func verifyScaledObjects(incomingSo *ScaledObject, action string, _ bool) error
metricscollector.RecordScaledObjectValidatingErrors(incomingSo.Namespace, action, "other-scaled-object")
return err
}

if getHpaName(so) == incomingSoHpaName {
err = fmt.Errorf("the HPA '%s' is already managed by the ScaledObject '%s'", so.Spec.Advanced.HorizontalPodAutoscalerConfig.Name, so.Name)
scaledobjectlog.Error(err, "validation error")
metricscollector.RecordScaledObjectValidatingErrors(incomingSo.Namespace, action, "other-scaled-object-hpa")
return err
}
}

// verify ScalingModifiers structure if defined in ScaledObject
Expand Down Expand Up @@ -544,3 +552,11 @@ func isContainerResourceLimitSet(ctx context.Context, namespace string, triggerT

return false
}

func getHpaName(so ScaledObject) string {
if so.Spec.Advanced == nil || so.Spec.Advanced.HorizontalPodAutoscalerConfig.Name == "" {
return fmt.Sprintf("keda-hpa-%s", so.Name)
}

return so.Spec.Advanced.HorizontalPodAutoscalerConfig.Name
}
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,27 @@ spec:
desiredReplicas: '1'
`

customHpaScaledObjectTemplate = `
apiVersion: keda.sh/v1alpha1
kind: ScaledObject
metadata:
name: {{.ScaledObjectName}}
namespace: {{.TestNamespace}}
spec:
scaleTargetRef:
name: {{.DeploymentName}}
advanced:
horizontalPodAutoscalerConfig:
name: {{.HpaName}}
triggers:
- type: cron
metadata:
timezone: Etc/UTC
start: 0 * * * *
end: 1 * * * *
desiredReplicas: '1'
`

hpaTemplate = `
apiVersion: autoscaling/v2
kind: HorizontalPodAutoscaler
Expand Down Expand Up @@ -179,6 +200,8 @@ func TestScaledObjectValidations(t *testing.T) {

testScaledWorkloadByOtherScaledObject(t, data)

testManagedHpaByOtherScaledObject(t, data)

testScaledWorkloadByOtherHpa(t, data)

testScaledWorkloadByOtherHpaWithOwnershipTransfer(t, data)
Expand Down Expand Up @@ -220,6 +243,25 @@ func testScaledWorkloadByOtherScaledObject(t *testing.T, data templateData) {
KubectlDeleteWithTemplate(t, data, "scaledObjectTemplate", scaledObjectTemplate)
}

func testManagedHpaByOtherScaledObject(t *testing.T, data templateData) {
t.Log("--- already managed hpa by other scaledobject---")

data.HpaName = hpaName

data.ScaledObjectName = scaledObject1Name
err := KubectlApplyWithErrors(t, data, "scaledObjectTemplate", customHpaScaledObjectTemplate)
assert.NoErrorf(t, err, "cannot deploy the scaledObject - %s", err)

data.ScaledObjectName = scaledObject2Name
data.DeploymentName = fmt.Sprintf("%s-other-deployment", testName)
err = KubectlApplyWithErrors(t, data, "scaledObjectTemplate", customHpaScaledObjectTemplate)
assert.Errorf(t, err, "can deploy the scaledObject - %s", err)
assert.Contains(t, err.Error(), fmt.Sprintf("the HPA '%s' is already managed by the ScaledObject '%s", hpaName, scaledObject1Name))

data.ScaledObjectName = scaledObject1Name
KubectlDeleteWithTemplate(t, data, "scaledObjectTemplate", scaledObjectTemplate)
}

func testScaledWorkloadByOtherHpa(t *testing.T, data templateData) {
t.Log("--- already scaled workload by other hpa---")

Expand Down
Loading