Skip to content

Commit

Permalink
Prevent multiple ScaledObjects managing one HPA
Browse files Browse the repository at this point in the history
Signed-off-by: wangrushen <[email protected]>
  • Loading branch information
dovics committed Sep 29, 2024
1 parent f6358d5 commit 04dc8c5
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ To learn more about active deprecations, we recommend checking [GitHub Discussio

### New

- **General**: Prevent multiple ScaledObjects managing one HPA ([#6130](https://github.com/kedacore/keda/issues/6130))
- **CloudEventSource**: Introduce ClusterCloudEventSource ([#3533](https://github.com/kedacore/keda/issues/3533))
- **CloudEventSource**: Provide ClusterCloudEventSource around the management of ScaledJobs resources ([#3523](https://github.com/kedacore/keda/issues/3523))
- **CloudEventSource**: Provide ClusterCloudEventSource around the management of TriggerAuthentication/ClusterTriggerAuthentication resources ([#3524](https://github.com/kedacore/keda/issues/3524))
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,24 @@ 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
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

0 comments on commit 04dc8c5

Please sign in to comment.