From 53065998c1f522bd87d627ea433f79f690b0698f Mon Sep 17 00:00:00 2001 From: Noah Kantrowitz Date: Fri, 30 Apr 2021 03:14:55 -0700 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fix=20incorrect=20handling=20whe?= =?UTF-8?q?n=20removing=20from=20the=20end=20of=20the=20triggers=20array.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- controllers/hpa.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/controllers/hpa.go b/controllers/hpa.go index 40c9a50741a..13b1d545e6f 100644 --- a/controllers/hpa.go +++ b/controllers/hpa.go @@ -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