fix: match HorizontalPodAutoscaler target quantities in SSA matcher

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/HorizontalPodAutoscalerSanitizer.java

@@ -0,0 +1,100 @@
+/*
+ * Copyright Java Operator SDK Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *         http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.javaoperatorsdk.operator.processing.dependent.kubernetes;
+
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.function.Function;
+
+import io.fabric8.kubernetes.api.model.GenericKubernetesResource;
+import io.fabric8.kubernetes.api.model.autoscaling.v2.HorizontalPodAutoscalerSpec;
+import io.fabric8.kubernetes.api.model.autoscaling.v2.MetricSpec;
+import io.fabric8.kubernetes.api.model.autoscaling.v2.MetricTarget;
+
+/**
+ * Sanitizes the target quantities ({@code value} and {@code averageValue}) of the metrics in a pair
+ * of {@link HorizontalPodAutoscalerSpec} instances.
+ *
+ * <p>When the number of metrics differs, the sanitizer returns early without fixing the actual map,
+ * because the specs differ anyway. A metric source that is only present on one side, for example
+ * because the metric type differs, is skipped for the same reason.
+ */
+class HorizontalPodAutoscalerSanitizer {
+
+  private static final Map<String, Function<MetricSpec, MetricTarget>> TARGET_BY_METRIC_SOURCE =
+      Map.of(
+          "external", m -> m.getExternal() == null ? null : m.getExternal().getTarget(),
+          "object", m -> m.getObject() == null ? null : m.getObject().getTarget(),
+          "pods", m -> m.getPods() == null ? null : m.getPods().getTarget(),
+          "resource", m -> m.getResource() == null ? null : m.getResource().getTarget(),
+          "containerResource",
+              m -> m.getContainerResource() == null ? null : m.getContainerResource().getTarget());
+
+  static void sanitizeHorizontalPodAutoscalerSpec(
+      final Map<String, Object> actualMap,
+      final HorizontalPodAutoscalerSpec actualSpec,
+      final HorizontalPodAutoscalerSpec desiredSpec) {
+    if (actualSpec == null || desiredSpec == null) {
+      return;
+    }
+    final List<MetricSpec> actualMetrics = actualSpec.getMetrics();
+    final List<MetricSpec> desiredMetrics = desiredSpec.getMetrics();
+    if (actualMetrics == null
+        || desiredMetrics == null
+        || actualMetrics.size() != desiredMetrics.size()) {
+      return;
+    }
+    for (int metricIndex = 0; metricIndex < desiredMetrics.size(); metricIndex++) {
+      final var actualMetric = actualMetrics.get(metricIndex);
+      final var desiredMetric = desiredMetrics.get(metricIndex);
+      for (var source : TARGET_BY_METRIC_SOURCE.entrySet()) {
+        sanitizeTarget(
+            actualMap,
+            metricIndex,
+            source.getKey(),
+            source.getValue().apply(actualMetric),
+            source.getValue().apply(desiredMetric));
+      }
+    }
+  }
+
+  @SuppressWarnings("unchecked")
+  private static void sanitizeTarget(
+      final Map<String, Object> actualMap,
+      final int metricIndex,
+      final String metricSource,
+      final MetricTarget actualTarget,
+      final MetricTarget desiredTarget) {
+    if (actualTarget == null || desiredTarget == null) {
+      return;
+    }
+    Optional.ofNullable(
+            GenericKubernetesResource.get(
+                actualMap, "spec", "metrics", metricIndex, metricSource, "target"))
+        .map(Map.class::cast)
+        .ifPresent(
+            m -> {
+              QuantitySanitizer.sanitizeQuantity(
+                  m, "value", actualTarget.getValue(), desiredTarget.getValue());
+              QuantitySanitizer.sanitizeQuantity(
+                  m,
+                  "averageValue",
+                  actualTarget.getAverageValue(),
+                  desiredTarget.getAverageValue());
+            });
+  }
+}

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/PodTemplateSpecSanitizer.java

@@ -139,26 +139,7 @@ private static void sanitizeQuantities(
                 "resources",
                 quantityPath))
         .map(Map.class::cast)
