diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/HorizontalPodAutoscalerSanitizer.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/HorizontalPodAutoscalerSanitizer.java new file mode 100644 index 0000000000..fd7f18135f --- /dev/null +++ b/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. + * + *

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> 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 actualMap, + final HorizontalPodAutoscalerSpec actualSpec, + final HorizontalPodAutoscalerSpec desiredSpec) { + if (actualSpec == null || desiredSpec == null) { + return; + } + final List actualMetrics = actualSpec.getMetrics(); + final List 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 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()); + }); + } +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/PodTemplateSpecSanitizer.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/PodTemplateSpecSanitizer.java index 9b6375c6a8..26ebaf2823 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/PodTemplateSpecSanitizer.java +++ b/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") diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/QuantitySanitizer.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/QuantitySanitizer.java new file mode 100644 index 0000000000..46fb18320e --- /dev/null +++ b/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 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 parent, + final Map actualQuantities, + final Map desiredQuantities) { + if (actualQuantities == null || desiredQuantities == null) { + return; + } + actualQuantities.forEach( + (key, actualQuantity) -> + sanitizeQuantity(parent, key, actualQuantity, desiredQuantities.get(key))); + } +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/SSABasedGenericKubernetesResourceMatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/SSABasedGenericKubernetesResourceMatcher.java index abec13290d..d31e4f6e7e 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/SSABasedGenericKubernetesResourceMatcher.java +++ b/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 actualMap) actualMap, actualDaemonSet.getSpec().getTemplate(), desiredDaemonSet.getSpec().getTemplate()); + } else if (actual instanceof HorizontalPodAutoscaler actualHorizontalPodAutoscaler + && desired instanceof HorizontalPodAutoscaler desiredHorizontalPodAutoscaler) { + sanitizeHorizontalPodAutoscalerSpec( + actualMap, + actualHorizontalPodAutoscaler.getSpec(), + desiredHorizontalPodAutoscaler.getSpec()); } } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/SSABasedGenericKubernetesResourceMatcherTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/SSABasedGenericKubernetesResourceMatcherTest.java index c4d2f2c77d..fda7c4fad3 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/SSABasedGenericKubernetesResourceMatcherTest.java +++ b/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) { diff --git a/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-hpa-quantities-desired.yaml b/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-hpa-quantities-desired.yaml new file mode 100644 index 0000000000..da585c5cbc --- /dev/null +++ b/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" diff --git a/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-hpa-quantities.yaml b/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-hpa-quantities.yaml new file mode 100644 index 0000000000..21b5487ce7 --- /dev/null +++ b/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-hpa-quantities.yaml @@ -0,0 +1,83 @@ +# +# 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. +# + +# actual HorizontalPodAutoscaler with target quantities in the canonical form stored by the API server +apiVersion: autoscaling/v2 +kind: HorizontalPodAutoscaler +metadata: + managedFields: + - manager: controller + operation: Apply + apiVersion: autoscaling/v2 + time: '2026-11-06T10:00:00Z' + fieldsType: FieldsV1 + fieldsV1: + f:spec: + f:maxReplicas: { } + f:metrics: { } + f:minReplicas: { } + f:scaleTargetRef: + f:apiVersion: { } + f:kind: { } + f:name: { } + name: "test" + uid: 0d6f5b8e-2c1a-4f3e-9b7d-5a4c3b2a1f00 +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: "1k" + - type: Object + object: + describedObject: + apiVersion: v1 + kind: Service + name: test + metric: + name: requests-per-second + target: + type: Value + value: "2" + - type: Pods + pods: + metric: + name: packets-per-second + target: + type: AverageValue + averageValue: "500m" + - type: Resource + resource: + name: memory + target: + type: AverageValue + averageValue: "1Gi" + - type: ContainerResource + containerResource: + name: cpu + container: app + target: + type: AverageValue + averageValue: "1500m"