From 8037c252a6228084f51ee70cf5002ee4452cc20a Mon Sep 17 00:00:00 2001 From: Conor Gallagher Date: Fri, 6 Nov 2026 11:27:35 +0000 Subject: [PATCH 1/3] fix: match HorizontalPodAutoscaler target quantities in SSA matcher The API server stores HPA metric target quantities in canonical form, so a desired averageValue of 1000 is stored as 1k and the dependent HPA was updated on every reconciliation. Normalise value and averageValue for all autoscaling/v2 metric sources, reusing the quantity check of PodTemplateSpecSanitizer. Fixes #3681 Signed-off-by: Conor Gallagher --- .../HorizontalPodAutoscalerSanitizer.java | 98 +++++++++++++++++++ .../kubernetes/PodTemplateSpecSanitizer.java | 21 +--- .../kubernetes/QuantitySanitizer.java | 61 ++++++++++++ ...BasedGenericKubernetesResourceMatcher.java | 8 ++ ...dGenericKubernetesResourceMatcherTest.java | 27 +++++ .../sample-hpa-quantities-desired.yaml | 67 +++++++++++++ .../kubernetes/sample-hpa-quantities.yaml | 83 ++++++++++++++++ .../HpaDesiredSanitizerDependentResource.java | 97 ++++++++++++++++++ .../HpaDesiredSanitizerIT.java | 91 +++++++++++++++++ .../HpaDesiredSanitizerReconciler.java | 34 +++++++ 10 files changed, 567 insertions(+), 20 deletions(-) create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/HorizontalPodAutoscalerSanitizer.java create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/QuantitySanitizer.java create mode 100644 operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-hpa-quantities-desired.yaml create mode 100644 operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-hpa-quantities.yaml create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/hpadesiredsanitizer/HpaDesiredSanitizerDependentResource.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/hpadesiredsanitizer/HpaDesiredSanitizerIT.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/hpadesiredsanitizer/HpaDesiredSanitizerReconciler.java 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..d505ac320e --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/HorizontalPodAutoscalerSanitizer.java @@ -0,0 +1,98 @@ +/* + * 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.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" diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/hpadesiredsanitizer/HpaDesiredSanitizerDependentResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/hpadesiredsanitizer/HpaDesiredSanitizerDependentResource.java new file mode 100644 index 0000000000..640614cff5 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/hpadesiredsanitizer/HpaDesiredSanitizerDependentResource.java @@ -0,0 +1,97 @@ +/* + * 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.dependent.hpadesiredsanitizer; + +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; + +import io.fabric8.kubernetes.api.model.Quantity; +import io.fabric8.kubernetes.api.model.autoscaling.v2.HorizontalPodAutoscaler; +import io.fabric8.kubernetes.api.model.autoscaling.v2.HorizontalPodAutoscalerBuilder; +import io.fabric8.kubernetes.api.model.autoscaling.v2.MetricSpecBuilder; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.dependent.statefulsetdesiredsanitizer.StatefulSetDesiredSanitizerCustomResource; +import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDKubernetesDependentResource; + +public class HpaDesiredSanitizerDependentResource + extends CRUDKubernetesDependentResource< + HorizontalPodAutoscaler, StatefulSetDesiredSanitizerCustomResource> { + + public static final Set matchedPrimaryValues = ConcurrentHashMap.newKeySet(); + public static volatile boolean nonMatchedAtLeastOnce; + + @Override + protected HorizontalPodAutoscaler desired( + StatefulSetDesiredSanitizerCustomResource primary, + Context context) { + var name = primary.getMetadata().getName(); + return new HorizontalPodAutoscalerBuilder() + .withNewMetadata() + .withName(name) + .withNamespace(primary.getMetadata().getNamespace()) + .endMetadata() + .withNewSpec() + .withMinReplicas(1) + .withMaxReplicas(4) + .withNewScaleTargetRef() + .withApiVersion("apps/v1") + .withKind("Deployment") + .withName(name) + .endScaleTargetRef() + .withMetrics( + // the API server stores 1000 as 1k and 0.5 as 500m + new MetricSpecBuilder() + .withType("External") + .withNewExternal() + .withNewMetric() + .withName("queue-depth") + .endMetric() + .withNewTarget() + .withType("Value") + .withValue(new Quantity("1000")) + .endTarget() + .endExternal() + .build(), + new MetricSpecBuilder() + .withType("Pods") + .withNewPods() + .withNewMetric() + .withName("packets-per-second") + .endMetric() + .withNewTarget() + .withType("AverageValue") + .withAverageValue(new Quantity("0.5")) + .endTarget() + .endPods() + .build()) + .endSpec() + .build(); + } + + @Override + public Result match( + HorizontalPodAutoscaler actualResource, + StatefulSetDesiredSanitizerCustomResource primary, + Context context) { + var res = super.match(actualResource, primary, context); + if (res.matched()) { + matchedPrimaryValues.add(primary.getSpec().getValue()); + } else { + nonMatchedAtLeastOnce = true; + } + return res; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/hpadesiredsanitizer/HpaDesiredSanitizerIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/hpadesiredsanitizer/HpaDesiredSanitizerIT.java new file mode 100644 index 0000000000..573d15530b --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/hpadesiredsanitizer/HpaDesiredSanitizerIT.java @@ -0,0 +1,91 @@ +/* + * 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.dependent.hpadesiredsanitizer; + +import java.time.Duration; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.fabric8.kubernetes.api.model.autoscaling.v2.HorizontalPodAutoscaler; +import io.javaoperatorsdk.annotation.Sample; +import io.javaoperatorsdk.operator.dependent.statefulsetdesiredsanitizer.StatefulSetDesiredSanitizerCustomResource; +import io.javaoperatorsdk.operator.dependent.statefulsetdesiredsanitizer.StatefulSetDesiredSanitizerSpec; +import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + +@Sample( + tldr = "Matching HorizontalPodAutoscaler target quantities with SSA", + description = + """ + Shows that the SSA based matcher treats numerically equal HorizontalPodAutoscaler \ + target quantities as equal, although the API server stores them in canonical form \ + (for example 1000 as 1k), so the HPA is not updated on every reconciliation. + """) +public class HpaDesiredSanitizerIT { + + public static final String TEST_1 = "test1"; + + @RegisterExtension + LocallyRunOperatorExtension extension = + LocallyRunOperatorExtension.builder() + .withReconciler(new HpaDesiredSanitizerReconciler()) + .build(); + + @Test + void testSSAMatcher() { + var resource = extension.create(testResource()); + + await() + .pollDelay(Duration.ofMillis(200)) + .untilAsserted( + () -> { + var hpa = extension.get(HorizontalPodAutoscaler.class, TEST_1); + assertThat(hpa).isNotNull(); + assertThat( + hpa.getSpec() + .getMetrics() + .get(0) + .getExternal() + .getTarget() + .getValue() + .toString()) + .isEqualTo("1k"); + }); + // make sure the changed primary is reconciled and matched at least once + resource.getSpec().setValue("changed value"); + extension.replace(resource); + + await() + .untilAsserted( + () -> + assertThat(HpaDesiredSanitizerDependentResource.matchedPrimaryValues) + .contains("changed value")); + assertThat(HpaDesiredSanitizerDependentResource.nonMatchedAtLeastOnce).isFalse(); + } + + StatefulSetDesiredSanitizerCustomResource testResource() { + var res = new StatefulSetDesiredSanitizerCustomResource(); + res.setMetadata(new ObjectMetaBuilder().withName(TEST_1).build()); + res.setSpec(new StatefulSetDesiredSanitizerSpec()); + res.getSpec().setValue("initial value"); + + return res; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/hpadesiredsanitizer/HpaDesiredSanitizerReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/hpadesiredsanitizer/HpaDesiredSanitizerReconciler.java new file mode 100644 index 0000000000..cf8ac05aaf --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/hpadesiredsanitizer/HpaDesiredSanitizerReconciler.java @@ -0,0 +1,34 @@ +/* + * 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.dependent.hpadesiredsanitizer; + +import io.javaoperatorsdk.operator.api.reconciler.*; +import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent; +import io.javaoperatorsdk.operator.dependent.statefulsetdesiredsanitizer.StatefulSetDesiredSanitizerCustomResource; + +@Workflow(dependents = {@Dependent(type = HpaDesiredSanitizerDependentResource.class)}) +@ControllerConfiguration +public class HpaDesiredSanitizerReconciler + implements Reconciler { + + @Override + public UpdateControl reconcile( + StatefulSetDesiredSanitizerCustomResource resource, + Context context) + throws Exception { + return UpdateControl.noUpdate(); + } +} From 9a3bc4f303e85d621ee2054fffcf555bf34299f1 Mon Sep 17 00:00:00 2001 From: Conor Gallagher Date: Fri, 6 Nov 2026 12:05:46 +0000 Subject: [PATCH 2/3] fix: guard against null metrics in HorizontalPodAutoscalerSanitizer Signed-off-by: Conor Gallagher --- .../kubernetes/HorizontalPodAutoscalerSanitizer.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 index d505ac320e..fd7f18135f 100644 --- 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 @@ -53,7 +53,9 @@ static void sanitizeHorizontalPodAutoscalerSpec( } final List actualMetrics = actualSpec.getMetrics(); final List desiredMetrics = desiredSpec.getMetrics(); - if (actualMetrics.size() != desiredMetrics.size()) { + if (actualMetrics == null + || desiredMetrics == null + || actualMetrics.size() != desiredMetrics.size()) { return; } for (int metricIndex = 0; metricIndex < desiredMetrics.size(); metricIndex++) { From 5f1ec0123c9d4a7fa00b72864f9534337d21860d Mon Sep 17 00:00:00 2001 From: Conor Gallagher Date: Fri, 6 Nov 2026 11:55:03 +0000 Subject: [PATCH 3/3] fix: match StatefulSet volumeClaimTemplates storage quantities in SSA matcher The API server stores PersistentVolumeClaim template storage requests in canonical form, so a desired storage of 1024Mi is stored as 1Gi and the dependent StatefulSet was updated on every reconciliation. Normalise the volumeClaimTemplates resource requests with the shared quantity check used for pod template resources. Signed-off-by: Conor Gallagher --- ...BasedGenericKubernetesResourceMatcher.java | 13 +++++ ...dGenericKubernetesResourceMatcherTest.java | 3 +- ...lates-desired-with-equivalent-storage.yaml | 50 +++++++++++++++++++ ...lSetDesiredSanitizerDependentResource.java | 10 ++++ 4 files changed, 75 insertions(+), 1 deletion(-) create mode 100644 operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-volumeclaimtemplates-desired-with-equivalent-storage.yaml 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 d31e4f6e7e..1945609079 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 @@ -231,6 +231,7 @@ protected void sanitizeState(R actual, R desired, Map actualMap) } } + @SuppressWarnings("unchecked") private static void sanitizeVolumeClaimTemplates( Map actualMap, StatefulSetSpec actualSpec, StatefulSetSpec desiredSpec) { int claims = desiredSpec.getVolumeClaimTemplates().size(); @@ -251,6 +252,18 @@ private static void sanitizeVolumeClaimTemplates( .map(Map.class::cast) .ifPresent(m -> m.remove("status")); } + var desiredResources = claim.getSpec().getResources(); + var actualResources = actualSpec.getVolumeClaimTemplates().get(i).getSpec().getResources(); + if (desiredResources != null && actualResources != null) { + Optional.ofNullable( + GenericKubernetesResource.get( + actualMap, "spec", "volumeClaimTemplates", i, "spec", "resources", "requests")) + .map(Map.class::cast) + .ifPresent( + m -> + QuantitySanitizer.sanitizeQuantities( + m, actualResources.getRequests(), desiredResources.getRequests())); + } } } 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 fda7c4fad3..eb7e54f492 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 @@ -253,7 +253,8 @@ void withFinalizer() { strings = { "sample-sts-volumeclaimtemplates-desired.yaml", "sample-sts-volumeclaimtemplates-desired-with-status.yaml", - "sample-sts-volumeclaimtemplates-desired-with-volumemode.yaml" + "sample-sts-volumeclaimtemplates-desired-with-volumemode.yaml", + "sample-sts-volumeclaimtemplates-desired-with-equivalent-storage.yaml" }) void testSanitizeState_statefulSetWithVolumeClaims(String desiredResourceFileName) { var desiredStatefulSet = loadResource(desiredResourceFileName, StatefulSet.class); diff --git a/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-volumeclaimtemplates-desired-with-equivalent-storage.yaml b/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-volumeclaimtemplates-desired-with-equivalent-storage.yaml new file mode 100644 index 0000000000..f8daf9d100 --- /dev/null +++ b/operator-framework-core/src/test/resources/io/javaoperatorsdk/operator/processing/dependent/kubernetes/sample-sts-volumeclaimtemplates-desired-with-equivalent-storage.yaml @@ -0,0 +1,50 @@ +# +# 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 StatefulSet with a VolumeClaimTemplate with a numerically equal, non-canonical storage quantity +apiVersion: apps/v1 +kind: StatefulSet +metadata: + name: "test" +spec: + replicas: 1 + selector: + matchLabels: + app: test-app + serviceName: "nginx-service" + template: + metadata: + labels: + app: test-app + spec: + containers: + - name: nginx + image: nginx:1.17.0 + ports: + - containerPort: 80 + volumeMounts: + - name: persistent-storage + mountPath: /usr/share/nginx/html + volumeClaimTemplates: + - metadata: + name: persistent-storage + spec: + accessModes: + - ReadWriteOnce + resources: + requests: + storage: 1024Mi + storageClassName: standard diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/statefulsetdesiredsanitizer/StatefulSetDesiredSanitizerDependentResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/statefulsetdesiredsanitizer/StatefulSetDesiredSanitizerDependentResource.java index e4bcaac460..2c40bedc25 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/statefulsetdesiredsanitizer/StatefulSetDesiredSanitizerDependentResource.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/statefulsetdesiredsanitizer/StatefulSetDesiredSanitizerDependentResource.java @@ -16,6 +16,7 @@ package io.javaoperatorsdk.operator.dependent.statefulsetdesiredsanitizer; import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.fabric8.kubernetes.api.model.Quantity; import io.fabric8.kubernetes.api.model.apps.StatefulSet; import io.javaoperatorsdk.operator.ReconcilerUtilsInternal; import io.javaoperatorsdk.operator.api.reconciler.Context; @@ -39,6 +40,15 @@ protected StatefulSet desired( .withName(primary.getMetadata().getName()) .withNamespace(primary.getMetadata().getNamespace()) .build()); + // the API server stores 1024Mi as 1Gi + template + .getSpec() + .getVolumeClaimTemplates() + .get(0) + .getSpec() + .getResources() + .getRequests() + .put("storage", new Quantity("1024Mi")); return template; }