From 07e0324aed5bc356a780b54cd7125948cb50315b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Tue, 27 Jan 2026 12:13:47 +0100 Subject: [PATCH] improve: move compare resource version methods to internal utils MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This should not be user facing. At least not in any obvious scenerio. Signed-off-by: Attila Mészáros --- .../operator/ReconcilerUtilsInternal.java | 120 +++++++++++++++ .../api/reconciler/ReconcileUtils.java | 119 --------------- .../source/informer/EventFilterDetails.java | 4 +- .../informer/ManagedInformerEventSource.java | 5 +- .../informer/TemporaryResourceCache.java | 8 +- .../operator/ReconcilerUtilsInternalTest.java | 137 +++++++++++++++++- .../api/reconciler/ReconcileUtilsTest.java | 136 ----------------- 7 files changed, 265 insertions(+), 264 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ReconcilerUtilsInternal.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ReconcilerUtilsInternal.java index 1523b792a5..26ae5af554 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ReconcilerUtilsInternal.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ReconcilerUtilsInternal.java @@ -31,6 +31,7 @@ import io.fabric8.kubernetes.client.utils.Serialization; import io.javaoperatorsdk.operator.api.reconciler.Constants; import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.NonComparableResourceVersionException; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; @SuppressWarnings("rawtypes") @@ -241,4 +242,123 @@ private static boolean matchesResourceType( } return false; } + + /** + * Compares resource versions of two resources. This is a convenience method that extracts the + * resource versions from the metadata and delegates to {@link + * #validateAndCompareResourceVersions(String, String)}. + * + * @param h1 first resource + * @param h2 second resource + * @return negative if h1 is older, zero if equal, positive if h1 is newer + * @throws NonComparableResourceVersionException if either resource version is invalid + */ + public static int validateAndCompareResourceVersions(HasMetadata h1, HasMetadata h2) { + return validateAndCompareResourceVersions( + h1.getMetadata().getResourceVersion(), h2.getMetadata().getResourceVersion()); + } + + /** + * Compares the resource versions of two Kubernetes resources. + * + *

This method extracts the resource versions from the metadata of both resources and delegates + * to {@link #compareResourceVersions(String, String)} for the actual comparison. + * + * @param h1 the first resource to compare + * @param h2 the second resource to compare + * @return a negative integer if h1's version is less than h2's version, zero if they are equal, + * or a positive integer if h1's version is greater than h2's version + * @see #compareResourceVersions(String, String) + */ + public static int compareResourceVersions(HasMetadata h1, HasMetadata h2) { + return compareResourceVersions( + h1.getMetadata().getResourceVersion(), h2.getMetadata().getResourceVersion()); + } + + /** + * Compares two resource version strings using a length-first, then lexicographic comparison + * algorithm. + * + *

The comparison is performed in two steps: + * + *

    + *
  1. First, compare the lengths of the version strings. A longer version string is considered + * greater than a shorter one. This works correctly for numeric versions because larger + * numbers have more digits (e.g., "100" > "99"). + *
  2. If the lengths are equal, perform a character-by-character lexicographic comparison until + * a difference is found. + *
+ * + *

This algorithm is more efficient than parsing the versions as numbers, especially for + * Kubernetes resource versions which are typically monotonically increasing numeric strings. + * + *

Note: This method does not validate that the input strings are numeric. For + * validated numeric comparison, use {@link #validateAndCompareResourceVersions(String, String)}. + * + * @param v1 the first resource version string + * @param v2 the second resource version string + * @return a negative integer if v1 is less than v2, zero if they are equal, or a positive integer + * if v1 is greater than v2 + * @see #validateAndCompareResourceVersions(String, String) + */ + public static int compareResourceVersions(String v1, String v2) { + int comparison = v1.length() - v2.length(); + if (comparison != 0) { + return comparison; + } + for (int i = 0; i < v2.length(); i++) { + int comp = v1.charAt(i) - v2.charAt(i); + if (comp != 0) { + return comp; + } + } + return 0; + } + + /** + * Compares two Kubernetes resource versions numerically. Kubernetes resource versions are + * expected to be numeric strings that increase monotonically. This method assumes both versions + * are valid numeric strings without leading zeros. + * + * @param v1 first resource version + * @param v2 second resource version + * @return negative if v1 is older, zero if equal, positive if v1 is newer + * @throws NonComparableResourceVersionException if either resource version is empty, has leading + * zeros, or contains non-numeric characters + */ + public static int validateAndCompareResourceVersions(String v1, String v2) { + int v1Length = validateResourceVersion(v1); + int v2Length = validateResourceVersion(v2); + int comparison = v1Length - v2Length; + if (comparison != 0) { + return comparison; + } + for (int i = 0; i < v2Length; i++) { + int comp = v1.charAt(i) - v2.charAt(i); + if (comp != 0) { + return comp; + } + } + return 0; + } + + private static int validateResourceVersion(String v1) { + int v1Length = v1.length(); + if (v1Length == 0) { + throw new NonComparableResourceVersionException("Resource version is empty"); + } + for (int i = 0; i < v1Length; i++) { + char char1 = v1.charAt(i); + if (char1 == '0') { + if (i == 0) { + throw new NonComparableResourceVersionException( + "Resource version cannot begin with 0: " + v1); + } + } else if (char1 < '0' || char1 > '9') { + throw new NonComparableResourceVersionException( + "Non numeric characters in resource version: " + v1); + } + } + return v1Length; + } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtils.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtils.java index 6876fb0f8a..ed02c56a01 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtils.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtils.java @@ -595,123 +595,4 @@ public static

