Skip to content

Commit 807ea2a

Browse files
ggarbclaude
andcommitted
fix(webhook): eliminate per-admission json.MarshalIndent and index verifyHpas
Three changes to reduce webhook memory pressure during SO creation bursts: 1. Replace unconditional json.MarshalIndent calls in ValidateCreate, ValidateUpdate, isRemovingFinalizer, and the verifyHpas loop with structured logr key-value logging. The marshals ran on every admission even when V(1) logging was disabled, generating 60-100 KB of transient garbage per request — at burst=60 this outpaced MADV_FREE and caused repeated OOMKills despite the 20 GiB limit. 2. Replace isRemovingFinalizer's JSON string comparison with reflect.DeepEqual, eliminating two spec marshals per update admission. 3. Add hpaScaleTargetNameIdx field index for HPA objects and switch verifyHpas to an indexed List, reducing it from O(N_hpas) to O(1) — same fix pattern as the A1d verifyScaledObjects change (#7681). Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
1 parent 966b7c5 commit 807ea2a

1 file changed

Lines changed: 23 additions & 19 deletions

File tree

apis/keda/v1alpha1/scaledobject_webhook.go

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,9 @@ package v1alpha1
1818

1919
import (
2020
"context"
21-
"encoding/json"
2221
"errors"
2322
"fmt"
23+
"reflect"
2424
"strconv"
2525
"strings"
2626

@@ -60,6 +60,11 @@ var cpuString = "cpu"
6060
const (
6161
scaleTargetRefNameIdx = "spec.scaleTargetRef.name"
6262
hpaNameIdx = "spec.hpaName"
63+
// hpaScaleTargetNameIdx indexes HPA objects by spec.scaleTargetRef.name so
64+
// verifyHpas can issue an O(1) lookup instead of listing every HPA in the
65+
// namespace. Index names are scoped per-GVK so reusing the same path string
66+
// as scaleTargetRefNameIdx is safe.
67+
hpaScaleTargetNameIdx = "spec.scaleTargetRef.name"
6368
)
6469

6570
func (so *ScaledObject) SetupWebhookWithManager(mgr ctrl.Manager, cacheMissFallback bool) error {
@@ -79,6 +84,12 @@ func (so *ScaledObject) SetupWebhookWithManager(mgr ctrl.Manager, cacheMissFallb
7984
}); err != nil {
8085
return fmt.Errorf("failed to register index %q: %w", hpaNameIdx, err)
8186
}
87+
if err := mgr.GetFieldIndexer().IndexField(ctx, &autoscalingv2.HorizontalPodAutoscaler{}, hpaScaleTargetNameIdx,
88+
func(obj client.Object) []string {
89+
return []string{obj.(*autoscalingv2.HorizontalPodAutoscaler).Spec.ScaleTargetRef.Name}
90+
}); err != nil {
91+
return fmt.Errorf("failed to register HPA index %q: %w", hpaScaleTargetNameIdx, err)
92+
}
8293

8394
err := setupKubernetesClients(mgr, cacheMissFallback)
8495
if err != nil {
@@ -150,14 +161,12 @@ var _ webhook.CustomValidator = &ScaledObjectCustomValidator{}
150161

151162
// ValidateCreate implements webhook.Validator so a webhook will be registered for the type
152163
func (so *ScaledObject) ValidateCreate(dryRun *bool) (admission.Warnings, error) {
153-
val, _ := json.MarshalIndent(so, "", " ")
154-
scaledobjectlog.V(1).Info(fmt.Sprintf("validating scaledobject creation for %s", string(val)))
164+
scaledobjectlog.V(1).Info("validating scaledobject creation", "name", so.Name)
155165
return validateWorkload(so, "create", *dryRun)
156166
}
157167

158168
func (so *ScaledObject) ValidateUpdate(old runtime.Object, dryRun *bool) (admission.Warnings, error) {
159-
val, _ := json.MarshalIndent(so, "", " ")
160-
scaledobjectlog.V(1).Info(fmt.Sprintf("validating scaledobject update for %s", string(val)))
169+
scaledobjectlog.V(1).Info("validating scaledobject update", "name", so.Name)
161170

162171
if isRemovingFinalizer(so, old) {
163172
scaledobjectlog.V(1).Info("finalizer removal, skipping validation")
@@ -173,13 +182,7 @@ func (so *ScaledObject) ValidateDelete(_ *bool) (admission.Warnings, error) {
173182

174183
func isRemovingFinalizer(so *ScaledObject, old runtime.Object) bool {
175184
oldSo := old.(*ScaledObject)
176-
177-
soSpec, _ := json.MarshalIndent(so.Spec, "", " ")
178-
oldSoSpec, _ := json.MarshalIndent(oldSo.Spec, "", " ")
179-
soSpecString := string(soSpec)
180-
oldSoSpecString := string(oldSoSpec)
181-
182-
return len(so.Finalizers) < len(oldSo.Finalizers) && soSpecString == oldSoSpecString
185+
return len(so.Finalizers) < len(oldSo.Finalizers) && reflect.DeepEqual(so.Spec, oldSo.Spec)
183186
}
184187

185188
func validateWorkload(so *ScaledObject, action string, dryRun bool) (admission.Warnings, error) {
@@ -194,7 +197,7 @@ func validateWorkload(so *ScaledObject, action string, dryRun bool) (admission.W
194197
}
195198

196199
for functionName, function := range verifyFunctions {
197-
scaledobjectlog.V(1).Info(fmt.Sprintf("calling %s to validate %s", functionName, so.Name))
200+
scaledobjectlog.V(1).Info("calling validator", "function", functionName, "name", so.Name)
198201
err := function(so, action, dryRun)
199202
if err != nil {
200203
return nil, err
@@ -262,10 +265,12 @@ func verifyTriggers(incomingObject interface{}, action string, _ bool) error {
262265

263266
func verifyHpas(incomingSo *ScaledObject, action string, _ bool) error {
264267
hpaList := &autoscalingv2.HorizontalPodAutoscalerList{}
265-
opt := &client.ListOptions{
266-
Namespace: incomingSo.Namespace,
267-
}
268-
err := kc.List(context.Background(), hpaList, opt)
268+
// Use the hpaScaleTargetNameIdx field index to narrow candidates to HPAs
269+
// that target the same workload name, avoiding an O(N) full-namespace scan.
270+
err := kc.List(context.Background(), hpaList,
271+
client.InNamespace(incomingSo.Namespace),
272+
client.MatchingFields{hpaScaleTargetNameIdx: incomingSo.Spec.ScaleTargetRef.Name},
273+
)
269274
if err != nil {
270275
return err
271276
}
@@ -281,8 +286,7 @@ func verifyHpas(incomingSo *ScaledObject, action string, _ bool) error {
281286
if hpa.Annotations[ValidationsHpaOwnershipAnnotation] == "false" {
282287
continue
283288
}
284-
val, _ := json.MarshalIndent(hpa, "", " ")
285-
scaledobjectlog.V(1).Info(fmt.Sprintf("checking hpa %s: %v", hpa.Name, string(val)))
289+
scaledobjectlog.V(1).Info("checking hpa", "name", hpa.Name)
286290

287291
hpaGvkr, err := ParseGVKR(restMapper, hpa.Spec.ScaleTargetRef.APIVersion, hpa.Spec.ScaleTargetRef.Kind)
288292
if err != nil {

0 commit comments

Comments
 (0)