-        .ifPresent(
-            m ->
-                actualResource.forEach(
-                    (key, actualQuantity) -> {
-                      final var desiredQuantity = desiredResource.get(key);
-                      if (desiredQuantity == null) {
-                        return;
-                      }
-                      // check if the string representation of the Quantity instances is equal
-                      if (actualQuantity.getAmount().equals(desiredQuantity.getAmount())
-                          && actualQuantity.getFormat().equals(desiredQuantity.getFormat())) {
-                        return;
-                      }
-                      // check if the numerical amount of the Quantity instances is equal
-                      if (actualQuantity.equals(desiredQuantity)) {
-                        // replace the actual Quantity with the desired Quantity to prevent a
-                        // resource update
-                        m.replace(key, desiredQuantity.toString());
-                      }
-                    }));
+        .ifPresent(m -> QuantitySanitizer.sanitizeQuantities(m, actualResource, desiredResource));
   }
 
   @SuppressWarnings("unchecked")

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/QuantitySanitizer.java

@@ -0,0 +1,61 @@
+/*
+ * Copyright Java Operator SDK Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *         http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.javaoperatorsdk.operator.processing.dependent.kubernetes;
+
+import java.util.Map;
+import java.util.Objects;
+
+import io.fabric8.kubernetes.api.model.Quantity;
+
+/**
+ * Replaces an actual {@link Quantity} with the desired one when both are numerically equal. The API
+ * server stores quantities in canonical form (for example {@code 1000} as {@code 1k}), which would
+ * otherwise cause a mismatch.
+ */
+class QuantitySanitizer {
+
+  static void sanitizeQuantity(
+      final Map<String, Object> parent,
+      final String key,
+      final Quantity actualQuantity,
+      final Quantity desiredQuantity) {
+    if (actualQuantity == null || desiredQuantity == null) {
+      return;
+    }
+    // check if the string representation of the Quantity instances is equal
+    if (Objects.equals(actualQuantity.getAmount(), desiredQuantity.getAmount())
+        && Objects.equals(actualQuantity.getFormat(), desiredQuantity.getFormat())) {
+      return;
+    }
+    // check if the numerical amount of the Quantity instances is equal
+    if (actualQuantity.equals(desiredQuantity)) {
+      // replace the actual Quantity with the desired Quantity to prevent a resource update
+      parent.replace(key, desiredQuantity.toString());
+    }
+  }
+
+  static void sanitizeQuantities(
+      final Map<String, Object> parent,
+      final Map<String, Quantity> actualQuantities,
+      final Map<String, Quantity> desiredQuantities) {
+    if (actualQuantities == null || desiredQuantities == null) {
+      return;
+    }
+    actualQuantities.forEach(
+        (key, actualQuantity) ->
+            sanitizeQuantity(parent, key, actualQuantity, desiredQuantities.get(key)));
+  }
+}

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/SSABasedGenericKubernetesResourceMatcher.java

@@ -39,6 +39,7 @@
 import io.fabric8.kubernetes.api.model.apps.ReplicaSet;
 import io.fabric8.kubernetes.api.model.apps.StatefulSet;
 import io.fabric8.kubernetes.api.model.apps.StatefulSetSpec;
+import io.fabric8.kubernetes.api.model.autoscaling.v2.HorizontalPodAutoscaler;
 import io.fabric8.kubernetes.client.utils.KubernetesSerialization;
 import io.javaoperatorsdk.operator.OperatorException;
 import io.javaoperatorsdk.operator.api.reconciler.Context;
@@ -47,6 +48,7 @@
 import com.github.difflib.DiffUtils;
 import com.github.difflib.UnifiedDiffUtils;
 