P addFinalizerWithSSA( e); } } - - /** - * Compares resource versions of two resources. This is a convenience method that extracts the - * resource versions from the metadata and delegates to {@link - * #validateAndCompareResourceVersions(String, String)}. - * - * @param h1 first resource - * @param h2 second resource - * @return negative if h1 is older, zero if equal, positive if h1 is newer - * @throws NonComparableResourceVersionException if either resource version is invalid - */ - public static int validateAndCompareResourceVersions(HasMetadata h1, HasMetadata h2) { - return validateAndCompareResourceVersions( - h1.getMetadata().getResourceVersion(), h2.getMetadata().getResourceVersion()); - } - - /** - * Compares the resource versions of two Kubernetes resources. - * - *

This method extracts the resource versions from the metadata of both resources and delegates - * to {@link #compareResourceVersions(String, String)} for the actual comparison. - * - * @param h1 the first resource to compare - * @param h2 the second resource to compare - * @return a negative integer if h1's version is less than h2's version, zero if they are equal, - * or a positive integer if h1's version is greater than h2's version - * @see #compareResourceVersions(String, String) - */ - public static int compareResourceVersions(HasMetadata h1, HasMetadata h2) { - return compareResourceVersions( - h1.getMetadata().getResourceVersion(), h2.getMetadata().getResourceVersion()); - } - - /** - * Compares two resource version strings using a length-first, then lexicographic comparison - * algorithm. - * - *

The comparison is performed in two steps: - * - *

    - *
  1. First, compare the lengths of the version strings. A longer version string is considered - * greater than a shorter one. This works correctly for numeric versions because larger - * numbers have more digits (e.g., "100" > "99"). - *
  2. If the lengths are equal, perform a character-by-character lexicographic comparison until - * a difference is found. - *
- * - *

This algorithm is more efficient than parsing the versions as numbers, especially for - * Kubernetes resource versions which are typically monotonically increasing numeric strings. - * - *

Note: This method does not validate that the input strings are numeric. For - * validated numeric comparison, use {@link #validateAndCompareResourceVersions(String, String)}. - * - * @param v1 the first resource version string - * @param v2 the second resource version string - * @return a negative integer if v1 is less than v2, zero if they are equal, or a positive integer - * if v1 is greater than v2 - * @see #validateAndCompareResourceVersions(String, String) - */ - public static int compareResourceVersions(String v1, String v2) { - int comparison = v1.length() - v2.length(); - if (comparison != 0) { - return comparison; - } - for (int i = 0; i < v2.length(); i++) { - int comp = v1.charAt(i) - v2.charAt(i); - if (comp != 0) { - return comp; - } - } - return 0; - } - - /** - * Compares two Kubernetes resource versions numerically. Kubernetes resource versions are - * expected to be numeric strings that increase monotonically. This method assumes both versions - * are valid numeric strings without leading zeros. - * - * @param v1 first resource version - * @param v2 second resource version - * @return negative if v1 is older, zero if equal, positive if v1 is newer - * @throws NonComparableResourceVersionException if either resource version is empty, has leading - * zeros, or contains non-numeric characters - */ - public static int validateAndCompareResourceVersions(String v1, String v2) { - int v1Length = validateResourceVersion(v1); - int v2Length = validateResourceVersion(v2); - int comparison = v1Length - v2Length; - if (comparison != 0) { - return comparison; - } - for (int i = 0; i < v2Length; i++) { - int comp = v1.charAt(i) - v2.charAt(i); - if (comp != 0) { - return comp; - } - } - return 0; - } - - private static int validateResourceVersion(String v1) { - int v1Length = v1.length(); - if (v1Length == 0) { - throw new NonComparableResourceVersionException("Resource version is empty"); - } - for (int i = 0; i < v1Length; i++) { - char char1 = v1.charAt(i); - if (char1 == '0') { - if (i == 0) { - throw new NonComparableResourceVersionException( - "Resource version cannot begin with 0: " + v1); - } - } else if (char1 < '0' || char1 > '9') { - throw new NonComparableResourceVersionException( - "Non numeric characters in resource version: " + v1); - } - } - return v1Length; - } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/EventFilterDetails.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/EventFilterDetails.java index 6a2d304976..8b573a986c 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/EventFilterDetails.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/EventFilterDetails.java @@ -17,7 +17,7 @@ import java.util.Optional; -import io.javaoperatorsdk.operator.api.reconciler.ReconcileUtils; +import io.javaoperatorsdk.operator.ReconcilerUtilsInternal; import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEvent; class EventFilterDetails { @@ -41,7 +41,7 @@ public void setLastEvent(ResourceEvent event) { public Optional getLatestEventAfterLastUpdateEvent(String updatedResourceVersion) { if (lastEvent != null && (updatedResourceVersion == null - || ReconcileUtils.compareResourceVersions( + || ReconcilerUtilsInternal.compareResourceVersions( lastEvent.getResource().orElseThrow().getMetadata().getResourceVersion(), updatedResourceVersion) > 0)) { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java index 9278400dde..dcfe687a2f 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java @@ -32,10 +32,10 @@ import io.fabric8.kubernetes.client.dsl.MixedOperation; import io.fabric8.kubernetes.client.informers.ResourceEventHandler; import io.javaoperatorsdk.operator.OperatorException; +import io.javaoperatorsdk.operator.ReconcilerUtilsInternal; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.config.Informable; import io.javaoperatorsdk.operator.api.config.NamespaceChangeable; -import io.javaoperatorsdk.operator.api.reconciler.ReconcileUtils; import io.javaoperatorsdk.operator.api.reconciler.dependent.RecentOperationCacheFiller; import io.javaoperatorsdk.operator.health.InformerHealthIndicator; import io.javaoperatorsdk.operator.health.InformerWrappingEventSourceHealthIndicator; @@ -181,7 +181,8 @@ public Optional get(ResourceID resourceID) { Optional resource = temporaryResourceCache.getResourceFromCache(resourceID); if (comparableResourceVersions && resource.isPresent() - && res.filter(r -> ReconcileUtils.compareResourceVersions(r, resource.orElseThrow()) > 0) + && res.filter( + r -> ReconcilerUtilsInternal.compareResourceVersions(r, resource.orElseThrow()) > 0) .isEmpty()) { log.debug("Latest resource found in temporary cache for Resource ID: {}", resourceID); return resource; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryResourceCache.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryResourceCache.java index eb76387a80..6e1d30c323 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryResourceCache.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryResourceCache.java @@ -24,7 +24,7 @@ import org.slf4j.LoggerFactory; import io.fabric8.kubernetes.api.model.HasMetadata; -import io.javaoperatorsdk.operator.api.reconciler.ReconcileUtils; +import io.javaoperatorsdk.operator.ReconcilerUtilsInternal; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource; import io.javaoperatorsdk.operator.processing.event.ResourceID; import io.javaoperatorsdk.operator.processing.event.source.ResourceAction; @@ -128,7 +128,7 @@ private synchronized EventHandling onEvent( var cached = cache.get(resourceId); EventHandling result = EventHandling.NEW; if (cached != null) { - int comp = ReconcileUtils.compareResourceVersions(resource, cached); + int comp = ReconcilerUtilsInternal.compareResourceVersions(resource, cached); if (comp >= 0 || unknownState) { cache.remove(resourceId); // we propagate event only for our update or newer other can be discarded since we know we @@ -174,7 +174,7 @@ public synchronized void putResource(T newResource) { // this also prevents resurrecting recently deleted entities for which the delete event // has already been processed if (latestResourceVersion != null - && ReconcileUtils.compareResourceVersions( + && ReconcilerUtilsInternal.compareResourceVersions( latestResourceVersion, newResource.getMetadata().getResourceVersion()) > 0) { log.debug( @@ -189,7 +189,7 @@ public synchronized void putResource(T newResource) { var cachedResource = getResourceFromCache(resourceId).orElse(null); if (cachedResource == null - || ReconcileUtils.compareResourceVersions(newResource, cachedResource) > 0) { + || ReconcilerUtilsInternal.compareResourceVersions(newResource, cachedResource) > 0) { log.debug( "Temporarily moving ahead to target version {} for resource id: {}", newResource.getMetadata().getResourceVersion(), diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ReconcilerUtilsInternalTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ReconcilerUtilsInternalTest.java index 12e45b9c23..129351e8af 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ReconcilerUtilsInternalTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ReconcilerUtilsInternalTest.java @@ -17,7 +17,10 @@ import java.net.URI; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import io.fabric8.kubernetes.api.model.*; import io.fabric8.kubernetes.api.model.apps.Deployment; @@ -29,6 +32,7 @@ import io.fabric8.kubernetes.model.annotation.Group; import io.fabric8.kubernetes.model.annotation.ShortNames; import io.fabric8.kubernetes.model.annotation.Version; +import io.javaoperatorsdk.operator.api.reconciler.NonComparableResourceVersionException; import io.javaoperatorsdk.operator.sample.simple.TestCustomReconciler; import io.javaoperatorsdk.operator.sample.simple.TestCustomResource; @@ -43,7 +47,7 @@ import static org.mockito.Mockito.when; class ReconcilerUtilsInternalTest { - + private static final Logger log = LoggerFactory.getLogger(ReconcilerUtilsInternalTest.class); public static final String RESOURCE_URI = "https://kubernetes.docker.internal:6443/apis/tomcatoperator.io/v1/tomcats"; @@ -183,4 +187,135 @@ public void setReplicas(Integer replicas) { this.replicas = replicas; } } + + // naive performance test that compares the work case scenario for the parsing and non-parsing + // variants + @Test + @Disabled + public void compareResourcePerformanceTest() { + var execNum = 30000000; + var startTime = System.currentTimeMillis(); + for (int i = 0; i < execNum; i++) { + var res = ReconcilerUtilsInternal.compareResourceVersions("123456788" + i, "123456789" + i); + } + var dur1 = System.currentTimeMillis() - startTime; + log.info("Duration without parsing: {}", dur1); + startTime = System.currentTimeMillis(); + for (int i = 0; i < execNum; i++) { + var res = Long.parseLong("123456788" + i) > Long.parseLong("123456789" + i); + } + var dur2 = System.currentTimeMillis() - startTime; + log.info("Duration with parsing: {}", dur2); + + assertThat(dur1).isLessThan(dur2); + } + + @Test + void validateAndCompareResourceVersionsTest() { + assertThat(ReconcilerUtilsInternal.validateAndCompareResourceVersions("11", "22")).isNegative(); + assertThat(ReconcilerUtilsInternal.validateAndCompareResourceVersions("22", "11")).isPositive(); + assertThat(ReconcilerUtilsInternal.validateAndCompareResourceVersions("1", "1")).isZero(); + assertThat(ReconcilerUtilsInternal.validateAndCompareResourceVersions("11", "11")).isZero(); + assertThat(ReconcilerUtilsInternal.validateAndCompareResourceVersions("123", "2")).isPositive(); + assertThat(ReconcilerUtilsInternal.validateAndCompareResourceVersions("3", "211")).isNegative(); + + assertThrows( + NonComparableResourceVersionException.class, + () -> ReconcilerUtilsInternal.validateAndCompareResourceVersions("aa", "22")); + assertThrows( + NonComparableResourceVersionException.class, + () -> ReconcilerUtilsInternal.validateAndCompareResourceVersions("11", "ba")); + assertThrows( + NonComparableResourceVersionException.class, + () -> ReconcilerUtilsInternal.validateAndCompareResourceVersions("", "22")); + assertThrows( + NonComparableResourceVersionException.class, + () -> ReconcilerUtilsInternal.validateAndCompareResourceVersions("11", "")); + assertThrows( + NonComparableResourceVersionException.class, + () -> ReconcilerUtilsInternal.validateAndCompareResourceVersions("01", "123")); + assertThrows( + NonComparableResourceVersionException.class, + () -> ReconcilerUtilsInternal.validateAndCompareResourceVersions("123", "01")); + assertThrows( + NonComparableResourceVersionException.class, + () -> ReconcilerUtilsInternal.validateAndCompareResourceVersions("3213", "123a")); + assertThrows( + NonComparableResourceVersionException.class, + () -> ReconcilerUtilsInternal.validateAndCompareResourceVersions("321", "123a")); + } + + @Test + void compareResourceVersionsWithStrings() { + // Test equal versions + assertThat(ReconcilerUtilsInternal.compareResourceVersions("1", "1")).isZero(); + assertThat(ReconcilerUtilsInternal.compareResourceVersions("123", "123")).isZero(); + + // Test different lengths - shorter version is less than longer version + assertThat(ReconcilerUtilsInternal.compareResourceVersions("1", "12")).isNegative(); + assertThat(ReconcilerUtilsInternal.compareResourceVersions("12", "1")).isPositive(); + assertThat(ReconcilerUtilsInternal.compareResourceVersions("99", "100")).isNegative(); + assertThat(ReconcilerUtilsInternal.compareResourceVersions("100", "99")).isPositive(); + assertThat(ReconcilerUtilsInternal.compareResourceVersions("9", "100")).isNegative(); + assertThat(ReconcilerUtilsInternal.compareResourceVersions("100", "9")).isPositive(); + + // Test same length - lexicographic comparison + assertThat(ReconcilerUtilsInternal.compareResourceVersions("1", "2")).isNegative(); + assertThat(ReconcilerUtilsInternal.compareResourceVersions("2", "1")).isPositive(); + assertThat(ReconcilerUtilsInternal.compareResourceVersions("11", "12")).isNegative(); + assertThat(ReconcilerUtilsInternal.compareResourceVersions("12", "11")).isPositive(); + assertThat(ReconcilerUtilsInternal.compareResourceVersions("99", "100")).isNegative(); + assertThat(ReconcilerUtilsInternal.compareResourceVersions("100", "99")).isPositive(); + assertThat(ReconcilerUtilsInternal.compareResourceVersions("123", "124")).isNegative(); + assertThat(ReconcilerUtilsInternal.compareResourceVersions("124", "123")).isPositive(); + + // Test with non-numeric strings (algorithm should still work character-wise) + assertThat(ReconcilerUtilsInternal.compareResourceVersions("a", "b")).isNegative(); + assertThat(ReconcilerUtilsInternal.compareResourceVersions("b", "a")).isPositive(); + assertThat(ReconcilerUtilsInternal.compareResourceVersions("abc", "abd")).isNegative(); + assertThat(ReconcilerUtilsInternal.compareResourceVersions("abd", "abc")).isPositive(); + + // Test edge cases with larger numbers + assertThat(ReconcilerUtilsInternal.compareResourceVersions("1234567890", "1234567891")) + .isNegative(); + assertThat(ReconcilerUtilsInternal.compareResourceVersions("1234567891", "1234567890")) + .isPositive(); + } + + @Test + void compareResourceVersionsWithHasMetadata() { + // Test equal versions + HasMetadata resource1 = createResourceWithVersion("123"); + HasMetadata resource2 = createResourceWithVersion("123"); + assertThat(ReconcilerUtilsInternal.compareResourceVersions(resource1, resource2)).isZero(); + + // Test different lengths + resource1 = createResourceWithVersion("1"); + resource2 = createResourceWithVersion("12"); + assertThat(ReconcilerUtilsInternal.compareResourceVersions(resource1, resource2)).isNegative(); + assertThat(ReconcilerUtilsInternal.compareResourceVersions(resource2, resource1)).isPositive(); + + // Test same length, different values + resource1 = createResourceWithVersion("100"); + resource2 = createResourceWithVersion("200"); + assertThat(ReconcilerUtilsInternal.compareResourceVersions(resource1, resource2)).isNegative(); + assertThat(ReconcilerUtilsInternal.compareResourceVersions(resource2, resource1)).isPositive(); + + // Test realistic Kubernetes resource versions + resource1 = createResourceWithVersion("12345"); + resource2 = createResourceWithVersion("12346"); + assertThat(ReconcilerUtilsInternal.compareResourceVersions(resource1, resource2)).isNegative(); + assertThat(ReconcilerUtilsInternal.compareResourceVersions(resource2, resource1)).isPositive(); + } + + private HasMetadata createResourceWithVersion(String resourceVersion) { + return new PodBuilder() + .withMetadata( + new ObjectMetaBuilder() + .withName("test-pod") + .withNamespace("default") + .withResourceVersion(resourceVersion) + .build()) + .build(); + } } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtilsTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtilsTest.java index 6d8c244c83..f76ec61e16 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtilsTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtilsTest.java @@ -20,14 +20,8 @@ import java.util.function.UnaryOperator; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import io.fabric8.kubernetes.api.model.HasMetadata; -import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; -import io.fabric8.kubernetes.api.model.PodBuilder; import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.KubernetesClientException; import io.fabric8.kubernetes.client.dsl.MixedOperation; @@ -47,7 +41,6 @@ class ReconcileUtilsTest { - private static final Logger log = LoggerFactory.getLogger(ReconcileUtilsTest.class); private static final String FINALIZER_NAME = "test.javaoperatorsdk.io/finalizer"; private Context context; @@ -80,113 +73,6 @@ void setupMocks() { when(mixedOperation.withName(any())).thenReturn(resourceOp); } - @Test - void validateAndCompareResourceVersionsTest() { - assertThat(ReconcileUtils.validateAndCompareResourceVersions("11", "22")).isNegative(); - assertThat(ReconcileUtils.validateAndCompareResourceVersions("22", "11")).isPositive(); - assertThat(ReconcileUtils.validateAndCompareResourceVersions("1", "1")).isZero(); - assertThat(ReconcileUtils.validateAndCompareResourceVersions("11", "11")).isZero(); - assertThat(ReconcileUtils.validateAndCompareResourceVersions("123", "2")).isPositive(); - assertThat(ReconcileUtils.validateAndCompareResourceVersions("3", "211")).isNegative(); - - assertThrows( - NonComparableResourceVersionException.class, - () -> ReconcileUtils.validateAndCompareResourceVersions("aa", "22")); - assertThrows( - NonComparableResourceVersionException.class, - () -> ReconcileUtils.validateAndCompareResourceVersions("11", "ba")); - assertThrows( - NonComparableResourceVersionException.class, - () -> ReconcileUtils.validateAndCompareResourceVersions("", "22")); - assertThrows( - NonComparableResourceVersionException.class, - () -> ReconcileUtils.validateAndCompareResourceVersions("11", "")); - assertThrows( - NonComparableResourceVersionException.class, - () -> ReconcileUtils.validateAndCompareResourceVersions("01", "123")); - assertThrows( - NonComparableResourceVersionException.class, - () -> ReconcileUtils.validateAndCompareResourceVersions("123", "01")); - assertThrows( - NonComparableResourceVersionException.class, - () -> ReconcileUtils.validateAndCompareResourceVersions("3213", "123a")); - assertThrows( - NonComparableResourceVersionException.class, - () -> ReconcileUtils.validateAndCompareResourceVersions("321", "123a")); - } - - @Test - void compareResourceVersionsWithStrings() { - // Test equal versions - assertThat(ReconcileUtils.compareResourceVersions("1", "1")).isZero(); - assertThat(ReconcileUtils.compareResourceVersions("123", "123")).isZero(); - - // Test different lengths - shorter version is less than longer version - assertThat(ReconcileUtils.compareResourceVersions("1", "12")).isNegative(); - assertThat(ReconcileUtils.compareResourceVersions("12", "1")).isPositive(); - assertThat(ReconcileUtils.compareResourceVersions("99", "100")).isNegative(); - assertThat(ReconcileUtils.compareResourceVersions("100", "99")).isPositive(); - assertThat(ReconcileUtils.compareResourceVersions("9", "100")).isNegative(); - assertThat(ReconcileUtils.compareResourceVersions("100", "9")).isPositive(); - - // Test same length - lexicographic comparison - assertThat(ReconcileUtils.compareResourceVersions("1", "2")).isNegative(); - assertThat(ReconcileUtils.compareResourceVersions("2", "1")).isPositive(); - assertThat(ReconcileUtils.compareResourceVersions("11", "12")).isNegative(); - assertThat(ReconcileUtils.compareResourceVersions("12", "11")).isPositive(); - assertThat(ReconcileUtils.compareResourceVersions("99", "100")).isNegative(); - assertThat(ReconcileUtils.compareResourceVersions("100", "99")).isPositive(); - assertThat(ReconcileUtils.compareResourceVersions("123", "124")).isNegative(); - assertThat(ReconcileUtils.compareResourceVersions("124", "123")).isPositive(); - - // Test with non-numeric strings (algorithm should still work character-wise) - assertThat(ReconcileUtils.compareResourceVersions("a", "b")).isNegative(); - assertThat(ReconcileUtils.compareResourceVersions("b", "a")).isPositive(); - assertThat(ReconcileUtils.compareResourceVersions("abc", "abd")).isNegative(); - assertThat(ReconcileUtils.compareResourceVersions("abd", "abc")).isPositive(); - - // Test edge cases with larger numbers - assertThat(ReconcileUtils.compareResourceVersions("1234567890", "1234567891")).isNegative(); - assertThat(ReconcileUtils.compareResourceVersions("1234567891", "1234567890")).isPositive(); - } - - @Test - void compareResourceVersionsWithHasMetadata() { - // Test equal versions - HasMetadata resource1 = createResourceWithVersion("123"); - HasMetadata resource2 = createResourceWithVersion("123"); - assertThat(ReconcileUtils.compareResourceVersions(resource1, resource2)).isZero(); - - // Test different lengths - resource1 = createResourceWithVersion("1"); - resource2 = createResourceWithVersion("12"); - assertThat(ReconcileUtils.compareResourceVersions(resource1, resource2)).isNegative(); - assertThat(ReconcileUtils.compareResourceVersions(resource2, resource1)).isPositive(); - - // Test same length, different values - resource1 = createResourceWithVersion("100"); - resource2 = createResourceWithVersion("200"); - assertThat(ReconcileUtils.compareResourceVersions(resource1, resource2)).isNegative(); - assertThat(ReconcileUtils.compareResourceVersions(resource2, resource1)).isPositive(); - - // Test realistic Kubernetes resource versions - resource1 = createResourceWithVersion("12345"); - resource2 = createResourceWithVersion("12346"); - assertThat(ReconcileUtils.compareResourceVersions(resource1, resource2)).isNegative(); - assertThat(ReconcileUtils.compareResourceVersions(resource2, resource1)).isPositive(); - } - - private HasMetadata createResourceWithVersion(String resourceVersion) { - return new PodBuilder() - .withMetadata( - new ObjectMetaBuilder() - .withName("test-pod") - .withNamespace("default") - .withResourceVersion(resourceVersion) - .build()) - .build(); - } - @Test void addsFinalizer() { var resource = TestUtils.testCustomResource1(); @@ -439,26 +325,4 @@ void resourcePatchThrowsWhenEventSourceIsNotManagedInformer() { assertThat(exception.getMessage()).contains("Target event source must be a subclass off"); assertThat(exception.getMessage()).contains("ManagedInformerEventSource"); } - - // naive performance test that compares the work case scenario for the parsing and non-parsing - // variants - @Test - @Disabled - public void compareResourcePerformanceTest() { - var execNum = 30000000; - var startTime = System.currentTimeMillis(); - for (int i = 0; i < execNum; i++) { - var res = ReconcileUtils.compareResourceVersions("123456788" + i, "123456789" + i); - } - var dur1 = System.currentTimeMillis() - startTime; - log.info("Duration without parsing: {}", dur1); - startTime = System.currentTimeMillis(); - for (int i = 0; i < execNum; i++) { - var res = Long.parseLong("123456788" + i) > Long.parseLong("123456789" + i); - } - var dur2 = System.currentTimeMillis() - startTime; - log.info("Duration with parsing: {}", dur2); - - assertThat(dur1).isLessThan(dur2); - } }