Skip to content

Commit

Permalink
🐛 Fix incorrect handling when removing from the end of the triggers a…
Browse files Browse the repository at this point in the history
…rray.

DeepDerivative considers it fine if the found HPA has extra elements in the array as long as the ones that are there in the computed HPA match the found. In our case, that causes updating the HPA to be skipped if you remove the last trigger (or any number as long as they are at the end). Checking the lengths first is quick and should cover all of these wonky cases.

Fixes #1699.

Signed-off-by: Noah Kantrowitz <noah@coderanger.net>
  • Loading branch information
coderanger committed Apr 30, 2021
1 parent f31aa14 commit 5306599
Showing 1 changed file with 2 additions and 1 deletion.
3 changes: 2 additions & 1 deletion controllers/hpa.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,8 @@ func (r *ScaledObjectReconciler) updateHPAIfNeeded(logger logr.Logger, scaledObj
return err
}

if !equality.Semantic.DeepDerivative(hpa.Spec, foundHpa.Spec) {
// DeepDerivative ignores extra entries in arrays which makes removing the last trigger not update things, so trigger and update any time the metrics count is different.
if len(hpa.Spec.Metrics) != len(foundHpa.Spec.Metrics) || !equality.Semantic.DeepDerivative(hpa.Spec, foundHpa.Spec) {
logger.V(1).Info("Found difference in the HPA spec accordint to ScaledObject", "currentHPA", foundHpa.Spec, "newHPA", hpa.Spec)
if r.Client.Update(context.TODO(), hpa) != nil {
foundHpa.Spec = hpa.Spec
Expand Down

0 comments on commit 5306599

Please sign in to comment.