+import static io.javaoperatorsdk.operator.processing.dependent.kubernetes.HorizontalPodAutoscalerSanitizer.sanitizeHorizontalPodAutoscalerSpec;
 import static io.javaoperatorsdk.operator.processing.dependent.kubernetes.PodTemplateSpecSanitizer.sanitizePodTemplateSpec;
 
 /**
@@ -220,6 +222,12 @@ protected void sanitizeState(R actual, R desired, Map<String, Object> actualMap)
           actualMap,
           actualDaemonSet.getSpec().getTemplate(),
           desiredDaemonSet.getSpec().getTemplate());
+    } else if (actual instanceof HorizontalPodAutoscaler actualHorizontalPodAutoscaler
+        && desired instanceof HorizontalPodAutoscaler desiredHorizontalPodAutoscaler) {
+      sanitizeHorizontalPodAutoscalerSpec(
+          actualMap,
+          actualHorizontalPodAutoscaler.getSpec(),
+          desiredHorizontalPodAutoscaler.getSpec());
     }
   }
 

operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/SSABasedGenericKubernetesResourceMatcherTest.java

@@ -25,11 +25,13 @@
 
 import io.fabric8.kubernetes.api.model.ConfigMap;
 import io.fabric8.kubernetes.api.model.HasMetadata;
+import io.fabric8.kubernetes.api.model.Quantity;
 import io.fabric8.kubernetes.api.model.Secret;
 import io.fabric8.kubernetes.api.model.apps.DaemonSet;
 import io.fabric8.kubernetes.api.model.apps.Deployment;
 import io.fabric8.kubernetes.api.model.apps.ReplicaSet;
 import io.fabric8.kubernetes.api.model.apps.StatefulSet;
+import io.fabric8.kubernetes.api.model.autoscaling.v2.HorizontalPodAutoscaler;
 import io.javaoperatorsdk.operator.MockKubernetesClient;
 import io.javaoperatorsdk.operator.OperatorException;
 import io.javaoperatorsdk.operator.ReconcilerUtilsInternal;
@@ -358,6 +360,31 @@ void testSanitizeState_daemonSet_withResourceTypeMismatch() {
     assertThat(matcher.matches(actualDaemonSet, desiredReplicaSet, mockedContext)).isFalse();
   }
 
+  @Test
+  void testSanitizeState_horizontalPodAutoscalerWithQuantities() {
+    var desiredHpa =
+        loadResource("sample-hpa-quantities-desired.yaml", HorizontalPodAutoscaler.class);
+    var actualHpa = loadResource("sample-hpa-quantities.yaml", HorizontalPodAutoscaler.class);
+
+    assertThat(matcher.matches(actualHpa, desiredHpa, mockedContext)).isTrue();
+  }
+
+  @Test
+  void testSanitizeState_horizontalPodAutoscalerWithQuantities_withMismatch() {
+    var desiredHpa =
+        loadResource("sample-hpa-quantities-desired.yaml", HorizontalPodAutoscaler.class);
+    desiredHpa
+        .getSpec()
+        .getMetrics()
+        .get(0)
+        .getExternal()
+        .getTarget()
+        .setAverageValue(new Quantity("2000"));
+    var actualHpa = loadResource("sample-hpa-quantities.yaml", HorizontalPodAutoscaler.class);
+
+    assertThat(matcher.matches(actualHpa, desiredHpa, mockedContext)).isFalse();
+  }
+
   @ParameterizedTest
   @ValueSource(booleans = {true, false})
   void testCustomMatcher_returnsExpectedMatchBasedOnReadOnlyLabel(boolean readOnly) {

operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-hpa-quantities-desired.yaml

@@ -0,0 +1,67 @@
+#
+# Copyright Java Operator SDK Authors
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+# desired HorizontalPodAutoscaler with numerically equal, non-canonical target quantities
+apiVersion: autoscaling/v2
+kind: HorizontalPodAutoscaler
+metadata:
+  name: "test"
+spec:
+  scaleTargetRef:
+    apiVersion: apps/v1
+    kind: Deployment
+    name: test
+  minReplicas: 1
+  maxReplicas: 4
+  metrics:
+    - type: External
+      external:
+        metric:
+          name: queue-depth
+        target:
+          type: AverageValue
+          averageValue: "1000"
+    - type: Object
+      object:
+        describedObject:
+          apiVersion: v1
+          kind: Service
+          name: test
+        metric:
+          name: requests-per-second
+        target:
+          type: Value
+          value: "2000m"
+    - type: Pods
+      pods:
+        metric:
+          name: packets-per-second
+        target:
+          type: AverageValue
+          averageValue: "0.5"
+    - type: Resource
+      resource:
+        name: memory
+        target:
+          type: AverageValue
+          averageValue: "1024Mi"
+    - type: ContainerResource
+      containerResource:
+        name: cpu
+        container: app
+        target:
+          type: AverageValue
+          averageValue: "1.5"