From 3aa6d17bdb1f8afa7e9ae4812e0842c73aadf1f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Fri, 11 Apr 2025 10:25:18 +0200 Subject: [PATCH 01/41] feat: resource cache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../processing/support/ResourceCache.java | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/support/ResourceCache.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/support/ResourceCache.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/support/ResourceCache.java new file mode 100644 index 0000000000..0f15114969 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/support/ResourceCache.java @@ -0,0 +1,55 @@ +package io.javaoperatorsdk.operator.processing.support; + +import java.util.concurrent.ConcurrentHashMap; +import java.util.function.BiPredicate; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.javaoperatorsdk.operator.processing.event.ResourceID; + +public class ResourceCache

{ + + private BiPredicate, P> evictionPredicate; + private ConcurrentHashMap> cache = new ConcurrentHashMap<>(); + + public ResourceCache(BiPredicate, P> evictionPredicate) { + this.evictionPredicate = evictionPredicate; + } + + public void cacheResource(P beforeUpdate, P afterUpdate) { + var resourceId = ResourceID.fromResource(beforeUpdate); + cache.put(resourceId, new Pair<>(beforeUpdate, afterUpdate)); + } + + public P getFreshResource(P newVersion) { + var resourceId = ResourceID.fromResource(newVersion); + var pair = cache.get(resourceId); + if (pair == null) { + return newVersion; + } + if (evictionPredicate.test(pair, newVersion)) { + cache.remove(resourceId); + return newVersion; + } else { + return pair.afterUpdate(); + } + } + + public record Pair(T beforeUpdate, T afterUpdate) {} + + public static class ResourceVersionComparePredicate + implements BiPredicate, T> { + @Override + public boolean test(Pair updatePair, T newVersion) { + return Long.parseLong(updatePair.afterUpdate().getMetadata().getResourceVersion()) + <= Long.parseLong(newVersion.getMetadata().getResourceVersion()); + } + } + + public static class ResourceVersionEqualityPredicate + implements BiPredicate, T> { + @Override + public boolean test(Pair updatePair, T newVersion) { + return !updatePair.beforeUpdate().equals(newVersion.getMetadata().getResourceVersion()); + } + } +} From 0125d66b9d288bc0456d5db43acf7491786e19eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Fri, 11 Apr 2025 10:28:56 +0200 Subject: [PATCH 02/41] wip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../operator/processing/support/ResourceCache.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/support/ResourceCache.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/support/ResourceCache.java index 0f15114969..c0c178f5cd 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/support/ResourceCache.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/support/ResourceCache.java @@ -15,6 +15,11 @@ public ResourceCache(BiPredicate, P> evictionPredicate) { this.evictionPredicate = evictionPredicate; } + public void cacheResource(P afterUpdate) { + var resourceId = ResourceID.fromResource(afterUpdate); + cache.put(resourceId, new Pair<>(null, afterUpdate)); + } + public void cacheResource(P beforeUpdate, P afterUpdate) { var resourceId = ResourceID.fromResource(beforeUpdate); cache.put(resourceId, new Pair<>(beforeUpdate, afterUpdate)); @@ -26,6 +31,10 @@ public P getFreshResource(P newVersion) { if (pair == null) { return newVersion; } + if (!newVersion.getMetadata().getUid().equals(pair.afterUpdate().getMetadata().getUid())) { + cache.remove(resourceId); + return newVersion; + } if (evictionPredicate.test(pair, newVersion)) { cache.remove(resourceId); return newVersion; From 8870c146abbb7851d0335c7d762e6d44e95ad116 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Mon, 14 Apr 2025 12:12:49 +0200 Subject: [PATCH 03/41] wip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../PrimaryUpdateAndCacheUtils.java | 25 +++++++++++++++++++ .../reconciler}/support/ResourceCache.java | 2 +- .../processing/event/EventSourceManager.java | 1 + .../event/EventSourceRetriever.java | 3 +++ 4 files changed, 30 insertions(+), 1 deletion(-) create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java rename operator-framework-core/src/main/java/io/javaoperatorsdk/operator/{processing => api/reconciler}/support/ResourceCache.java (97%) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java new file mode 100644 index 0000000000..511c834518 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java @@ -0,0 +1,25 @@ +package io.javaoperatorsdk.operator.api.reconciler; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.javaoperatorsdk.operator.processing.event.ResourceID; + +public class PrimaryUpdateAndCacheUtils { + + public static

P updateAndCacheStatus(P primary, Context

context) { + if (primary.getMetadata().getResourceVersion() == null) { + throw new IllegalStateException( + "Primary resource version is null, it is expected to set resource version for updates" + + " with for cache"); + } + var updatedResource = context.getClient().resource(primary).updateStatus(); + context + .eventSourceRetriever() + .getControllerEventSource() + .handleRecentResourceUpdate(ResourceID.fromResource(primary), updatedResource, primary); + return updatedResource; + } + + public static

P patchAndCacheStatus(P primary) { + return null; + } +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/support/ResourceCache.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/support/ResourceCache.java similarity index 97% rename from operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/support/ResourceCache.java rename to operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/support/ResourceCache.java index c0c178f5cd..11ac1636d0 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/support/ResourceCache.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/support/ResourceCache.java @@ -1,4 +1,4 @@ -package io.javaoperatorsdk.operator.processing.support; +package io.javaoperatorsdk.operator.api.reconciler.support; import java.util.concurrent.ConcurrentHashMap; import java.util.function.BiPredicate; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java index 02b91f6dd0..8b07bf110b 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java @@ -208,6 +208,7 @@ public Stream> getEventSourcesStream() { return eventSources.flatMappedSources(); } + @Override public ControllerEventSource

getControllerEventSource() { return eventSources.controllerEventSource(); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceRetriever.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceRetriever.java index 066a7f5808..c5a219a026 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceRetriever.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceRetriever.java @@ -6,6 +6,7 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.api.reconciler.EventSourceContext; import io.javaoperatorsdk.operator.processing.event.source.EventSource; +import io.javaoperatorsdk.operator.processing.event.source.controller.ControllerEventSource; public interface EventSourceRetriever

{ @@ -17,6 +18,8 @@ default EventSource getEventSourceFor(Class dependentType) { List> getEventSourcesFor(Class dependentType); + ControllerEventSource

getControllerEventSource(); + /** * Registers (and starts) the specified {@link EventSource} dynamically during the reconciliation. * If an EventSource is already registered with the specified name, the registration will be From bff907cf971be912f3f213eea8d80ac1234a47b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Mon, 14 Apr 2025 12:44:00 +0200 Subject: [PATCH 04/41] wip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../api/reconciler/PrimaryUpdateAndCacheUtils.java | 12 +++++++++++- ...ourceCache.java => UserPrimaryResourceCache.java} | 10 +++++----- ...va => TemporaryUserPrimaryResourceCacheTest.java} | 2 +- 3 files changed, 17 insertions(+), 7 deletions(-) rename operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/support/{ResourceCache.java => UserPrimaryResourceCache.java} (86%) rename operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/{TemporaryResourceCacheTest.java => TemporaryUserPrimaryResourceCacheTest.java} (99%) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java index 511c834518..39426f2805 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java @@ -1,6 +1,8 @@ package io.javaoperatorsdk.operator.api.reconciler; import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.client.KubernetesClient; +import io.javaoperatorsdk.operator.api.reconciler.support.UserPrimaryResourceCache; import io.javaoperatorsdk.operator.processing.event.ResourceID; public class PrimaryUpdateAndCacheUtils { @@ -19,7 +21,15 @@ public static

P updateAndCacheStatus(P primary, Context< return updatedResource; } - public static

P patchAndCacheStatus(P primary) { + public static

P patchAndCacheStatus( + P primary, Context

context, UserPrimaryResourceCache

cache) { + + return null; + } + + public static

P patchAndCacheStatus( + P primary, KubernetesClient client, UserPrimaryResourceCache

cache) { + return null; } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/support/ResourceCache.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/support/UserPrimaryResourceCache.java similarity index 86% rename from operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/support/ResourceCache.java rename to operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/support/UserPrimaryResourceCache.java index 11ac1636d0..89678a2c89 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/support/ResourceCache.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/support/UserPrimaryResourceCache.java @@ -6,13 +6,13 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.processing.event.ResourceID; -public class ResourceCache

{ +public class UserPrimaryResourceCache

{ - private BiPredicate, P> evictionPredicate; + private BiPredicate, P> evictionCondition; private ConcurrentHashMap> cache = new ConcurrentHashMap<>(); - public ResourceCache(BiPredicate, P> evictionPredicate) { - this.evictionPredicate = evictionPredicate; + public UserPrimaryResourceCache(BiPredicate, P> evictionCondition) { + this.evictionCondition = evictionCondition; } public void cacheResource(P afterUpdate) { @@ -35,7 +35,7 @@ public P getFreshResource(P newVersion) { cache.remove(resourceId); return newVersion; } - if (evictionPredicate.test(pair, newVersion)) { + if (evictionCondition.test(pair, newVersion)) { cache.remove(resourceId); return newVersion; } else { diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryResourceCacheTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryUserPrimaryResourceCacheTest.java similarity index 99% rename from operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryResourceCacheTest.java rename to operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryUserPrimaryResourceCacheTest.java index d31408beb6..c88901c8f5 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryResourceCacheTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryUserPrimaryResourceCacheTest.java @@ -19,7 +19,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -class TemporaryResourceCacheTest { +class TemporaryUserPrimaryResourceCacheTest { public static final String RESOURCE_VERSION = "2"; From 32823e0b51147f2175bbdb12fcee9b1823527b23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Mon, 14 Apr 2025 13:28:21 +0200 Subject: [PATCH 05/41] wip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../PrimaryUpdateAndCacheUtils.java | 32 ++++++++++++++++--- .../support/UserPrimaryResourceCache.java | 20 +++++++----- .../informer/TemporaryResourceCache.java | 8 ++--- 3 files changed, 44 insertions(+), 16 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java index 39426f2805..6209c7b488 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java @@ -1,5 +1,10 @@ package io.javaoperatorsdk.operator.api.reconciler; +import java.util.function.BiFunction; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.client.KubernetesClient; import io.javaoperatorsdk.operator.api.reconciler.support.UserPrimaryResourceCache; @@ -7,6 +12,8 @@ public class PrimaryUpdateAndCacheUtils { + private static final Logger log = LoggerFactory.getLogger(PrimaryUpdateAndCacheUtils.class); + public static

P updateAndCacheStatus(P primary, Context

context) { if (primary.getMetadata().getResourceVersion() == null) { throw new IllegalStateException( @@ -24,12 +31,29 @@ public static

P updateAndCacheStatus(P primary, Context< public static

P patchAndCacheStatus( P primary, Context

context, UserPrimaryResourceCache

cache) { - return null; + return patchAndCacheStatus( + primary, + context.getClient(), + cache, + (P p, KubernetesClient c) -> { + if (context + .getControllerConfiguration() + .getConfigurationService() + .useSSAToPatchPrimaryResource()) { + return c.resource(p).serverSideApply(); + } else { + return c.resource(p).patchStatus(); + } + }); } public static

P patchAndCacheStatus( - P primary, KubernetesClient client, UserPrimaryResourceCache

cache) { - - return null; + P primary, + KubernetesClient client, + UserPrimaryResourceCache

cache, + BiFunction patch) { + var updatedResource = patch.apply(primary, client); + cache.cacheResource(primary, updatedResource); + return updatedResource; } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/support/UserPrimaryResourceCache.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/support/UserPrimaryResourceCache.java index 89678a2c89..3b5f88e6b3 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/support/UserPrimaryResourceCache.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/support/UserPrimaryResourceCache.java @@ -8,11 +8,11 @@ public class UserPrimaryResourceCache

{ - private BiPredicate, P> evictionCondition; - private ConcurrentHashMap> cache = new ConcurrentHashMap<>(); + private final BiPredicate, P> evictionPredicate; + private final ConcurrentHashMap> cache = new ConcurrentHashMap<>(); - public UserPrimaryResourceCache(BiPredicate, P> evictionCondition) { - this.evictionCondition = evictionCondition; + public UserPrimaryResourceCache(BiPredicate, P> evictionPredicate) { + this.evictionPredicate = evictionPredicate; } public void cacheResource(P afterUpdate) { @@ -35,7 +35,7 @@ public P getFreshResource(P newVersion) { cache.remove(resourceId); return newVersion; } - if (evictionCondition.test(pair, newVersion)) { + if (evictionPredicate.test(pair, newVersion)) { cache.remove(resourceId); return newVersion; } else { @@ -45,7 +45,7 @@ public P getFreshResource(P newVersion) { public record Pair(T beforeUpdate, T afterUpdate) {} - public static class ResourceVersionComparePredicate + public static class ResourceVersionParsingEvictionPredicate implements BiPredicate, T> { @Override public boolean test(Pair updatePair, T newVersion) { @@ -54,11 +54,15 @@ public boolean test(Pair updatePair, T newVersion) { } } - public static class ResourceVersionEqualityPredicate + public static class EqualityPredicateForOptimisticUpdate implements BiPredicate, T> { @Override public boolean test(Pair updatePair, T newVersion) { - return !updatePair.beforeUpdate().equals(newVersion.getMetadata().getResourceVersion()); + return !updatePair + .beforeUpdate() + .getMetadata() + .getResourceVersion() + .equals(newVersion.getMetadata().getResourceVersion()); } } } 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 247cdb9aa5..9ec5b3694c 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 @@ -9,7 +9,7 @@ import org.slf4j.LoggerFactory; import io.fabric8.kubernetes.api.model.HasMetadata; -import io.javaoperatorsdk.operator.api.config.informer.InformerEventSourceConfiguration; +import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource; import io.javaoperatorsdk.operator.processing.event.ResourceID; @@ -167,9 +167,9 @@ public synchronized boolean isKnownResourceVersion(T resource) { } /** - * @return true if {@link InformerEventSourceConfiguration#parseResourceVersions()} is enabled and - * the resourceVersion of newResource is numerically greater than cachedResource, otherwise - * false + * @return true if {@link ConfigurationService#parseResourceVersionsForEventFilteringAndCaching()} + * is enabled and the resourceVersion of newResource is numerically greater than + * cachedResource, otherwise false */ private boolean isLaterResourceVersion(ResourceID resourceId, T newResource, T cachedResource) { try { From b016bf0ae6d7421bd9ff71a781d3e955a35136f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Mon, 14 Apr 2025 17:04:58 +0200 Subject: [PATCH 06/41] wip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../PrimaryUpdateAndCacheUtils.java | 96 +++++++++++++++---- .../support/UserPrimaryResourceCache.java | 16 +--- 2 files changed, 81 insertions(+), 31 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java index 6209c7b488..8c8bc16900 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java @@ -1,50 +1,90 @@ package io.javaoperatorsdk.operator.api.reconciler; import java.util.function.BiFunction; +import java.util.function.UnaryOperator; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.client.KubernetesClient; +import io.fabric8.kubernetes.client.dsl.base.PatchContext; +import io.fabric8.kubernetes.client.dsl.base.PatchType; import io.javaoperatorsdk.operator.api.reconciler.support.UserPrimaryResourceCache; import io.javaoperatorsdk.operator.processing.event.ResourceID; public class PrimaryUpdateAndCacheUtils { + private PrimaryUpdateAndCacheUtils() {} + private static final Logger log = LoggerFactory.getLogger(PrimaryUpdateAndCacheUtils.class); public static

P updateAndCacheStatus(P primary, Context

context) { - if (primary.getMetadata().getResourceVersion() == null) { - throw new IllegalStateException( - "Primary resource version is null, it is expected to set resource version for updates" - + " with for cache"); - } - var updatedResource = context.getClient().resource(primary).updateStatus(); + return patchAndCacheStatusWithLock( + primary, context, (p, c) -> c.resource(primary).updateStatus()); + } + + public static

P patchAndCacheStatusWithLock( + P primary, Context

context) { + return patchAndCacheStatusWithLock( + primary, context, (p, c) -> c.resource(primary).patchStatus()); + } + + public static

P editAndCacheStatusWithLock( + P primary, Context

context, UnaryOperator

operation) { + return patchAndCacheStatusWithLock( + primary, context, (p, c) -> c.resource(primary).editStatus(operation)); + } + + public static

P patchAndCacheStatusWithLock( + P primary, Context

context, BiFunction patch) { + checkResourceVersionPresent(primary); + var updatedResource = patch.apply(primary, context.getClient()); context .eventSourceRetriever() .getControllerEventSource() .handleRecentResourceUpdate(ResourceID.fromResource(primary), updatedResource, primary); - return updatedResource; + return null; } - public static

P patchAndCacheStatus( - P primary, Context

context, UserPrimaryResourceCache

cache) { + public static

P ssaPatchAndCacheStatusWithLock( + P primary, P freshResourceWithStatus, Context

context) { + checkResourceVersionPresent(freshResourceWithStatus); + var res = + context + .getClient() + .resource(freshResourceWithStatus) + .subresource("status") + .patch( + new PatchContext.Builder() + .withForce(true) + .withFieldManager(context.getControllerConfiguration().fieldManager()) + .withPatchType(PatchType.SERVER_SIDE_APPLY) + .build()); + context + .eventSourceRetriever() + .getControllerEventSource() + .handleRecentResourceUpdate(ResourceID.fromResource(primary), res, primary); + return res; + } + + public static

P ssaPatchAndCacheStatus( + P primary, P freshResource, Context

context, UserPrimaryResourceCache

cache) { + logWarnIfResourceVersionPresent(freshResource); return patchAndCacheStatus( primary, context.getClient(), cache, - (P p, KubernetesClient c) -> { - if (context - .getControllerConfiguration() - .getConfigurationService() - .useSSAToPatchPrimaryResource()) { - return c.resource(p).serverSideApply(); - } else { - return c.resource(p).patchStatus(); - } - }); + (P p, KubernetesClient c) -> + c.resource(freshResource) + .subresource("status") + .patch( + new PatchContext.Builder() + .withForce(true) + .withFieldManager(context.getControllerConfiguration().fieldManager()) + .withPatchType(PatchType.SERVER_SIDE_APPLY) + .build())); } public static

P patchAndCacheStatus( @@ -56,4 +96,22 @@ public static

P patchAndCacheStatus( cache.cacheResource(primary, updatedResource); return updatedResource; } + + private static

void checkResourceVersionPresent(P primary) { + if (primary.getMetadata().getResourceVersion() == null) { + throw new IllegalStateException( + "Primary resource version is null, it is expected to set resource version for updates for caching. Name: %s namespace: %s" + .formatted(primary.getMetadata().getName(), primary.getMetadata().getNamespace())); + } + } + + private static

void logWarnIfResourceVersionPresent(P primary) { + if (primary.getMetadata().getResourceVersion() != null) { + log.warn( + "Primary resource version is NOT null, for caching with optimistic locking use" + + " alternative methods. Name: {} namespace: {}", + primary.getMetadata().getName(), + primary.getMetadata().getNamespace()); + } + } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/support/UserPrimaryResourceCache.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/support/UserPrimaryResourceCache.java index 3b5f88e6b3..624b0b7787 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/support/UserPrimaryResourceCache.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/support/UserPrimaryResourceCache.java @@ -43,6 +43,10 @@ public P getFreshResource(P newVersion) { } } + public void cleanup(P resource) { + cache.remove(ResourceID.fromResource(resource)); + } + public record Pair(T beforeUpdate, T afterUpdate) {} public static class ResourceVersionParsingEvictionPredicate @@ -53,16 +57,4 @@ public boolean test(Pair updatePair, T newVersion) { <= Long.parseLong(newVersion.getMetadata().getResourceVersion()); } } - - public static class EqualityPredicateForOptimisticUpdate - implements BiPredicate, T> { - @Override - public boolean test(Pair updatePair, T newVersion) { - return !updatePair - .beforeUpdate() - .getMetadata() - .getResourceVersion() - .equals(newVersion.getMetadata().getResourceVersion()); - } - } } From 00fd9e6be09c65990a045bd7b158d2d19fc22b28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Mon, 14 Apr 2025 17:46:47 +0200 Subject: [PATCH 07/41] wip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- operator-framework-core/pom.xml | 5 ++ .../PrimaryUpdateAndCacheUtils.java | 7 +-- ...ceCache.java => PrimaryResourceCache.java} | 4 +- .../support/PrimaryResourceCacheTest.java | 52 +++++++++++++++++++ ...=> TemporaryPrimaryResourceCacheTest.java} | 2 +- 5 files changed, 64 insertions(+), 6 deletions(-) rename operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/support/{UserPrimaryResourceCache.java => PrimaryResourceCache.java} (93%) create mode 100644 operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/support/PrimaryResourceCacheTest.java rename operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/{TemporaryUserPrimaryResourceCacheTest.java => TemporaryPrimaryResourceCacheTest.java} (99%) diff --git a/operator-framework-core/pom.xml b/operator-framework-core/pom.xml index cad50ebc32..da6d3395a3 100644 --- a/operator-framework-core/pom.xml +++ b/operator-framework-core/pom.xml @@ -79,6 +79,11 @@ awaitility test + + io.fabric8 + kube-api-test-client-inject + test + diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java index 8c8bc16900..b934138ed3 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java @@ -10,9 +10,10 @@ import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.dsl.base.PatchContext; import io.fabric8.kubernetes.client.dsl.base.PatchType; -import io.javaoperatorsdk.operator.api.reconciler.support.UserPrimaryResourceCache; +import io.javaoperatorsdk.operator.api.reconciler.support.PrimaryResourceCache; import io.javaoperatorsdk.operator.processing.event.ResourceID; +// todo javadoc public class PrimaryUpdateAndCacheUtils { private PrimaryUpdateAndCacheUtils() {} @@ -70,7 +71,7 @@ public static

P ssaPatchAndCacheStatusWithLock( } public static

P ssaPatchAndCacheStatus( - P primary, P freshResource, Context

context, UserPrimaryResourceCache

cache) { + P primary, P freshResource, Context

context, PrimaryResourceCache

cache) { logWarnIfResourceVersionPresent(freshResource); return patchAndCacheStatus( primary, @@ -90,7 +91,7 @@ public static

P ssaPatchAndCacheStatus( public static

P patchAndCacheStatus( P primary, KubernetesClient client, - UserPrimaryResourceCache

cache, + PrimaryResourceCache

cache, BiFunction patch) { var updatedResource = patch.apply(primary, client); cache.cacheResource(primary, updatedResource); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/support/UserPrimaryResourceCache.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/support/PrimaryResourceCache.java similarity index 93% rename from operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/support/UserPrimaryResourceCache.java rename to operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/support/PrimaryResourceCache.java index 624b0b7787..9197cb8744 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/support/UserPrimaryResourceCache.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/support/PrimaryResourceCache.java @@ -6,12 +6,12 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.processing.event.ResourceID; -public class UserPrimaryResourceCache

{ +public class PrimaryResourceCache

{ private final BiPredicate, P> evictionPredicate; private final ConcurrentHashMap> cache = new ConcurrentHashMap<>(); - public UserPrimaryResourceCache(BiPredicate, P> evictionPredicate) { + public PrimaryResourceCache(BiPredicate, P> evictionPredicate) { this.evictionPredicate = evictionPredicate; } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/support/PrimaryResourceCacheTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/support/PrimaryResourceCacheTest.java new file mode 100644 index 0000000000..6511b8195d --- /dev/null +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/support/PrimaryResourceCacheTest.java @@ -0,0 +1,52 @@ +package io.javaoperatorsdk.operator.api.reconciler.support; + +import org.junit.jupiter.api.Test; + +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.javaoperatorsdk.operator.sample.simple.TestCustomResource; +import io.javaoperatorsdk.operator.sample.simple.TestCustomResourceSpec; + +import static org.assertj.core.api.Assertions.assertThat; + +class PrimaryResourceCacheTest { + + @Test + void flowWithResourceVersionParsingEvictionPredicate() { + var cache = + new PrimaryResourceCache( + new PrimaryResourceCache.ResourceVersionParsingEvictionPredicate<>()); + + var newCR = customResource("2"); + var cr = cache.getFreshResource(newCR); + assertThat(cr).isSameAs(newCR); + // todo break these down by spec + cache.cacheResource(newCR); + cr = cache.getFreshResource(customResource("1")); + + assertThat(cr).isSameAs(newCR); + + var newestCR = customResource("3"); + cr = cache.getFreshResource(newestCR); + + assertThat(cr).isSameAs(newestCR); + } + + @Test + void customResourceSpecificEvictionPredicate() { + // todo + } + + private TestCustomResource customResource(String resourceVersion) { + var cr = new TestCustomResource(); + cr.setMetadata( + new ObjectMetaBuilder() + .withName("test1") + .withNamespace("default") + .withUid("uid") + .withResourceVersion(resourceVersion) + .build()); + cr.setSpec(new TestCustomResourceSpec()); + cr.getSpec().setKey("key"); + return cr; + } +} diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryUserPrimaryResourceCacheTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryPrimaryResourceCacheTest.java similarity index 99% rename from operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryUserPrimaryResourceCacheTest.java rename to operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryPrimaryResourceCacheTest.java index c88901c8f5..e62888832f 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryUserPrimaryResourceCacheTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryPrimaryResourceCacheTest.java @@ -19,7 +19,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -class TemporaryUserPrimaryResourceCacheTest { +class TemporaryPrimaryResourceCacheTest { public static final String RESOURCE_VERSION = "2"; From 3b99f7803fe0e0c8a860b34e4294c155808f8498 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Tue, 15 Apr 2025 14:08:02 +0200 Subject: [PATCH 08/41] Integration tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../PrimaryUpdateAndCacheUtils.java | 128 +++++++++++++++++- .../support/PrimaryResourceCacheTest.java | 55 +++++--- .../PeriodicTriggerEventSource.java | 55 ++++++++ ...StatusPatchPrimaryCacheCustomResource.java | 14 ++ .../StatusPatchPrimaryCacheIT.java | 48 +++++++ .../StatusPatchPrimaryCacheReconciler.java | 91 +++++++++++++ .../StatusPatchPrimaryCacheSpec.java | 15 ++ .../StatusPatchPrimaryCacheStatus.java | 15 ++ ...tatusPatchCacheWithLockCustomResource.java | 14 ++ .../withlock/StatusPatchCacheWithLockIT.java | 48 +++++++ .../StatusPatchCacheWithLockReconciler.java | 70 ++++++++++ .../StatusPatchCacheWithLockSpec.java | 14 ++ .../StatusPatchCacheWithLockStatus.java | 15 ++ 13 files changed, 561 insertions(+), 21 deletions(-) create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/PeriodicTriggerEventSource.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheCustomResource.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheIT.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheReconciler.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheSpec.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheStatus.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockCustomResource.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockIT.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockReconciler.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockSpec.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockStatus.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java index b934138ed3..68630f8c91 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java @@ -13,30 +13,67 @@ import io.javaoperatorsdk.operator.api.reconciler.support.PrimaryResourceCache; import io.javaoperatorsdk.operator.processing.event.ResourceID; -// todo javadoc public class PrimaryUpdateAndCacheUtils { private PrimaryUpdateAndCacheUtils() {} private static final Logger log = LoggerFactory.getLogger(PrimaryUpdateAndCacheUtils.class); - public static

P updateAndCacheStatus(P primary, Context

context) { + /** + * Makes sure that the up-to-date primary resource will be present during the next reconciliation. + * Using update (PUT) method. + * + * @param primary resource + * @param context of reconciliation + * @return updated resource + * @param

primary resource type + */ + public static

P updateAndCacheStatusWithLock( + P primary, Context

context) { return patchAndCacheStatusWithLock( primary, context, (p, c) -> c.resource(primary).updateStatus()); } + /** + * Makes sure that the up-to-date primary resource will be present during the next reconciliation. + * Using JSON Merge patch. + * + * @param primary resource + * @param context of reconciliation + * @return updated resource + * @param

primary resource type + */ public static

P patchAndCacheStatusWithLock( P primary, Context

context) { return patchAndCacheStatusWithLock( primary, context, (p, c) -> c.resource(primary).patchStatus()); } + /** + * Makes sure that the up-to-date primary resource will be present during the next reconciliation. + * Using JSON Patch. + * + * @param primary resource + * @param context of reconciliation + * @return updated resource + * @param

primary resource type + */ public static

P editAndCacheStatusWithLock( P primary, Context

context, UnaryOperator

operation) { return patchAndCacheStatusWithLock( primary, context, (p, c) -> c.resource(primary).editStatus(operation)); } + /** + * Makes sure that the up-to-date primary resource will be present during the next reconciliation. + * + * @param primary resource + * @param context of reconciliation + * @param patch free implementation of cache - make sure you use optimistic locking during the + * update + * @return the updated resource. + * @param

primary resource type + */ public static

P patchAndCacheStatusWithLock( P primary, Context

context, BiFunction patch) { checkResourceVersionPresent(primary); @@ -48,6 +85,16 @@ public static

P patchAndCacheStatusWithLock( return null; } + /** + * Makes sure that the up-to-date primary resource will be present during the next reconciliation. + * Using Server Side Apply. + * + * @param primary resource + * @param freshResourceWithStatus - fresh resource with target state + * @param context of reconciliation + * @return the updated resource. + * @param

primary resource type + */ public static

P ssaPatchAndCacheStatusWithLock( P primary, P freshResourceWithStatus, Context

context) { checkResourceVersionPresent(freshResourceWithStatus); @@ -70,15 +117,26 @@ public static

P ssaPatchAndCacheStatusWithLock( return res; } + /** + * Patches the resource and adds it to the {@link PrimaryResourceCache} provided. Optimistic + * locking is not required. + * + * @param primary resource + * @param freshResourceWithStatus - fresh resource with target state + * @param context of reconciliation + * @param cache - resource cache managed by user + * @return the updated resource. + * @param

primary resource type + */ public static

P ssaPatchAndCacheStatus( - P primary, P freshResource, Context

context, PrimaryResourceCache

cache) { - logWarnIfResourceVersionPresent(freshResource); + P primary, P freshResourceWithStatus, Context

context, PrimaryResourceCache

cache) { + logWarnIfResourceVersionPresent(freshResourceWithStatus); return patchAndCacheStatus( primary, context.getClient(), cache, (P p, KubernetesClient c) -> - c.resource(freshResource) + c.resource(freshResourceWithStatus) .subresource("status") .patch( new PatchContext.Builder() @@ -88,6 +146,66 @@ public static

P ssaPatchAndCacheStatus( .build())); } + /** + * Patches the resource with JSON Patch and adds it to the {@link PrimaryResourceCache} provided. + * Optimistic locking is not required. + * + * @param primary resource* + * @param context of reconciliation + * @param cache - resource cache managed by user + * @return the updated resource. + * @param

primary resource type + */ + public static

P edithAndCacheStatus( + P primary, Context

context, PrimaryResourceCache

cache, UnaryOperator

operation) { + logWarnIfResourceVersionPresent(primary); + return patchAndCacheStatus( + primary, + context.getClient(), + cache, + (P p, KubernetesClient c) -> c.resource(primary).editStatus(operation)); + } + + /** + * Patches the resource with JSON Merge patch and adds it to the {@link PrimaryResourceCache} + * provided. Optimistic locking is not required. + * + * @param primary resource* + * @param context of reconciliation + * @param cache - resource cache managed by user + * @return the updated resource. + * @param

primary resource type + */ + public static

P patchAndCacheStatus( + P primary, Context

context, PrimaryResourceCache

cache) { + logWarnIfResourceVersionPresent(primary); + return patchAndCacheStatus( + primary, + context.getClient(), + cache, + (P p, KubernetesClient c) -> c.resource(primary).patchStatus()); + } + + /** + * Updates the resource and adds it to the {@link PrimaryResourceCache} provided. Optimistic + * locking is not required. + * + * @param primary resource* + * @param context of reconciliation + * @param cache - resource cache managed by user + * @return the updated resource. + * @param

primary resource type + */ + public static

P updateAndCacheStatus( + P primary, Context

context, PrimaryResourceCache

cache) { + logWarnIfResourceVersionPresent(primary); + return patchAndCacheStatus( + primary, + context.getClient(), + cache, + (P p, KubernetesClient c) -> c.resource(primary).updateStatus()); + } + public static

P patchAndCacheStatus( P primary, KubernetesClient client, diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/support/PrimaryResourceCacheTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/support/PrimaryResourceCacheTest.java index 6511b8195d..9026c0d2cd 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/support/PrimaryResourceCacheTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/support/PrimaryResourceCacheTest.java @@ -10,30 +10,53 @@ class PrimaryResourceCacheTest { + PrimaryResourceCache versionParsingCache = + new PrimaryResourceCache<>( + new PrimaryResourceCache.ResourceVersionParsingEvictionPredicate<>()); + @Test - void flowWithResourceVersionParsingEvictionPredicate() { - var cache = - new PrimaryResourceCache( - new PrimaryResourceCache.ResourceVersionParsingEvictionPredicate<>()); + void returnsThePassedValueIfCacheIsEmpty() { + var cr = customResource("1"); + + var res = versionParsingCache.getFreshResource(cr); - var newCR = customResource("2"); - var cr = cache.getFreshResource(newCR); - assertThat(cr).isSameAs(newCR); - // todo break these down by spec - cache.cacheResource(newCR); - cr = cache.getFreshResource(customResource("1")); + assertThat(cr).isSameAs(res); + } - assertThat(cr).isSameAs(newCR); + @Test + void returnsTheCachedIfNotEvictedAccordingToPredicate() { + var cr = customResource("2"); - var newestCR = customResource("3"); - cr = cache.getFreshResource(newestCR); + versionParsingCache.cacheResource(cr); - assertThat(cr).isSameAs(newestCR); + var res = versionParsingCache.getFreshResource(customResource("1")); + assertThat(cr).isSameAs(res); } @Test - void customResourceSpecificEvictionPredicate() { - // todo + void ifMoreFreshPassedCachedIsEvicted() { + var cr = customResource("2"); + versionParsingCache.cacheResource(cr); + var newCR = customResource("3"); + + var res = versionParsingCache.getFreshResource(newCR); + var resOnOlder = versionParsingCache.getFreshResource(cr); + + assertThat(newCR).isSameAs(res); + assertThat(resOnOlder).isSameAs(cr); + assertThat(newCR).isNotSameAs(cr); + } + + @Test + void cleanupRemovesCachedResources() { + var cr = customResource("2"); + versionParsingCache.cacheResource(cr); + + versionParsingCache.cleanup(customResource("3")); + + var olderCR = customResource("1"); + var res = versionParsingCache.getFreshResource(olderCR); + assertThat(olderCR).isSameAs(res); } private TestCustomResource customResource(String resourceVersion) { diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/PeriodicTriggerEventSource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/PeriodicTriggerEventSource.java new file mode 100644 index 0000000000..5402e72a52 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/PeriodicTriggerEventSource.java @@ -0,0 +1,55 @@ +package io.javaoperatorsdk.operator.baseapi.statuscache; + +import java.util.Set; +import java.util.Timer; +import java.util.TimerTask; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.javaoperatorsdk.operator.OperatorException; +import io.javaoperatorsdk.operator.processing.event.Event; +import io.javaoperatorsdk.operator.processing.event.ResourceID; +import io.javaoperatorsdk.operator.processing.event.source.AbstractEventSource; +import io.javaoperatorsdk.operator.processing.event.source.IndexerResourceCache; + +public class PeriodicTriggerEventSource

+ extends AbstractEventSource { + + public static final int DEFAULT_PERIOD = 30; + private final Timer timer = new Timer(); + private final IndexerResourceCache

primaryCache; + private final int period; + + public PeriodicTriggerEventSource(IndexerResourceCache

primaryCache) { + this(primaryCache, DEFAULT_PERIOD); + } + + public PeriodicTriggerEventSource(IndexerResourceCache

primaryCache, int period) { + super(Void.class); + this.primaryCache = primaryCache; + this.period = period; + } + + @Override + public Set getSecondaryResources(P primary) { + return Set.of(); + } + + @Override + public void start() throws OperatorException { + super.start(); + timer.schedule( + new TimerTask() { + @Override + public void run() { + primaryCache + .list() + .forEach( + r -> { + getEventHandler().handleEvent(new Event(ResourceID.fromResource(r))); + }); + } + }, + 0, + period); + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheCustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheCustomResource.java new file mode 100644 index 0000000000..84b145cac3 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheCustomResource.java @@ -0,0 +1,14 @@ +package io.javaoperatorsdk.operator.baseapi.statuscache.primarycache; + +import io.fabric8.kubernetes.api.model.Namespaced; +import io.fabric8.kubernetes.client.CustomResource; +import io.fabric8.kubernetes.model.annotation.Group; +import io.fabric8.kubernetes.model.annotation.ShortNames; +import io.fabric8.kubernetes.model.annotation.Version; + +@Group("sample.javaoperatorsdk") +@Version("v1") +@ShortNames("spc") +public class StatusPatchPrimaryCacheCustomResource + extends CustomResource + implements Namespaced {} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheIT.java new file mode 100644 index 0000000000..6cf46cf17a --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheIT.java @@ -0,0 +1,48 @@ +package io.javaoperatorsdk.operator.baseapi.statuscache.primarycache; + +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.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + +public class StatusPatchPrimaryCacheIT { + + public static final String TEST_1 = "test1"; + + @RegisterExtension + LocallyRunOperatorExtension extension = + LocallyRunOperatorExtension.builder() + .withReconciler(StatusPatchPrimaryCacheReconciler.class) + .build(); + + @Test + void testStatusAlwaysUpToDate() { + var reconciler = extension.getReconcilerOfType(StatusPatchPrimaryCacheReconciler.class); + + extension.create(testResource()); + + // the reconciliation id periodically triggered, the status values should be increasing + // monotonically + await() + .pollDelay(Duration.ofSeconds(1)) + .pollInterval(Duration.ofMillis(30)) + .untilAsserted( + () -> { + assertThat(reconciler.errorPresent).isFalse(); + assertThat(reconciler.latestValue).isGreaterThan(10); + }); + } + + StatusPatchPrimaryCacheCustomResource testResource() { + var res = new StatusPatchPrimaryCacheCustomResource(); + res.setMetadata(new ObjectMetaBuilder().withName(TEST_1).build()); + res.setSpec(new StatusPatchPrimaryCacheSpec()); + return res; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheReconciler.java new file mode 100644 index 0000000000..bcb0a0a00a --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheReconciler.java @@ -0,0 +1,91 @@ +package io.javaoperatorsdk.operator.baseapi.statuscache.primarycache; + +import java.util.List; + +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.javaoperatorsdk.operator.api.reconciler.Cleaner; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.DeleteControl; +import io.javaoperatorsdk.operator.api.reconciler.EventSourceContext; +import io.javaoperatorsdk.operator.api.reconciler.PrimaryUpdateAndCacheUtils; +import io.javaoperatorsdk.operator.api.reconciler.Reconciler; +import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; +import io.javaoperatorsdk.operator.api.reconciler.support.PrimaryResourceCache; +import io.javaoperatorsdk.operator.baseapi.statuscache.PeriodicTriggerEventSource; +import io.javaoperatorsdk.operator.processing.event.source.EventSource; + +@ControllerConfiguration +public class StatusPatchPrimaryCacheReconciler + implements Reconciler, + Cleaner { + + public volatile int latestValue = 0; + public volatile boolean errorPresent = false; + + private PrimaryResourceCache cache = + new PrimaryResourceCache<>( + (statusPatchCacheCustomResourcePair, statusPatchCacheCustomResource) -> + statusPatchCacheCustomResource.getStatus().getValue() + >= statusPatchCacheCustomResourcePair.afterUpdate().getStatus().getValue()); + + @Override + public UpdateControl reconcile( + StatusPatchPrimaryCacheCustomResource primary, + Context context) + throws InterruptedException { + + primary = cache.getFreshResource(primary); + + if (primary.getStatus() != null && primary.getStatus().getValue() != latestValue) { + errorPresent = false; + throw new IllegalStateException( + "status is not up to date. Latest value: " + + latestValue + + " status values: " + + primary.getStatus().getValue()); + } + + var freshCopy = createFreshCopy(primary); + // setting the resource version + freshCopy.getMetadata().setResourceVersion(primary.getMetadata().getResourceVersion()); + freshCopy + .getStatus() + .setValue(primary.getStatus() == null ? 1 : primary.getStatus().getValue() + 1); + + var updated = + PrimaryUpdateAndCacheUtils.ssaPatchAndCacheStatus(primary, freshCopy, context, cache); + latestValue = updated.getStatus().getValue(); + + return UpdateControl.noUpdate(); + } + + @Override + public List> prepareEventSources( + EventSourceContext context) { + // periodic event triggering for testing purposes + return List.of(new PeriodicTriggerEventSource<>(context.getPrimaryCache())); + } + + private StatusPatchPrimaryCacheCustomResource createFreshCopy( + StatusPatchPrimaryCacheCustomResource resource) { + var res = new StatusPatchPrimaryCacheCustomResource(); + res.setMetadata( + new ObjectMetaBuilder() + .withName(resource.getMetadata().getName()) + .withNamespace(resource.getMetadata().getNamespace()) + .build()); + res.setStatus(new StatusPatchPrimaryCacheStatus()); + + return res; + } + + @Override + public DeleteControl cleanup( + StatusPatchPrimaryCacheCustomResource resource, + Context context) + throws Exception { + cache.cleanup(resource); + return DeleteControl.defaultDelete(); + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheSpec.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheSpec.java new file mode 100644 index 0000000000..90630c1ae8 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheSpec.java @@ -0,0 +1,15 @@ +package io.javaoperatorsdk.operator.baseapi.statuscache.primarycache; + +public class StatusPatchPrimaryCacheSpec { + + private boolean messageInStatus = true; + + public boolean isMessageInStatus() { + return messageInStatus; + } + + public StatusPatchPrimaryCacheSpec setMessageInStatus(boolean messageInStatus) { + this.messageInStatus = messageInStatus; + return this; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheStatus.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheStatus.java new file mode 100644 index 0000000000..0687d5576a --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheStatus.java @@ -0,0 +1,15 @@ +package io.javaoperatorsdk.operator.baseapi.statuscache.primarycache; + +public class StatusPatchPrimaryCacheStatus { + + private Integer value = 0; + + public Integer getValue() { + return value; + } + + public StatusPatchPrimaryCacheStatus setValue(Integer value) { + this.value = value; + return this; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockCustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockCustomResource.java new file mode 100644 index 0000000000..d84a992a13 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockCustomResource.java @@ -0,0 +1,14 @@ +package io.javaoperatorsdk.operator.baseapi.statuscache.withlock; + +import io.fabric8.kubernetes.api.model.Namespaced; +import io.fabric8.kubernetes.client.CustomResource; +import io.fabric8.kubernetes.model.annotation.Group; +import io.fabric8.kubernetes.model.annotation.ShortNames; +import io.fabric8.kubernetes.model.annotation.Version; + +@Group("sample.javaoperatorsdk") +@Version("v1") +@ShortNames("spcl") +public class StatusPatchCacheWithLockCustomResource + extends CustomResource + implements Namespaced {} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockIT.java new file mode 100644 index 0000000000..370dab81f3 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockIT.java @@ -0,0 +1,48 @@ +package io.javaoperatorsdk.operator.baseapi.statuscache.withlock; + +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.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + +public class StatusPatchCacheWithLockIT { + + public static final String TEST_1 = "test1"; + + @RegisterExtension + LocallyRunOperatorExtension extension = + LocallyRunOperatorExtension.builder() + .withReconciler(StatusPatchCacheWithLockReconciler.class) + .build(); + + @Test + void testStatusAlwaysUpToDate() { + var reconciler = extension.getReconcilerOfType(StatusPatchCacheWithLockReconciler.class); + + extension.create(testResource()); + + // the reconciliation id periodically triggered, the status values should be increasing + // monotonically + await() + .pollDelay(Duration.ofSeconds(1)) + .pollInterval(Duration.ofMillis(30)) + .untilAsserted( + () -> { + assertThat(reconciler.errorPresent).isFalse(); + assertThat(reconciler.latestValue).isGreaterThan(10); + }); + } + + StatusPatchCacheWithLockCustomResource testResource() { + var res = new StatusPatchCacheWithLockCustomResource(); + res.setMetadata(new ObjectMetaBuilder().withName(TEST_1).build()); + res.setSpec(new StatusPatchCacheWithLockSpec()); + return res; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockReconciler.java new file mode 100644 index 0000000000..2ecb98f087 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockReconciler.java @@ -0,0 +1,70 @@ +package io.javaoperatorsdk.operator.baseapi.statuscache.withlock; + +import java.util.List; + +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.EventSourceContext; +import io.javaoperatorsdk.operator.api.reconciler.PrimaryUpdateAndCacheUtils; +import io.javaoperatorsdk.operator.api.reconciler.Reconciler; +import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; +import io.javaoperatorsdk.operator.baseapi.statuscache.PeriodicTriggerEventSource; +import io.javaoperatorsdk.operator.processing.event.source.EventSource; + +@ControllerConfiguration +public class StatusPatchCacheWithLockReconciler + implements Reconciler { + + public volatile int latestValue = 0; + public volatile boolean errorPresent = false; + + @Override + public UpdateControl reconcile( + StatusPatchCacheWithLockCustomResource resource, + Context context) + throws InterruptedException { + + if (resource.getStatus() != null && resource.getStatus().getValue() != latestValue) { + errorPresent = false; + throw new IllegalStateException( + "status is not up to date. Latest value: " + + latestValue + + " status values: " + + resource.getStatus().getValue()); + } + + var freshCopy = createFreshCopy(resource); + // setting the resource version + freshCopy.getMetadata().setResourceVersion(resource.getMetadata().getResourceVersion()); + freshCopy + .getStatus() + .setValue(resource.getStatus() == null ? 1 : resource.getStatus().getValue() + 1); + + var updated = + PrimaryUpdateAndCacheUtils.ssaPatchAndCacheStatusWithLock(resource, freshCopy, context); + latestValue = updated.getStatus().getValue(); + + return UpdateControl.noUpdate(); + } + + @Override + public List> prepareEventSources( + EventSourceContext context) { + // periodic event triggering for testing purposes + return List.of(new PeriodicTriggerEventSource<>(context.getPrimaryCache())); + } + + private StatusPatchCacheWithLockCustomResource createFreshCopy( + StatusPatchCacheWithLockCustomResource resource) { + var res = new StatusPatchCacheWithLockCustomResource(); + res.setMetadata( + new ObjectMetaBuilder() + .withName(resource.getMetadata().getName()) + .withNamespace(resource.getMetadata().getNamespace()) + .build()); + res.setStatus(new StatusPatchCacheWithLockStatus()); + + return res; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockSpec.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockSpec.java new file mode 100644 index 0000000000..12cd1ac3e1 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockSpec.java @@ -0,0 +1,14 @@ +package io.javaoperatorsdk.operator.baseapi.statuscache.withlock; + +public class StatusPatchCacheWithLockSpec { + + private int counter = 0; + + public int getCounter() { + return counter; + } + + public void setCounter(int counter) { + this.counter = counter; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockStatus.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockStatus.java new file mode 100644 index 0000000000..8d1e559308 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockStatus.java @@ -0,0 +1,15 @@ +package io.javaoperatorsdk.operator.baseapi.statuscache.withlock; + +public class StatusPatchCacheWithLockStatus { + + private Integer value = 0; + + public Integer getValue() { + return value; + } + + public StatusPatchCacheWithLockStatus setValue(Integer value) { + this.value = value; + return this; + } +} From 1812851aea079a7620d7c047a3a9a145e8409cc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Tue, 15 Apr 2025 14:21:08 +0200 Subject: [PATCH 09/41] wip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../api/reconciler/support/PrimaryResourceCache.java | 5 +++++ .../baseapi/statuscache/PeriodicTriggerEventSource.java | 5 +---- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/support/PrimaryResourceCache.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/support/PrimaryResourceCache.java index 9197cb8744..4da73ab8b1 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/support/PrimaryResourceCache.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/support/PrimaryResourceCache.java @@ -15,6 +15,10 @@ public PrimaryResourceCache(BiPredicate, P> evictionPredicate) { this.evictionPredicate = evictionPredicate; } + public PrimaryResourceCache() { + this(new ResourceVersionParsingEvictionPredicate<>()); + } + public void cacheResource(P afterUpdate) { var resourceId = ResourceID.fromResource(afterUpdate); cache.put(resourceId, new Pair<>(null, afterUpdate)); @@ -49,6 +53,7 @@ public void cleanup(P resource) { public record Pair(T beforeUpdate, T afterUpdate) {} + /** This works in general, but it does not strictly follow the contract with k8s API */ public static class ResourceVersionParsingEvictionPredicate implements BiPredicate, T> { @Override diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/PeriodicTriggerEventSource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/PeriodicTriggerEventSource.java index 5402e72a52..366777409a 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/PeriodicTriggerEventSource.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/PeriodicTriggerEventSource.java @@ -43,10 +43,7 @@ public void start() throws OperatorException { public void run() { primaryCache .list() - .forEach( - r -> { - getEventHandler().handleEvent(new Event(ResourceID.fromResource(r))); - }); + .forEach(r -> getEventHandler().handleEvent(new Event(ResourceID.fromResource(r)))); } }, 0, From e09472a79c58978de6c61e7467c03225d6455e1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Tue, 15 Apr 2025 14:22:17 +0200 Subject: [PATCH 10/41] fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- operator-framework-core/pom.xml | 5 ----- 1 file changed, 5 deletions(-) diff --git a/operator-framework-core/pom.xml b/operator-framework-core/pom.xml index da6d3395a3..cad50ebc32 100644 --- a/operator-framework-core/pom.xml +++ b/operator-framework-core/pom.xml @@ -79,11 +79,6 @@ awaitility test - - io.fabric8 - kube-api-test-client-inject - test - From 608fb09b3219412f1608fe17c4c9e6f0df1f2523 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Tue, 15 Apr 2025 17:21:24 +0200 Subject: [PATCH 11/41] Update operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java Co-authored-by: Martin Stefanko --- .../operator/api/reconciler/PrimaryUpdateAndCacheUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java index 68630f8c91..89641c5e62 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java @@ -156,7 +156,7 @@ public static

P ssaPatchAndCacheStatus( * @return the updated resource. * @param

primary resource type */ - public static

P edithAndCacheStatus( + public static

P editAndCacheStatus( P primary, Context

context, PrimaryResourceCache

cache, UnaryOperator

operation) { logWarnIfResourceVersionPresent(primary); return patchAndCacheStatus( From 21b2ef5d2ac951d56d7d286a5fbf27a388b85e21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Tue, 15 Apr 2025 17:22:29 +0200 Subject: [PATCH 12/41] fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../primarycache/StatusPatchPrimaryCacheReconciler.java | 2 +- .../withlock/StatusPatchCacheWithLockReconciler.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheReconciler.java index bcb0a0a00a..85c3987944 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheReconciler.java @@ -38,7 +38,7 @@ public UpdateControl reconcile( primary = cache.getFreshResource(primary); if (primary.getStatus() != null && primary.getStatus().getValue() != latestValue) { - errorPresent = false; + errorPresent = true; throw new IllegalStateException( "status is not up to date. Latest value: " + latestValue diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockReconciler.java index 2ecb98f087..8b85de26d9 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockReconciler.java @@ -26,7 +26,7 @@ public UpdateControl reconcile( throws InterruptedException { if (resource.getStatus() != null && resource.getStatus().getValue() != latestValue) { - errorPresent = false; + errorPresent = true; throw new IllegalStateException( "status is not up to date. Latest value: " + latestValue From 870db5739dcd380d4208b3a0a0a4d38fbf8eb9e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Tue, 15 Apr 2025 17:37:07 +0200 Subject: [PATCH 13/41] additional test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../reconciler/support/PrimaryResourceCacheTest.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/support/PrimaryResourceCacheTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/support/PrimaryResourceCacheTest.java index 9026c0d2cd..58e3ce8a0a 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/support/PrimaryResourceCacheTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/support/PrimaryResourceCacheTest.java @@ -59,6 +59,18 @@ void cleanupRemovesCachedResources() { assertThat(olderCR).isSameAs(res); } + @Test + void removesIfNewResourceWithDifferentUid() { + var cr = customResource("2"); + versionParsingCache.cacheResource(cr); + var crWithDifferentUid = customResource("1"); + cr.getMetadata().setUid("otheruid"); + + var res = versionParsingCache.getFreshResource(crWithDifferentUid); + + assertThat(res).isSameAs(crWithDifferentUid); + } + private TestCustomResource customResource(String resourceVersion) { var cr = new TestCustomResource(); cr.setMetadata( From 9c58fd45a0a600528cb90104eca7be868f138719 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Tue, 15 Apr 2025 17:39:54 +0200 Subject: [PATCH 14/41] doc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../primarycache/StatusPatchPrimaryCacheReconciler.java | 1 + 1 file changed, 1 insertion(+) diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheReconciler.java index 85c3987944..2b38513173 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheReconciler.java @@ -23,6 +23,7 @@ public class StatusPatchPrimaryCacheReconciler public volatile int latestValue = 0; public volatile boolean errorPresent = false; + // We on purpose don't use the provided predicate to show what a custom could look like. private PrimaryResourceCache cache = new PrimaryResourceCache<>( (statusPatchCacheCustomResourcePair, statusPatchCacheCustomResource) -> From e48134250644db04b721eb38f35a331d052e290d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Tue, 15 Apr 2025 17:44:25 +0200 Subject: [PATCH 15/41] Update operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java Co-authored-by: Antonio <122279781+afalhambra-hivemq@users.noreply.github.com> --- .../operator/api/reconciler/PrimaryUpdateAndCacheUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java index 89641c5e62..6d8a541dd9 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java @@ -82,7 +82,7 @@ public static

P patchAndCacheStatusWithLock( .eventSourceRetriever() .getControllerEventSource() .handleRecentResourceUpdate(ResourceID.fromResource(primary), updatedResource, primary); - return null; + return updatedResource; } /** From 51f1ca0cb7d24d1229991de05b7cf47f292d1347 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Wed, 16 Apr 2025 10:02:10 +0200 Subject: [PATCH 16/41] Update operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheIT.java Co-authored-by: Antonio <122279781+afalhambra-hivemq@users.noreply.github.com> --- .../statuscache/primarycache/StatusPatchPrimaryCacheIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheIT.java index 6cf46cf17a..751d1362d0 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheIT.java @@ -11,7 +11,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.awaitility.Awaitility.await; -public class StatusPatchPrimaryCacheIT { +class StatusPatchPrimaryCacheIT { public static final String TEST_1 = "test1"; From 3409053ca7379d1a726c6996919334c193b3046b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Wed, 16 Apr 2025 10:02:39 +0200 Subject: [PATCH 17/41] Update operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java Co-authored-by: Antonio <122279781+afalhambra-hivemq@users.noreply.github.com> --- .../operator/api/reconciler/PrimaryUpdateAndCacheUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java index 6d8a541dd9..840b94bf46 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java @@ -227,7 +227,7 @@ private static

void checkResourceVersionPresent(P primar private static

void logWarnIfResourceVersionPresent(P primary) { if (primary.getMetadata().getResourceVersion() != null) { log.warn( - "Primary resource version is NOT null, for caching with optimistic locking use" + "Primary resource version is NOT null, for caching with optimistic locking use" + " alternative methods. Name: {} namespace: {}", primary.getMetadata().getName(), primary.getMetadata().getNamespace()); From 68ca625c08ac44290dab97a72150e9f7245bed33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Thu, 17 Apr 2025 11:34:33 +0200 Subject: [PATCH 18/41] remove with lock versions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../PrimaryUpdateAndCacheUtils.java | 99 ------------------- ...StatusPatchPrimaryCacheCustomResource.java | 2 +- .../StatusPatchPrimaryCacheIT.java | 2 +- .../StatusPatchPrimaryCacheReconciler.java | 3 +- .../StatusPatchPrimaryCacheSpec.java | 2 +- .../StatusPatchPrimaryCacheStatus.java | 2 +- ...tatusPatchCacheWithLockCustomResource.java | 14 --- .../withlock/StatusPatchCacheWithLockIT.java | 48 --------- .../StatusPatchCacheWithLockReconciler.java | 70 ------------- .../StatusPatchCacheWithLockSpec.java | 14 --- .../StatusPatchCacheWithLockStatus.java | 15 --- 11 files changed, 5 insertions(+), 266 deletions(-) rename operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/{primarycache => }/StatusPatchPrimaryCacheCustomResource.java (87%) rename operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/{primarycache => }/StatusPatchPrimaryCacheIT.java (95%) rename operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/{primarycache => }/StatusPatchPrimaryCacheReconciler.java (95%) rename operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/{primarycache => }/StatusPatchPrimaryCacheSpec.java (81%) rename operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/{primarycache => }/StatusPatchPrimaryCacheStatus.java (77%) delete mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockCustomResource.java delete mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockIT.java delete mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockReconciler.java delete mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockSpec.java delete mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockStatus.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java index 840b94bf46..a580244a55 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java @@ -11,7 +11,6 @@ import io.fabric8.kubernetes.client.dsl.base.PatchContext; import io.fabric8.kubernetes.client.dsl.base.PatchType; import io.javaoperatorsdk.operator.api.reconciler.support.PrimaryResourceCache; -import io.javaoperatorsdk.operator.processing.event.ResourceID; public class PrimaryUpdateAndCacheUtils { @@ -19,104 +18,6 @@ private PrimaryUpdateAndCacheUtils() {} private static final Logger log = LoggerFactory.getLogger(PrimaryUpdateAndCacheUtils.class); - /** - * Makes sure that the up-to-date primary resource will be present during the next reconciliation. - * Using update (PUT) method. - * - * @param primary resource - * @param context of reconciliation - * @return updated resource - * @param

primary resource type - */ - public static

P updateAndCacheStatusWithLock( - P primary, Context

context) { - return patchAndCacheStatusWithLock( - primary, context, (p, c) -> c.resource(primary).updateStatus()); - } - - /** - * Makes sure that the up-to-date primary resource will be present during the next reconciliation. - * Using JSON Merge patch. - * - * @param primary resource - * @param context of reconciliation - * @return updated resource - * @param

primary resource type - */ - public static

P patchAndCacheStatusWithLock( - P primary, Context

context) { - return patchAndCacheStatusWithLock( - primary, context, (p, c) -> c.resource(primary).patchStatus()); - } - - /** - * Makes sure that the up-to-date primary resource will be present during the next reconciliation. - * Using JSON Patch. - * - * @param primary resource - * @param context of reconciliation - * @return updated resource - * @param

primary resource type - */ - public static

P editAndCacheStatusWithLock( - P primary, Context

context, UnaryOperator

operation) { - return patchAndCacheStatusWithLock( - primary, context, (p, c) -> c.resource(primary).editStatus(operation)); - } - - /** - * Makes sure that the up-to-date primary resource will be present during the next reconciliation. - * - * @param primary resource - * @param context of reconciliation - * @param patch free implementation of cache - make sure you use optimistic locking during the - * update - * @return the updated resource. - * @param

primary resource type - */ - public static

P patchAndCacheStatusWithLock( - P primary, Context

context, BiFunction patch) { - checkResourceVersionPresent(primary); - var updatedResource = patch.apply(primary, context.getClient()); - context - .eventSourceRetriever() - .getControllerEventSource() - .handleRecentResourceUpdate(ResourceID.fromResource(primary), updatedResource, primary); - return updatedResource; - } - - /** - * Makes sure that the up-to-date primary resource will be present during the next reconciliation. - * Using Server Side Apply. - * - * @param primary resource - * @param freshResourceWithStatus - fresh resource with target state - * @param context of reconciliation - * @return the updated resource. - * @param

primary resource type - */ - public static

P ssaPatchAndCacheStatusWithLock( - P primary, P freshResourceWithStatus, Context

context) { - checkResourceVersionPresent(freshResourceWithStatus); - var res = - context - .getClient() - .resource(freshResourceWithStatus) - .subresource("status") - .patch( - new PatchContext.Builder() - .withForce(true) - .withFieldManager(context.getControllerConfiguration().fieldManager()) - .withPatchType(PatchType.SERVER_SIDE_APPLY) - .build()); - - context - .eventSourceRetriever() - .getControllerEventSource() - .handleRecentResourceUpdate(ResourceID.fromResource(primary), res, primary); - return res; - } - /** * Patches the resource and adds it to the {@link PrimaryResourceCache} provided. Optimistic * locking is not required. diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheCustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/StatusPatchPrimaryCacheCustomResource.java similarity index 87% rename from operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheCustomResource.java rename to operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/StatusPatchPrimaryCacheCustomResource.java index 84b145cac3..20d939a375 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheCustomResource.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/StatusPatchPrimaryCacheCustomResource.java @@ -1,4 +1,4 @@ -package io.javaoperatorsdk.operator.baseapi.statuscache.primarycache; +package io.javaoperatorsdk.operator.baseapi.statuscache; import io.fabric8.kubernetes.api.model.Namespaced; import io.fabric8.kubernetes.client.CustomResource; diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/StatusPatchPrimaryCacheIT.java similarity index 95% rename from operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheIT.java rename to operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/StatusPatchPrimaryCacheIT.java index 751d1362d0..0645c1ac5a 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/StatusPatchPrimaryCacheIT.java @@ -1,4 +1,4 @@ -package io.javaoperatorsdk.operator.baseapi.statuscache.primarycache; +package io.javaoperatorsdk.operator.baseapi.statuscache; import java.time.Duration; diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/StatusPatchPrimaryCacheReconciler.java similarity index 95% rename from operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheReconciler.java rename to operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/StatusPatchPrimaryCacheReconciler.java index 2b38513173..4fd3d2b796 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/StatusPatchPrimaryCacheReconciler.java @@ -1,4 +1,4 @@ -package io.javaoperatorsdk.operator.baseapi.statuscache.primarycache; +package io.javaoperatorsdk.operator.baseapi.statuscache; import java.util.List; @@ -12,7 +12,6 @@ import io.javaoperatorsdk.operator.api.reconciler.Reconciler; import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; import io.javaoperatorsdk.operator.api.reconciler.support.PrimaryResourceCache; -import io.javaoperatorsdk.operator.baseapi.statuscache.PeriodicTriggerEventSource; import io.javaoperatorsdk.operator.processing.event.source.EventSource; @ControllerConfiguration diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheSpec.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/StatusPatchPrimaryCacheSpec.java similarity index 81% rename from operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheSpec.java rename to operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/StatusPatchPrimaryCacheSpec.java index 90630c1ae8..c7a29c3b3c 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheSpec.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/StatusPatchPrimaryCacheSpec.java @@ -1,4 +1,4 @@ -package io.javaoperatorsdk.operator.baseapi.statuscache.primarycache; +package io.javaoperatorsdk.operator.baseapi.statuscache; public class StatusPatchPrimaryCacheSpec { diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheStatus.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/StatusPatchPrimaryCacheStatus.java similarity index 77% rename from operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheStatus.java rename to operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/StatusPatchPrimaryCacheStatus.java index 0687d5576a..fedf015790 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheStatus.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/StatusPatchPrimaryCacheStatus.java @@ -1,4 +1,4 @@ -package io.javaoperatorsdk.operator.baseapi.statuscache.primarycache; +package io.javaoperatorsdk.operator.baseapi.statuscache; public class StatusPatchPrimaryCacheStatus { diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockCustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockCustomResource.java deleted file mode 100644 index d84a992a13..0000000000 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockCustomResource.java +++ /dev/null @@ -1,14 +0,0 @@ -package io.javaoperatorsdk.operator.baseapi.statuscache.withlock; - -import io.fabric8.kubernetes.api.model.Namespaced; -import io.fabric8.kubernetes.client.CustomResource; -import io.fabric8.kubernetes.model.annotation.Group; -import io.fabric8.kubernetes.model.annotation.ShortNames; -import io.fabric8.kubernetes.model.annotation.Version; - -@Group("sample.javaoperatorsdk") -@Version("v1") -@ShortNames("spcl") -public class StatusPatchCacheWithLockCustomResource - extends CustomResource - implements Namespaced {} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockIT.java deleted file mode 100644 index 370dab81f3..0000000000 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockIT.java +++ /dev/null @@ -1,48 +0,0 @@ -package io.javaoperatorsdk.operator.baseapi.statuscache.withlock; - -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.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.awaitility.Awaitility.await; - -public class StatusPatchCacheWithLockIT { - - public static final String TEST_1 = "test1"; - - @RegisterExtension - LocallyRunOperatorExtension extension = - LocallyRunOperatorExtension.builder() - .withReconciler(StatusPatchCacheWithLockReconciler.class) - .build(); - - @Test - void testStatusAlwaysUpToDate() { - var reconciler = extension.getReconcilerOfType(StatusPatchCacheWithLockReconciler.class); - - extension.create(testResource()); - - // the reconciliation id periodically triggered, the status values should be increasing - // monotonically - await() - .pollDelay(Duration.ofSeconds(1)) - .pollInterval(Duration.ofMillis(30)) - .untilAsserted( - () -> { - assertThat(reconciler.errorPresent).isFalse(); - assertThat(reconciler.latestValue).isGreaterThan(10); - }); - } - - StatusPatchCacheWithLockCustomResource testResource() { - var res = new StatusPatchCacheWithLockCustomResource(); - res.setMetadata(new ObjectMetaBuilder().withName(TEST_1).build()); - res.setSpec(new StatusPatchCacheWithLockSpec()); - return res; - } -} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockReconciler.java deleted file mode 100644 index 8b85de26d9..0000000000 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockReconciler.java +++ /dev/null @@ -1,70 +0,0 @@ -package io.javaoperatorsdk.operator.baseapi.statuscache.withlock; - -import java.util.List; - -import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; -import io.javaoperatorsdk.operator.api.reconciler.Context; -import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; -import io.javaoperatorsdk.operator.api.reconciler.EventSourceContext; -import io.javaoperatorsdk.operator.api.reconciler.PrimaryUpdateAndCacheUtils; -import io.javaoperatorsdk.operator.api.reconciler.Reconciler; -import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; -import io.javaoperatorsdk.operator.baseapi.statuscache.PeriodicTriggerEventSource; -import io.javaoperatorsdk.operator.processing.event.source.EventSource; - -@ControllerConfiguration -public class StatusPatchCacheWithLockReconciler - implements Reconciler { - - public volatile int latestValue = 0; - public volatile boolean errorPresent = false; - - @Override - public UpdateControl reconcile( - StatusPatchCacheWithLockCustomResource resource, - Context context) - throws InterruptedException { - - if (resource.getStatus() != null && resource.getStatus().getValue() != latestValue) { - errorPresent = true; - throw new IllegalStateException( - "status is not up to date. Latest value: " - + latestValue - + " status values: " - + resource.getStatus().getValue()); - } - - var freshCopy = createFreshCopy(resource); - // setting the resource version - freshCopy.getMetadata().setResourceVersion(resource.getMetadata().getResourceVersion()); - freshCopy - .getStatus() - .setValue(resource.getStatus() == null ? 1 : resource.getStatus().getValue() + 1); - - var updated = - PrimaryUpdateAndCacheUtils.ssaPatchAndCacheStatusWithLock(resource, freshCopy, context); - latestValue = updated.getStatus().getValue(); - - return UpdateControl.noUpdate(); - } - - @Override - public List> prepareEventSources( - EventSourceContext context) { - // periodic event triggering for testing purposes - return List.of(new PeriodicTriggerEventSource<>(context.getPrimaryCache())); - } - - private StatusPatchCacheWithLockCustomResource createFreshCopy( - StatusPatchCacheWithLockCustomResource resource) { - var res = new StatusPatchCacheWithLockCustomResource(); - res.setMetadata( - new ObjectMetaBuilder() - .withName(resource.getMetadata().getName()) - .withNamespace(resource.getMetadata().getNamespace()) - .build()); - res.setStatus(new StatusPatchCacheWithLockStatus()); - - return res; - } -} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockSpec.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockSpec.java deleted file mode 100644 index 12cd1ac3e1..0000000000 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockSpec.java +++ /dev/null @@ -1,14 +0,0 @@ -package io.javaoperatorsdk.operator.baseapi.statuscache.withlock; - -public class StatusPatchCacheWithLockSpec { - - private int counter = 0; - - public int getCounter() { - return counter; - } - - public void setCounter(int counter) { - this.counter = counter; - } -} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockStatus.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockStatus.java deleted file mode 100644 index 8d1e559308..0000000000 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockStatus.java +++ /dev/null @@ -1,15 +0,0 @@ -package io.javaoperatorsdk.operator.baseapi.statuscache.withlock; - -public class StatusPatchCacheWithLockStatus { - - private Integer value = 0; - - public Integer getValue() { - return value; - } - - public StatusPatchCacheWithLockStatus setValue(Integer value) { - this.value = value; - return this; - } -} From 84eec7be19c01876968f89d529b2e6b4f13410dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Thu, 17 Apr 2025 14:04:15 +0200 Subject: [PATCH 19/41] remove not used code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../api/reconciler/PrimaryUpdateAndCacheUtils.java | 8 -------- 1 file changed, 8 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java index a580244a55..5d2d85377e 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java @@ -117,14 +117,6 @@ public static

P patchAndCacheStatus( return updatedResource; } - private static

void checkResourceVersionPresent(P primary) { - if (primary.getMetadata().getResourceVersion() == null) { - throw new IllegalStateException( - "Primary resource version is null, it is expected to set resource version for updates for caching. Name: %s namespace: %s" - .formatted(primary.getMetadata().getName(), primary.getMetadata().getNamespace())); - } - } - private static

void logWarnIfResourceVersionPresent(P primary) { if (primary.getMetadata().getResourceVersion() != null) { log.warn( From 42b9eadca5907e88fe75f9e2d813fd5fc1aa738b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Thu, 17 Apr 2025 14:42:20 +0200 Subject: [PATCH 20/41] Revert "remove not used code" This reverts commit 84eec7be19c01876968f89d529b2e6b4f13410dd. --- .../api/reconciler/PrimaryUpdateAndCacheUtils.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java index 5d2d85377e..a580244a55 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java @@ -117,6 +117,14 @@ public static

P patchAndCacheStatus( return updatedResource; } + private static

void checkResourceVersionPresent(P primary) { + if (primary.getMetadata().getResourceVersion() == null) { + throw new IllegalStateException( + "Primary resource version is null, it is expected to set resource version for updates for caching. Name: %s namespace: %s" + .formatted(primary.getMetadata().getName(), primary.getMetadata().getNamespace())); + } + } + private static

void logWarnIfResourceVersionPresent(P primary) { if (primary.getMetadata().getResourceVersion() != null) { log.warn( From d51f0e3cf27d6f08f48a2ac5b74b04f8eda4a6a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Thu, 17 Apr 2025 14:42:37 +0200 Subject: [PATCH 21/41] Revert "remove with lock versions" This reverts commit 68ca625c08ac44290dab97a72150e9f7245bed33. --- .../PrimaryUpdateAndCacheUtils.java | 99 +++++++++++++++++++ ...StatusPatchPrimaryCacheCustomResource.java | 2 +- .../StatusPatchPrimaryCacheIT.java | 2 +- .../StatusPatchPrimaryCacheReconciler.java | 3 +- .../StatusPatchPrimaryCacheSpec.java | 2 +- .../StatusPatchPrimaryCacheStatus.java | 2 +- ...tatusPatchCacheWithLockCustomResource.java | 14 +++ .../withlock/StatusPatchCacheWithLockIT.java | 48 +++++++++ .../StatusPatchCacheWithLockReconciler.java | 70 +++++++++++++ .../StatusPatchCacheWithLockSpec.java | 14 +++ .../StatusPatchCacheWithLockStatus.java | 15 +++ 11 files changed, 266 insertions(+), 5 deletions(-) rename operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/{ => primarycache}/StatusPatchPrimaryCacheCustomResource.java (87%) rename operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/{ => primarycache}/StatusPatchPrimaryCacheIT.java (95%) rename operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/{ => primarycache}/StatusPatchPrimaryCacheReconciler.java (95%) rename operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/{ => primarycache}/StatusPatchPrimaryCacheSpec.java (81%) rename operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/{ => primarycache}/StatusPatchPrimaryCacheStatus.java (77%) create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockCustomResource.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockIT.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockReconciler.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockSpec.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockStatus.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java index a580244a55..840b94bf46 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java @@ -11,6 +11,7 @@ import io.fabric8.kubernetes.client.dsl.base.PatchContext; import io.fabric8.kubernetes.client.dsl.base.PatchType; import io.javaoperatorsdk.operator.api.reconciler.support.PrimaryResourceCache; +import io.javaoperatorsdk.operator.processing.event.ResourceID; public class PrimaryUpdateAndCacheUtils { @@ -18,6 +19,104 @@ private PrimaryUpdateAndCacheUtils() {} private static final Logger log = LoggerFactory.getLogger(PrimaryUpdateAndCacheUtils.class); + /** + * Makes sure that the up-to-date primary resource will be present during the next reconciliation. + * Using update (PUT) method. + * + * @param primary resource + * @param context of reconciliation + * @return updated resource + * @param

primary resource type + */ + public static

P updateAndCacheStatusWithLock( + P primary, Context

context) { + return patchAndCacheStatusWithLock( + primary, context, (p, c) -> c.resource(primary).updateStatus()); + } + + /** + * Makes sure that the up-to-date primary resource will be present during the next reconciliation. + * Using JSON Merge patch. + * + * @param primary resource + * @param context of reconciliation + * @return updated resource + * @param

primary resource type + */ + public static

P patchAndCacheStatusWithLock( + P primary, Context

context) { + return patchAndCacheStatusWithLock( + primary, context, (p, c) -> c.resource(primary).patchStatus()); + } + + /** + * Makes sure that the up-to-date primary resource will be present during the next reconciliation. + * Using JSON Patch. + * + * @param primary resource + * @param context of reconciliation + * @return updated resource + * @param

primary resource type + */ + public static

P editAndCacheStatusWithLock( + P primary, Context

context, UnaryOperator

operation) { + return patchAndCacheStatusWithLock( + primary, context, (p, c) -> c.resource(primary).editStatus(operation)); + } + + /** + * Makes sure that the up-to-date primary resource will be present during the next reconciliation. + * + * @param primary resource + * @param context of reconciliation + * @param patch free implementation of cache - make sure you use optimistic locking during the + * update + * @return the updated resource. + * @param

primary resource type + */ + public static

P patchAndCacheStatusWithLock( + P primary, Context

context, BiFunction patch) { + checkResourceVersionPresent(primary); + var updatedResource = patch.apply(primary, context.getClient()); + context + .eventSourceRetriever() + .getControllerEventSource() + .handleRecentResourceUpdate(ResourceID.fromResource(primary), updatedResource, primary); + return updatedResource; + } + + /** + * Makes sure that the up-to-date primary resource will be present during the next reconciliation. + * Using Server Side Apply. + * + * @param primary resource + * @param freshResourceWithStatus - fresh resource with target state + * @param context of reconciliation + * @return the updated resource. + * @param

primary resource type + */ + public static

P ssaPatchAndCacheStatusWithLock( + P primary, P freshResourceWithStatus, Context

context) { + checkResourceVersionPresent(freshResourceWithStatus); + var res = + context + .getClient() + .resource(freshResourceWithStatus) + .subresource("status") + .patch( + new PatchContext.Builder() + .withForce(true) + .withFieldManager(context.getControllerConfiguration().fieldManager()) + .withPatchType(PatchType.SERVER_SIDE_APPLY) + .build()); + + context + .eventSourceRetriever() + .getControllerEventSource() + .handleRecentResourceUpdate(ResourceID.fromResource(primary), res, primary); + return res; + } + /** * Patches the resource and adds it to the {@link PrimaryResourceCache} provided. Optimistic * locking is not required. diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/StatusPatchPrimaryCacheCustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheCustomResource.java similarity index 87% rename from operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/StatusPatchPrimaryCacheCustomResource.java rename to operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheCustomResource.java index 20d939a375..84b145cac3 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/StatusPatchPrimaryCacheCustomResource.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheCustomResource.java @@ -1,4 +1,4 @@ -package io.javaoperatorsdk.operator.baseapi.statuscache; +package io.javaoperatorsdk.operator.baseapi.statuscache.primarycache; import io.fabric8.kubernetes.api.model.Namespaced; import io.fabric8.kubernetes.client.CustomResource; diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/StatusPatchPrimaryCacheIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheIT.java similarity index 95% rename from operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/StatusPatchPrimaryCacheIT.java rename to operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheIT.java index 0645c1ac5a..751d1362d0 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/StatusPatchPrimaryCacheIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheIT.java @@ -1,4 +1,4 @@ -package io.javaoperatorsdk.operator.baseapi.statuscache; +package io.javaoperatorsdk.operator.baseapi.statuscache.primarycache; import java.time.Duration; diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/StatusPatchPrimaryCacheReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheReconciler.java similarity index 95% rename from operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/StatusPatchPrimaryCacheReconciler.java rename to operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheReconciler.java index 4fd3d2b796..2b38513173 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/StatusPatchPrimaryCacheReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheReconciler.java @@ -1,4 +1,4 @@ -package io.javaoperatorsdk.operator.baseapi.statuscache; +package io.javaoperatorsdk.operator.baseapi.statuscache.primarycache; import java.util.List; @@ -12,6 +12,7 @@ import io.javaoperatorsdk.operator.api.reconciler.Reconciler; import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; import io.javaoperatorsdk.operator.api.reconciler.support.PrimaryResourceCache; +import io.javaoperatorsdk.operator.baseapi.statuscache.PeriodicTriggerEventSource; import io.javaoperatorsdk.operator.processing.event.source.EventSource; @ControllerConfiguration diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/StatusPatchPrimaryCacheSpec.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheSpec.java similarity index 81% rename from operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/StatusPatchPrimaryCacheSpec.java rename to operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheSpec.java index c7a29c3b3c..90630c1ae8 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/StatusPatchPrimaryCacheSpec.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheSpec.java @@ -1,4 +1,4 @@ -package io.javaoperatorsdk.operator.baseapi.statuscache; +package io.javaoperatorsdk.operator.baseapi.statuscache.primarycache; public class StatusPatchPrimaryCacheSpec { diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/StatusPatchPrimaryCacheStatus.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheStatus.java similarity index 77% rename from operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/StatusPatchPrimaryCacheStatus.java rename to operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheStatus.java index fedf015790..0687d5576a 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/StatusPatchPrimaryCacheStatus.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheStatus.java @@ -1,4 +1,4 @@ -package io.javaoperatorsdk.operator.baseapi.statuscache; +package io.javaoperatorsdk.operator.baseapi.statuscache.primarycache; public class StatusPatchPrimaryCacheStatus { diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockCustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockCustomResource.java new file mode 100644 index 0000000000..d84a992a13 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockCustomResource.java @@ -0,0 +1,14 @@ +package io.javaoperatorsdk.operator.baseapi.statuscache.withlock; + +import io.fabric8.kubernetes.api.model.Namespaced; +import io.fabric8.kubernetes.client.CustomResource; +import io.fabric8.kubernetes.model.annotation.Group; +import io.fabric8.kubernetes.model.annotation.ShortNames; +import io.fabric8.kubernetes.model.annotation.Version; + +@Group("sample.javaoperatorsdk") +@Version("v1") +@ShortNames("spcl") +public class StatusPatchCacheWithLockCustomResource + extends CustomResource + implements Namespaced {} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockIT.java new file mode 100644 index 0000000000..370dab81f3 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockIT.java @@ -0,0 +1,48 @@ +package io.javaoperatorsdk.operator.baseapi.statuscache.withlock; + +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.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + +public class StatusPatchCacheWithLockIT { + + public static final String TEST_1 = "test1"; + + @RegisterExtension + LocallyRunOperatorExtension extension = + LocallyRunOperatorExtension.builder() + .withReconciler(StatusPatchCacheWithLockReconciler.class) + .build(); + + @Test + void testStatusAlwaysUpToDate() { + var reconciler = extension.getReconcilerOfType(StatusPatchCacheWithLockReconciler.class); + + extension.create(testResource()); + + // the reconciliation id periodically triggered, the status values should be increasing + // monotonically + await() + .pollDelay(Duration.ofSeconds(1)) + .pollInterval(Duration.ofMillis(30)) + .untilAsserted( + () -> { + assertThat(reconciler.errorPresent).isFalse(); + assertThat(reconciler.latestValue).isGreaterThan(10); + }); + } + + StatusPatchCacheWithLockCustomResource testResource() { + var res = new StatusPatchCacheWithLockCustomResource(); + res.setMetadata(new ObjectMetaBuilder().withName(TEST_1).build()); + res.setSpec(new StatusPatchCacheWithLockSpec()); + return res; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockReconciler.java new file mode 100644 index 0000000000..8b85de26d9 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockReconciler.java @@ -0,0 +1,70 @@ +package io.javaoperatorsdk.operator.baseapi.statuscache.withlock; + +import java.util.List; + +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.EventSourceContext; +import io.javaoperatorsdk.operator.api.reconciler.PrimaryUpdateAndCacheUtils; +import io.javaoperatorsdk.operator.api.reconciler.Reconciler; +import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; +import io.javaoperatorsdk.operator.baseapi.statuscache.PeriodicTriggerEventSource; +import io.javaoperatorsdk.operator.processing.event.source.EventSource; + +@ControllerConfiguration +public class StatusPatchCacheWithLockReconciler + implements Reconciler { + + public volatile int latestValue = 0; + public volatile boolean errorPresent = false; + + @Override + public UpdateControl reconcile( + StatusPatchCacheWithLockCustomResource resource, + Context context) + throws InterruptedException { + + if (resource.getStatus() != null && resource.getStatus().getValue() != latestValue) { + errorPresent = true; + throw new IllegalStateException( + "status is not up to date. Latest value: " + + latestValue + + " status values: " + + resource.getStatus().getValue()); + } + + var freshCopy = createFreshCopy(resource); + // setting the resource version + freshCopy.getMetadata().setResourceVersion(resource.getMetadata().getResourceVersion()); + freshCopy + .getStatus() + .setValue(resource.getStatus() == null ? 1 : resource.getStatus().getValue() + 1); + + var updated = + PrimaryUpdateAndCacheUtils.ssaPatchAndCacheStatusWithLock(resource, freshCopy, context); + latestValue = updated.getStatus().getValue(); + + return UpdateControl.noUpdate(); + } + + @Override + public List> prepareEventSources( + EventSourceContext context) { + // periodic event triggering for testing purposes + return List.of(new PeriodicTriggerEventSource<>(context.getPrimaryCache())); + } + + private StatusPatchCacheWithLockCustomResource createFreshCopy( + StatusPatchCacheWithLockCustomResource resource) { + var res = new StatusPatchCacheWithLockCustomResource(); + res.setMetadata( + new ObjectMetaBuilder() + .withName(resource.getMetadata().getName()) + .withNamespace(resource.getMetadata().getNamespace()) + .build()); + res.setStatus(new StatusPatchCacheWithLockStatus()); + + return res; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockSpec.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockSpec.java new file mode 100644 index 0000000000..12cd1ac3e1 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockSpec.java @@ -0,0 +1,14 @@ +package io.javaoperatorsdk.operator.baseapi.statuscache.withlock; + +public class StatusPatchCacheWithLockSpec { + + private int counter = 0; + + public int getCounter() { + return counter; + } + + public void setCounter(int counter) { + this.counter = counter; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockStatus.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockStatus.java new file mode 100644 index 0000000000..8d1e559308 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockStatus.java @@ -0,0 +1,15 @@ +package io.javaoperatorsdk.operator.baseapi.statuscache.withlock; + +public class StatusPatchCacheWithLockStatus { + + private Integer value = 0; + + public Integer getValue() { + return value; + } + + public StatusPatchCacheWithLockStatus setValue(Integer value) { + this.value = value; + return this; + } +} From 14c63bb59bbf982d44d676ca7f35f90a3793708c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Thu, 17 Apr 2025 14:47:22 +0200 Subject: [PATCH 22/41] wip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../PrimaryUpdateAndCacheUtils.java | 20 ++++++++----------- .../StatusPatchCacheWithLockReconciler.java | 3 ++- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java index 840b94bf46..8b960434ee 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java @@ -28,10 +28,8 @@ private PrimaryUpdateAndCacheUtils() {} * @return updated resource * @param

primary resource type */ - public static

P updateAndCacheStatusWithLock( - P primary, Context

context) { - return patchAndCacheStatusWithLock( - primary, context, (p, c) -> c.resource(primary).updateStatus()); + public static

P updateAndCacheStatusWith(P primary, Context

context) { + return patchAndCacheStatusWith(primary, context, (p, c) -> c.resource(primary).updateStatus()); } /** @@ -43,10 +41,8 @@ public static

P updateAndCacheStatusWithLock( * @return updated resource * @param

primary resource type */ - public static

P patchAndCacheStatusWithLock( - P primary, Context

context) { - return patchAndCacheStatusWithLock( - primary, context, (p, c) -> c.resource(primary).patchStatus()); + public static

P patchAndCacheStatusWith(P primary, Context

context) { + return patchAndCacheStatusWith(primary, context, (p, c) -> c.resource(primary).patchStatus()); } /** @@ -58,9 +54,9 @@ public static

P patchAndCacheStatusWithLock( * @return updated resource * @param

primary resource type */ - public static

P editAndCacheStatusWithLock( + public static

P editAndCacheStatusWith( P primary, Context

context, UnaryOperator

operation) { - return patchAndCacheStatusWithLock( + return patchAndCacheStatusWith( primary, context, (p, c) -> c.resource(primary).editStatus(operation)); } @@ -74,7 +70,7 @@ public static

P editAndCacheStatusWithLock( * @return the updated resource. * @param

primary resource type */ - public static

P patchAndCacheStatusWithLock( + public static

P patchAndCacheStatusWith( P primary, Context

context, BiFunction patch) { checkResourceVersionPresent(primary); var updatedResource = patch.apply(primary, context.getClient()); @@ -95,7 +91,7 @@ public static

P patchAndCacheStatusWithLock( * @return the updated resource. * @param

primary resource type */ - public static

P ssaPatchAndCacheStatusWithLock( + public static

P ssaPatchAndCacheStatusWith( P primary, P freshResourceWithStatus, Context

context) { checkResourceVersionPresent(freshResourceWithStatus); var res = diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockReconciler.java index 8b85de26d9..8e6e87fba0 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockReconciler.java @@ -41,8 +41,9 @@ public UpdateControl reconcile( .getStatus() .setValue(resource.getStatus() == null ? 1 : resource.getStatus().getValue() + 1); + resource.getMetadata().setResourceVersion(null); var updated = - PrimaryUpdateAndCacheUtils.ssaPatchAndCacheStatusWithLock(resource, freshCopy, context); + PrimaryUpdateAndCacheUtils.ssaPatchAndCacheStatusWith(resource, freshCopy, context); latestValue = updated.getStatus().getValue(); return UpdateControl.noUpdate(); From e9bcfbefa0907a03b02c9da6226d0dc06e3d54e2 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Thu, 17 Apr 2025 19:30:09 +0200 Subject: [PATCH 23/41] fix: typos and start improving javadoc Signed-off-by: Chris Laprun --- .../source/informer/InformerEventSource.java | 70 ++++++++++--------- .../StatusPatchPrimaryCacheIT.java | 2 +- .../StatusPatchPrimaryCacheReconciler.java | 4 +- .../withlock/StatusPatchCacheWithLockIT.java | 2 +- 4 files changed, 41 insertions(+), 37 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java index 688a88ae22..40ceaa975b 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java @@ -13,6 +13,7 @@ import io.fabric8.kubernetes.client.dsl.MixedOperation; import io.fabric8.kubernetes.client.informers.ResourceEventHandler; import io.javaoperatorsdk.operator.api.config.informer.InformerEventSourceConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.api.reconciler.EventSourceContext; import io.javaoperatorsdk.operator.processing.event.Event; import io.javaoperatorsdk.operator.processing.event.EventHandler; @@ -20,50 +21,53 @@ import io.javaoperatorsdk.operator.processing.event.source.PrimaryToSecondaryMapper; /** - * Wraps informer(s) so it is connected to the eventing system of the framework. Note that since - * it's it is built on top of Informers, it also support caching resources using caching from - * fabric8 client Informer caches and additional caches described below. + * Wraps informer(s) so they are connected to the eventing system of the framework. Note that since + * this is built on top of Fabric8 client Informers, it also supports caching resources using caching from + * informer caches as well as additional caches described below. * *

InformerEventSource also supports two features to better handle events and caching of - * resources on top of Informers from fabric8 Kubernetes client. These two features implementation - * wise are related to each other:
+ * resources on top of Informers from the Fabric8 Kubernetes client. These two features + * are related to each other as follows: * - *

1. API that allows to make sure the cache contains the fresh resource after an update. This is - * important for {@link io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource} and - * mainly for {@link - * io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource} so after - * reconcile if getResource() called always return the fresh resource. To achieve this - * handleRecentResourceUpdate() and handleRecentResourceCreate() needs to be called explicitly after - * resource created/updated using the kubernetes client. (These calls are done automatically by - * KubernetesDependentResource implementation.). In the background this will store the new resource - * in a temporary cache {@link TemporaryResourceCache} which do additional checks. After a new event - * is received the cachec object is removed from this cache, since in general then it is already in - * the cache of informer.
+ *

    + *
  1. Ensuring the cache contains the fresh resource after an update. This is + * important for {@link + * io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource} and mainly for + * {@link + * io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource} so + * that {@link io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource#getSecondaryResource(HasMetadata, Context)} always returns the latest version of the resource after a reconciliation. To achieve this + * {@link #handleRecentResourceUpdate(ResourceID, HasMetadata, HasMetadata)} and {@link #handleRecentResourceCreate(ResourceID, HasMetadata)} need to be called explicitly + * after a resource is created or updated using the kubernetes client. These calls are done + * automatically by the KubernetesDependentResource implementation. In the background this will + * store the new resource in a temporary cache {@link TemporaryResourceCache} which does + * additional checks. After a new event is received the cached object is removed from this + * cache, since it is then usually already in the informer cache. + *
  2. Avoiding unneeded reconciliations after resources are created or updated. This filters out events that are the results of updates and creates made + * by the controller itself because we typically don't want the associated informer to trigger an event causing a useless reconciliation (as the change originates from the reconciler itself). This is achieved + * by + * TODO: update as this mentions methods that don't exist anymore and isn't very clear * - *

    2. Additional API is provided that is meant to be used with the combination of the previous - * one, and the goal is to filter out events that are the results of updates and creates made by the - * controller itself. For example if in reconciler a ConfigMaps is created, there should be an - * Informer in place to handle change events of that ConfigMap, but since it has bean created (or - * updated) by the reconciler this should not trigger an additional reconciliation by default. In - * order to achieve this prepareForCreateOrUpdateEventFiltering(..) method needs to be called before - * the operation of the k8s client. And the operation from point 1. after the k8s client call. See - * it's usage in CreateUpdateEventFilterTestReconciler integration test for the usage. (Again this - * is managed for the developer if using dependent resources.)
    - * Roughly it works in a way that before the K8S API call is made, we set mark the resource ID, and - * from that point informer won't propagate events further just will start record them. After the - * client operation is done, it's checked and analysed what events were received and based on that - * it will propagate event or not and/or put the new resource into the temporal cache - so if the - * event not arrived yet about the update will be able to filter it in the future. + * {@link #prepareForCreateOrUpdateEventFiltering(..) method needs to be called before the operation + * of the k8s client. And the operation from point 1. after the k8s client call. See its + * usage in CreateUpdateEventFilterTestReconciler integration test for the usage. (Again this + * is managed for the developer if using dependent resources.)
    + * Roughly it works in a way that before the K8S API call is made, we set mark the resource + * ID, and from that point informer won't propagate events further just will start record + * them. After the client operation is done, it's checked and analysed what events were + * received and based on that it will propagate event or not and/or put the new resource into + * the temporal cache - so if the event not arrived yet about the update will be able to + * filter it in the future. + *

* - * @param resource type watching - * @param

type of the primary resource + * @param resource type being watched + * @param

type of the associated primary resource */ public class InformerEventSource extends ManagedInformerEventSource> implements ResourceEventHandler { - private static final Logger log = LoggerFactory.getLogger(InformerEventSource.class); public static final String PREVIOUS_ANNOTATION_KEY = "javaoperatorsdk.io/previous"; + private static final Logger log = LoggerFactory.getLogger(InformerEventSource.class); // we need direct control for the indexer to propagate the just update resource also to the index private final PrimaryToSecondaryIndex primaryToSecondaryIndex; private final PrimaryToSecondaryMapper

primaryToSecondaryMapper; diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheIT.java index 751d1362d0..a884ec0758 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheIT.java @@ -27,7 +27,7 @@ void testStatusAlwaysUpToDate() { extension.create(testResource()); - // the reconciliation id periodically triggered, the status values should be increasing + // the reconciliation is periodically triggered, the status values should be increasing // monotonically await() .pollDelay(Duration.ofSeconds(1)) diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheReconciler.java index 2b38513173..879cc1e3d8 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheReconciler.java @@ -23,8 +23,8 @@ public class StatusPatchPrimaryCacheReconciler public volatile int latestValue = 0; public volatile boolean errorPresent = false; - // We on purpose don't use the provided predicate to show what a custom could look like. - private PrimaryResourceCache cache = + // We on purpose don't use the provided predicate to show what a custom one could look like. + private final PrimaryResourceCache cache = new PrimaryResourceCache<>( (statusPatchCacheCustomResourcePair, statusPatchCacheCustomResource) -> statusPatchCacheCustomResource.getStatus().getValue() diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockIT.java index 370dab81f3..ec687c45d5 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockIT.java @@ -27,7 +27,7 @@ void testStatusAlwaysUpToDate() { extension.create(testResource()); - // the reconciliation id periodically triggered, the status values should be increasing + // the reconciliation is periodically triggered, the status values should be increasing // monotonically await() .pollDelay(Duration.ofSeconds(1)) From e8ede1a384a7bf87a1423f44315c813448c274d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Tue, 22 Apr 2025 13:04:42 +0200 Subject: [PATCH 24/41] refactor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../PrimaryUpdateAndCacheUtils.java | 10 --------- .../StatusPatchCacheCustomResource.java} | 7 +++---- .../StatusPatchCacheIT.java} | 14 ++++++------- .../StatusPatchCacheReconciler.java} | 21 ++++++++----------- .../StatusPatchCacheSpec.java} | 4 ++-- .../internal/StatusPatchCacheStatus.java | 15 +++++++++++++ .../StatusPatchCacheWithLockStatus.java | 15 ------------- 7 files changed, 36 insertions(+), 50 deletions(-) rename operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/{withlock/StatusPatchCacheWithLockCustomResource.java => internal/StatusPatchCacheCustomResource.java} (59%) rename operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/{withlock/StatusPatchCacheWithLockIT.java => internal/StatusPatchCacheIT.java} (75%) rename operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/{withlock/StatusPatchCacheWithLockReconciler.java => internal/StatusPatchCacheReconciler.java} (72%) rename operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/{withlock/StatusPatchCacheWithLockSpec.java => internal/StatusPatchCacheSpec.java} (59%) create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/internal/StatusPatchCacheStatus.java delete mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockStatus.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java index 8b960434ee..de6d8c7852 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java @@ -72,7 +72,6 @@ public static

P editAndCacheStatusWith( */ public static

P patchAndCacheStatusWith( P primary, Context

context, BiFunction patch) { - checkResourceVersionPresent(primary); var updatedResource = patch.apply(primary, context.getClient()); context .eventSourceRetriever() @@ -93,7 +92,6 @@ public static

P patchAndCacheStatusWith( */ public static

P ssaPatchAndCacheStatusWith( P primary, P freshResourceWithStatus, Context

context) { - checkResourceVersionPresent(freshResourceWithStatus); var res = context .getClient() @@ -212,14 +210,6 @@ public static

P patchAndCacheStatus( return updatedResource; } - private static

void checkResourceVersionPresent(P primary) { - if (primary.getMetadata().getResourceVersion() == null) { - throw new IllegalStateException( - "Primary resource version is null, it is expected to set resource version for updates for caching. Name: %s namespace: %s" - .formatted(primary.getMetadata().getName(), primary.getMetadata().getNamespace())); - } - } - private static

void logWarnIfResourceVersionPresent(P primary) { if (primary.getMetadata().getResourceVersion() != null) { log.warn( diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockCustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/internal/StatusPatchCacheCustomResource.java similarity index 59% rename from operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockCustomResource.java rename to operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/internal/StatusPatchCacheCustomResource.java index d84a992a13..2a2d8b83fd 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockCustomResource.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/internal/StatusPatchCacheCustomResource.java @@ -1,4 +1,4 @@ -package io.javaoperatorsdk.operator.baseapi.statuscache.withlock; +package io.javaoperatorsdk.operator.baseapi.statuscache.internal; import io.fabric8.kubernetes.api.model.Namespaced; import io.fabric8.kubernetes.client.CustomResource; @@ -9,6 +9,5 @@ @Group("sample.javaoperatorsdk") @Version("v1") @ShortNames("spcl") -public class StatusPatchCacheWithLockCustomResource - extends CustomResource - implements Namespaced {} +public class StatusPatchCacheCustomResource + extends CustomResource implements Namespaced {} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/internal/StatusPatchCacheIT.java similarity index 75% rename from operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockIT.java rename to operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/internal/StatusPatchCacheIT.java index ec687c45d5..f78511f250 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/internal/StatusPatchCacheIT.java @@ -1,4 +1,4 @@ -package io.javaoperatorsdk.operator.baseapi.statuscache.withlock; +package io.javaoperatorsdk.operator.baseapi.statuscache.internal; import java.time.Duration; @@ -11,19 +11,19 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.awaitility.Awaitility.await; -public class StatusPatchCacheWithLockIT { +public class StatusPatchCacheIT { public static final String TEST_1 = "test1"; @RegisterExtension LocallyRunOperatorExtension extension = LocallyRunOperatorExtension.builder() - .withReconciler(StatusPatchCacheWithLockReconciler.class) + .withReconciler(StatusPatchCacheReconciler.class) .build(); @Test void testStatusAlwaysUpToDate() { - var reconciler = extension.getReconcilerOfType(StatusPatchCacheWithLockReconciler.class); + var reconciler = extension.getReconcilerOfType(StatusPatchCacheReconciler.class); extension.create(testResource()); @@ -39,10 +39,10 @@ void testStatusAlwaysUpToDate() { }); } - StatusPatchCacheWithLockCustomResource testResource() { - var res = new StatusPatchCacheWithLockCustomResource(); + StatusPatchCacheCustomResource testResource() { + var res = new StatusPatchCacheCustomResource(); res.setMetadata(new ObjectMetaBuilder().withName(TEST_1).build()); - res.setSpec(new StatusPatchCacheWithLockSpec()); + res.setSpec(new StatusPatchCacheSpec()); return res; } } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/internal/StatusPatchCacheReconciler.java similarity index 72% rename from operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockReconciler.java rename to operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/internal/StatusPatchCacheReconciler.java index 8e6e87fba0..fc4bf2ca9b 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/internal/StatusPatchCacheReconciler.java @@ -1,4 +1,4 @@ -package io.javaoperatorsdk.operator.baseapi.statuscache.withlock; +package io.javaoperatorsdk.operator.baseapi.statuscache.internal; import java.util.List; @@ -13,16 +13,14 @@ import io.javaoperatorsdk.operator.processing.event.source.EventSource; @ControllerConfiguration -public class StatusPatchCacheWithLockReconciler - implements Reconciler { +public class StatusPatchCacheReconciler implements Reconciler { public volatile int latestValue = 0; public volatile boolean errorPresent = false; @Override - public UpdateControl reconcile( - StatusPatchCacheWithLockCustomResource resource, - Context context) + public UpdateControl reconcile( + StatusPatchCacheCustomResource resource, Context context) throws InterruptedException { if (resource.getStatus() != null && resource.getStatus().getValue() != latestValue) { @@ -50,21 +48,20 @@ public UpdateControl reconcile( } @Override - public List> prepareEventSources( - EventSourceContext context) { + public List> prepareEventSources( + EventSourceContext context) { // periodic event triggering for testing purposes return List.of(new PeriodicTriggerEventSource<>(context.getPrimaryCache())); } - private StatusPatchCacheWithLockCustomResource createFreshCopy( - StatusPatchCacheWithLockCustomResource resource) { - var res = new StatusPatchCacheWithLockCustomResource(); + private StatusPatchCacheCustomResource createFreshCopy(StatusPatchCacheCustomResource resource) { + var res = new StatusPatchCacheCustomResource(); res.setMetadata( new ObjectMetaBuilder() .withName(resource.getMetadata().getName()) .withNamespace(resource.getMetadata().getNamespace()) .build()); - res.setStatus(new StatusPatchCacheWithLockStatus()); + res.setStatus(new StatusPatchCacheStatus()); return res; } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockSpec.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/internal/StatusPatchCacheSpec.java similarity index 59% rename from operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockSpec.java rename to operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/internal/StatusPatchCacheSpec.java index 12cd1ac3e1..d1426fd943 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockSpec.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/internal/StatusPatchCacheSpec.java @@ -1,6 +1,6 @@ -package io.javaoperatorsdk.operator.baseapi.statuscache.withlock; +package io.javaoperatorsdk.operator.baseapi.statuscache.internal; -public class StatusPatchCacheWithLockSpec { +public class StatusPatchCacheSpec { private int counter = 0; diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/internal/StatusPatchCacheStatus.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/internal/StatusPatchCacheStatus.java new file mode 100644 index 0000000000..00bc4b6f04 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/internal/StatusPatchCacheStatus.java @@ -0,0 +1,15 @@ +package io.javaoperatorsdk.operator.baseapi.statuscache.internal; + +public class StatusPatchCacheStatus { + + private Integer value = 0; + + public Integer getValue() { + return value; + } + + public StatusPatchCacheStatus setValue(Integer value) { + this.value = value; + return this; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockStatus.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockStatus.java deleted file mode 100644 index 8d1e559308..0000000000 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/withlock/StatusPatchCacheWithLockStatus.java +++ /dev/null @@ -1,15 +0,0 @@ -package io.javaoperatorsdk.operator.baseapi.statuscache.withlock; - -public class StatusPatchCacheWithLockStatus { - - private Integer value = 0; - - public Integer getValue() { - return value; - } - - public StatusPatchCacheWithLockStatus setValue(Integer value) { - this.value = value; - return this; - } -} From a71eafe103f932e6e857590f65c3b482bb15910f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Tue, 22 Apr 2025 14:29:29 +0200 Subject: [PATCH 25/41] docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../en/docs/documentation/reconciler.md | 118 ++++++++++++++++++ .../PrimaryUpdateAndCacheUtils.java | 16 +-- .../internal/StatusPatchCacheReconciler.java | 6 +- .../StatusPatchPrimaryCacheReconciler.java | 5 +- 4 files changed, 129 insertions(+), 16 deletions(-) diff --git a/docs/content/en/docs/documentation/reconciler.md b/docs/content/en/docs/documentation/reconciler.md index 26a2a20d61..9ba8ea3201 100644 --- a/docs/content/en/docs/documentation/reconciler.md +++ b/docs/content/en/docs/documentation/reconciler.md @@ -169,3 +169,121 @@ You can specify the name of the finalizer to use for your `Reconciler` using the annotation. If you do not specify a finalizer name, one will be automatically generated for you. From v5, by default, the finalizer is added using Server Side Apply. See also `UpdateControl` in docs. + +### Making sure primary is up to date for the next reconciliation + +When you implement a reconciler as a final step (but maybe also multiple times during one reconciliation), you +usually update the status subresource with the information that was available during the reconciliation. +Sometimes this is referred to as the last observed state. +When the resource is updated, the framework does not cache the resource directly from the response of the update. +Instead, the underlying informer eventually receives an event with the updated resource and caches the resource. +Therefore, it can happen that on next reconciliation the primary resource is not up-to-date regarding your updated (note that other event sources +can trigger the reconciliation meanwhile). This is not usually a problem, since the status is not used as an input, +the reconciliation runs again, and the status us updated again. The caches are eventually consistent. + +However, there are cases when you would like to store some state in the status, typically generated +IDs of external resources. +See related topic in Kubernetes docs: [Representing Allocated Values](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#representing-allocated-values). +In this case, it is reasonable to expect to have the state always available for the next reconciliation, +to avoid generating the resource again and other race conditions. + +Therefore, +the framework provides facilities +to cover these use cases withing [`PrimaryUpdateAndCacheUtils`](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java#L16). +These utility methods come in two flavors: + +#### Using internal cache + +In almost all cases for this purpose, you can use internal caches: + +```java + @Override +public UpdateControl reconcile( + StatusPatchCacheCustomResource resource, Context context) { + + // omitted logic + + // update with SSA requires a fresh copy + var freshCopy = createFreshCopy(primary); + + freshCopy.getStatus().setValue(statusWithState()); + + var updatedResource = PrimaryUpdateAndCacheUtils.ssaPatchAndCacheStatus(resource, freshCopy, context); + + return UpdateControl.noUpdate(); + } +``` + +In the background `PrimaryUpdateAndCacheUtils.ssaPatchAndCacheStatusWith` puts the result of the update into an internal +cache of the event source of primary resource, and will make sure that the next reconciliation will contain the most +recent version of the resource. Note that it is not necessarily the version from the update response, it can be newer +since other parties can do additional updates meanwhile, but if not explicitly modified, it will contain the up-to-date +status. + +See related integration test [here](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/internal). + +This approach works with the default configuration of the framework and should be good to go in most of the cases. +Without going further into the details, this won't work if `ConfigurtionService.parseResourceVersionsForEventFilteringAndCaching` +is set to `false` (more precisely there are some edge cases when it won't work). For that case framework provides the following solution: + +#### Using `PrimaryResourceCache` cache + +As an alternative, you can use an explicit caching approach that the framework supports: + +```java + +// We on purpose don't use the provided predicate to show what a custom one could look like. + private final PrimaryResourceCache cache = + new PrimaryResourceCache<>( + (statusPatchCacheCustomResourcePair, statusPatchCacheCustomResource) -> + statusPatchCacheCustomResource.getStatus().getValue() + >= statusPatchCacheCustomResourcePair.afterUpdate().getStatus().getValue()); + + @Override + public UpdateControl reconcile( + StatusPatchPrimaryCacheCustomResource primary, + Context context) { + + // + primary = cache.getFreshResource(primary); + + // omitted logic + + var freshCopy = createFreshCopy(primary); + + freshCopy.getStatus().setValue(statusWithState()); + + var updated = + PrimaryUpdateAndCacheUtils.ssaPatchAndCacheStatus(primary, freshCopy, context, cache); + + return UpdateControl.noUpdate(); + } + + @Override + public DeleteControl cleanup( + StatusPatchPrimaryCacheCustomResource resource, + Context context) + throws Exception { + // cleanup the cache on resource deletion + cache.cleanup(resource); + return DeleteControl.defaultDelete(); + } + +``` + +[`PrimaryResourceCache`](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/support/PrimaryResourceCache.java) +is designed for this purpose. +As shown in the example above, it is up to you to provide a predicate to determine if the resource is more recent than the one available. +In other words, when to evict the resource from the cache. Typically, as show in the [integration test](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache) +you can have a counter in status to check on that. + +Since all of this happens explicitly, you cannot use it for now with managed dependent resources and workflows. + +#### Additional remarks + +As shown in the integration tests, there is no optimistic locking used when updating the +[resource](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/internal/StatusPatchCacheReconciler.java#L41) +(in other works `metadata.resourceVersion` is set to `null`). +This is desired since you don't want the patch to fail on update. + +In addition, you can configure retry for in fabric8 client. diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java index de6d8c7852..a783053ea4 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java @@ -28,8 +28,8 @@ private PrimaryUpdateAndCacheUtils() {} * @return updated resource * @param

primary resource type */ - public static

P updateAndCacheStatusWith(P primary, Context

context) { - return patchAndCacheStatusWith(primary, context, (p, c) -> c.resource(primary).updateStatus()); + public static

P updateAndCacheStatus(P primary, Context

context) { + return patchAndCacheStatus(primary, context, (p, c) -> c.resource(primary).updateStatus()); } /** @@ -41,8 +41,8 @@ public static

P updateAndCacheStatusWith(P primary, Cont * @return updated resource * @param

primary resource type */ - public static

P patchAndCacheStatusWith(P primary, Context

context) { - return patchAndCacheStatusWith(primary, context, (p, c) -> c.resource(primary).patchStatus()); + public static

P patchAndCacheStatus(P primary, Context

context) { + return patchAndCacheStatus(primary, context, (p, c) -> c.resource(primary).patchStatus()); } /** @@ -54,9 +54,9 @@ public static

P patchAndCacheStatusWith(P primary, Conte * @return updated resource * @param

primary resource type */ - public static

P editAndCacheStatusWith( + public static

P editAndCacheStatus( P primary, Context

context, UnaryOperator

operation) { - return patchAndCacheStatusWith( + return patchAndCacheStatus( primary, context, (p, c) -> c.resource(primary).editStatus(operation)); } @@ -70,7 +70,7 @@ public static

P editAndCacheStatusWith( * @return the updated resource. * @param

primary resource type */ - public static

P patchAndCacheStatusWith( + public static

P patchAndCacheStatus( P primary, Context

context, BiFunction patch) { var updatedResource = patch.apply(primary, context.getClient()); context @@ -90,7 +90,7 @@ public static

P patchAndCacheStatusWith( * @return the updated resource. * @param

primary resource type */ - public static

P ssaPatchAndCacheStatusWith( + public static

P ssaPatchAndCacheStatus( P primary, P freshResourceWithStatus, Context

context) { var res = context diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/internal/StatusPatchCacheReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/internal/StatusPatchCacheReconciler.java index fc4bf2ca9b..cf26bf0231 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/internal/StatusPatchCacheReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/internal/StatusPatchCacheReconciler.java @@ -20,8 +20,7 @@ public class StatusPatchCacheReconciler implements Reconciler reconcile( - StatusPatchCacheCustomResource resource, Context context) - throws InterruptedException { + StatusPatchCacheCustomResource resource, Context context) { if (resource.getStatus() != null && resource.getStatus().getValue() != latestValue) { errorPresent = true; @@ -40,8 +39,7 @@ public UpdateControl reconcile( .setValue(resource.getStatus() == null ? 1 : resource.getStatus().getValue() + 1); resource.getMetadata().setResourceVersion(null); - var updated = - PrimaryUpdateAndCacheUtils.ssaPatchAndCacheStatusWith(resource, freshCopy, context); + var updated = PrimaryUpdateAndCacheUtils.ssaPatchAndCacheStatus(resource, freshCopy, context); latestValue = updated.getStatus().getValue(); return UpdateControl.noUpdate(); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheReconciler.java index 879cc1e3d8..c25fcddfec 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache/StatusPatchPrimaryCacheReconciler.java @@ -33,8 +33,7 @@ public class StatusPatchPrimaryCacheReconciler @Override public UpdateControl reconcile( StatusPatchPrimaryCacheCustomResource primary, - Context context) - throws InterruptedException { + Context context) { primary = cache.getFreshResource(primary); @@ -48,8 +47,6 @@ public UpdateControl reconcile( } var freshCopy = createFreshCopy(primary); - // setting the resource version - freshCopy.getMetadata().setResourceVersion(primary.getMetadata().getResourceVersion()); freshCopy .getStatus() .setValue(primary.getStatus() == null ? 1 : primary.getStatus().getValue() + 1); From eff1ccb00f3ca781b7372e5d8969616dc85cce51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Fri, 25 Apr 2025 09:56:47 +0200 Subject: [PATCH 26/41] Update operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java Co-authored-by: Antonio <122279781+afalhambra-hivemq@users.noreply.github.com> --- .../operator/api/reconciler/PrimaryUpdateAndCacheUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java index a783053ea4..bec784d228 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java @@ -184,7 +184,7 @@ public static

P patchAndCacheStatus( * Updates the resource and adds it to the {@link PrimaryResourceCache} provided. Optimistic * locking is not required. * - * @param primary resource* + * @param primary resource * @param context of reconciliation * @param cache - resource cache managed by user * @return the updated resource. From 217629f0e2e845461083e024b3259a6f6c96b116 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Fri, 25 Apr 2025 09:56:58 +0200 Subject: [PATCH 27/41] Update operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java Co-authored-by: Antonio <122279781+afalhambra-hivemq@users.noreply.github.com> --- .../operator/api/reconciler/PrimaryUpdateAndCacheUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java index bec784d228..50f7f7d413 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java @@ -164,7 +164,7 @@ public static

P editAndCacheStatus( * Patches the resource with JSON Merge patch and adds it to the {@link PrimaryResourceCache} * provided. Optimistic locking is not required. * - * @param primary resource* + * @param primary resource * @param context of reconciliation * @param cache - resource cache managed by user * @return the updated resource. From a8e7efc7edaa4c657383ec074a77e205e67ab748 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Fri, 25 Apr 2025 09:57:27 +0200 Subject: [PATCH 28/41] Update docs/content/en/docs/documentation/reconciler.md Co-authored-by: Antonio <122279781+afalhambra-hivemq@users.noreply.github.com> --- docs/content/en/docs/documentation/reconciler.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/content/en/docs/documentation/reconciler.md b/docs/content/en/docs/documentation/reconciler.md index 9ba8ea3201..b5a1ddd25b 100644 --- a/docs/content/en/docs/documentation/reconciler.md +++ b/docs/content/en/docs/documentation/reconciler.md @@ -189,7 +189,7 @@ to avoid generating the resource again and other race conditions. Therefore, the framework provides facilities -to cover these use cases withing [`PrimaryUpdateAndCacheUtils`](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java#L16). +to cover these use cases with [`PrimaryUpdateAndCacheUtils`](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java#L16). These utility methods come in two flavors: #### Using internal cache From a1d303d14612e93bbd2df1ea3c099295e59deba6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Fri, 25 Apr 2025 09:57:34 +0200 Subject: [PATCH 29/41] Update operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java Co-authored-by: Antonio <122279781+afalhambra-hivemq@users.noreply.github.com> --- .../operator/api/reconciler/PrimaryUpdateAndCacheUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java index 50f7f7d413..b896bf0b89 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java @@ -144,7 +144,7 @@ public static

P ssaPatchAndCacheStatus( * Patches the resource with JSON Patch and adds it to the {@link PrimaryResourceCache} provided. * Optimistic locking is not required. * - * @param primary resource* + * @param primary resource * @param context of reconciliation * @param cache - resource cache managed by user * @return the updated resource. From 7b58dca067dd26dda4d85381dc9f28c8c7181639 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Fri, 25 Apr 2025 09:58:14 +0200 Subject: [PATCH 30/41] Update docs/content/en/docs/documentation/reconciler.md Co-authored-by: Antonio <122279781+afalhambra-hivemq@users.noreply.github.com> --- docs/content/en/docs/documentation/reconciler.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/content/en/docs/documentation/reconciler.md b/docs/content/en/docs/documentation/reconciler.md index b5a1ddd25b..b25c555803 100644 --- a/docs/content/en/docs/documentation/reconciler.md +++ b/docs/content/en/docs/documentation/reconciler.md @@ -214,7 +214,7 @@ public UpdateControl reconcile( } ``` -In the background `PrimaryUpdateAndCacheUtils.ssaPatchAndCacheStatusWith` puts the result of the update into an internal +In the background `PrimaryUpdateAndCacheUtils.ssaPatchAndCacheStatus` puts the result of the update into an internal cache of the event source of primary resource, and will make sure that the next reconciliation will contain the most recent version of the resource. Note that it is not necessarily the version from the update response, it can be newer since other parties can do additional updates meanwhile, but if not explicitly modified, it will contain the up-to-date From a656160a7634fe009652a48246b37cba03f975d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Fri, 25 Apr 2025 10:40:44 +0200 Subject: [PATCH 31/41] improvements MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../en/docs/documentation/reconciler.md | 4 +- .../PrimaryUpdateAndCacheUtils.java | 81 ++++++++++--------- .../internal/StatusPatchCacheReconciler.java | 1 - 3 files changed, 45 insertions(+), 41 deletions(-) diff --git a/docs/content/en/docs/documentation/reconciler.md b/docs/content/en/docs/documentation/reconciler.md index b25c555803..f49f6844e6 100644 --- a/docs/content/en/docs/documentation/reconciler.md +++ b/docs/content/en/docs/documentation/reconciler.md @@ -277,7 +277,9 @@ As shown in the example above, it is up to you to provide a predicate to determi In other words, when to evict the resource from the cache. Typically, as show in the [integration test](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache) you can have a counter in status to check on that. -Since all of this happens explicitly, you cannot use it for now with managed dependent resources and workflows. +Since all of this happens explicitly, you cannot use this approach for managed dependent resources and workflows; +Since passing of the primary resource to the dependent resource always comes directly from the underlying informer event +source cache. #### Additional remarks diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java index b896bf0b89..cc4709f8ef 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java @@ -1,18 +1,23 @@ package io.javaoperatorsdk.operator.api.reconciler; -import java.util.function.BiFunction; +import java.util.function.Supplier; import java.util.function.UnaryOperator; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import io.fabric8.kubernetes.api.model.HasMetadata; -import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.dsl.base.PatchContext; import io.fabric8.kubernetes.client.dsl.base.PatchType; import io.javaoperatorsdk.operator.api.reconciler.support.PrimaryResourceCache; import io.javaoperatorsdk.operator.processing.event.ResourceID; +/** + * Utility methods to patch the primary resource state and store it to the related cache, to make + * sure that fresh resource is present for the next reconciliation. The main use case for such + * updates is to store state is resource status. Use of optimistic locking is not desired for such + * updates, since we don't want to patch fail and lose information that we want to store. + */ public class PrimaryUpdateAndCacheUtils { private PrimaryUpdateAndCacheUtils() {} @@ -29,7 +34,9 @@ private PrimaryUpdateAndCacheUtils() {} * @param

primary resource type */ public static

P updateAndCacheStatus(P primary, Context

context) { - return patchAndCacheStatus(primary, context, (p, c) -> c.resource(primary).updateStatus()); + logWarnIfResourceVersionPresent(primary); + return patchAndCacheStatus( + primary, context, () -> context.getClient().resource(primary).updateStatus()); } /** @@ -42,7 +49,9 @@ public static

P updateAndCacheStatus(P primary, Context< * @param

primary resource type */ public static

P patchAndCacheStatus(P primary, Context

context) { - return patchAndCacheStatus(primary, context, (p, c) -> c.resource(primary).patchStatus()); + logWarnIfResourceVersionPresent(primary); + return patchAndCacheStatus( + primary, context, () -> context.getClient().resource(primary).patchStatus()); } /** @@ -56,8 +65,9 @@ public static

P patchAndCacheStatus(P primary, Context

P editAndCacheStatus( P primary, Context

context, UnaryOperator

operation) { + logWarnIfResourceVersionPresent(primary); return patchAndCacheStatus( - primary, context, (p, c) -> c.resource(primary).editStatus(operation)); + primary, context, () -> context.getClient().resource(primary).editStatus(operation)); } /** @@ -65,14 +75,13 @@ public static

P editAndCacheStatus( * * @param primary resource * @param context of reconciliation - * @param patch free implementation of cache - make sure you use optimistic locking during the - * update + * @param patch free implementation of cache * @return the updated resource. * @param

primary resource type */ public static

P patchAndCacheStatus( - P primary, Context

context, BiFunction patch) { - var updatedResource = patch.apply(primary, context.getClient()); + P primary, Context

context, Supplier

patch) { + var updatedResource = patch.get(); context .eventSourceRetriever() .getControllerEventSource() @@ -92,6 +101,7 @@ public static

P patchAndCacheStatus( */ public static

P ssaPatchAndCacheStatus( P primary, P freshResourceWithStatus, Context

context) { + logWarnIfResourceVersionPresent(freshResourceWithStatus); var res = context .getClient() @@ -112,8 +122,7 @@ public static

P ssaPatchAndCacheStatus( } /** - * Patches the resource and adds it to the {@link PrimaryResourceCache} provided. Optimistic - * locking is not required. + * Patches the resource and adds it to the {@link PrimaryResourceCache} provided. * * @param primary resource * @param freshResourceWithStatus - fresh resource with target state @@ -127,10 +136,11 @@ public static

P ssaPatchAndCacheStatus( logWarnIfResourceVersionPresent(freshResourceWithStatus); return patchAndCacheStatus( primary, - context.getClient(), cache, - (P p, KubernetesClient c) -> - c.resource(freshResourceWithStatus) + () -> + context + .getClient() + .resource(freshResourceWithStatus) .subresource("status") .patch( new PatchContext.Builder() @@ -142,7 +152,6 @@ public static

P ssaPatchAndCacheStatus( /** * Patches the resource with JSON Patch and adds it to the {@link PrimaryResourceCache} provided. - * Optimistic locking is not required. * * @param primary resource * @param context of reconciliation @@ -154,15 +163,12 @@ public static

P editAndCacheStatus( P primary, Context

context, PrimaryResourceCache

cache, UnaryOperator

operation) { logWarnIfResourceVersionPresent(primary); return patchAndCacheStatus( - primary, - context.getClient(), - cache, - (P p, KubernetesClient c) -> c.resource(primary).editStatus(operation)); + primary, cache, () -> context.getClient().resource(primary).editStatus(operation)); } /** * Patches the resource with JSON Merge patch and adds it to the {@link PrimaryResourceCache} - * provided. Optimistic locking is not required. + * provided. * * @param primary resource * @param context of reconciliation @@ -174,15 +180,11 @@ public static

P patchAndCacheStatus( P primary, Context

context, PrimaryResourceCache

cache) { logWarnIfResourceVersionPresent(primary); return patchAndCacheStatus( - primary, - context.getClient(), - cache, - (P p, KubernetesClient c) -> c.resource(primary).patchStatus()); + primary, cache, () -> context.getClient().resource(primary).patchStatus()); } /** - * Updates the resource and adds it to the {@link PrimaryResourceCache} provided. Optimistic - * locking is not required. + * Updates the resource and adds it to the {@link PrimaryResourceCache} provided. * * @param primary resource * @param context of reconciliation @@ -194,18 +196,21 @@ public static

P updateAndCacheStatus( P primary, Context

context, PrimaryResourceCache

cache) { logWarnIfResourceVersionPresent(primary); return patchAndCacheStatus( - primary, - context.getClient(), - cache, - (P p, KubernetesClient c) -> c.resource(primary).updateStatus()); + primary, cache, () -> context.getClient().resource(primary).updateStatus()); } + /** + * Updates the resource using the user provided implementation anc caches the result. + * + * @param primary resource + * @param cache resource cache managed by user + * @param patch implementation of resource update* + * @return the updated resource. + * @param

primary resource type + */ public static

P patchAndCacheStatus( - P primary, - KubernetesClient client, - PrimaryResourceCache

cache, - BiFunction patch) { - var updatedResource = patch.apply(primary, client); + P primary, PrimaryResourceCache

cache, Supplier

patch) { + var updatedResource = patch.get(); cache.cacheResource(primary, updatedResource); return updatedResource; } @@ -213,10 +218,8 @@ public static

P patchAndCacheStatus( private static

void logWarnIfResourceVersionPresent(P primary) { if (primary.getMetadata().getResourceVersion() != null) { log.warn( - "Primary resource version is NOT null, for caching with optimistic locking use" - + " alternative methods. Name: {} namespace: {}", - primary.getMetadata().getName(), - primary.getMetadata().getNamespace()); + "The metadata.resourceVersion of primary resource is NOT null, " + + "using optimistic locking is discouraged for this purpose. "); } } } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/internal/StatusPatchCacheReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/internal/StatusPatchCacheReconciler.java index cf26bf0231..76575fe9c5 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/internal/StatusPatchCacheReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/internal/StatusPatchCacheReconciler.java @@ -38,7 +38,6 @@ public UpdateControl reconcile( .getStatus() .setValue(resource.getStatus() == null ? 1 : resource.getStatus().getValue() + 1); - resource.getMetadata().setResourceVersion(null); var updated = PrimaryUpdateAndCacheUtils.ssaPatchAndCacheStatus(resource, freshCopy, context); latestValue = updated.getStatus().getValue(); From 1f1e1d0501de4df11c21ca20c194010b7a22395c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Fri, 25 Apr 2025 10:50:57 +0200 Subject: [PATCH 32/41] docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- docs/content/en/docs/documentation/reconciler.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/content/en/docs/documentation/reconciler.md b/docs/content/en/docs/documentation/reconciler.md index f49f6844e6..aa8a918ae9 100644 --- a/docs/content/en/docs/documentation/reconciler.md +++ b/docs/content/en/docs/documentation/reconciler.md @@ -216,9 +216,9 @@ public UpdateControl reconcile( In the background `PrimaryUpdateAndCacheUtils.ssaPatchAndCacheStatus` puts the result of the update into an internal cache of the event source of primary resource, and will make sure that the next reconciliation will contain the most -recent version of the resource. Note that it is not necessarily the version from the update response, it can be newer -since other parties can do additional updates meanwhile, but if not explicitly modified, it will contain the up-to-date -status. +recent version of the resource. Note that it is not necessarily the version of the resource you got as response from the update , +it can be newer since other parties can do additional updates meanwhile, but if not explicitly modified, +it will contain the up-to-date status. See related integration test [here](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/internal). From 95a9f2eafff7fe27820585815a077ac94f9555a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Fri, 25 Apr 2025 10:53:57 +0200 Subject: [PATCH 33/41] improve MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../statuscache/internal/StatusPatchCacheReconciler.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/internal/StatusPatchCacheReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/internal/StatusPatchCacheReconciler.java index 76575fe9c5..8a3a72a901 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/internal/StatusPatchCacheReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/internal/StatusPatchCacheReconciler.java @@ -32,8 +32,7 @@ public UpdateControl reconcile( } var freshCopy = createFreshCopy(resource); - // setting the resource version - freshCopy.getMetadata().setResourceVersion(resource.getMetadata().getResourceVersion()); + freshCopy .getStatus() .setValue(resource.getStatus() == null ? 1 : resource.getStatus().getValue() + 1); From 720a4c5cce9c5421378a3c07b95fc078ee903107 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Fri, 25 Apr 2025 10:56:53 +0200 Subject: [PATCH 34/41] docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- docs/content/en/docs/documentation/reconciler.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/content/en/docs/documentation/reconciler.md b/docs/content/en/docs/documentation/reconciler.md index aa8a918ae9..7ad4cfba68 100644 --- a/docs/content/en/docs/documentation/reconciler.md +++ b/docs/content/en/docs/documentation/reconciler.md @@ -228,7 +228,8 @@ is set to `false` (more precisely there are some edge cases when it won't work). #### Using `PrimaryResourceCache` cache -As an alternative, you can use an explicit caching approach that the framework supports: +As an alternative, for very rare cases when `ConfigurtionService.parseResourceVersionsForEventFilteringAndCaching` +needs to be set to `false` you can use an explicit caching approach: ```java From a510cc700ab7b6edc86cf2761b845444419394c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Fri, 25 Apr 2025 12:58:27 +0200 Subject: [PATCH 35/41] wip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- docs/content/en/docs/documentation/reconciler.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/content/en/docs/documentation/reconciler.md b/docs/content/en/docs/documentation/reconciler.md index 7ad4cfba68..7b927c311a 100644 --- a/docs/content/en/docs/documentation/reconciler.md +++ b/docs/content/en/docs/documentation/reconciler.md @@ -245,7 +245,7 @@ needs to be set to `false` you can use an explicit caching approach: StatusPatchPrimaryCacheCustomResource primary, Context context) { - // + // cache will compare the current and the cached resource and return the more recent. (And evic the old) primary = cache.getFreshResource(primary); // omitted logic @@ -278,7 +278,7 @@ As shown in the example above, it is up to you to provide a predicate to determi In other words, when to evict the resource from the cache. Typically, as show in the [integration test](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache) you can have a counter in status to check on that. -Since all of this happens explicitly, you cannot use this approach for managed dependent resources and workflows; +Since all of this happens explicitly, you cannot use this approach for managed dependent resources and workflows (you can still use not managed); Since passing of the primary resource to the dependent resource always comes directly from the underlying informer event source cache. From 0a79dd75ac3472f03ccfe5a3757102e4d464483a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Fri, 25 Apr 2025 13:05:36 +0200 Subject: [PATCH 36/41] wip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- docs/content/en/docs/documentation/reconciler.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/content/en/docs/documentation/reconciler.md b/docs/content/en/docs/documentation/reconciler.md index 7b927c311a..02999c77f0 100644 --- a/docs/content/en/docs/documentation/reconciler.md +++ b/docs/content/en/docs/documentation/reconciler.md @@ -205,7 +205,6 @@ public UpdateControl reconcile( // update with SSA requires a fresh copy var freshCopy = createFreshCopy(primary); - freshCopy.getStatus().setValue(statusWithState()); var updatedResource = PrimaryUpdateAndCacheUtils.ssaPatchAndCacheStatus(resource, freshCopy, context); @@ -226,7 +225,7 @@ This approach works with the default configuration of the framework and should b Without going further into the details, this won't work if `ConfigurtionService.parseResourceVersionsForEventFilteringAndCaching` is set to `false` (more precisely there are some edge cases when it won't work). For that case framework provides the following solution: -#### Using `PrimaryResourceCache` cache +#### Fallback approach: using `PrimaryResourceCache` cache As an alternative, for very rare cases when `ConfigurtionService.parseResourceVersionsForEventFilteringAndCaching` needs to be set to `false` you can use an explicit caching approach: From f12955b123f4d5a947d98fb8c9e7fe40b1e14020 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Mon, 28 Apr 2025 19:24:55 +0200 Subject: [PATCH 37/41] fix: wording Co-authored-by: Antonio <122279781+afalhambra-hivemq@users.noreply.github.com> --- docs/content/en/docs/documentation/reconciler.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/content/en/docs/documentation/reconciler.md b/docs/content/en/docs/documentation/reconciler.md index 02999c77f0..f8d174b089 100644 --- a/docs/content/en/docs/documentation/reconciler.md +++ b/docs/content/en/docs/documentation/reconciler.md @@ -177,7 +177,7 @@ usually update the status subresource with the information that was available du Sometimes this is referred to as the last observed state. When the resource is updated, the framework does not cache the resource directly from the response of the update. Instead, the underlying informer eventually receives an event with the updated resource and caches the resource. -Therefore, it can happen that on next reconciliation the primary resource is not up-to-date regarding your updated (note that other event sources +Therefore, it can happen that on next reconciliation the primary resource is not up-to-date regarding your updated status subresource (note that other event sources can trigger the reconciliation meanwhile). This is not usually a problem, since the status is not used as an input, the reconciliation runs again, and the status us updated again. The caches are eventually consistent. From 717933bfca80a3f9650684168f32dbfc84445631 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Mon, 28 Apr 2025 20:02:07 +0200 Subject: [PATCH 38/41] fix: wording & typos Signed-off-by: Chris Laprun --- .../en/docs/documentation/reconciler.md | 68 +++++++++---------- 1 file changed, 31 insertions(+), 37 deletions(-) diff --git a/docs/content/en/docs/documentation/reconciler.md b/docs/content/en/docs/documentation/reconciler.md index f8d174b089..aded1ca574 100644 --- a/docs/content/en/docs/documentation/reconciler.md +++ b/docs/content/en/docs/documentation/reconciler.md @@ -170,26 +170,20 @@ annotation. If you do not specify a finalizer name, one will be automatically ge From v5, by default, the finalizer is added using Server Side Apply. See also `UpdateControl` in docs. -### Making sure primary is up to date for the next reconciliation - -When you implement a reconciler as a final step (but maybe also multiple times during one reconciliation), you -usually update the status subresource with the information that was available during the reconciliation. -Sometimes this is referred to as the last observed state. -When the resource is updated, the framework does not cache the resource directly from the response of the update. -Instead, the underlying informer eventually receives an event with the updated resource and caches the resource. -Therefore, it can happen that on next reconciliation the primary resource is not up-to-date regarding your updated status subresource (note that other event sources -can trigger the reconciliation meanwhile). This is not usually a problem, since the status is not used as an input, -the reconciliation runs again, and the status us updated again. The caches are eventually consistent. - -However, there are cases when you would like to store some state in the status, typically generated -IDs of external resources. -See related topic in Kubernetes docs: [Representing Allocated Values](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#representing-allocated-values). -In this case, it is reasonable to expect to have the state always available for the next reconciliation, -to avoid generating the resource again and other race conditions. - -Therefore, -the framework provides facilities -to cover these use cases with [`PrimaryUpdateAndCacheUtils`](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java#L16). +### Making sure the primary resource is up to date for the next reconciliation + +It is typical to want to update the status subresource with the information that is available during the reconciliation. +This is sometimes referred to as the last observed state. When the primary resource is updated, though, the framework +does not cache the resource directly, relying instead on the propagation of the update to the underlying informer's +cache. It can, therefore, happen that, if other events trigger other reconciliations before the informer cache gets +updated, your reconciler does not see the latest version of the primary resource. While this might not typically be a +problem in most cases, as caches eventually become consistent, depending on your reconciliation logic, you might still +require the latest status version possible, for example if the status subresource is used as a communication mechanism, +see [Representing Allocated Values](https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#representing-allocated-values) +from the Kubernetes docs for more details. + +The framework provides utilities to help with these use cases with +[`PrimaryUpdateAndCacheUtils`](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java). These utility methods come in two flavors: #### Using internal cache @@ -214,20 +208,19 @@ public UpdateControl reconcile( ``` In the background `PrimaryUpdateAndCacheUtils.ssaPatchAndCacheStatus` puts the result of the update into an internal -cache of the event source of primary resource, and will make sure that the next reconciliation will contain the most -recent version of the resource. Note that it is not necessarily the version of the resource you got as response from the update , -it can be newer since other parties can do additional updates meanwhile, but if not explicitly modified, -it will contain the up-to-date status. +cache and will make sure that the next reconciliation will contain the most recent version of the resource. Note that it +is not necessarily the version of the resource you got as response from the update, it can be newer since other parties +can do additional updates meanwhile, but if not explicitly modified, it will contain the up-to-date status. See related integration test [here](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/internal). This approach works with the default configuration of the framework and should be good to go in most of the cases. -Without going further into the details, this won't work if `ConfigurtionService.parseResourceVersionsForEventFilteringAndCaching` +Without going further into the details, this won't work if `ConfigurationService.parseResourceVersionsForEventFilteringAndCaching` is set to `false` (more precisely there are some edge cases when it won't work). For that case framework provides the following solution: #### Fallback approach: using `PrimaryResourceCache` cache -As an alternative, for very rare cases when `ConfigurtionService.parseResourceVersionsForEventFilteringAndCaching` +As an alternative, for very rare cases when `ConfigurationService.parseResourceVersionsForEventFilteringAndCaching` needs to be set to `false` you can use an explicit caching approach: ```java @@ -244,7 +237,7 @@ needs to be set to `false` you can use an explicit caching approach: StatusPatchPrimaryCacheCustomResource primary, Context context) { - // cache will compare the current and the cached resource and return the more recent. (And evic the old) + // cache will compare the current and the cached resource and return the more recent. (And evict the old) primary = cache.getFreshResource(primary); // omitted logic @@ -272,20 +265,21 @@ needs to be set to `false` you can use an explicit caching approach: ``` [`PrimaryResourceCache`](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/support/PrimaryResourceCache.java) -is designed for this purpose. -As shown in the example above, it is up to you to provide a predicate to determine if the resource is more recent than the one available. -In other words, when to evict the resource from the cache. Typically, as show in the [integration test](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache) -you can have a counter in status to check on that. +is designed for this purpose. As shown in the example above, it is up to you to provide a predicate to determine if the +resource is more recent than the one available. In other words, when to evict the resource from the cache. Typically, as +shown in +the [integration test](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/primarycache) +you can have a counter in status to check on that. -Since all of this happens explicitly, you cannot use this approach for managed dependent resources and workflows (you can still use not managed); -Since passing of the primary resource to the dependent resource always comes directly from the underlying informer event -source cache. +Since all of this happens explicitly, you cannot use this approach for managed dependent resources and workflows and +will need to use the unmanaged approach instead. This is due to the fact that managed dependent resources always get +their associated primary resource from the underlying informer event source cache. #### Additional remarks As shown in the integration tests, there is no optimistic locking used when updating the [resource](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/internal/StatusPatchCacheReconciler.java#L41) -(in other works `metadata.resourceVersion` is set to `null`). -This is desired since you don't want the patch to fail on update. +(in other words `metadata.resourceVersion` is set to `null`). This is desired since you don't want the patch to fail on +update. -In addition, you can configure retry for in fabric8 client. +In addition, you can configure the Fabric8 client retry. From 966aee7563d59ba72d5315ddd5887753591d073d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Tue, 29 Apr 2025 13:34:09 +0200 Subject: [PATCH 39/41] add link for fabric8 configuration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- docs/content/en/docs/documentation/reconciler.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/content/en/docs/documentation/reconciler.md b/docs/content/en/docs/documentation/reconciler.md index aded1ca574..b9ede8aa95 100644 --- a/docs/content/en/docs/documentation/reconciler.md +++ b/docs/content/en/docs/documentation/reconciler.md @@ -282,4 +282,4 @@ As shown in the integration tests, there is no optimistic locking used when upda (in other words `metadata.resourceVersion` is set to `null`). This is desired since you don't want the patch to fail on update. -In addition, you can configure the Fabric8 client retry. +In addition, you can configure the [Fabric8 client retry](https://github.com/fabric8io/kubernetes-client?tab=readme-ov-file#configuring-the-client). From 68471f748dcc988c3f929818d9f1bf5ee2e69d87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Tue, 29 Apr 2025 13:35:48 +0200 Subject: [PATCH 40/41] docs improve MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../operator/api/reconciler/PrimaryUpdateAndCacheUtils.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java index cc4709f8ef..174f7667f6 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java @@ -122,7 +122,7 @@ public static

P ssaPatchAndCacheStatus( } /** - * Patches the resource and adds it to the {@link PrimaryResourceCache} provided. + * Patches the resource and adds it to the {@link PrimaryResourceCache}. * * @param primary resource * @param freshResourceWithStatus - fresh resource with target state @@ -151,7 +151,7 @@ public static

P ssaPatchAndCacheStatus( } /** - * Patches the resource with JSON Patch and adds it to the {@link PrimaryResourceCache} provided. + * Patches the resource with JSON Patch and adds it to the {@link PrimaryResourceCache}. * * @param primary resource * @param context of reconciliation @@ -184,7 +184,7 @@ public static

P patchAndCacheStatus( } /** - * Updates the resource and adds it to the {@link PrimaryResourceCache} provided. + * Updates the resource and adds it to the {@link PrimaryResourceCache}. * * @param primary resource * @param context of reconciliation From fb090a37056fe0412b8b3bba5650420d124087b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Tue, 29 Apr 2025 13:42:10 +0200 Subject: [PATCH 41/41] docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../source/informer/InformerEventSource.java | 46 ++++++++----------- 1 file changed, 19 insertions(+), 27 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java index 40ceaa975b..b52dc278f2 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java @@ -22,41 +22,33 @@ /** * Wraps informer(s) so they are connected to the eventing system of the framework. Note that since - * this is built on top of Fabric8 client Informers, it also supports caching resources using caching from - * informer caches as well as additional caches described below. + * this is built on top of Fabric8 client Informers, it also supports caching resources using + * caching from informer caches as well as additional caches described below. * *

InformerEventSource also supports two features to better handle events and caching of - * resources on top of Informers from the Fabric8 Kubernetes client. These two features - * are related to each other as follows: + * resources on top of Informers from the Fabric8 Kubernetes client. These two features are related + * to each other as follows: * *

    - *
  1. Ensuring the cache contains the fresh resource after an update. This is - * important for {@link - * io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource} and mainly for - * {@link + *
  2. Ensuring the cache contains the fresh resource after an update. This is important for + * {@link io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource} and mainly + * for {@link * io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource} so - * that {@link io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource#getSecondaryResource(HasMetadata, Context)} always returns the latest version of the resource after a reconciliation. To achieve this - * {@link #handleRecentResourceUpdate(ResourceID, HasMetadata, HasMetadata)} and {@link #handleRecentResourceCreate(ResourceID, HasMetadata)} need to be called explicitly + * that {@link + * io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource#getSecondaryResource(HasMetadata, + * Context)} always returns the latest version of the resource after a reconciliation. To + * achieve this {@link #handleRecentResourceUpdate(ResourceID, HasMetadata, HasMetadata)} and + * {@link #handleRecentResourceCreate(ResourceID, HasMetadata)} need to be called explicitly * after a resource is created or updated using the kubernetes client. These calls are done - * automatically by the KubernetesDependentResource implementation. In the background this will - * store the new resource in a temporary cache {@link TemporaryResourceCache} which does + * automatically by the KubernetesDependentResource implementation. In the background this + * will store the new resource in a temporary cache {@link TemporaryResourceCache} which does * additional checks. After a new event is received the cached object is removed from this * cache, since it is then usually already in the informer cache. - *
  3. Avoiding unneeded reconciliations after resources are created or updated. This filters out events that are the results of updates and creates made - * by the controller itself because we typically don't want the associated informer to trigger an event causing a useless reconciliation (as the change originates from the reconciler itself). This is achieved - * by - * TODO: update as this mentions methods that don't exist anymore and isn't very clear - * - * {@link #prepareForCreateOrUpdateEventFiltering(..) method needs to be called before the operation - * of the k8s client. And the operation from point 1. after the k8s client call. See its - * usage in CreateUpdateEventFilterTestReconciler integration test for the usage. (Again this - * is managed for the developer if using dependent resources.)
    - * Roughly it works in a way that before the K8S API call is made, we set mark the resource - * ID, and from that point informer won't propagate events further just will start record - * them. After the client operation is done, it's checked and analysed what events were - * received and based on that it will propagate event or not and/or put the new resource into - * the temporal cache - so if the event not arrived yet about the update will be able to - * filter it in the future. + *
  4. Avoiding unneeded reconciliations after resources are created or updated. This filters out + * events that are the results of updates and creates made by the controller itself because we + * typically don't want the associated informer to trigger an event causing a useless + * reconciliation (as the change originates from the reconciler itself). For the details see + * {@link #canSkipEvent(HasMetadata, HasMetadata, ResourceID)} and related usage. *
* * @param resource type being watched