From ef947bb11aeea9b0a666df255831aec30c1bb818 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Thu, 25 Dec 2025 18:40:47 +0100 Subject: [PATCH 01/21] feat: ReconcileUtils for strongly consistent updates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../io/javaoperatorsdk/operator/Operator.java | 2 +- ...tils.java => ReconcilerUtilsInternal.java} | 4 +- .../config/AbstractConfigurationService.java | 4 +- .../api/config/BaseConfigurationService.java | 4 +- .../api/config/ControllerConfiguration.java | 12 +- .../informer/InformerConfiguration.java | 4 +- .../api/reconciler/AbstractUpdateControl.java | 36 + .../reconciler/ErrorStatusUpdateControl.java | 2 +- .../PrimaryUpdateAndCacheUtils.java | 4 + .../api/reconciler/ReconcileUtils.java | 983 ++++++++++++++++++ .../api/reconciler/UpdateControl.java | 2 +- .../KubernetesDependentResource.java | 6 +- .../event/ReconciliationDispatcher.java | 229 +--- .../controller/ControllerEventSource.java | 32 +- .../source/informer/InformerEventSource.java | 10 +- .../source/informer/InformerManager.java | 4 +- .../source/informer/InformerWrapper.java | 6 +- .../informer/ManagedInformerEventSource.java | 54 +- .../informer/TemporaryResourceCache.java | 57 +- .../javaoperatorsdk/operator/OperatorIT.java | 2 +- ....java => ReconcilerUtilsInternalTest.java} | 28 +- .../api/reconciler/ReconcileUtilsTest.java | 94 ++ .../GenericKubernetesResourceMatcherTest.java | 4 +- .../GenericResourceUpdaterTest.java | 4 +- ...dGenericKubernetesResourceMatcherTest.java | 4 +- .../event/ReconciliationDispatcherTest.java | 223 ++-- .../controller/ControllerEventSourceTest.java | 34 +- .../TemporaryPrimaryResourceCacheTest.java | 27 +- .../junit/LocallyRunOperatorExtension.java | 10 +- .../baseapi/LeaderElectionPermissionIT.java | 6 +- .../BuiltInResourceCleanerIT.java | 4 +- .../filterpatchevent/FilterPatchEventIT.java | 108 ++ .../FilterPatchEventTestCustomResource.java | 28 + ...terPatchEventTestCustomResourceStatus.java | 30 + .../FilterPatchEventTestReconciler.java | 55 + .../InfrastructureClientIT.java | 12 +- ...PatchResourceAndStatusNoSSAReconciler.java | 2 +- .../PatchResourceWithSSAReconciler.java | 3 +- .../PatchWithSSAITBase.java | 1 + .../baseapi/simple/ReconcilerExecutorIT.java | 2 +- .../baseapi/simple/TestReconciler.java | 96 +- .../specupdate/SSASpecUpdateReconciler.java | 18 +- .../subresource/SubResourceUpdateIT.java | 6 +- ...TriggerReconcilerOnAllEventReconciler.java | 6 +- .../SelectiveFinalizerHandlingReconciler.java | 6 +- .../config/BaseConfigurationServiceTest.java | 6 +- .../DefaultConfigurationServiceTest.java | 4 +- .../ExternalStateReconciler.java | 6 +- .../InformerRelatedBehaviorITS.java | 14 +- .../ServiceDependentResource.java | 2 +- .../ServiceDependentResource.java | 2 +- .../StandaloneDependentTestReconciler.java | 4 +- ...lSetDesiredSanitizerDependentResource.java | 4 +- .../dependent/BaseService.java | 4 +- .../dependent/BaseStatefulSet.java | 4 +- .../DeploymentDependentResource.java | 4 +- .../sample/DeploymentDependentResource.java | 4 +- .../sample/ServiceDependentResource.java | 5 +- .../operator/sample/Utils.java | 2 +- .../operator/sample/WebPageReconciler.java | 7 +- .../DeploymentDependentResource.java | 2 +- .../ServiceDependentResource.java | 2 +- 62 files changed, 1763 insertions(+), 581 deletions(-) rename operator-framework-core/src/main/java/io/javaoperatorsdk/operator/{ReconcilerUtils.java => ReconcilerUtilsInternal.java} (99%) create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/AbstractUpdateControl.java create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtils.java rename operator-framework-core/src/test/java/io/javaoperatorsdk/operator/{ReconcilerUtilsTest.java => ReconcilerUtilsInternalTest.java} (84%) create mode 100644 operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtilsTest.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/filterpatchevent/FilterPatchEventIT.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/filterpatchevent/FilterPatchEventTestCustomResource.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/filterpatchevent/FilterPatchEventTestCustomResourceStatus.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/filterpatchevent/FilterPatchEventTestReconciler.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java index 5adc90182d..0cfe0e997a 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java @@ -263,7 +263,7 @@ public

RegisteredController

register( "Cannot register reconciler with name " + reconciler.getClass().getCanonicalName() + " reconciler named " - + ReconcilerUtils.getNameFor(reconciler) + + ReconcilerUtilsInternal.getNameFor(reconciler) + " because its configuration cannot be found.\n" + " Known reconcilers are: " + configurationService.getKnownReconcilerNames()); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ReconcilerUtils.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ReconcilerUtilsInternal.java similarity index 99% rename from operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ReconcilerUtils.java rename to operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ReconcilerUtilsInternal.java index 354c2aa420..1523b792a5 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ReconcilerUtils.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ReconcilerUtilsInternal.java @@ -34,7 +34,7 @@ import io.javaoperatorsdk.operator.api.reconciler.Reconciler; @SuppressWarnings("rawtypes") -public class ReconcilerUtils { +public class ReconcilerUtilsInternal { private static final String FINALIZER_NAME_SUFFIX = "/finalizer"; protected static final String MISSING_GROUP_SUFFIX = ".javaoperatorsdk.io"; @@ -46,7 +46,7 @@ public class ReconcilerUtils { Pattern.compile(".*http(s?)://[^/]*/api(s?)/(\\S*).*"); // NOSONAR: input is controlled // prevent instantiation of util class - private ReconcilerUtils() {} + private ReconcilerUtilsInternal() {} public static boolean isFinalizerValid(String finalizer) { return HasMetadata.validateFinalizer(finalizer); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AbstractConfigurationService.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AbstractConfigurationService.java index b85ee03fcb..a1b37d6fe9 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AbstractConfigurationService.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AbstractConfigurationService.java @@ -22,7 +22,7 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.client.KubernetesClient; -import io.javaoperatorsdk.operator.ReconcilerUtils; +import io.javaoperatorsdk.operator.ReconcilerUtilsInternal; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; /** @@ -145,7 +145,7 @@ private String getReconcilersNameMessage() { } protected String keyFor(Reconciler reconciler) { - return ReconcilerUtils.getNameFor(reconciler); + return ReconcilerUtilsInternal.getNameFor(reconciler); } @SuppressWarnings("unused") diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/BaseConfigurationService.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/BaseConfigurationService.java index 0a7d3ece04..6b7579b6a8 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/BaseConfigurationService.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/BaseConfigurationService.java @@ -28,7 +28,7 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.client.KubernetesClient; -import io.javaoperatorsdk.operator.ReconcilerUtils; +import io.javaoperatorsdk.operator.ReconcilerUtilsInternal; import io.javaoperatorsdk.operator.api.config.Utils.Configurator; import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceConfigurationResolver; import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; @@ -265,7 +265,7 @@ private

ResolvedControllerConfiguration

controllerCon io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration annotation) { final var resourceClass = getResourceClassResolver().getPrimaryResourceClass(reconcilerClass); - final var name = ReconcilerUtils.getNameFor(reconcilerClass); + final var name = ReconcilerUtilsInternal.getNameFor(reconcilerClass); final var generationAware = valueOrDefaultFromAnnotation( annotation, diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java index 8bddc8479e..63177b614f 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java @@ -20,7 +20,7 @@ import java.util.Set; import io.fabric8.kubernetes.api.model.HasMetadata; -import io.javaoperatorsdk.operator.ReconcilerUtils; +import io.javaoperatorsdk.operator.ReconcilerUtilsInternal; import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; import io.javaoperatorsdk.operator.api.config.workflow.WorkflowSpec; import io.javaoperatorsdk.operator.api.reconciler.MaxReconciliationInterval; @@ -42,16 +42,18 @@ default String getName() { } default String getFinalizerName() { - return ReconcilerUtils.getDefaultFinalizerName(getResourceClass()); + return ReconcilerUtilsInternal.getDefaultFinalizerName(getResourceClass()); } static String ensureValidName(String name, String reconcilerClassName) { - return name != null ? name : ReconcilerUtils.getDefaultReconcilerName(reconcilerClassName); + return name != null + ? name + : ReconcilerUtilsInternal.getDefaultReconcilerName(reconcilerClassName); } static String ensureValidFinalizerName(String finalizer, String resourceTypeName) { if (finalizer != null && !finalizer.isBlank()) { - if (ReconcilerUtils.isFinalizerValid(finalizer)) { + if (ReconcilerUtilsInternal.isFinalizerValid(finalizer)) { return finalizer; } else { throw new IllegalArgumentException( @@ -61,7 +63,7 @@ static String ensureValidFinalizerName(String finalizer, String resourceTypeName + " for details"); } } else { - return ReconcilerUtils.getDefaultFinalizerName(resourceTypeName); + return ReconcilerUtilsInternal.getDefaultFinalizerName(resourceTypeName); } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java index 30a1a32e8a..f6caa4fe4d 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java @@ -25,7 +25,7 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.client.informers.cache.ItemStore; import io.javaoperatorsdk.operator.OperatorException; -import io.javaoperatorsdk.operator.ReconcilerUtils; +import io.javaoperatorsdk.operator.ReconcilerUtilsInternal; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.config.Utils; import io.javaoperatorsdk.operator.api.reconciler.Constants; @@ -92,7 +92,7 @@ private InformerConfiguration(Class resourceClass) { // controller // where GenericKubernetesResource now does not apply ? GenericKubernetesResource.class.getSimpleName() - : ReconcilerUtils.getResourceTypeName(resourceClass); + : ReconcilerUtilsInternal.getResourceTypeName(resourceClass); } @SuppressWarnings({"rawtypes", "unchecked"}) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/AbstractUpdateControl.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/AbstractUpdateControl.java new file mode 100644 index 0000000000..881cd210b9 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/AbstractUpdateControl.java @@ -0,0 +1,36 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.api.reconciler; + +public abstract class AbstractUpdateControl> extends BaseControl { + + private boolean filterPatchEvent = true; + + /** + * The event from resource primary updates are filtered by default, thus does not trigger the + * reconciliation. Setting this to false will turn the filtering off. + * + * @since 5.3.0 + */ + public T filterPatchEvent(boolean filterPatchEvent) { + this.filterPatchEvent = filterPatchEvent; + return (T) this; + } + + public boolean isFilterPatchEvent() { + return filterPatchEvent; + } +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ErrorStatusUpdateControl.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ErrorStatusUpdateControl.java index 2636fea879..d51cb72720 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ErrorStatusUpdateControl.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ErrorStatusUpdateControl.java @@ -21,7 +21,7 @@ import io.fabric8.kubernetes.api.model.HasMetadata; public class ErrorStatusUpdateControl

- extends BaseControl> { + extends AbstractUpdateControl> { private final P resource; private boolean noRetry = false; 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 11dfd21648..31c825e673 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 @@ -45,7 +45,11 @@ * caches the updated resource from the response in an overlay cache on top of the Informer cache. * If the update fails, it reads the primary resource from the cluster, applies the modifications * again and retries the update. + * + * @deprecated Use {@link ReconcileUtils} that contains the more efficient up-to-date versions of + * the target utils. */ +@Deprecated(forRemoval = true) public class PrimaryUpdateAndCacheUtils { public static final int DEFAULT_MAX_RETRY = 10; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtils.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtils.java new file mode 100644 index 0000000000..eb8ba95737 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtils.java @@ -0,0 +1,983 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.api.reconciler; + +import java.lang.reflect.InvocationTargetException; +import java.util.function.Predicate; +import java.util.function.UnaryOperator; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.api.model.ObjectMeta; +import io.fabric8.kubernetes.client.KubernetesClient; +import io.fabric8.kubernetes.client.KubernetesClientException; +import io.fabric8.kubernetes.client.dsl.base.PatchContext; +import io.fabric8.kubernetes.client.dsl.base.PatchType; +import io.javaoperatorsdk.operator.OperatorException; +import io.javaoperatorsdk.operator.processing.event.ResourceID; +import io.javaoperatorsdk.operator.processing.event.source.informer.ManagedInformerEventSource; + +import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getUID; +import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getVersion; + +public class ReconcileUtils { + + private static final Logger log = LoggerFactory.getLogger(ReconcileUtils.class); + + public static final int DEFAULT_MAX_RETRY = 10; + + private ReconcileUtils() {} + + /** + * Server-Side Apply the resource and filters out the resulting event. This is a convenience + * method that calls {@link #serverSideApply(Context, HasMetadata, boolean)} with filterEvent set + * to true. + * + * @param context of reconciler + * @param resource fresh resource for server side apply + * @return updated resource + * @param resource type + * @see #serverSideApply(Context, HasMetadata, boolean) + */ + public static R serverSideApply( + Context context, R resource) { + return serverSideApply(context, resource, true); + } + + /** + * Updates the resource and caches the response if needed, thus making sure that next + * reconciliation will contain to updated resource. Or more recent one if someone did an update + * after our update. + * + *

Optionally also can filter out the event, what is the result of this update. + * + *

You are free to control the optimistic locking by setting the resource version in resource + * metadata. In case of SSA we advise not to do updates with optimistic locking. + * + * @param context of reconciler + * @param resource fresh resource for server side apply + * @param filterEvent if true the event from this update will be filtered out so won't trigger the + * reconciliation + * @return updated resource + * @param resource type + */ + public static R serverSideApply( + Context context, R resource, boolean filterEvent) { + return resourcePatch( + context, + resource, + r -> + context + .getClient() + .resource(r) + .patch( + new PatchContext.Builder() + .withForce(true) + .withFieldManager(context.getControllerConfiguration().fieldManager()) + .withPatchType(PatchType.SERVER_SIDE_APPLY) + .build()), + filterEvent); + } + + /** + * Server-Side Apply the resource status subresource and filters out the resulting event. This is + * a convenience method that calls {@link #serverSideApplyStatus(Context, HasMetadata, boolean)} + * with filterEvent set to true. + * + * @param context of reconciler + * @param resource fresh resource for server side apply + * @return updated resource + * @param resource type + * @see #serverSideApplyStatus(Context, HasMetadata, boolean) + */ + public static R serverSideApplyStatus( + Context context, R resource) { + return serverSideApplyStatus(context, resource, true); + } + + /** + * Server-Side Apply the resource status subresource. Updates the resource status and caches the + * response if needed, ensuring the next reconciliation will contain the updated resource. + * + * @param context of reconciler + * @param resource fresh resource for server side apply + * @param filterEvent if true the event from this update will be filtered out so won't trigger the + * reconciliation + * @return updated resource + * @param resource type + */ + public static R serverSideApplyStatus( + Context context, R resource, boolean filterEvent) { + return resourcePatch( + context, + resource, + r -> + context + .getClient() + .resource(r) + .subresource("status") + .patch( + new PatchContext.Builder() + .withForce(true) + .withFieldManager(context.getControllerConfiguration().fieldManager()) + .withPatchType(PatchType.SERVER_SIDE_APPLY) + .build()), + filterEvent); + } + + /** + * Server-Side Apply the primary resource and filters out the resulting event. This is a + * convenience method that calls {@link #serverSideApplyPrimary(Context, HasMetadata, boolean)} + * with filterEvent set to true. + * + * @param context of reconciler + * @param resource primary resource for server side apply + * @return updated resource + * @param

primary resource type + * @see #serverSideApplyPrimary(Context, HasMetadata, boolean) + */ + public static

P serverSideApplyPrimary(Context

context, P resource) { + return serverSideApplyPrimary(context, resource, true); + } + + /** + * Server-Side Apply the primary resource. Updates the primary resource and caches the response + * using the controller's event source, ensuring the next reconciliation will contain the updated + * resource. + * + * @param context of reconciler + * @param resource primary resource for server side apply + * @param filterEvent if true the event from this update will be filtered out so won't trigger the + * reconciliation + * @return updated resource + * @param

primary resource type + */ + public static

P serverSideApplyPrimary( + Context

context, P resource, boolean filterEvent) { + return resourcePatch( + resource, + r -> + context + .getClient() + .resource(r) + .patch( + new PatchContext.Builder() + .withForce(true) + .withFieldManager(context.getControllerConfiguration().fieldManager()) + .withPatchType(PatchType.SERVER_SIDE_APPLY) + .build()), + context.eventSourceRetriever().getControllerEventSource(), + filterEvent); + } + + /** + * Server-Side Apply the primary resource status subresource and filters out the resulting event. + * This is a convenience method that calls {@link #serverSideApplyPrimaryStatus(Context, + * HasMetadata, boolean)} with filterEvent set to true. + * + * @param context of reconciler + * @param resource primary resource for server side apply + * @return updated resource + * @param

primary resource type + * @see #serverSideApplyPrimaryStatus(Context, HasMetadata, boolean) + */ + public static

P serverSideApplyPrimaryStatus( + Context

context, P resource) { + return serverSideApplyPrimaryStatus(context, resource, true); + } + + /** + * Server-Side Apply the primary resource status subresource. Updates the primary resource status + * and caches the response using the controller's event source. + * + * @param context of reconciler + * @param resource primary resource for server side apply + * @param filterEvent if true the event from this update will be filtered out so won't trigger the + * reconciliation + * @return updated resource + * @param

primary resource type + */ + public static

P serverSideApplyPrimaryStatus( + Context

context, P resource, boolean filterEvent) { + return resourcePatch( + resource, + r -> + context + .getClient() + .resource(r) + .subresource("status") + .patch( + new PatchContext.Builder() + .withForce(true) + .withFieldManager(context.getControllerConfiguration().fieldManager()) + .withPatchType(PatchType.SERVER_SIDE_APPLY) + .build()), + context.eventSourceRetriever().getControllerEventSource(), + filterEvent); + } + + /** + * Updates the resource and filters out the resulting event. This is a convenience method that + * calls {@link #update(Context, HasMetadata, boolean)} with filterEvent set to true. Uses + * optimistic locking based on the resource version. + * + * @param context of reconciler + * @param resource resource to update + * @return updated resource + * @param resource type + * @see #update(Context, HasMetadata, boolean) + */ + public static R update( + Context context, R resource) { + return update(context, resource, true); + } + + /** + * Updates the resource with optimistic locking based on the resource version. Caches the response + * if needed, ensuring the next reconciliation will contain the updated resource. + * + * @param context of reconciler + * @param resource resource to update + * @param filterEvent if true the event from this update will be filtered out so won't trigger the + * reconciliation + * @return updated resource + * @param resource type + */ + public static R update( + Context context, R resource, boolean filterEvent) { + return resourcePatch( + context, resource, r -> context.getClient().resource(r).update(), filterEvent); + } + + /** + * Updates the resource status subresource and filters out the resulting event. This is a + * convenience method that calls {@link #updateStatus(Context, HasMetadata, boolean)} with + * filterEvent set to true. + * + * @param context of reconciler + * @param resource resource to update + * @return updated resource + * @param resource type + * @see #updateStatus(Context, HasMetadata, boolean) + */ + public static R updateStatus( + Context context, R resource) { + return updateStatus(context, resource, true); + } + + /** + * Updates the resource status subresource with optimistic locking. Caches the response if needed. + * + * @param context of reconciler + * @param resource resource to update + * @param filterEvent if true the event from this update will be filtered out so won't trigger the + * reconciliation + * @return updated resource + * @param resource type + */ + public static R updateStatus( + Context context, R resource, boolean filterEvent) { + return resourcePatch( + context, resource, r -> context.getClient().resource(r).updateStatus(), filterEvent); + } + + /** + * Updates the primary resource and filters out the resulting event. This is a convenience method + * that calls {@link #updatePrimary(Context, HasMetadata, boolean)} with filterEvent set to true. + * + * @param context of reconciler + * @param resource primary resource to update + * @return updated resource + * @param resource type + * @see #updatePrimary(Context, HasMetadata, boolean) + */ + public static R updatePrimary( + Context context, R resource) { + return updatePrimary(context, resource, true); + } + + /** + * Updates the primary resource with optimistic locking. Caches the response using the + * controller's event source. + * + * @param context of reconciler + * @param resource primary resource to update + * @param filterEvent if true the event from this update will be filtered out so won't trigger the + * reconciliation + * @return updated resource + * @param resource type + */ + public static R updatePrimary( + Context context, R resource, boolean filterEvent) { + return resourcePatch( + resource, + r -> context.getClient().resource(r).update(), + context.eventSourceRetriever().getControllerEventSource(), + filterEvent); + } + + /** + * Updates the primary resource status subresource and filters out the resulting event. This is a + * convenience method that calls {@link #updatePrimaryStatus(Context, HasMetadata, boolean)} with + * filterEvent set to true. + * + * @param context of reconciler + * @param resource primary resource to update + * @return updated resource + * @param resource type + * @see #updatePrimaryStatus(Context, HasMetadata, boolean) + */ + public static R updatePrimaryStatus( + Context context, R resource) { + return updatePrimaryStatus(context, resource, true); + } + + /** + * Updates the primary resource status subresource with optimistic locking. Caches the response + * using the controller's event source. + * + * @param context of reconciler + * @param resource primary resource to update + * @param filterEvent if true the event from this update will be filtered out so won't trigger the + * reconciliation + * @return updated resource + * @param resource type + */ + public static R updatePrimaryStatus( + Context context, R resource, boolean filterEvent) { + return resourcePatch( + resource, + r -> context.getClient().resource(r).updateStatus(), + context.eventSourceRetriever().getControllerEventSource(), + filterEvent); + } + + /** + * Applies a JSON Patch to the resource and filters out the resulting event. This is a convenience + * method that calls {@link #jsonPatch(Context, HasMetadata, UnaryOperator, boolean)} with + * filterEvent set to true. + * + * @param context of reconciler + * @param resource resource to patch + * @param unaryOperator function to modify the resource + * @return updated resource + * @param resource type + * @see #jsonPatch(Context, HasMetadata, UnaryOperator, boolean) + */ + public static R jsonPatch( + Context context, R resource, UnaryOperator unaryOperator) { + return jsonPatch(context, resource, unaryOperator, true); + } + + /** + * Applies a JSON Patch to the resource. The unaryOperator function is used to modify the + * resource, and the differences are sent as a JSON Patch to the Kubernetes API server. + * + * @param context of reconciler + * @param resource resource to patch + * @param unaryOperator function to modify the resource + * @param filterEvent if true the event from this update will be filtered out so won't trigger the + * reconciliation + * @return updated resource + * @param resource type + */ + public static R jsonPatch( + Context context, + R resource, + UnaryOperator unaryOperator, + boolean filterEvent) { + return resourcePatch( + context, resource, r -> context.getClient().resource(r).edit(unaryOperator), filterEvent); + } + + /** + * Applies a JSON Patch to the resource status subresource and filters out the resulting event. + * This is a convenience method that calls {@link #jsonPatchStatus(Context, HasMetadata, + * UnaryOperator, boolean)} with filterEvent set to true. + * + * @param context of reconciler + * @param resource resource to patch + * @param unaryOperator function to modify the resource + * @return updated resource + * @param resource type + * @see #jsonPatchStatus(Context, HasMetadata, UnaryOperator, boolean) + */ + public static R jsonPatchStatus( + Context context, R resource, UnaryOperator unaryOperator) { + return jsonPatchStatus(context, resource, unaryOperator, true); + } + + /** + * Applies a JSON Patch to the resource status subresource. The unaryOperator function is used to + * modify the resource status, and the differences are sent as a JSON Patch. + * + * @param context of reconciler + * @param resource resource to patch + * @param unaryOperator function to modify the resource + * @param filterEvent if true the event from this update will be filtered out so won't trigger the + * reconciliation + * @return updated resource + * @param resource type + */ + public static R jsonPatchStatus( + Context context, + R resource, + UnaryOperator unaryOperator, + boolean filterEvent) { + return resourcePatch( + context, + resource, + r -> context.getClient().resource(r).editStatus(unaryOperator), + filterEvent); + } + + /** + * Applies a JSON Patch to the primary resource and filters out the resulting event. This is a + * convenience method that calls {@link #jsonPatchPrimary(Context, HasMetadata, UnaryOperator, + * boolean)} with filterEvent set to true. + * + * @param context of reconciler + * @param resource primary resource to patch + * @param unaryOperator function to modify the resource + * @return updated resource + * @param resource type + * @see #jsonPatchPrimary(Context, HasMetadata, UnaryOperator, boolean) + */ + public static R jsonPatchPrimary( + Context context, R resource, UnaryOperator unaryOperator) { + return jsonPatchPrimary(context, resource, unaryOperator, true); + } + + /** + * Applies a JSON Patch to the primary resource. Caches the response using the controller's event + * source. + * + * @param context of reconciler + * @param resource primary resource to patch + * @param unaryOperator function to modify the resource + * @param filterEvent if true the event from this update will be filtered out so won't trigger the + * reconciliation + * @return updated resource + * @param resource type + */ + public static R jsonPatchPrimary( + Context context, + R resource, + UnaryOperator unaryOperator, + boolean filterEvent) { + return resourcePatch( + resource, + r -> context.getClient().resource(r).edit(unaryOperator), + context.eventSourceRetriever().getControllerEventSource(), + filterEvent); + } + + /** + * Applies a JSON Patch to the primary resource status subresource and filters out the resulting + * event. This is a convenience method that calls {@link #jsonPatchPrimaryStatus(Context, + * HasMetadata, UnaryOperator, boolean)} with filterEvent set to true. + * + * @param context of reconciler + * @param resource primary resource to patch + * @param unaryOperator function to modify the resource + * @return updated resource + * @param resource type + * @see #jsonPatchPrimaryStatus(Context, HasMetadata, UnaryOperator, boolean) + */ + public static R jsonPatchPrimaryStatus( + Context context, R resource, UnaryOperator unaryOperator) { + return jsonPatchPrimaryStatus(context, resource, unaryOperator, true); + } + + /** + * Applies a JSON Patch to the primary resource status subresource. Caches the response using the + * controller's event source. + * + * @param context of reconciler + * @param resource primary resource to patch + * @param unaryOperator function to modify the resource + * @param filterEvent if true the event from this update will be filtered out so won't trigger the + * reconciliation + * @return updated resource + * @param resource type + */ + public static R jsonPatchPrimaryStatus( + Context context, + R resource, + UnaryOperator unaryOperator, + boolean filterEvent) { + return resourcePatch( + resource, + r -> context.getClient().resource(r).editStatus(unaryOperator), + context.eventSourceRetriever().getControllerEventSource(), + filterEvent); + } + + /** + * Applies a JSON Merge Patch to the resource and filters out the resulting event. This is a + * convenience method that calls {@link #jsonMergePatch(Context, HasMetadata, boolean)} with + * filterEvent set to true. JSON Merge Patch (RFC 7386) is a simpler patching strategy compared to + * JSON Patch. + * + * @param context of reconciler + * @param resource resource to patch + * @return updated resource + * @param resource type + * @see #jsonMergePatch(Context, HasMetadata, boolean) + */ + public static R jsonMergePatch( + Context context, R resource) { + return jsonMergePatch(context, resource, true); + } + + /** + * Applies a JSON Merge Patch to the resource. JSON Merge Patch (RFC 7386) is a simpler patching + * strategy that merges the provided resource with the existing resource on the server. + * + * @param context of reconciler + * @param resource resource to patch + * @param filterEvent if true the event from this update will be filtered out so won't trigger the + * reconciliation + * @return updated resource + * @param resource type + */ + public static R jsonMergePatch( + Context context, R resource, boolean filterEvent) { + return resourcePatch( + context, resource, r -> context.getClient().resource(r).patch(), filterEvent); + } + + /** + * Applies a JSON Merge Patch to the resource status subresource and filters out the resulting + * event. This is a convenience method that calls {@link #jsonMergePatchStatus(Context, + * HasMetadata, boolean)} with filterEvent set to true. + * + * @param context of reconciler + * @param resource resource to patch + * @return updated resource + * @param resource type + * @see #jsonMergePatchStatus(Context, HasMetadata, boolean) + */ + public static R jsonMergePatchStatus( + Context context, R resource) { + return jsonMergePatchStatus(context, resource, true); + } + + /** + * Applies a JSON Merge Patch to the resource status subresource. Merges the provided resource + * status with the existing resource status on the server. + * + * @param context of reconciler + * @param resource resource to patch + * @param filterEvent if true the event from this update will be filtered out so won't trigger the + * reconciliation + * @return updated resource + * @param resource type + */ + public static R jsonMergePatchStatus( + Context context, R resource, boolean filterEvent) { + return resourcePatch( + context, resource, r -> context.getClient().resource(r).patchStatus(), filterEvent); + } + + /** + * Applies a JSON Merge Patch to the primary resource and filters out the resulting event. This is + * a convenience method that calls {@link #jsonMergePatchPrimary(Context, HasMetadata, boolean)} + * with filterEvent set to true. + * + * @param context of reconciler + * @param resource primary resource to patch + * @return updated resource + * @param resource type + * @see #jsonMergePatchPrimary(Context, HasMetadata, boolean) + */ + public static R jsonMergePatchPrimary( + Context context, R resource) { + return jsonMergePatchPrimary(context, resource, true); + } + + /** + * Applies a JSON Merge Patch to the primary resource. Caches the response using the controller's + * event source. + * + * @param context of reconciler + * @param resource primary resource to patch + * @param filterEvent if true the event from this update will be filtered out so won't trigger the + * reconciliation + * @return updated resource + * @param resource type + */ + public static R jsonMergePatchPrimary( + Context context, R resource, boolean filterEvent) { + return resourcePatch( + resource, + r -> context.getClient().resource(r).patch(), + context.eventSourceRetriever().getControllerEventSource(), + filterEvent); + } + + /** + * Applies a JSON Merge Patch to the primary resource status subresource and filters out the + * resulting event. This is a convenience method that calls {@link + * #jsonMergePatchPrimaryStatus(Context, HasMetadata, boolean)} with filterEvent set to true. + * + * @param context of reconciler + * @param resource primary resource to patch + * @return updated resource + * @param resource type + * @see #jsonMergePatchPrimaryStatus(Context, HasMetadata, boolean) + */ + public static R jsonMergePatchPrimaryStatus( + Context context, R resource) { + return jsonMergePatchPrimaryStatus(context, resource, true); + } + + /** + * Applies a JSON Merge Patch to the primary resource status subresource. Caches the response + * using the controller's event source. + * + * @param context of reconciler + * @param resource primary resource to patch + * @param filterEvent if true the event from this update will be filtered out so won't trigger the + * reconciliation + * @return updated resource + * @param resource type + */ + public static R jsonMergePatchPrimaryStatus( + Context context, R resource, boolean filterEvent) { + return resourcePatch( + resource, + r -> context.getClient().resource(r).patch(), + context.eventSourceRetriever().getControllerEventSource(), + filterEvent); + } + + /** + * Internal utility method to patch a resource and cache the result. Automatically discovers the + * event source for the resource type and delegates to {@link #resourcePatch(HasMetadata, + * UnaryOperator, ManagedInformerEventSource, boolean)}. + * + * @param context of reconciler + * @param resource resource to patch + * @param updateOperation operation to perform (update, patch, edit, etc.) + * @param filterEvent if true the event from this update will be filtered out + * @return updated resource + * @param resource type + * @throws IllegalStateException if no event source or multiple event sources are found + */ + public static R resourcePatch( + Context context, R resource, UnaryOperator updateOperation, boolean filterEvent) { + + var esList = context.eventSourceRetriever().getEventSourcesFor(resource.getClass()); + if (esList.isEmpty()) { + throw new IllegalStateException("No event source found for type: " + resource.getClass()); + } + if (esList.size() > 1) { + throw new IllegalStateException( + "Multiple event sources found for: " + + resource.getClass() + + " please provide the target event source"); + } + var es = esList.get(0); + if (es instanceof ManagedInformerEventSource mes) { + return resourcePatch(resource, updateOperation, mes, filterEvent); + } else { + throw new IllegalStateException( + "Target event source must be a subclass off " + + ManagedInformerEventSource.class.getName()); + } + } + + /** + * Internal utility method to patch a resource and cache the result using the specified event + * source. This method either filters out the resulting event or allows it to trigger + * reconciliation based on the filterEvent parameter. + * + * @param resource resource to patch + * @param updateOperation operation to perform (update, patch, edit, etc.) + * @param ies the managed informer event source to use for caching + * @param filterEvent if true the event from this update will be filtered out + * @return updated resource + * @param resource type + */ + @SuppressWarnings("unchecked") + public static R resourcePatch( + R resource, + UnaryOperator updateOperation, + ManagedInformerEventSource ies, + boolean filterEvent) { + return filterEvent + ? (R) ies.eventFilteringUpdateAndCacheResource(resource, updateOperation) + : (R) ies.updateAndCacheResource(resource, updateOperation); + } + + /** + * Adds the default finalizer (from controller configuration) to the primary resource. This is a + * convenience method that calls {@link #addFinalizer(Context, String)} with the configured + * finalizer name. + * + * @param context of reconciler + * @return updated resource from the server response + * @param

primary resource type + * @see #addFinalizer(Context, String) + */ + public static

P addFinalizer(Context

context) { + return addFinalizer(context, context.getControllerConfiguration().getFinalizerName()); + } + + /** + * Adds finalizer to the resource using JSON Patch. Retries conflicts and unprocessable content + * (HTTP 422), see {@link PrimaryUpdateAndCacheUtils#conflictRetryingPatch(KubernetesClient, + * HasMetadata, UnaryOperator, Predicate)} for details on retry. It does not try to add finalizer + * if there is already a finalizer or resource is marked for deletion. + * + * @return updated resource from the server response + */ + public static

P addFinalizer(Context

context, String finalizerName) { + var resource = context.getPrimaryResource(); + if (resource.isMarkedForDeletion() || resource.hasFinalizer(finalizerName)) { + return resource; + } + return conflictRetryingPatch( + context, + r -> { + r.addFinalizer(finalizerName); + return r; + }, + r -> !r.hasFinalizer(finalizerName)); + } + + /** + * Removes the default finalizer (from controller configuration) from the primary resource. This + * is a convenience method that calls {@link #removeFinalizer(Context, String)} with the + * configured finalizer name. + * + * @param context of reconciler + * @return updated resource from the server response + * @param

primary resource type + * @see #removeFinalizer(Context, String) + */ + public static

P removeFinalizer(Context

context) { + return removeFinalizer(context, context.getControllerConfiguration().getFinalizerName()); + } + + /** + * Removes the target finalizer from target resource. Uses JSON Patch and handles retries, see + * {@link PrimaryUpdateAndCacheUtils#conflictRetryingPatch(KubernetesClient, HasMetadata, + * UnaryOperator, Predicate)} for details. It does not try to remove finalizer if finalizer is not + * present on the resource. + * + * @return updated resource from the server response + */ + public static

P removeFinalizer( + Context

context, String finalizerName) { + var resource = context.getPrimaryResource(); + if (!resource.hasFinalizer(finalizerName)) { + return resource; + } + return conflictRetryingPatch( + context, + r -> { + r.removeFinalizer(finalizerName); + return r; + }, + r -> r.hasFinalizer(finalizerName)); + } + + /** + * Patches the resource using JSON Patch. In case the server responds with conflict (HTTP 409) or + * unprocessable content (HTTP 422) it retries the operation up to the maximum number defined in + * {@link ReconcileUtils#DEFAULT_MAX_RETRY}. + * + * @param context reconciliation context + * @param resourceChangesOperator changes to be done on the resource before update + * @param preCondition condition to check if the patch operation still needs to be performed or + * not. + * @return updated resource from the server or unchanged if the precondition does not hold. + * @param

resource type + */ + @SuppressWarnings("unchecked") + public static

P conflictRetryingPatch( + Context

context, UnaryOperator

resourceChangesOperator, Predicate

preCondition) { + var resource = context.getPrimaryResource(); + var client = context.getClient(); + if (log.isDebugEnabled()) { + log.debug("Conflict retrying update for: {}", ResourceID.fromResource(resource)); + } + int retryIndex = 0; + while (true) { + try { + if (!preCondition.test(resource)) { + return resource; + } + return jsonPatchPrimary(context, resource, resourceChangesOperator, false); + } catch (KubernetesClientException e) { + log.trace("Exception during patch for resource: {}", resource); + retryIndex++; + // only retry on conflict (409) and unprocessable content (422) which + // can happen if JSON Patch is not a valid request since there was + // a concurrent request which already removed another finalizer: + // List element removal from a list is by index in JSON Patch + // so if addressing a second finalizer but first is meanwhile removed + // it is a wrong request. + if (e.getCode() != 409 && e.getCode() != 422) { + throw e; + } + if (retryIndex >= DEFAULT_MAX_RETRY) { + throw new OperatorException( + "Exceeded maximum (" + + DEFAULT_MAX_RETRY + + ") retry attempts to patch resource: " + + ResourceID.fromResource(resource)); + } + log.debug( + "Retrying patch for resource name: {}, namespace: {}; HTTP code: {}", + resource.getMetadata().getName(), + resource.getMetadata().getNamespace(), + e.getCode()); + var operation = client.resources(resource.getClass()); + if (resource.getMetadata().getNamespace() != null) { + resource = + (P) + operation + .inNamespace(resource.getMetadata().getNamespace()) + .withName(resource.getMetadata().getName()) + .get(); + } else { + resource = (P) operation.withName(resource.getMetadata().getName()).get(); + } + } + } + } + + /** + * Adds the default finalizer (from controller configuration) to the primary resource using + * Server-Side Apply. This is a convenience method that calls {@link #addFinalizerWithSSA(Context, + * String)} with the configured finalizer name. + * + * @param context of reconciler + * @return the patched resource from the server response + * @param

primary resource type + * @see #addFinalizerWithSSA(Context, String) + */ + public static

P addFinalizerWithSSA(Context

context) { + return addFinalizerWithSSA(context, context.getControllerConfiguration().getFinalizerName()); + } + + /** + * Adds finalizer using Server-Side Apply. In the background this method creates a fresh copy of + * the target resource, setting only name, namespace and finalizer. Does not use optimistic + * locking for the patch. + * + * @param context of reconciler + * @param finalizerName name of the finalizer to add + * @return the patched resource from the server response + * @param

primary resource type + */ + public static

P addFinalizerWithSSA( + Context

context, String finalizerName) { + var originalResource = context.getPrimaryResource(); + if (log.isDebugEnabled()) { + log.debug( + "Adding finalizer (using SSA) for resource: {} version: {}", + getUID(originalResource), + getVersion(originalResource)); + } + try { + P resource = (P) originalResource.getClass().getConstructor().newInstance(); + ObjectMeta objectMeta = new ObjectMeta(); + objectMeta.setName(originalResource.getMetadata().getName()); + objectMeta.setNamespace(originalResource.getMetadata().getNamespace()); + resource.setMetadata(objectMeta); + resource.addFinalizer(finalizerName); + + return serverSideApplyPrimary(context, resource, false); + } catch (InstantiationException + | IllegalAccessException + | InvocationTargetException + | NoSuchMethodException e) { + throw new RuntimeException( + "Issue with creating custom resource instance with reflection." + + " Custom Resources must provide a no-arg constructor. Class: " + + originalResource.getClass().getName(), + e); + } + } + + /** + * Compares resource versions of two resources. This is a convenience method that extracts the + * resource versions from the metadata and delegates to {@link #compareResourceVersions(String, + * String)}. + * + * @param h1 first resource + * @param h2 second resource + * @return negative if h1 is older, zero if equal, positive if h1 is newer + * @throws NonComparableResourceVersionException if either resource version is invalid + */ + public static int compareResourceVersions(HasMetadata h1, HasMetadata h2) { + return compareResourceVersions( + h1.getMetadata().getResourceVersion(), h2.getMetadata().getResourceVersion()); + } + + /** + * Compares two Kubernetes resource versions numerically. Kubernetes resource versions are + * expected to be numeric strings that increase monotonically. This method assumes both versions + * are valid numeric strings without leading zeros. + * + * @param v1 first resource version + * @param v2 second resource version + * @return negative if v1 is older, zero if equal, positive if v1 is newer + * @throws NonComparableResourceVersionException if either resource version is empty, has leading + * zeros, or contains non-numeric characters + */ + public static int compareResourceVersions(String v1, String v2) { + int v1Length = validateResourceVersion(v1); + int v2Length = validateResourceVersion(v2); + int comparison = v1Length - v2Length; + if (comparison != 0) { + return comparison; + } + for (int i = 0; i < v2Length; i++) { + int comp = v1.charAt(i) - v2.charAt(i); + if (comp != 0) { + return comp; + } + } + return 0; + } + + private static int validateResourceVersion(String v1) { + int v1Length = v1.length(); + if (v1Length == 0) { + throw new NonComparableResourceVersionException("Resource version is empty"); + } + for (int i = 0; i < v1Length; i++) { + char char1 = v1.charAt(i); + if (char1 == '0') { + if (i == 0) { + throw new NonComparableResourceVersionException( + "Resource version cannot begin with 0: " + v1); + } + } else if (char1 < '0' || char1 > '9') { + throw new NonComparableResourceVersionException( + "Non numeric characters in resource version: " + v1); + } + } + return v1Length; + } +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/UpdateControl.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/UpdateControl.java index 08611ff06f..48b1bf4b50 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/UpdateControl.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/UpdateControl.java @@ -20,7 +20,7 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.client.CustomResource; -public class UpdateControl

extends BaseControl> { +public class UpdateControl

extends AbstractUpdateControl> { private final P resource; private final boolean patchResource; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java index 5d53b807cc..b9ea27b190 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java @@ -75,9 +75,8 @@ public void configureWith(KubernetesDependentResourceConfig config) { protected R handleCreate(R desired, P primary, Context

context) { return eventSource() .orElseThrow() - .updateAndCacheResource( + .eventFilteringUpdateAndCacheResource( desired, - context, toCreate -> KubernetesDependentResource.super.handleCreate(toCreate, primary, context)); } @@ -85,9 +84,8 @@ protected R handleCreate(R desired, P primary, Context

context) { protected R handleUpdate(R actual, R desired, P primary, Context

context) { return eventSource() .orElseThrow() - .updateAndCacheResource( + .eventFilteringUpdateAndCacheResource( desired, - context, toUpdate -> KubernetesDependentResource.super.handleUpdate(actual, toUpdate, primary, context)); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java index da4ae9835a..0a1ff7d429 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java @@ -15,31 +15,23 @@ */ package io.javaoperatorsdk.operator.processing.event; -import java.lang.reflect.InvocationTargetException; import java.net.HttpURLConnection; -import java.util.function.Function; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.slf4j.event.Level; import io.fabric8.kubernetes.api.model.HasMetadata; -import io.fabric8.kubernetes.api.model.KubernetesResourceList; -import io.fabric8.kubernetes.api.model.Namespaced; -import io.fabric8.kubernetes.api.model.ObjectMeta; import io.fabric8.kubernetes.client.KubernetesClientException; -import io.fabric8.kubernetes.client.dsl.MixedOperation; -import io.fabric8.kubernetes.client.dsl.Resource; -import io.fabric8.kubernetes.client.dsl.base.PatchContext; -import io.fabric8.kubernetes.client.dsl.base.PatchType; import io.javaoperatorsdk.operator.OperatorException; -import io.javaoperatorsdk.operator.ReconcilerUtils; +import io.javaoperatorsdk.operator.ReconcilerUtilsInternal; import io.javaoperatorsdk.operator.api.config.Cloner; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.BaseControl; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.api.reconciler.DefaultContext; import io.javaoperatorsdk.operator.api.reconciler.DeleteControl; +import io.javaoperatorsdk.operator.api.reconciler.ReconcileUtils; import io.javaoperatorsdk.operator.api.reconciler.RetryInfo; import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; import io.javaoperatorsdk.operator.processing.Controller; @@ -49,8 +41,6 @@ /** Handles calls and results of a Reconciler and finalizer related logic */ class ReconciliationDispatcher

{ - public static final int MAX_UPDATE_RETRY = 10; - private static final Logger log = LoggerFactory.getLogger(ReconciliationDispatcher.class); private final Controller

controller; @@ -76,7 +66,6 @@ public ReconciliationDispatcher(Controller

controller) { this( controller, new CustomResourceFacade<>( - controller.getCRClient(), controller.getConfiguration(), controller.getConfiguration().getConfigurationService().getResourceCloner())); } @@ -119,7 +108,7 @@ && shouldNotDispatchToCleanupWhenMarkedForDeletion(originalResource)) { // checking the cleaner for all-event-mode if (!triggerOnAllEvents() && markedForDeletion) { - return handleCleanup(resourceForExecution, originalResource, context, executionScope); + return handleCleanup(resourceForExecution, context, executionScope); } else { return handleReconcile(executionScope, resourceForExecution, originalResource, context); } @@ -148,9 +137,9 @@ private PostExecutionControl

handleReconcile( */ P updatedResource; if (useSSA) { - updatedResource = addFinalizerWithSSA(originalResource); + updatedResource = ReconcileUtils.addFinalizerWithSSA(context); } else { - updatedResource = updateCustomResourceWithFinalizer(resourceForExecution, originalResource); + updatedResource = ReconcileUtils.addFinalizer(context); } return PostExecutionControl.onlyFinalizerAdded(updatedResource); } else { @@ -194,7 +183,8 @@ private PostExecutionControl

reconcileExecution( } if (updateControl.isPatchResource()) { - updatedCustomResource = patchResource(toUpdate, originalResource); + updatedCustomResource = + patchResource(context, toUpdate, originalResource, updateControl.isFilterPatchEvent()); if (!useSSA) { toUpdate .getMetadata() @@ -203,7 +193,8 @@ private PostExecutionControl

reconcileExecution( } if (updateControl.isPatchStatus()) { - customResourceFacade.patchStatus(toUpdate, originalResource); + customResourceFacade.patchStatus( + context, toUpdate, originalResource, updateControl.isFilterPatchEvent()); } return createPostExecutionControl(updatedCustomResource, updateControl, executionScope); } @@ -241,7 +232,10 @@ public boolean isLastAttempt() { try { updatedResource = customResourceFacade.patchStatus( - errorStatusUpdateControl.getResource().orElseThrow(), originalResource); + context, + errorStatusUpdateControl.getResource().orElseThrow(), + originalResource, + errorStatusUpdateControl.isFilterPatchEvent()); } catch (Exception ex) { int code = ex instanceof KubernetesClientException kcex ? kcex.getCode() : -1; Level exceptionLevel = Level.ERROR; @@ -317,10 +311,7 @@ private void updatePostExecutionControlWithReschedule( } private PostExecutionControl

handleCleanup( - P resourceForExecution, - P originalResource, - Context

context, - ExecutionScope

executionScope) { + P resourceForExecution, Context

context, ExecutionScope

executionScope) { if (log.isDebugEnabled()) { log.debug( "Executing delete for resource: {} with version: {}", @@ -334,24 +325,7 @@ private PostExecutionControl

handleCleanup( // cleanup is finished, nothing left to be done final var finalizerName = configuration().getFinalizerName(); if (deleteControl.isRemoveFinalizer() && resourceForExecution.hasFinalizer(finalizerName)) { - P customResource = - conflictRetryingPatch( - resourceForExecution, - originalResource, - r -> { - // the operator might not be allowed to retrieve the resource on a retry, e.g. - // when its - // permissions are removed by deleting the namespace concurrently - if (r == null) { - log.warn( - "Could not remove finalizer on null resource: {} with version: {}", - getUID(resourceForExecution), - getVersion(resourceForExecution)); - return false; - } - return r.removeFinalizer(finalizerName); - }, - true); + P customResource = ReconcileUtils.removeFinalizer(context); return PostExecutionControl.customResourceFinalizerRemoved(customResource); } } @@ -367,50 +341,14 @@ private PostExecutionControl

handleCleanup( return postExecutionControl; } - @SuppressWarnings("unchecked") - private P addFinalizerWithSSA(P originalResource) { - log.debug( - "Adding finalizer (using SSA) for resource: {} version: {}", - getUID(originalResource), - getVersion(originalResource)); - try { - P resource = (P) originalResource.getClass().getConstructor().newInstance(); - ObjectMeta objectMeta = new ObjectMeta(); - objectMeta.setName(originalResource.getMetadata().getName()); - objectMeta.setNamespace(originalResource.getMetadata().getNamespace()); - resource.setMetadata(objectMeta); - resource.addFinalizer(configuration().getFinalizerName()); - return customResourceFacade.patchResourceWithSSA(resource); - } catch (InstantiationException - | IllegalAccessException - | InvocationTargetException - | NoSuchMethodException e) { - throw new RuntimeException( - "Issue with creating custom resource instance with reflection." - + " Custom Resources must provide a no-arg constructor. Class: " - + originalResource.getClass().getName(), - e); + private P patchResource(Context

context, P resource, P originalResource, boolean filterEvent) { + if (log.isDebugEnabled()) { + log.debug( + "Updating resource: {} with version: {}; SSA: {}", + resource.getMetadata().getName(), + getVersion(resource), + useSSA); } - } - - private P updateCustomResourceWithFinalizer(P resourceForExecution, P originalResource) { - log.debug( - "Adding finalizer for resource: {} version: {}", - getUID(originalResource), - getVersion(originalResource)); - return conflictRetryingPatch( - resourceForExecution, - originalResource, - r -> r.addFinalizer(configuration().getFinalizerName()), - false); - } - - private P patchResource(P resource, P originalResource) { - log.debug( - "Updating resource: {} with version: {}; SSA: {}", - getUID(resource), - getVersion(resource), - useSSA); log.trace("Resource before update: {}", resource); final var finalizerName = configuration().getFinalizerName(); @@ -418,64 +356,13 @@ private P patchResource(P resource, P originalResource) { // addFinalizer already prevents adding an already present finalizer so no need to check resource.addFinalizer(finalizerName); } - return customResourceFacade.patchResource(resource, originalResource); + return customResourceFacade.patchResource(context, resource, originalResource, filterEvent); } ControllerConfiguration

configuration() { return controller.getConfiguration(); } - public P conflictRetryingPatch( - P resource, - P originalResource, - Function modificationFunction, - boolean forceNotUseSSA) { - if (log.isDebugEnabled()) { - log.debug("Conflict retrying update for: {}", ResourceID.fromResource(resource)); - } - int retryIndex = 0; - while (true) { - try { - var modified = modificationFunction.apply(resource); - if (Boolean.FALSE.equals(modified)) { - return resource; - } - if (forceNotUseSSA) { - return customResourceFacade.patchResourceWithoutSSA(resource, originalResource); - } else { - return customResourceFacade.patchResource(resource, originalResource); - } - } catch (KubernetesClientException e) { - log.trace("Exception during patch for resource: {}", resource); - retryIndex++; - // only retry on conflict (409) and unprocessable content (422) which - // can happen if JSON Patch is not a valid request since there was - // a concurrent request which already removed another finalizer: - // List element removal from a list is by index in JSON Patch - // so if addressing a second finalizer but first is meanwhile removed - // it is a wrong request. - if (e.getCode() != 409 && e.getCode() != 422) { - throw e; - } - if (retryIndex >= MAX_UPDATE_RETRY) { - throw new OperatorException( - "Exceeded maximum (" - + MAX_UPDATE_RETRY - + ") retry attempts to patch resource: " - + ResourceID.fromResource(resource)); - } - log.debug( - "Retrying patch for resource name: {}, namespace: {}; HTTP code: {}", - resource.getMetadata().getName(), - resource.getMetadata().getNamespace(), - e.getCode()); - resource = - customResourceFacade.getResource( - resource.getMetadata().getNamespace(), resource.getMetadata().getName()); - } - } - } - private void validateExecutionScope(ExecutionScope

executionScope) { if (!triggerOnAllEvents() && (executionScope.isDeleteEvent() || executionScope.isDeleteFinalStateUnknown())) { @@ -488,34 +375,16 @@ private void validateExecutionScope(ExecutionScope

executionScope) { // created to support unit testing static class CustomResourceFacade { - private final MixedOperation, Resource> resourceOperation; private final boolean useSSA; - private final String fieldManager; private final Cloner cloner; - public CustomResourceFacade( - MixedOperation, Resource> resourceOperation, - ControllerConfiguration configuration, - Cloner cloner) { - this.resourceOperation = resourceOperation; + public CustomResourceFacade(ControllerConfiguration configuration, Cloner cloner) { this.useSSA = configuration.getConfigurationService().useSSAToPatchPrimaryResource(); - this.fieldManager = configuration.fieldManager(); this.cloner = cloner; } - public R getResource(String namespace, String name) { - if (namespace != null) { - return resourceOperation.inNamespace(namespace).withName(name).get(); - } else { - return resourceOperation.withName(name).get(); - } - } - - public R patchResourceWithoutSSA(R resource, R originalResource) { - return resource(originalResource).edit(r -> resource); - } - - public R patchResource(R resource, R originalResource) { + public R patchResource( + Context context, R resource, R originalResource, boolean filterEvent) { if (log.isDebugEnabled()) { log.debug( "Trying to replace resource {}, version: {}", @@ -523,35 +392,29 @@ public R patchResource(R resource, R originalResource) { resource.getMetadata().getResourceVersion()); } if (useSSA) { - return patchResourceWithSSA(resource); + return ReconcileUtils.serverSideApplyPrimary(context, resource, filterEvent); } else { - return resource(originalResource).edit(r -> resource); + return ReconcileUtils.jsonPatchPrimary( + context, originalResource, r -> resource, filterEvent); } } - public R patchStatus(R resource, R originalResource) { + public R patchStatus(Context context, R resource, R originalResource, boolean filterEvent) { log.trace("Patching status for resource: {} with ssa: {}", resource, useSSA); if (useSSA) { var managedFields = resource.getMetadata().getManagedFields(); try { resource.getMetadata().setManagedFields(null); - var res = resource(resource); - return res.subresource("status") - .patch( - new PatchContext.Builder() - .withFieldManager(fieldManager) - .withForce(true) - .withPatchType(PatchType.SERVER_SIDE_APPLY) - .build()); + return ReconcileUtils.serverSideApplyPrimaryStatus(context, resource, filterEvent); } finally { resource.getMetadata().setManagedFields(managedFields); } } else { - return editStatus(resource, originalResource); + return editStatus(context, resource, originalResource, filterEvent); } } - private R editStatus(R resource, R originalResource) { + private R editStatus(Context context, R resource, R originalResource, boolean filterEvent) { String resourceVersion = resource.getMetadata().getResourceVersion(); // the cached resource should not be changed in any circumstances // that can lead to all kinds of race conditions. @@ -559,34 +422,20 @@ private R editStatus(R resource, R originalResource) { try { clonedOriginal.getMetadata().setResourceVersion(null); resource.getMetadata().setResourceVersion(null); - var res = resource(clonedOriginal); - return res.editStatus( + return ReconcileUtils.jsonPatchPrimaryStatus( + context, + clonedOriginal, r -> { - ReconcilerUtils.setStatus(r, ReconcilerUtils.getStatus(resource)); + ReconcilerUtilsInternal.setStatus(r, ReconcilerUtilsInternal.getStatus(resource)); return r; - }); + }, + filterEvent); } finally { // restore initial resource version clonedOriginal.getMetadata().setResourceVersion(resourceVersion); resource.getMetadata().setResourceVersion(resourceVersion); } } - - public R patchResourceWithSSA(R resource) { - return resource(resource) - .patch( - new PatchContext.Builder() - .withFieldManager(fieldManager) - .withForce(true) - .withPatchType(PatchType.SERVER_SIDE_APPLY) - .build()); - } - - private Resource resource(R resource) { - return resource instanceof Namespaced - ? resourceOperation.inNamespace(resource.getMetadata().getNamespace()).resource(resource) - : resourceOperation.resource(resource); - } } private boolean triggerOnAllEvents() { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerEventSource.java index f7ed9fdc8e..c323f4841d 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerEventSource.java @@ -32,7 +32,7 @@ import io.javaoperatorsdk.operator.processing.event.source.filter.OnUpdateFilter; import io.javaoperatorsdk.operator.processing.event.source.informer.ManagedInformerEventSource; -import static io.javaoperatorsdk.operator.ReconcilerUtils.handleKubernetesClientException; +import static io.javaoperatorsdk.operator.ReconcilerUtilsInternal.handleKubernetesClientException; import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getVersion; import static io.javaoperatorsdk.operator.processing.event.source.controller.InternalEventFilters.*; @@ -81,21 +81,26 @@ public synchronized void start() { } } - public void eventReceived( - ResourceAction action, T resource, T oldResource, Boolean deletedFinalStateUnknown) { + public synchronized void eventReceived( + ResourceAction action, + T resource, + T oldResource, + Boolean deletedFinalStateUnknown, + boolean filterEvent) { try { if (log.isDebugEnabled()) { log.debug( - "Event received for resource: {} version: {} uuid: {} action: {}", + "Event received for resource: {} version: {} uuid: {} action: {} filter event: {}", ResourceID.fromResource(resource), getVersion(resource), resource.getMetadata().getUid(), - action); + action, + filterEvent); log.trace("Event Old resource: {},\n new resource: {}", oldResource, resource); } MDCUtils.addResourceInfo(resource); controller.getEventSourceManager().broadcastOnResourceEvent(action, resource, oldResource); - if (isAcceptedByFilters(action, resource, oldResource)) { + if (isAcceptedByFilters(action, resource, oldResource) && !filterEvent) { if (deletedFinalStateUnknown != null) { getEventHandler() .handleEvent( @@ -132,20 +137,23 @@ private boolean isAcceptedByFilters(ResourceAction action, T resource, T oldReso @Override public void onAdd(T resource) { - super.onAdd(resource); - eventReceived(ResourceAction.ADDED, resource, null, null); + var knownResourceVersion = temporaryResourceCache.onAddOrUpdateEvent(resource); + eventReceived(ResourceAction.ADDED, resource, null, null, knownResourceVersion); } @Override public void onUpdate(T oldCustomResource, T newCustomResource) { - super.onUpdate(oldCustomResource, newCustomResource); - eventReceived(ResourceAction.UPDATED, newCustomResource, oldCustomResource, null); + var knownResourceVersion = temporaryResourceCache.onAddOrUpdateEvent(newCustomResource); + eventReceived( + ResourceAction.UPDATED, newCustomResource, oldCustomResource, null, knownResourceVersion); } @Override public void onDelete(T resource, boolean deletedFinalStateUnknown) { - super.onDelete(resource, deletedFinalStateUnknown); - eventReceived(ResourceAction.DELETED, resource, null, deletedFinalStateUnknown); + temporaryResourceCache.onDeleteEvent(resource, deletedFinalStateUnknown); + // delete event is quite special here, that requires special care, since we clean up caches on + // delete event. + eventReceived(ResourceAction.DELETED, resource, null, deletedFinalStateUnknown, false); } @Override 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 0feb3dc2a8..3f25490534 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 @@ -17,7 +17,6 @@ import java.util.Optional; import java.util.Set; -import java.util.function.UnaryOperator; import java.util.stream.Collectors; import org.slf4j.Logger; @@ -28,7 +27,6 @@ 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; @@ -148,7 +146,7 @@ public synchronized void onDelete(R resource, boolean b) { resourceType().getSimpleName()); } primaryToSecondaryIndex.onDelete(resource); - super.onDelete(resource, b); + temporaryResourceCache.onDeleteEvent(resource, b); if (acceptedByDeleteFilters(resource, b)) { propagateEvent(resource); } @@ -233,15 +231,15 @@ public Set getSecondaryResources(P primary) { @Override public void handleRecentResourceUpdate( ResourceID resourceID, R resource, R previousVersionOfResource) { - handleRecentCreateOrUpdate(Operation.UPDATE, resource, previousVersionOfResource); + handleRecentCreateOrUpdate(resource); } @Override public void handleRecentResourceCreate(ResourceID resourceID, R resource) { - handleRecentCreateOrUpdate(Operation.ADD, resource, null); + handleRecentCreateOrUpdate(resource); } - private void handleRecentCreateOrUpdate(Operation operation, R newResource, R oldResource) { + private void handleRecentCreateOrUpdate(R newResource) { primaryToSecondaryIndex.onAddOrUpdate(newResource); temporaryResourceCache.putResource(newResource); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java index abd2b6a752..42e06c9d9a 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java @@ -32,7 +32,7 @@ import io.fabric8.kubernetes.client.dsl.Resource; import io.fabric8.kubernetes.client.informers.ResourceEventHandler; import io.javaoperatorsdk.operator.OperatorException; -import io.javaoperatorsdk.operator.ReconcilerUtils; +import io.javaoperatorsdk.operator.ReconcilerUtilsInternal; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.config.Informable; import io.javaoperatorsdk.operator.api.config.informer.InformerConfiguration; @@ -253,7 +253,7 @@ public String toString() { final var informerConfig = configuration.getInformerConfig(); final var selector = informerConfig.getLabelSelector(); return "InformerManager [" - + ReconcilerUtils.getResourceTypeNameWithVersion(configuration.getResourceClass()) + + ReconcilerUtilsInternal.getResourceTypeNameWithVersion(configuration.getResourceClass()) + "] watching: " + informerConfig.getEffectiveNamespaces(controllerConfiguration) + (selector != null ? " selector: " + selector : ""); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.java index c3a4a9f2c1..60497bc0c9 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.java @@ -35,7 +35,7 @@ import io.fabric8.kubernetes.client.informers.SharedIndexInformer; import io.fabric8.kubernetes.client.informers.cache.Cache; import io.javaoperatorsdk.operator.OperatorException; -import io.javaoperatorsdk.operator.ReconcilerUtils; +import io.javaoperatorsdk.operator.ReconcilerUtilsInternal; import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.health.InformerHealthIndicator; import io.javaoperatorsdk.operator.health.Status; @@ -131,7 +131,7 @@ public void start() throws OperatorException { } } catch (Exception e) { - ReconcilerUtils.handleKubernetesClientException( + ReconcilerUtilsInternal.handleKubernetesClientException( e, HasMetadata.getFullResourceName(informer.getApiTypeClass())); throw new OperatorException( "Couldn't start informer for " + versionedFullResourceName() + " resources", e); @@ -143,7 +143,7 @@ private String versionedFullResourceName() { if (apiTypeClass.isAssignableFrom(GenericKubernetesResource.class)) { return GenericKubernetesResource.class.getSimpleName(); } - return ReconcilerUtils.getResourceTypeNameWithVersion(apiTypeClass); + return ReconcilerUtilsInternal.getResourceTypeNameWithVersion(apiTypeClass); } @Override diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java index af30617d92..6f3d732967 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java @@ -22,6 +22,7 @@ import java.util.Set; import java.util.function.Function; import java.util.function.Predicate; +import java.util.function.UnaryOperator; import java.util.stream.Stream; import org.slf4j.Logger; @@ -34,7 +35,7 @@ import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.config.Informable; import io.javaoperatorsdk.operator.api.config.NamespaceChangeable; -import io.javaoperatorsdk.operator.api.reconciler.PrimaryUpdateAndCacheUtils; +import io.javaoperatorsdk.operator.api.reconciler.ReconcileUtils; import io.javaoperatorsdk.operator.api.reconciler.dependent.RecentOperationCacheFiller; import io.javaoperatorsdk.operator.health.InformerHealthIndicator; import io.javaoperatorsdk.operator.health.InformerWrappingEventSourceHealthIndicator; @@ -71,21 +72,6 @@ protected ManagedInformerEventSource( this.configuration = configuration; } - @Override - public void onAdd(R resource) { - temporaryResourceCache.onAddOrUpdateEvent(resource); - } - - @Override - public void onUpdate(R oldObj, R newObj) { - temporaryResourceCache.onAddOrUpdateEvent(newObj); - } - - @Override - public void onDelete(R obj, boolean deletedFinalStateUnknown) { - temporaryResourceCache.onDeleteEvent(obj, deletedFinalStateUnknown); - } - protected InformerManager manager() { return cache; } @@ -97,6 +83,37 @@ public void changeNamespaces(Set namespaces) { } } + /** + * Updates the resource and makes sure that the response is available for the next reconciliation. + * It does not filter the event produced by this update. + */ + public R updateAndCacheResource(R resourceToUpdate, UnaryOperator updateMethod) { + ResourceID id = ResourceID.fromResource(resourceToUpdate); + var updated = updateMethod.apply(resourceToUpdate); + handleRecentResourceUpdate(id, updated, resourceToUpdate); + return updated; + } + + /** + * Updates the resource and makes sure that the response is available for the next reconciliation. + * Also makes sure that the even produced by this update is filtered, thus does not trigger the + * reconciliation. + */ + public R eventFilteringUpdateAndCacheResource(R resourceToUpdate, UnaryOperator updateMethod) { + ResourceID id = ResourceID.fromResource(resourceToUpdate); + if (log.isDebugEnabled()) { + log.debug("Update and cache: {}", id); + } + try { + temporaryResourceCache.startEventFilteringModify(id); + var updated = updateMethod.apply(resourceToUpdate); + handleRecentResourceUpdate(id, updated, resourceToUpdate); + return updated; + } finally { + temporaryResourceCache.doneEventFilterModify(id); + } + } + @SuppressWarnings("unchecked") @Override public synchronized void start() { @@ -137,10 +154,7 @@ public Optional get(ResourceID resourceID) { Optional resource = temporaryResourceCache.getResourceFromCache(resourceID); if (comparableResourceVersions && resource.isPresent() - && res.filter( - r -> - PrimaryUpdateAndCacheUtils.compareResourceVersions(r, resource.orElseThrow()) - > 0) + && res.filter(r -> ReconcileUtils.compareResourceVersions(r, resource.orElseThrow()) > 0) .isEmpty()) { log.debug("Latest resource found in temporary cache for Resource ID: {}", resourceID); return resource; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryResourceCache.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryResourceCache.java index d918be447d..6b6d0c3d95 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 @@ -17,6 +17,7 @@ import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.locks.ReentrantLock; @@ -24,7 +25,7 @@ import org.slf4j.LoggerFactory; import io.fabric8.kubernetes.api.model.HasMetadata; -import io.javaoperatorsdk.operator.api.reconciler.PrimaryUpdateAndCacheUtils; +import io.javaoperatorsdk.operator.api.reconciler.ReconcileUtils; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource; import io.javaoperatorsdk.operator.processing.event.ResourceID; @@ -55,13 +56,14 @@ public class TemporaryResourceCache { private final Map cache = new ConcurrentHashMap<>(); private final boolean comparableResourceVersions; private final Map activelyModifying = new ConcurrentHashMap<>(); + private final Set skipFiltering = ConcurrentHashMap.newKeySet(); private String latestResourceVersion; public TemporaryResourceCache(boolean comparableResourceVersions) { this.comparableResourceVersions = comparableResourceVersions; } - public void startModifying(ResourceID id) { + public void startEventFilteringModify(ResourceID id) { if (!comparableResourceVersions) { return; } @@ -78,7 +80,7 @@ public void startModifying(ResourceID id) { .lock(); } - public void doneModifying(ResourceID id) { + public void doneEventFilterModify(ResourceID id) { if (!comparableResourceVersions) { return; } @@ -95,42 +97,62 @@ public void onDeleteEvent(T resource, boolean unknownState) { } /** - * @return true if the resourceVersion was already known + * @return true if the resourceVersion was already known and not skipped for event filtering */ public boolean onAddOrUpdateEvent(T resource) { return onEvent(resource, false); } private boolean onEvent(T resource, boolean unknownState) { - ReentrantLock lock = activelyModifying.get(ResourceID.fromResource(resource)); + var resourceId = ResourceID.fromResource(resource); + if (log.isDebugEnabled()) { + log.debug( + "Processing event for resource id: {} version: {} ", + resourceId, + resource.getMetadata().getResourceVersion()); + } + ReentrantLock lock = activelyModifying.get(resourceId); if (lock != null) { + log.trace("Lock for event filtering resource id: {}", resourceId); + // note that this is a special case of lock striping; event handling happens + // always on the same thread of the informer we lock only if the update is happening + // for the same resource (not any resource), and if the event comes from the current update + // this should be locked for a very short time, since that update request already send at this + // point. lock.lock(); // wait for the modification to finish lock.unlock(); // simply unlock as the event is guaranteed after the modification + log.trace("Unlock for event resource id: {}", resourceId); } - boolean[] known = new boolean[1]; + boolean[] filter = new boolean[1]; synchronized (this) { if (!unknownState) { latestResourceVersion = resource.getMetadata().getResourceVersion(); } cache.computeIfPresent( - ResourceID.fromResource(resource), + resourceId, (id, cached) -> { boolean remove = unknownState; if (!unknownState) { - int comp = PrimaryUpdateAndCacheUtils.compareResourceVersions(resource, cached); + int comp = ReconcileUtils.compareResourceVersions(resource, cached); if (comp >= 0) { remove = true; } - if (comp <= 0) { - known[0] = true; + if (comp < 0) { + filter[0] = true; + } else if (comp == 0) { + filter[0] = !skipFiltering.remove(resourceId); + } else { + skipFiltering.remove(resourceId); } + } else { + skipFiltering.remove(resourceId); } if (remove) { return null; } return cached; }); - return known[0]; + return filter[0]; } } @@ -141,6 +163,7 @@ public synchronized void putResource(T newResource) { } var resourceId = ResourceID.fromResource(newResource); + skipFiltering.remove(resourceId); if (newResource.getMetadata().getResourceVersion() == null) { log.warn( @@ -157,7 +180,7 @@ public synchronized void putResource(T newResource) { // this also prevents resurrecting recently deleted entities for which the delete event // has already been processed if (latestResourceVersion != null - && PrimaryUpdateAndCacheUtils.compareResourceVersions( + && ReconcileUtils.compareResourceVersions( latestResourceVersion, newResource.getMetadata().getResourceVersion()) > 0) { log.debug( @@ -172,16 +195,24 @@ public synchronized void putResource(T newResource) { var cachedResource = getResourceFromCache(resourceId).orElse(null); if (cachedResource == null - || PrimaryUpdateAndCacheUtils.compareResourceVersions(newResource, cachedResource) > 0) { + || ReconcileUtils.compareResourceVersions(newResource, cachedResource) > 0) { log.debug( "Temporarily moving ahead to target version {} for resource id: {}", newResource.getMetadata().getResourceVersion(), resourceId); cache.put(resourceId, newResource); + if (!isFilteringModification(resourceId)) { + log.debug("Add resource id to skipFiltering: {}", resourceId); + skipFiltering.add(resourceId); + } } } public synchronized Optional getResourceFromCache(ResourceID resourceID) { return Optional.ofNullable(cache.get(resourceID)); } + + private boolean isFilteringModification(ResourceID resourceId) { + return activelyModifying.containsKey(resourceId); + } } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/OperatorIT.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/OperatorIT.java index c87c986f99..e5dae6ca80 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/OperatorIT.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/OperatorIT.java @@ -45,7 +45,7 @@ void shouldBePossibleToRetrieveNumberOfRegisteredControllers() { void shouldBePossibleToRetrieveRegisteredControllerByName() { final var operator = new Operator(); final var reconciler = new FooReconciler(); - final var name = ReconcilerUtils.getNameFor(reconciler); + final var name = ReconcilerUtilsInternal.getNameFor(reconciler); var registeredControllers = operator.getRegisteredControllers(); assertTrue(operator.getRegisteredController(name).isEmpty()); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ReconcilerUtilsTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ReconcilerUtilsInternalTest.java similarity index 84% rename from operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ReconcilerUtilsTest.java rename to operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ReconcilerUtilsInternalTest.java index 3bbe2a894b..12e45b9c23 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ReconcilerUtilsTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ReconcilerUtilsInternalTest.java @@ -32,17 +32,17 @@ import io.javaoperatorsdk.operator.sample.simple.TestCustomReconciler; import io.javaoperatorsdk.operator.sample.simple.TestCustomResource; -import static io.javaoperatorsdk.operator.ReconcilerUtils.getDefaultFinalizerName; -import static io.javaoperatorsdk.operator.ReconcilerUtils.getDefaultNameFor; -import static io.javaoperatorsdk.operator.ReconcilerUtils.getDefaultReconcilerName; -import static io.javaoperatorsdk.operator.ReconcilerUtils.handleKubernetesClientException; -import static io.javaoperatorsdk.operator.ReconcilerUtils.isFinalizerValid; +import static io.javaoperatorsdk.operator.ReconcilerUtilsInternal.getDefaultFinalizerName; +import static io.javaoperatorsdk.operator.ReconcilerUtilsInternal.getDefaultNameFor; +import static io.javaoperatorsdk.operator.ReconcilerUtilsInternal.getDefaultReconcilerName; +import static io.javaoperatorsdk.operator.ReconcilerUtilsInternal.handleKubernetesClientException; +import static io.javaoperatorsdk.operator.ReconcilerUtilsInternal.isFinalizerValid; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.*; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -class ReconcilerUtilsTest { +class ReconcilerUtilsInternalTest { public static final String RESOURCE_URI = "https://kubernetes.docker.internal:6443/apis/tomcatoperator.io/v1/tomcats"; @@ -71,7 +71,7 @@ void equalsSpecObject() { var d1 = createTestDeployment(); var d2 = createTestDeployment(); - assertThat(ReconcilerUtils.specsEqual(d1, d2)).isTrue(); + assertThat(ReconcilerUtilsInternal.specsEqual(d1, d2)).isTrue(); } @Test @@ -80,7 +80,7 @@ void equalArbitraryDifferentSpecsOfObjects() { var d2 = createTestDeployment(); d2.getSpec().getTemplate().getSpec().setHostname("otherhost"); - assertThat(ReconcilerUtils.specsEqual(d1, d2)).isFalse(); + assertThat(ReconcilerUtilsInternal.specsEqual(d1, d2)).isFalse(); } @Test @@ -89,7 +89,7 @@ void getsSpecWithReflection() { deployment.setSpec(new DeploymentSpec()); deployment.getSpec().setReplicas(5); - DeploymentSpec spec = (DeploymentSpec) ReconcilerUtils.getSpec(deployment); + DeploymentSpec spec = (DeploymentSpec) ReconcilerUtilsInternal.getSpec(deployment); assertThat(spec.getReplicas()).isEqualTo(5); } @@ -97,10 +97,10 @@ void getsSpecWithReflection() { void properlyHandlesNullSpec() { Namespace ns = new Namespace(); - final var spec = ReconcilerUtils.getSpec(ns); + final var spec = ReconcilerUtilsInternal.getSpec(ns); assertThat(spec).isNull(); - ReconcilerUtils.setSpec(ns, null); + ReconcilerUtilsInternal.setSpec(ns, null); } @Test @@ -111,7 +111,7 @@ void setsSpecWithReflection() { DeploymentSpec newSpec = new DeploymentSpec(); newSpec.setReplicas(1); - ReconcilerUtils.setSpec(deployment, newSpec); + ReconcilerUtilsInternal.setSpec(deployment, newSpec); assertThat(deployment.getSpec().getReplicas()).isEqualTo(1); } @@ -124,7 +124,7 @@ void setsSpecCustomResourceWithReflection() { TomcatSpec newSpec = new TomcatSpec(); newSpec.setReplicas(1); - ReconcilerUtils.setSpec(tomcat, newSpec); + ReconcilerUtilsInternal.setSpec(tomcat, newSpec); assertThat(tomcat.getSpec().getReplicas()).isEqualTo(1); } @@ -132,7 +132,7 @@ void setsSpecCustomResourceWithReflection() { @Test void loadYamlAsBuilder() { DeploymentBuilder builder = - ReconcilerUtils.loadYaml(DeploymentBuilder.class, getClass(), "deployment.yaml"); + ReconcilerUtilsInternal.loadYaml(DeploymentBuilder.class, getClass(), "deployment.yaml"); builder.accept(ContainerBuilder.class, c -> c.withImage("my-image")); Deployment deployment = builder.editMetadata().withName("my-deployment").and().build(); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtilsTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtilsTest.java new file mode 100644 index 0000000000..5480eb081a --- /dev/null +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtilsTest.java @@ -0,0 +1,94 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.api.reconciler; + +import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import static io.javaoperatorsdk.operator.api.reconciler.ReconcileUtils.compareResourceVersions; +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.*; + +class ReconcileUtilsTest { + + private static final Logger log = LoggerFactory.getLogger(ReconcileUtilsTest.class); + + @Test + public void compareResourceVersionsTest() { + assertThat(compareResourceVersions("11", "22")).isNegative(); + assertThat(compareResourceVersions("22", "11")).isPositive(); + assertThat(compareResourceVersions("1", "1")).isZero(); + assertThat(compareResourceVersions("11", "11")).isZero(); + assertThat(compareResourceVersions("123", "2")).isPositive(); + assertThat(compareResourceVersions("3", "211")).isNegative(); + + assertThrows( + NonComparableResourceVersionException.class, () -> compareResourceVersions("aa", "22")); + assertThrows( + NonComparableResourceVersionException.class, () -> compareResourceVersions("11", "ba")); + assertThrows( + NonComparableResourceVersionException.class, () -> compareResourceVersions("", "22")); + assertThrows( + NonComparableResourceVersionException.class, () -> compareResourceVersions("11", "")); + assertThrows( + NonComparableResourceVersionException.class, () -> compareResourceVersions("01", "123")); + assertThrows( + NonComparableResourceVersionException.class, () -> compareResourceVersions("123", "01")); + assertThrows( + NonComparableResourceVersionException.class, () -> compareResourceVersions("3213", "123a")); + assertThrows( + NonComparableResourceVersionException.class, () -> compareResourceVersions("321", "123a")); + } + + // naive performance test that compares the work case scenario for the parsing and non-parsing + // variants + @Test + @Disabled + public void compareResourcePerformanceTest() { + var execNum = 30000000; + var startTime = System.currentTimeMillis(); + for (int i = 0; i < execNum; i++) { + var res = compareResourceVersions("123456788", "123456789"); + } + var dur1 = System.currentTimeMillis() - startTime; + log.info("Duration without parsing: {}", dur1); + startTime = System.currentTimeMillis(); + for (int i = 0; i < execNum; i++) { + var res = Long.parseLong("123456788") > Long.parseLong("123456789"); + } + var dur2 = System.currentTimeMillis() - startTime; + log.info("Duration with parsing: {}", dur2); + + assertThat(dur1).isLessThan(dur2); + } + + @Test + void retriesAddingFinalizerWithoutSSA() { + // todo + } + + @Test + void nullResourceIsGracefullyHandledOnFinalizerRemovalRetry() { + // todo + } + + @Test + void retriesFinalizerRemovalWithFreshResource() { + // todo + } +} diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcherTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcherTest.java index 8a920b28b9..8dd7283fb9 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcherTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcherTest.java @@ -26,7 +26,7 @@ import io.fabric8.kubernetes.api.model.apps.DeploymentStatusBuilder; import io.fabric8.kubernetes.client.KubernetesClient; import io.javaoperatorsdk.operator.MockKubernetesClient; -import io.javaoperatorsdk.operator.ReconcilerUtils; +import io.javaoperatorsdk.operator.ReconcilerUtilsInternal; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.api.reconciler.DefaultContext; @@ -198,7 +198,7 @@ ConfigMap createConfigMap() { } Deployment createDeployment() { - return ReconcilerUtils.loadYaml( + return ReconcilerUtilsInternal.loadYaml( Deployment.class, GenericKubernetesResourceMatcherTest.class, "nginx-deployment.yaml"); } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericResourceUpdaterTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericResourceUpdaterTest.java index 3b6580c5d3..70d664f652 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericResourceUpdaterTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericResourceUpdaterTest.java @@ -25,7 +25,7 @@ import io.fabric8.kubernetes.api.model.*; import io.fabric8.kubernetes.api.model.apps.Deployment; import io.javaoperatorsdk.operator.MockKubernetesClient; -import io.javaoperatorsdk.operator.ReconcilerUtils; +import io.javaoperatorsdk.operator.ReconcilerUtilsInternal; import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.Context; @@ -131,7 +131,7 @@ void checkServiceAccount() { } Deployment createDeployment() { - return ReconcilerUtils.loadYaml( + return ReconcilerUtilsInternal.loadYaml( Deployment.class, GenericResourceUpdaterTest.class, "nginx-deployment.yaml"); } } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/SSABasedGenericKubernetesResourceMatcherTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/SSABasedGenericKubernetesResourceMatcherTest.java index bbcfa704b5..c4d2f2c77d 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/SSABasedGenericKubernetesResourceMatcherTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/SSABasedGenericKubernetesResourceMatcherTest.java @@ -32,7 +32,7 @@ import io.fabric8.kubernetes.api.model.apps.StatefulSet; import io.javaoperatorsdk.operator.MockKubernetesClient; import io.javaoperatorsdk.operator.OperatorException; -import io.javaoperatorsdk.operator.ReconcilerUtils; +import io.javaoperatorsdk.operator.ReconcilerUtilsInternal; import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.Context; @@ -419,7 +419,7 @@ void testSortListItems() { } private static R loadResource(String fileName, Class clazz) { - return ReconcilerUtils.loadYaml( + return ReconcilerUtilsInternal.loadYaml( clazz, SSABasedGenericKubernetesResourceMatcherTest.class, fileName); } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java index cc9df317ae..017e84dcfa 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java @@ -26,12 +26,11 @@ import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; import org.mockito.ArgumentMatchers; -import org.mockito.stubbing.Answer; +import org.mockito.MockedStatic; import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.api.model.ObjectMeta; import io.fabric8.kubernetes.client.CustomResource; -import io.fabric8.kubernetes.client.KubernetesClientException; import io.fabric8.kubernetes.client.utils.KubernetesSerialization; import io.javaoperatorsdk.operator.MockKubernetesClient; import io.javaoperatorsdk.operator.OperatorException; @@ -46,6 +45,7 @@ import io.javaoperatorsdk.operator.api.reconciler.DefaultContext; import io.javaoperatorsdk.operator.api.reconciler.DeleteControl; import io.javaoperatorsdk.operator.api.reconciler.ErrorStatusUpdateControl; +import io.javaoperatorsdk.operator.api.reconciler.ReconcileUtils; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; import io.javaoperatorsdk.operator.api.reconciler.RetryInfo; import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; @@ -56,10 +56,8 @@ import io.javaoperatorsdk.operator.sample.simple.TestCustomResource; import static io.javaoperatorsdk.operator.TestUtils.markForDeletion; -import static io.javaoperatorsdk.operator.processing.event.ReconciliationDispatcher.MAX_UPDATE_RETRY; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.*; -import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.Mockito.*; @SuppressWarnings({"unchecked", "rawtypes"}) @@ -154,28 +152,26 @@ public boolean useFinalizer() { @Test void addFinalizerOnNewResource() { - assertFalse(testCustomResource.hasFinalizer(DEFAULT_FINALIZER)); - reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); - verify(reconciler, never()).reconcile(ArgumentMatchers.eq(testCustomResource), any()); - verify(customResourceFacade, times(1)) - .patchResourceWithSSA( - argThat(testCustomResource -> testCustomResource.hasFinalizer(DEFAULT_FINALIZER))); + try (MockedStatic mockedReconcileUtils = mockStatic(ReconcileUtils.class)) { + assertFalse(testCustomResource.hasFinalizer(DEFAULT_FINALIZER)); + reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); + verify(reconciler, never()).reconcile(ArgumentMatchers.eq(testCustomResource), any()); + mockedReconcileUtils.verify(() -> ReconcileUtils.addFinalizerWithSSA(any()), times(1)); + } } @Test void addFinalizerOnNewResourceWithoutSSA() { - initConfigService(false); - final ReconciliationDispatcher dispatcher = - init(testCustomResource, reconciler, null, customResourceFacade, true); - - assertFalse(testCustomResource.hasFinalizer(DEFAULT_FINALIZER)); - dispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); - verify(reconciler, never()).reconcile(ArgumentMatchers.eq(testCustomResource), any()); - verify(customResourceFacade, times(1)) - .patchResource( - argThat(testCustomResource -> testCustomResource.hasFinalizer(DEFAULT_FINALIZER)), - any()); - assertThat(testCustomResource.hasFinalizer(DEFAULT_FINALIZER)).isTrue(); + try (MockedStatic mockedReconcileUtils = mockStatic(ReconcileUtils.class)) { + initConfigService(false, false); + final ReconciliationDispatcher dispatcher = + init(testCustomResource, reconciler, null, customResourceFacade, true); + + assertFalse(testCustomResource.hasFinalizer(DEFAULT_FINALIZER)); + dispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); + verify(reconciler, never()).reconcile(ArgumentMatchers.eq(testCustomResource), any()); + mockedReconcileUtils.verify(() -> ReconcileUtils.addFinalizer(any()), times(1)); + } } @Test @@ -190,13 +186,15 @@ void patchesBothResourceAndStatusIfFinalizerSet() { testCustomResource.addFinalizer(DEFAULT_FINALIZER); reconciler.reconcile = (r, c) -> UpdateControl.patchResourceAndStatus(testCustomResource); - when(customResourceFacade.patchResource(eq(testCustomResource), any())) + when(customResourceFacade.patchResource(any(), eq(testCustomResource), any(), anyBoolean())) .thenReturn(testCustomResource); reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); - verify(customResourceFacade, times(1)).patchResource(eq(testCustomResource), any()); - verify(customResourceFacade, times(1)).patchStatus(eq(testCustomResource), any()); + verify(customResourceFacade, times(1)) + .patchResource(any(), eq(testCustomResource), any(), eq(true)); + verify(customResourceFacade, times(1)) + .patchStatus(any(), eq(testCustomResource), any(), eq(true)); } @Test @@ -207,8 +205,9 @@ void patchesStatus() { reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); - verify(customResourceFacade, times(1)).patchStatus(eq(testCustomResource), any()); - verify(customResourceFacade, never()).patchResource(any(), any()); + verify(customResourceFacade, times(1)) + .patchStatus(any(), eq(testCustomResource), any(), eq(true)); + verify(customResourceFacade, never()).patchResource(any(), any(), any(), eq(true)); } @Test @@ -232,86 +231,16 @@ void callsDeleteIfObjectHasFinalizerAndMarkedForDelete() { @Test void removesDefaultFinalizerOnDeleteIfSet() { - testCustomResource.addFinalizer(DEFAULT_FINALIZER); - markForDeletion(testCustomResource); - - var postExecControl = - reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); + try (MockedStatic mockedReconcileUtils = mockStatic(ReconcileUtils.class)) { + testCustomResource.addFinalizer(DEFAULT_FINALIZER); + markForDeletion(testCustomResource); - assertThat(postExecControl.isFinalizerRemoved()).isTrue(); - verify(customResourceFacade, times(1)).patchResourceWithoutSSA(eq(testCustomResource), any()); - } + var postExecControl = + reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); - @Test - void retriesFinalizerRemovalWithFreshResource() { - testCustomResource.addFinalizer(DEFAULT_FINALIZER); - markForDeletion(testCustomResource); - var resourceWithFinalizer = TestUtils.testCustomResource(); - resourceWithFinalizer.addFinalizer(DEFAULT_FINALIZER); - when(customResourceFacade.patchResourceWithoutSSA(eq(testCustomResource), any())) - .thenThrow(new KubernetesClientException(null, 409, null)) - .thenReturn(testCustomResource); - when(customResourceFacade.getResource(any(), any())).thenReturn(resourceWithFinalizer); - - var postExecControl = - reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); - - assertThat(postExecControl.isFinalizerRemoved()).isTrue(); - verify(customResourceFacade, times(2)).patchResourceWithoutSSA(any(), any()); - verify(customResourceFacade, times(1)).getResource(any(), any()); - } - - @Test - void nullResourceIsGracefullyHandledOnFinalizerRemovalRetry() { - // simulate the operator not able or not be allowed to get the custom resource during the retry - // of the finalizer removal - testCustomResource.addFinalizer(DEFAULT_FINALIZER); - markForDeletion(testCustomResource); - when(customResourceFacade.patchResourceWithoutSSA(any(), any())) - .thenThrow(new KubernetesClientException(null, 409, null)); - when(customResourceFacade.getResource(any(), any())).thenReturn(null); - - var postExecControl = - reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); - - assertThat(postExecControl.isFinalizerRemoved()).isTrue(); - verify(customResourceFacade, times(1)).patchResourceWithoutSSA(eq(testCustomResource), any()); - verify(customResourceFacade, times(1)).getResource(any(), any()); - } - - @Test - void throwsExceptionIfFinalizerRemovalRetryExceeded() { - testCustomResource.addFinalizer(DEFAULT_FINALIZER); - markForDeletion(testCustomResource); - when(customResourceFacade.patchResourceWithoutSSA(any(), any())) - .thenThrow(new KubernetesClientException(null, 409, null)); - when(customResourceFacade.getResource(any(), any())) - .thenAnswer((Answer) invocationOnMock -> createResourceWithFinalizer()); - - var postExecControl = - reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); - - assertThat(postExecControl.isFinalizerRemoved()).isFalse(); - assertThat(postExecControl.getRuntimeException()).isPresent(); - assertThat(postExecControl.getRuntimeException().get()).isInstanceOf(OperatorException.class); - verify(customResourceFacade, times(MAX_UPDATE_RETRY)).patchResourceWithoutSSA(any(), any()); - verify(customResourceFacade, times(MAX_UPDATE_RETRY - 1)).getResource(any(), any()); - } - - @Test - void throwsExceptionIfFinalizerRemovalClientExceptionIsNotConflict() { - testCustomResource.addFinalizer(DEFAULT_FINALIZER); - markForDeletion(testCustomResource); - when(customResourceFacade.patchResourceWithoutSSA(any(), any())) - .thenThrow(new KubernetesClientException(null, 400, null)); - - var res = - reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); - - assertThat(res.getRuntimeException()).isPresent(); - assertThat(res.getRuntimeException().get()).isInstanceOf(KubernetesClientException.class); - verify(customResourceFacade, times(1)).patchResourceWithoutSSA(any(), any()); - verify(customResourceFacade, never()).getResource(any(), any()); + assertThat(postExecControl.isFinalizerRemoved()).isTrue(); + mockedReconcileUtils.verify(() -> ReconcileUtils.removeFinalizer(any()), times(1)); + } } @Test @@ -354,7 +283,7 @@ void doesNotRemovesTheSetFinalizerIfTheDeleteNotMethodInstructsIt() { reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); assertEquals(1, testCustomResource.getMetadata().getFinalizers().size()); - verify(customResourceFacade, never()).patchResource(any(), any()); + verify(customResourceFacade, never()).patchResource(any(), any(), any(), eq(true)); } @Test @@ -364,23 +293,26 @@ void doesNotUpdateTheResourceIfNoUpdateUpdateControlIfFinalizerSet() { reconciler.reconcile = (r, c) -> UpdateControl.noUpdate(); reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); - verify(customResourceFacade, never()).patchResource(any(), any()); - verify(customResourceFacade, never()).patchStatus(eq(testCustomResource), any()); + verify(customResourceFacade, never()).patchResource(any(), any(), any(), eq(true)); + verify(customResourceFacade, never()) + .patchStatus(any(), eq(testCustomResource), any(), eq(true)); } @Test void addsFinalizerIfNotMarkedForDeletionAndEmptyCustomResourceReturned() { - removeFinalizers(testCustomResource); - reconciler.reconcile = (r, c) -> UpdateControl.noUpdate(); - when(customResourceFacade.patchResourceWithSSA(any())).thenReturn(testCustomResource); - - var postExecControl = - reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); - - verify(customResourceFacade, times(1)) - .patchResourceWithSSA(argThat(a -> !a.getMetadata().getFinalizers().isEmpty())); - assertThat(postExecControl.updateIsStatusPatch()).isFalse(); - assertThat(postExecControl.getUpdatedCustomResource()).isPresent(); + try (MockedStatic mockedReconcileUtils = mockStatic(ReconcileUtils.class)) { + removeFinalizers(testCustomResource); + reconciler.reconcile = (r, c) -> UpdateControl.noUpdate(); + mockedReconcileUtils + .when(() -> ReconcileUtils.addFinalizerWithSSA(any())) + .thenReturn(testCustomResource); + var postExecControl = + reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); + + mockedReconcileUtils.verify(() -> ReconcileUtils.addFinalizerWithSSA(any()), times(1)); + assertThat(postExecControl.updateIsStatusPatch()).isFalse(); + assertThat(postExecControl.getUpdatedCustomResource()).isPresent(); + } } @Test @@ -390,7 +322,7 @@ void doesNotCallDeleteIfMarkedForDeletionButNotOurFinalizer() { reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); - verify(customResourceFacade, never()).patchResource(any(), any()); + verify(customResourceFacade, never()).patchResource(any(), any(), any(), eq(true)); verify(reconciler, never()).cleanup(eq(testCustomResource), any()); } @@ -471,7 +403,7 @@ void doesNotUpdatesObservedGenerationIfStatusIsNotPatchedWhenUsingSSA() throws E CustomResourceFacade facade = mock(CustomResourceFacade.class); when(config.isGenerationAware()).thenReturn(true); when(reconciler.reconcile(any(), any())).thenReturn(UpdateControl.noUpdate()); - when(facade.patchStatus(any(), any())).thenReturn(observedGenResource); + when(facade.patchStatus(any(), any(), any(), anyBoolean())).thenReturn(observedGenResource); var dispatcher = init(observedGenResource, reconciler, config, facade, true); PostExecutionControl control = @@ -489,12 +421,12 @@ void doesNotPatchObservedGenerationOnCustomResourcePatch() throws Exception { when(config.isGenerationAware()).thenReturn(true); when(reconciler.reconcile(any(), any())) .thenReturn(UpdateControl.patchResource(observedGenResource)); - when(facade.patchResource(any(), any())).thenReturn(observedGenResource); + when(facade.patchResource(any(), any(), any(), anyBoolean())).thenReturn(observedGenResource); var dispatcher = init(observedGenResource, reconciler, config, facade, false); dispatcher.handleExecution(executionScopeWithCREvent(observedGenResource)); - verify(facade, never()).patchStatus(any(), any()); + verify(facade, never()).patchStatus(any(), any(), any(), anyBoolean()); } @Test @@ -529,7 +461,8 @@ public boolean isLastAttempt() { false) .setResource(testCustomResource)); - verify(customResourceFacade, times(1)).patchStatus(eq(testCustomResource), any()); + verify(customResourceFacade, times(1)) + .patchStatus(any(), eq(testCustomResource), any(), eq(true)); verify(reconciler, times(1)).updateErrorStatus(eq(testCustomResource), any(), any()); } @@ -550,7 +483,8 @@ void callErrorStatusHandlerEvenOnFirstError() { var postExecControl = reconciliationDispatcher.handleExecution( new ExecutionScope(null, null, false, false).setResource(testCustomResource)); - verify(customResourceFacade, times(1)).patchStatus(eq(testCustomResource), any()); + verify(customResourceFacade, times(1)) + .patchStatus(any(), eq(testCustomResource), any(), eq(true)); verify(reconciler, times(1)).updateErrorStatus(eq(testCustomResource), any(), any()); assertThat(postExecControl.exceptionDuringExecution()).isTrue(); } @@ -573,7 +507,8 @@ void errorHandlerCanInstructNoRetryWithUpdate() { new ExecutionScope(null, null, false, false).setResource(testCustomResource)); verify(reconciler, times(1)).updateErrorStatus(eq(testCustomResource), any(), any()); - verify(customResourceFacade, times(1)).patchStatus(eq(testCustomResource), any()); + verify(customResourceFacade, times(1)) + .patchStatus(any(), eq(testCustomResource), any(), eq(true)); assertThat(postExecControl.exceptionDuringExecution()).isFalse(); } @@ -595,7 +530,8 @@ void errorHandlerCanInstructNoRetryNoUpdate() { new ExecutionScope(null, null, false, false).setResource(testCustomResource)); verify(reconciler, times(1)).updateErrorStatus(eq(testCustomResource), any(), any()); - verify(customResourceFacade, times(0)).patchStatus(eq(testCustomResource), any()); + verify(customResourceFacade, times(0)) + .patchStatus(any(), eq(testCustomResource), any(), eq(true)); assertThat(postExecControl.exceptionDuringExecution()).isFalse(); } @@ -611,7 +547,8 @@ void errorStatusHandlerCanPatchResource() { reconciliationDispatcher.handleExecution( new ExecutionScope(null, null, false, false).setResource(testCustomResource)); - verify(customResourceFacade, times(1)).patchStatus(eq(testCustomResource), any()); + verify(customResourceFacade, times(1)) + .patchStatus(any(), eq(testCustomResource), any(), eq(true)); verify(reconciler, times(1)).updateErrorStatus(eq(testCustomResource), any(), any()); } @@ -659,30 +596,6 @@ void canSkipSchedulingMaxDelayIf() { assertThat(control.getReScheduleDelay()).isNotPresent(); } - @Test - void retriesAddingFinalizerWithoutSSA() { - initConfigService(false); - reconciliationDispatcher = - init(testCustomResource, reconciler, null, customResourceFacade, true); - - removeFinalizers(testCustomResource); - reconciler.reconcile = (r, c) -> UpdateControl.noUpdate(); - when(customResourceFacade.patchResource(any(), any())) - .thenThrow(new KubernetesClientException(null, 409, null)) - .thenReturn(testCustomResource); - when(customResourceFacade.getResource(any(), any())) - .then( - (Answer) - invocationOnMock -> { - testCustomResource.getFinalizers().clear(); - return testCustomResource; - }); - - reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); - - verify(customResourceFacade, times(2)).patchResource(any(), any()); - } - @Test void reSchedulesFromErrorHandler() { var delay = 1000L; @@ -751,12 +664,6 @@ private ObservedGenCustomResource createObservedGenCustomResource() { return observedGenCustomResource; } - TestCustomResource createResourceWithFinalizer() { - var resourceWithFinalizer = TestUtils.testCustomResource(); - resourceWithFinalizer.addFinalizer(DEFAULT_FINALIZER); - return resourceWithFinalizer; - } - private void removeFinalizers(CustomResource customResource) { customResource.getMetadata().getFinalizers().clear(); } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerEventSourceTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerEventSourceTest.java index dcd10b4225..0cfb66711a 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerEventSourceTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerEventSourceTest.java @@ -22,7 +22,7 @@ import org.junit.jupiter.api.Test; import io.javaoperatorsdk.operator.MockKubernetesClient; -import io.javaoperatorsdk.operator.ReconcilerUtils; +import io.javaoperatorsdk.operator.ReconcilerUtilsInternal; import io.javaoperatorsdk.operator.TestUtils; import io.javaoperatorsdk.operator.api.config.BaseConfigurationService; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; @@ -46,7 +46,7 @@ class ControllerEventSourceTest extends AbstractEventSourceTestBase, EventHandler> { public static final String FINALIZER = - ReconcilerUtils.getDefaultFinalizerName(TestCustomResource.class); + ReconcilerUtilsInternal.getDefaultFinalizerName(TestCustomResource.class); private final TestController testController = new TestController(true); private final ControllerConfiguration controllerConfig = mock(ControllerConfiguration.class); @@ -68,10 +68,10 @@ void skipsEventHandlingIfGenerationNotIncreased() { TestCustomResource oldCustomResource = TestUtils.testCustomResource(); oldCustomResource.getMetadata().setFinalizers(List.of(FINALIZER)); - source.eventReceived(ResourceAction.UPDATED, customResource, oldCustomResource, null); + source.eventReceived(ResourceAction.UPDATED, customResource, oldCustomResource, null, false); verify(eventHandler, times(1)).handleEvent(any()); - source.eventReceived(ResourceAction.UPDATED, customResource, customResource, null); + source.eventReceived(ResourceAction.UPDATED, customResource, customResource, null, false); verify(eventHandler, times(1)).handleEvent(any()); } @@ -79,12 +79,12 @@ void skipsEventHandlingIfGenerationNotIncreased() { void dontSkipEventHandlingIfMarkedForDeletion() { TestCustomResource customResource1 = TestUtils.testCustomResource(); - source.eventReceived(ResourceAction.UPDATED, customResource1, customResource1, null); + source.eventReceived(ResourceAction.UPDATED, customResource1, customResource1, null, false); verify(eventHandler, times(1)).handleEvent(any()); // mark for deletion customResource1.getMetadata().setDeletionTimestamp(LocalDateTime.now().toString()); - source.eventReceived(ResourceAction.UPDATED, customResource1, customResource1, null); + source.eventReceived(ResourceAction.UPDATED, customResource1, customResource1, null, false); verify(eventHandler, times(2)).handleEvent(any()); } @@ -92,11 +92,11 @@ void dontSkipEventHandlingIfMarkedForDeletion() { void normalExecutionIfGenerationChanges() { TestCustomResource customResource1 = TestUtils.testCustomResource(); - source.eventReceived(ResourceAction.UPDATED, customResource1, customResource1, null); + source.eventReceived(ResourceAction.UPDATED, customResource1, customResource1, null, false); verify(eventHandler, times(1)).handleEvent(any()); customResource1.getMetadata().setGeneration(2L); - source.eventReceived(ResourceAction.UPDATED, customResource1, customResource1, null); + source.eventReceived(ResourceAction.UPDATED, customResource1, customResource1, null, false); verify(eventHandler, times(2)).handleEvent(any()); } @@ -107,10 +107,10 @@ void handlesAllEventIfNotGenerationAware() { TestCustomResource customResource1 = TestUtils.testCustomResource(); - source.eventReceived(ResourceAction.UPDATED, customResource1, customResource1, null); + source.eventReceived(ResourceAction.UPDATED, customResource1, customResource1, null, false); verify(eventHandler, times(1)).handleEvent(any()); - source.eventReceived(ResourceAction.UPDATED, customResource1, customResource1, null); + source.eventReceived(ResourceAction.UPDATED, customResource1, customResource1, null, false); verify(eventHandler, times(2)).handleEvent(any()); } @@ -118,7 +118,7 @@ void handlesAllEventIfNotGenerationAware() { void eventWithNoGenerationProcessedIfNoFinalizer() { TestCustomResource customResource1 = TestUtils.testCustomResource(); - source.eventReceived(ResourceAction.UPDATED, customResource1, customResource1, null); + source.eventReceived(ResourceAction.UPDATED, customResource1, customResource1, null, false); verify(eventHandler, times(1)).handleEvent(any()); } @@ -127,7 +127,7 @@ void eventWithNoGenerationProcessedIfNoFinalizer() { void callsBroadcastsOnResourceEvents() { TestCustomResource customResource1 = TestUtils.testCustomResource(); - source.eventReceived(ResourceAction.UPDATED, customResource1, customResource1, null); + source.eventReceived(ResourceAction.UPDATED, customResource1, customResource1, null, false); verify(testController.getEventSourceManager(), times(1)) .broadcastOnResourceEvent( @@ -143,8 +143,8 @@ void filtersOutEventsOnAddAndUpdate() { source = new ControllerEventSource<>(new TestController(onAddFilter, onUpdatePredicate, null)); setUpSource(source, true, controllerConfig); - source.eventReceived(ResourceAction.ADDED, cr, null, null); - source.eventReceived(ResourceAction.UPDATED, cr, cr, null); + source.eventReceived(ResourceAction.ADDED, cr, null, null, false); + source.eventReceived(ResourceAction.UPDATED, cr, cr, null, false); verify(eventHandler, never()).handleEvent(any()); } @@ -156,9 +156,9 @@ void genericFilterFiltersOutAddUpdateAndDeleteEvents() { source = new ControllerEventSource<>(new TestController(null, null, res -> false)); setUpSource(source, true, controllerConfig); - source.eventReceived(ResourceAction.ADDED, cr, null, null); - source.eventReceived(ResourceAction.UPDATED, cr, cr, null); - source.eventReceived(ResourceAction.DELETED, cr, cr, true); + source.eventReceived(ResourceAction.ADDED, cr, null, null, false); + source.eventReceived(ResourceAction.UPDATED, cr, cr, null, false); + source.eventReceived(ResourceAction.DELETED, cr, cr, true, false); verify(eventHandler, never()).handleEvent(any()); } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryPrimaryResourceCacheTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryPrimaryResourceCacheTest.java index 4b12148015..c665cf8396 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryPrimaryResourceCacheTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryPrimaryResourceCacheTest.java @@ -122,7 +122,7 @@ void nonComparableResourceVersionsDisables() { void lockedEventBeforePut() throws Exception { var testResource = testResource(); - temporaryResourceCache.startModifying(ResourceID.fromResource(testResource)); + temporaryResourceCache.startEventFilteringModify(ResourceID.fromResource(testResource)); ExecutorService ex = Executors.newSingleThreadExecutor(); try { @@ -130,7 +130,7 @@ void lockedEventBeforePut() throws Exception { temporaryResourceCache.putResource(testResource); assertThat(result.isDone()).isFalse(); - temporaryResourceCache.doneModifying(ResourceID.fromResource(testResource)); + temporaryResourceCache.doneEventFilterModify(ResourceID.fromResource(testResource)); assertThat(result.get(10, TimeUnit.SECONDS)).isTrue(); } finally { ex.shutdownNow(); @@ -149,7 +149,28 @@ void putBeforeEvent() { nextResource.getMetadata().setResourceVersion("3"); temporaryResourceCache.putResource(nextResource); - // now expect an event with the matching resourceVersion to be known after the put + // the result is false since the put was not part of event filtering update + result = temporaryResourceCache.onAddOrUpdateEvent(nextResource); + assertThat(result).isFalse(); + } + + @Test + void putBeforeEventWithEventFiltering() { + var testResource = testResource(); + + // first ensure an event is not known + var result = temporaryResourceCache.onAddOrUpdateEvent(testResource); + assertThat(result).isFalse(); + + var nextResource = testResource(); + nextResource.getMetadata().setResourceVersion("3"); + var resourceId = ResourceID.fromResource(testResource); + + temporaryResourceCache.startEventFilteringModify(resourceId); + temporaryResourceCache.putResource(nextResource); + temporaryResourceCache.doneEventFilterModify(resourceId); + + // the result is false since the put was not part of event filtering update result = temporaryResourceCache.onAddOrUpdateEvent(nextResource); assertThat(result).isTrue(); } diff --git a/operator-framework-junit/src/main/java/io/javaoperatorsdk/operator/junit/LocallyRunOperatorExtension.java b/operator-framework-junit/src/main/java/io/javaoperatorsdk/operator/junit/LocallyRunOperatorExtension.java index 0a33293b53..1faa545f54 100644 --- a/operator-framework-junit/src/main/java/io/javaoperatorsdk/operator/junit/LocallyRunOperatorExtension.java +++ b/operator-framework-junit/src/main/java/io/javaoperatorsdk/operator/junit/LocallyRunOperatorExtension.java @@ -44,7 +44,7 @@ import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.LocalPortForward; import io.javaoperatorsdk.operator.Operator; -import io.javaoperatorsdk.operator.ReconcilerUtils; +import io.javaoperatorsdk.operator.ReconcilerUtilsInternal; import io.javaoperatorsdk.operator.RegisteredController; import io.javaoperatorsdk.operator.api.config.ConfigurationServiceOverrider; import io.javaoperatorsdk.operator.api.config.ControllerConfigurationOverrider; @@ -140,7 +140,7 @@ public static Builder builder() { } public static void applyCrd(Class resourceClass, KubernetesClient client) { - applyCrd(ReconcilerUtils.getResourceTypeName(resourceClass), client); + applyCrd(ReconcilerUtilsInternal.getResourceTypeName(resourceClass), client); } /** @@ -192,7 +192,7 @@ private static void applyCrd(String crdString, String path, KubernetesClient cli * @param crClass the custom resource class for which we want to apply the CRD */ public void applyCrd(Class crClass) { - applyCrd(ReconcilerUtils.getResourceTypeName(crClass)); + applyCrd(ReconcilerUtilsInternal.getResourceTypeName(crClass)); } /** @@ -203,7 +203,7 @@ public void applyCrd(Class crClass) { * * @param resourceTypeName the resource type name associated with the CRD to be applied, * typically, given a resource type, its name would be obtained using {@link - * ReconcilerUtils#getResourceTypeName(Class)} + * ReconcilerUtilsInternal#getResourceTypeName(Class)} */ public void applyCrd(String resourceTypeName) { // first attempt to use a manually defined CRD @@ -296,7 +296,7 @@ protected void before(ExtensionContext context) { ref.controllerConfigurationOverrider.accept(oconfig); } - final var resourceTypeName = ReconcilerUtils.getResourceTypeName(resourceClass); + final var resourceTypeName = ReconcilerUtilsInternal.getResourceTypeName(resourceClass); // only try to apply a CRD for the reconciler if it is associated to a CR if (CustomResource.class.isAssignableFrom(resourceClass)) { applyCrd(resourceTypeName); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/LeaderElectionPermissionIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/LeaderElectionPermissionIT.java index db99324ae2..457de54ca3 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/LeaderElectionPermissionIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/LeaderElectionPermissionIT.java @@ -26,7 +26,7 @@ import io.javaoperatorsdk.annotation.Sample; import io.javaoperatorsdk.operator.Operator; import io.javaoperatorsdk.operator.OperatorException; -import io.javaoperatorsdk.operator.ReconcilerUtils; +import io.javaoperatorsdk.operator.ReconcilerUtilsInternal; import io.javaoperatorsdk.operator.api.config.LeaderElectionConfiguration; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; @@ -87,14 +87,14 @@ public UpdateControl reconcile(ConfigMap resource, Context private void applyRoleBinding() { var clusterRoleBinding = - ReconcilerUtils.loadYaml( + ReconcilerUtilsInternal.loadYaml( RoleBinding.class, this.getClass(), "leader-elector-stop-noaccess-role-binding.yaml"); adminClient.resource(clusterRoleBinding).createOrReplace(); } private void applyRole() { var role = - ReconcilerUtils.loadYaml( + ReconcilerUtilsInternal.loadYaml( Role.class, this.getClass(), "leader-elector-stop-role-noaccess.yaml"); adminClient.resource(role).createOrReplace(); } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/builtinresourcecleaner/BuiltInResourceCleanerIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/builtinresourcecleaner/BuiltInResourceCleanerIT.java index 9667c22486..18e076e2bf 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/builtinresourcecleaner/BuiltInResourceCleanerIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/builtinresourcecleaner/BuiltInResourceCleanerIT.java @@ -24,7 +24,7 @@ import io.fabric8.kubernetes.api.model.Service; import io.javaoperatorsdk.annotation.Sample; -import io.javaoperatorsdk.operator.ReconcilerUtils; +import io.javaoperatorsdk.operator.ReconcilerUtilsInternal; import io.javaoperatorsdk.operator.dependent.standalonedependent.StandaloneDependentResourceIT; import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; @@ -85,7 +85,7 @@ void cleanerIsCalledOnBuiltInResource() { Service testService() { Service service = - ReconcilerUtils.loadYaml( + ReconcilerUtilsInternal.loadYaml( Service.class, StandaloneDependentResourceIT.class, "/io/javaoperatorsdk/operator/service-template.yaml"); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/filterpatchevent/FilterPatchEventIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/filterpatchevent/FilterPatchEventIT.java new file mode 100644 index 0000000000..6f27925e21 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/filterpatchevent/FilterPatchEventIT.java @@ -0,0 +1,108 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.baseapi.filterpatchevent; + +import java.time.Duration; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.fabric8.kubernetes.api.model.ObjectMeta; +import io.javaoperatorsdk.annotation.Sample; +import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + +@Sample( + tldr = "Controlling patch event filtering in UpdateControl", + description = + """ + Demonstrates how to use the filterPatchEvent parameter in UpdateControl to control \ + whether patch operations trigger subsequent reconciliation events. When filterPatchEvent \ + is true (default), patch events are filtered out to prevent reconciliation loops. When \ + false, patch events trigger reconciliation, allowing for controlled event propagation. + """) +class FilterPatchEventIT { + + public static final int POLL_DELAY = 150; + public static final String NAME = "test1"; + public static final String UPDATED = "updated"; + + FilterPatchEventTestReconciler reconciler = new FilterPatchEventTestReconciler(); + + @RegisterExtension + LocallyRunOperatorExtension extension = + LocallyRunOperatorExtension.builder().withReconciler(reconciler).build(); + + @Test + void patchEventFilteredWhenFlagIsTrue() { + reconciler.setFilterPatchEvent(true); + var resource = createTestResource(); + extension.create(resource); + + // Wait for the reconciliation to complete and the resource to be updated + await() + .pollDelay(Duration.ofMillis(POLL_DELAY)) + .untilAsserted( + () -> { + var updated = extension.get(FilterPatchEventTestCustomResource.class, NAME); + assertThat(updated.getStatus()).isNotNull(); + assertThat(updated.getStatus().getValue()).isEqualTo(UPDATED); + }); + + // With filterPatchEvent=true, reconciliation should only run once + // (triggered by the initial create, but not by the patch operation) + int executions = reconciler.getNumberOfExecutions(); + assertThat(executions).isEqualTo(1); + } + + @Test + void patchEventNotFilteredWhenFlagIsFalse() { + reconciler.setFilterPatchEvent(false); + var resource = createTestResource(); + extension.create(resource); + + // Wait for the reconciliation to complete and the resource to be updated + await() + .atMost(Duration.ofSeconds(5)) + .untilAsserted( + () -> { + var updated = extension.get(FilterPatchEventTestCustomResource.class, NAME); + assertThat(updated.getStatus()).isNotNull(); + assertThat(updated.getStatus().getValue()).isEqualTo(UPDATED); + }); + + // Wait for potential additional reconciliations + await() + .pollDelay(Duration.ofMillis(POLL_DELAY)) + .atMost(Duration.ofSeconds(5)) + .untilAsserted( + () -> { + int executions = reconciler.getNumberOfExecutions(); + // With filterPatchEvent=false, reconciliation should run at least twice + // (once for create and at least once for the patch event) + assertThat(executions).isGreaterThanOrEqualTo(2); + }); + } + + private FilterPatchEventTestCustomResource createTestResource() { + FilterPatchEventTestCustomResource resource = new FilterPatchEventTestCustomResource(); + resource.setMetadata(new ObjectMeta()); + resource.getMetadata().setName(NAME); + return resource; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/filterpatchevent/FilterPatchEventTestCustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/filterpatchevent/FilterPatchEventTestCustomResource.java new file mode 100644 index 0000000000..7f8b4838de --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/filterpatchevent/FilterPatchEventTestCustomResource.java @@ -0,0 +1,28 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.baseapi.filterpatchevent; + +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("fpe") +public class FilterPatchEventTestCustomResource + extends CustomResource implements Namespaced {} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/filterpatchevent/FilterPatchEventTestCustomResourceStatus.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/filterpatchevent/FilterPatchEventTestCustomResourceStatus.java new file mode 100644 index 0000000000..1c7aeafadd --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/filterpatchevent/FilterPatchEventTestCustomResourceStatus.java @@ -0,0 +1,30 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.baseapi.filterpatchevent; + +public class FilterPatchEventTestCustomResourceStatus { + + private String value; + + public String getValue() { + return value; + } + + public FilterPatchEventTestCustomResourceStatus setValue(String value) { + this.value = value; + return this; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/filterpatchevent/FilterPatchEventTestReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/filterpatchevent/FilterPatchEventTestReconciler.java new file mode 100644 index 0000000000..3af28f5aa3 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/filterpatchevent/FilterPatchEventTestReconciler.java @@ -0,0 +1,55 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.baseapi.filterpatchevent; + +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; + +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.Reconciler; +import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; + +import static io.javaoperatorsdk.operator.baseapi.filterpatchevent.FilterPatchEventIT.UPDATED; + +@ControllerConfiguration(generationAwareEventProcessing = false) +public class FilterPatchEventTestReconciler + implements Reconciler { + + private final AtomicInteger numberOfExecutions = new AtomicInteger(0); + private final AtomicBoolean filterPatchEvent = new AtomicBoolean(false); + + @Override + public UpdateControl reconcile( + FilterPatchEventTestCustomResource resource, + Context context) { + numberOfExecutions.incrementAndGet(); + + // Update the spec value to trigger a patch operation + resource.setStatus(new FilterPatchEventTestCustomResourceStatus()); + resource.getStatus().setValue(UPDATED); + + return UpdateControl.patchStatus(resource).filterPatchEvent(filterPatchEvent.get()); + } + + public int getNumberOfExecutions() { + return numberOfExecutions.get(); + } + + public void setFilterPatchEvent(boolean b) { + filterPatchEvent.set(b); + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/infrastructureclient/InfrastructureClientIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/infrastructureclient/InfrastructureClientIT.java index 59faaae90b..eb39fa0657 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/infrastructureclient/InfrastructureClientIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/infrastructureclient/InfrastructureClientIT.java @@ -28,7 +28,7 @@ import io.fabric8.kubernetes.client.ConfigBuilder; import io.fabric8.kubernetes.client.KubernetesClientBuilder; import io.fabric8.kubernetes.client.KubernetesClientException; -import io.javaoperatorsdk.operator.ReconcilerUtils; +import io.javaoperatorsdk.operator.ReconcilerUtilsInternal; import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; import static org.assertj.core.api.Assertions.assertThat; @@ -127,23 +127,25 @@ void shouldNotAccessNotPermittedResources() { private void applyClusterRoleBinding(String filename) { var clusterRoleBinding = - ReconcilerUtils.loadYaml(ClusterRoleBinding.class, this.getClass(), filename); + ReconcilerUtilsInternal.loadYaml(ClusterRoleBinding.class, this.getClass(), filename); operator.getInfrastructureKubernetesClient().resource(clusterRoleBinding).serverSideApply(); } private void applyClusterRole(String filename) { - var clusterRole = ReconcilerUtils.loadYaml(ClusterRole.class, this.getClass(), filename); + var clusterRole = + ReconcilerUtilsInternal.loadYaml(ClusterRole.class, this.getClass(), filename); operator.getInfrastructureKubernetesClient().resource(clusterRole).serverSideApply(); } private void removeClusterRoleBinding(String filename) { var clusterRoleBinding = - ReconcilerUtils.loadYaml(ClusterRoleBinding.class, this.getClass(), filename); + ReconcilerUtilsInternal.loadYaml(ClusterRoleBinding.class, this.getClass(), filename); operator.getInfrastructureKubernetesClient().resource(clusterRoleBinding).delete(); } private void removeClusterRole(String filename) { - var clusterRole = ReconcilerUtils.loadYaml(ClusterRole.class, this.getClass(), filename); + var clusterRole = + ReconcilerUtilsInternal.loadYaml(ClusterRole.class, this.getClass(), filename); operator.getInfrastructureKubernetesClient().resource(clusterRole).delete(); } } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/patchresourceandstatusnossa/PatchResourceAndStatusNoSSAReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/patchresourceandstatusnossa/PatchResourceAndStatusNoSSAReconciler.java index e091896597..eb19f9e249 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/patchresourceandstatusnossa/PatchResourceAndStatusNoSSAReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/patchresourceandstatusnossa/PatchResourceAndStatusNoSSAReconciler.java @@ -45,7 +45,7 @@ public UpdateControl reconcile( Context context) { numberOfExecutions.addAndGet(1); - log.info("Value: " + resource.getSpec().getValue()); + log.info("Value: {}", resource.getSpec().getValue()); if (removeAnnotation) { resource.getMetadata().getAnnotations().remove(TEST_ANNOTATION); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/patchresourcewithssa/PatchResourceWithSSAReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/patchresourcewithssa/PatchResourceWithSSAReconciler.java index c241c4cd4f..78ece627dd 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/patchresourcewithssa/PatchResourceWithSSAReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/patchresourcewithssa/PatchResourceWithSSAReconciler.java @@ -41,7 +41,8 @@ public UpdateControl reconcile( if (resource.getSpec().getControllerManagedValue() == null) { res.setSpec(new PatchResourceWithSSASpec()); res.getSpec().setControllerManagedValue(ADDED_VALUE); - return UpdateControl.patchResource(res); + // test assumes we will run this in the next reconciliation + return UpdateControl.patchResource(res).filterPatchEvent(false); } else { res.setStatus(new PatchResourceWithSSAStatus()); res.getStatus().setSuccessfullyReconciled(true); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/patchresourcewithssa/PatchWithSSAITBase.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/patchresourcewithssa/PatchWithSSAITBase.java index 9f2ca81543..2a8314ecb9 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/patchresourcewithssa/PatchWithSSAITBase.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/patchresourcewithssa/PatchWithSSAITBase.java @@ -49,6 +49,7 @@ void reconcilerPatchesResourceWithSSA() { .isEqualTo(PatchResourceWithSSAReconciler.ADDED_VALUE); // finalizer is added to the SSA patch in the background by the framework assertThat(actualResource.getMetadata().getFinalizers()).isNotEmpty(); + assertThat(actualResource.getStatus()).isNotNull(); assertThat(actualResource.getStatus().isSuccessfullyReconciled()).isTrue(); // one for resource, one for subresource assertThat( diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/simple/ReconcilerExecutorIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/simple/ReconcilerExecutorIT.java index b6790e4085..54d639c05a 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/simple/ReconcilerExecutorIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/simple/ReconcilerExecutorIT.java @@ -52,7 +52,7 @@ void configMapGetsCreatedForTestCustomResource() { awaitResourcesCreatedOrUpdated(); awaitStatusUpdated(); - assertThat(TestUtils.getNumberOfExecutions(operator)).isEqualTo(2); + assertThat(TestUtils.getNumberOfExecutions(operator)).isEqualTo(1); } @Test diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/simple/TestReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/simple/TestReconciler.java index b614b97f3a..49dbe80554 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/simple/TestReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/simple/TestReconciler.java @@ -16,6 +16,7 @@ package io.javaoperatorsdk.operator.baseapi.simple; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicInteger; @@ -25,8 +26,11 @@ import io.fabric8.kubernetes.api.model.ConfigMap; import io.fabric8.kubernetes.api.model.ConfigMapBuilder; import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; -import io.javaoperatorsdk.operator.ReconcilerUtils; +import io.javaoperatorsdk.operator.ReconcilerUtilsInternal; +import io.javaoperatorsdk.operator.api.config.informer.InformerEventSourceConfiguration; import io.javaoperatorsdk.operator.api.reconciler.*; +import io.javaoperatorsdk.operator.processing.event.source.EventSource; +import io.javaoperatorsdk.operator.processing.event.source.informer.InformerEventSource; import io.javaoperatorsdk.operator.support.TestExecutionInfoProvider; @ControllerConfiguration(generationAwareEventProcessing = false) @@ -38,7 +42,7 @@ public class TestReconciler private static final Logger log = LoggerFactory.getLogger(TestReconciler.class); public static final String FINALIZER_NAME = - ReconcilerUtils.getDefaultFinalizerName(TestCustomResource.class); + ReconcilerUtilsInternal.getDefaultFinalizerName(TestCustomResource.class); private final AtomicInteger numberOfExecutions = new AtomicInteger(0); private final AtomicInteger numberOfCleanupExecutions = new AtomicInteger(0); @@ -52,32 +56,6 @@ public void setUpdateStatus(boolean updateStatus) { this.updateStatus = updateStatus; } - @Override - public DeleteControl cleanup(TestCustomResource resource, Context context) { - numberOfCleanupExecutions.incrementAndGet(); - - var statusDetail = - context - .getClient() - .configMaps() - .inNamespace(resource.getMetadata().getNamespace()) - .withName(resource.getSpec().getConfigMapName()) - .delete(); - - if (statusDetail.size() == 1 && statusDetail.get(0).getCauses().isEmpty()) { - log.info( - "Deleted ConfigMap {} for resource: {}", - resource.getSpec().getConfigMapName(), - resource.getMetadata().getName()); - } else { - log.error( - "Failed to delete ConfigMap {} for resource: {}", - resource.getSpec().getConfigMapName(), - resource.getMetadata().getName()); - } - return DeleteControl.defaultDelete(); - } - @Override public UpdateControl reconcile( TestCustomResource resource, Context context) { @@ -85,22 +63,13 @@ public UpdateControl reconcile( if (!resource.getMetadata().getFinalizers().contains(FINALIZER_NAME)) { throw new IllegalStateException("Finalizer is not present."); } - final var kubernetesClient = context.getClient(); - ConfigMap existingConfigMap = - kubernetesClient - .configMaps() - .inNamespace(resource.getMetadata().getNamespace()) - .withName(resource.getSpec().getConfigMapName()) - .get(); + + var existingConfigMap = context.getSecondaryResource(ConfigMap.class).orElse(null); if (existingConfigMap != null) { existingConfigMap.setData(configMapData(resource)); - // existingConfigMap.getMetadata().setResourceVersion(null); - kubernetesClient - .configMaps() - .inNamespace(resource.getMetadata().getNamespace()) - .resource(existingConfigMap) - .createOrReplace(); + log.info("Updating config map"); + ReconcileUtils.serverSideApply(context, existingConfigMap); } else { Map labels = new HashMap<>(); labels.put("managedBy", TestReconciler.class.getSimpleName()); @@ -114,11 +83,8 @@ public UpdateControl reconcile( .build()) .withData(configMapData(resource)) .build(); - kubernetesClient - .configMaps() - .inNamespace(resource.getMetadata().getNamespace()) - .resource(newConfigMap) - .createOrReplace(); + log.info("Creating config map"); + ReconcileUtils.serverSideApply(context, newConfigMap); } if (updateStatus) { var statusUpdateResource = new TestCustomResource(); @@ -129,11 +95,49 @@ public UpdateControl reconcile( .build()); resource.setStatus(new TestCustomResourceStatus()); resource.getStatus().setConfigMapStatus("ConfigMap Ready"); + log.info("Patching status"); return UpdateControl.patchStatus(resource); } return UpdateControl.noUpdate(); } + @Override + public DeleteControl cleanup(TestCustomResource resource, Context context) { + numberOfCleanupExecutions.incrementAndGet(); + + var statusDetail = + context + .getClient() + .configMaps() + .inNamespace(resource.getMetadata().getNamespace()) + .withName(resource.getSpec().getConfigMapName()) + .delete(); + + if (statusDetail.size() == 1 && statusDetail.get(0).getCauses().isEmpty()) { + log.info( + "Deleted ConfigMap {} for resource: {}", + resource.getSpec().getConfigMapName(), + resource.getMetadata().getName()); + } else { + log.error( + "Failed to delete ConfigMap {} for resource: {}", + resource.getSpec().getConfigMapName(), + resource.getMetadata().getName()); + } + return DeleteControl.defaultDelete(); + } + + @Override + public List> prepareEventSources( + EventSourceContext context) { + InformerEventSource es = + new InformerEventSource<>( + InformerEventSourceConfiguration.from(ConfigMap.class, TestCustomResource.class) + .build(), + context); + return List.of(es); + } + private Map configMapData(TestCustomResource resource) { Map data = new HashMap<>(); data.put(resource.getSpec().getKey(), resource.getSpec().getValue()); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/ssaissue/specupdate/SSASpecUpdateReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/ssaissue/specupdate/SSASpecUpdateReconciler.java index c1dca492ca..ccdbfdd181 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/ssaissue/specupdate/SSASpecUpdateReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/ssaissue/specupdate/SSASpecUpdateReconciler.java @@ -15,6 +15,9 @@ */ package io.javaoperatorsdk.operator.baseapi.ssaissue.specupdate; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; import io.javaoperatorsdk.operator.api.reconciler.Cleaner; import io.javaoperatorsdk.operator.api.reconciler.Context; @@ -27,18 +30,21 @@ public class SSASpecUpdateReconciler implements Reconciler, Cleaner { + private static final Logger log = LoggerFactory.getLogger(SSASpecUpdateReconciler.class); + @Override public UpdateControl reconcile( SSASpecUpdateCustomResource resource, Context context) { var copy = createFreshCopy(resource); copy.getSpec().setValue("value"); - context - .getClient() - .resource(copy) - .fieldManager(context.getControllerConfiguration().fieldManager()) - .serverSideApply(); - + var res = + context + .getClient() + .resource(copy) + .fieldManager(context.getControllerConfiguration().fieldManager()) + .serverSideApply(); + log.info("res: {}", res); return UpdateControl.noUpdate(); } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/subresource/SubResourceUpdateIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/subresource/SubResourceUpdateIT.java index 1ea9ca96ce..a86220439c 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/subresource/SubResourceUpdateIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/subresource/SubResourceUpdateIT.java @@ -59,7 +59,7 @@ void updatesSubResourceStatus() { // wait for sure, there are no more events waitXms(WAIT_AFTER_EXECUTION); // there is no event on status update processed - assertThat(TestUtils.getNumberOfExecutions(operator)).isEqualTo(2); + assertThat(TestUtils.getNumberOfExecutions(operator)).isEqualTo(1); } @Test @@ -73,7 +73,7 @@ void updatesSubResourceStatusNoFinalizer() { // wait for sure, there are no more events waitXms(WAIT_AFTER_EXECUTION); // there is no event on status update processed - assertThat(TestUtils.getNumberOfExecutions(operator)).isEqualTo(2); + assertThat(TestUtils.getNumberOfExecutions(operator)).isEqualTo(1); } /** Note that we check on controller impl if there is finalizer on execution. */ @@ -87,7 +87,7 @@ void ifNoFinalizerPresentFirstAddsTheFinalizerThenExecutesControllerAgain() { // wait for sure, there are no more events waitXms(WAIT_AFTER_EXECUTION); // there is no event on status update processed - assertThat(TestUtils.getNumberOfExecutions(operator)).isEqualTo(2); + assertThat(TestUtils.getNumberOfExecutions(operator)).isEqualTo(1); } /** diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/triggerallevent/eventing/TriggerReconcilerOnAllEventReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/triggerallevent/eventing/TriggerReconcilerOnAllEventReconciler.java index 0b8c0ff1e6..2217662402 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/triggerallevent/eventing/TriggerReconcilerOnAllEventReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/triggerallevent/eventing/TriggerReconcilerOnAllEventReconciler.java @@ -22,7 +22,7 @@ import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; -import io.javaoperatorsdk.operator.api.reconciler.PrimaryUpdateAndCacheUtils; +import io.javaoperatorsdk.operator.api.reconciler.ReconcileUtils; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; @@ -75,7 +75,7 @@ public UpdateControl reconcile( if (!primary.isMarkedForDeletion() && getUseFinalizer() && !primary.hasFinalizer(FINALIZER)) { log.info("Adding finalizer"); - PrimaryUpdateAndCacheUtils.addFinalizer(context, FINALIZER); + ReconcileUtils.addFinalizer(context, FINALIZER); return UpdateControl.noUpdate(); } @@ -98,7 +98,7 @@ public UpdateControl reconcile( setEventOnMarkedForDeletion(true); if (getUseFinalizer() && primary.hasFinalizer(FINALIZER)) { log.info("Removing finalizer"); - PrimaryUpdateAndCacheUtils.removeFinalizer(context, FINALIZER); + ReconcileUtils.removeFinalizer(context, FINALIZER); } } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/triggerallevent/finalizerhandling/SelectiveFinalizerHandlingReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/triggerallevent/finalizerhandling/SelectiveFinalizerHandlingReconciler.java index 9b3cd5683f..f9198d0eae 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/triggerallevent/finalizerhandling/SelectiveFinalizerHandlingReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/triggerallevent/finalizerhandling/SelectiveFinalizerHandlingReconciler.java @@ -17,7 +17,7 @@ import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; -import io.javaoperatorsdk.operator.api.reconciler.PrimaryUpdateAndCacheUtils; +import io.javaoperatorsdk.operator.api.reconciler.ReconcileUtils; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; @@ -37,11 +37,11 @@ public UpdateControl reconci } if (resource.getSpec().getUseFinalizer()) { - PrimaryUpdateAndCacheUtils.addFinalizer(context, FINALIZER); + ReconcileUtils.addFinalizer(context, FINALIZER); } if (resource.isMarkedForDeletion()) { - PrimaryUpdateAndCacheUtils.removeFinalizer(context, FINALIZER); + ReconcileUtils.removeFinalizer(context, FINALIZER); } return UpdateControl.noUpdate(); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/BaseConfigurationServiceTest.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/BaseConfigurationServiceTest.java index 370f09509f..ffd0f6b904 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/BaseConfigurationServiceTest.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/BaseConfigurationServiceTest.java @@ -29,7 +29,7 @@ import io.fabric8.kubernetes.api.model.ConfigMap; import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.api.model.Service; -import io.javaoperatorsdk.operator.ReconcilerUtils; +import io.javaoperatorsdk.operator.ReconcilerUtilsInternal; import io.javaoperatorsdk.operator.api.config.AnnotationConfigurable; import io.javaoperatorsdk.operator.api.config.BaseConfigurationService; import io.javaoperatorsdk.operator.api.config.dependent.ConfigurationConverter; @@ -133,13 +133,13 @@ void missingAnnotationCreatesDefaultConfig() { final var reconciler = new MissingAnnotationReconciler(); var config = configFor(reconciler); - assertThat(config.getName()).isEqualTo(ReconcilerUtils.getNameFor(reconciler)); + assertThat(config.getName()).isEqualTo(ReconcilerUtilsInternal.getNameFor(reconciler)); assertThat(config.getRetry()).isInstanceOf(GenericRetry.class); assertThat(config.getRateLimiter()).isInstanceOf(LinearRateLimiter.class); assertThat(config.maxReconciliationInterval()).hasValue(Duration.ofHours(DEFAULT_INTERVAL)); assertThat(config.fieldManager()).isEqualTo(config.getName()); assertThat(config.getFinalizerName()) - .isEqualTo(ReconcilerUtils.getDefaultFinalizerName(config.getResourceClass())); + .isEqualTo(ReconcilerUtilsInternal.getDefaultFinalizerName(config.getResourceClass())); final var informerConfig = config.getInformerConfig(); assertThat(informerConfig.getLabelSelector()).isNull(); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/DefaultConfigurationServiceTest.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/DefaultConfigurationServiceTest.java index 1b328ccaf9..fa31575b9e 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/DefaultConfigurationServiceTest.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/DefaultConfigurationServiceTest.java @@ -20,7 +20,7 @@ import io.fabric8.kubernetes.client.CustomResource; import io.fabric8.kubernetes.model.annotation.Group; import io.fabric8.kubernetes.model.annotation.Version; -import io.javaoperatorsdk.operator.ReconcilerUtils; +import io.javaoperatorsdk.operator.ReconcilerUtilsInternal; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; @@ -40,7 +40,7 @@ void returnsValuesFromControllerAnnotationFinalizer() { assertEquals( CustomResource.getCRDName(TestCustomResource.class), configuration.getResourceTypeName()); assertEquals( - ReconcilerUtils.getDefaultFinalizerName(TestCustomResource.class), + ReconcilerUtilsInternal.getDefaultFinalizerName(TestCustomResource.class), configuration.getFinalizerName()); assertEquals(TestCustomResource.class, configuration.getResourceClass()); assertFalse(configuration.isGenerationAware()); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/externalstate/ExternalStateReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/externalstate/ExternalStateReconciler.java index 89d1dee94b..5a9d9a7f06 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/externalstate/ExternalStateReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/externalstate/ExternalStateReconciler.java @@ -109,10 +109,8 @@ private void createExternalResource( // Making sure that the created resources are in the cache for the next reconciliation. // This is critical in this case, since on next reconciliation if it would not be in the cache // it would be created again. - configMapEventSource.updateAndCacheResource( - configMap, - context, - toCreate -> context.getClient().configMaps().resource(toCreate).create()); + configMapEventSource.eventFilteringUpdateAndCacheResource( + configMap, toCreate -> context.getClient().configMaps().resource(toCreate).create()); externalResourceEventSource.handleRecentResourceCreate(primaryID, createdResource); } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/informerrelatedbehavior/InformerRelatedBehaviorITS.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/informerrelatedbehavior/InformerRelatedBehaviorITS.java index 221d7363a3..ce98af58e0 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/informerrelatedbehavior/InformerRelatedBehaviorITS.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/informerrelatedbehavior/InformerRelatedBehaviorITS.java @@ -34,7 +34,7 @@ import io.fabric8.kubernetes.client.utils.KubernetesResourceUtil; import io.javaoperatorsdk.operator.Operator; import io.javaoperatorsdk.operator.OperatorException; -import io.javaoperatorsdk.operator.ReconcilerUtils; +import io.javaoperatorsdk.operator.ReconcilerUtilsInternal; import io.javaoperatorsdk.operator.health.InformerHealthIndicator; import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; import io.javaoperatorsdk.operator.processing.event.source.controller.ControllerEventSource; @@ -399,23 +399,25 @@ private void setFullResourcesAccess() { private void addRoleBindingsToTestNamespaces() { var role = - ReconcilerUtils.loadYaml(Role.class, this.getClass(), "rbac-test-only-main-ns-access.yaml"); + ReconcilerUtilsInternal.loadYaml( + Role.class, this.getClass(), "rbac-test-only-main-ns-access.yaml"); adminClient.resource(role).inNamespace(actualNamespace).createOrReplace(); var roleBinding = - ReconcilerUtils.loadYaml( + ReconcilerUtilsInternal.loadYaml( RoleBinding.class, this.getClass(), "rbac-test-only-main-ns-access-binding.yaml"); adminClient.resource(roleBinding).inNamespace(actualNamespace).createOrReplace(); } private void applyClusterRoleBinding() { var clusterRoleBinding = - ReconcilerUtils.loadYaml( + ReconcilerUtilsInternal.loadYaml( ClusterRoleBinding.class, this.getClass(), "rbac-test-role-binding.yaml"); adminClient.resource(clusterRoleBinding).createOrReplace(); } private void applyClusterRole(String filename) { - var clusterRole = ReconcilerUtils.loadYaml(ClusterRole.class, this.getClass(), filename); + var clusterRole = + ReconcilerUtilsInternal.loadYaml(ClusterRole.class, this.getClass(), filename); adminClient.resource(clusterRole).createOrReplace(); } @@ -431,7 +433,7 @@ private Namespace namespace(String name) { private void removeClusterRoleBinding() { var clusterRoleBinding = - ReconcilerUtils.loadYaml( + ReconcilerUtilsInternal.loadYaml( ClusterRoleBinding.class, this.getClass(), "rbac-test-role-binding.yaml"); adminClient.resource(clusterRoleBinding).delete(); } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/servicestrictmatcher/ServiceDependentResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/servicestrictmatcher/ServiceDependentResource.java index 1bb34de16c..fb243251f3 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/servicestrictmatcher/ServiceDependentResource.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/servicestrictmatcher/ServiceDependentResource.java @@ -26,7 +26,7 @@ import io.javaoperatorsdk.operator.processing.dependent.kubernetes.GenericKubernetesResourceMatcher; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependent; -import static io.javaoperatorsdk.operator.ReconcilerUtils.loadYaml; +import static io.javaoperatorsdk.operator.ReconcilerUtilsInternal.loadYaml; @KubernetesDependent public class ServiceDependentResource diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/ssalegacymatcher/ServiceDependentResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/ssalegacymatcher/ServiceDependentResource.java index 7cd65bd7ef..6a998b3ea4 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/ssalegacymatcher/ServiceDependentResource.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/ssalegacymatcher/ServiceDependentResource.java @@ -25,7 +25,7 @@ import io.javaoperatorsdk.operator.processing.dependent.kubernetes.GenericKubernetesResourceMatcher; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependent; -import static io.javaoperatorsdk.operator.ReconcilerUtils.loadYaml; +import static io.javaoperatorsdk.operator.ReconcilerUtilsInternal.loadYaml; @KubernetesDependent public class ServiceDependentResource diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/standalonedependent/StandaloneDependentTestReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/standalonedependent/StandaloneDependentTestReconciler.java index 6f97be1be7..92f033d681 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/standalonedependent/StandaloneDependentTestReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/standalonedependent/StandaloneDependentTestReconciler.java @@ -20,7 +20,7 @@ import io.fabric8.kubernetes.api.model.apps.Deployment; import io.fabric8.kubernetes.client.KubernetesClientException; -import io.javaoperatorsdk.operator.ReconcilerUtils; +import io.javaoperatorsdk.operator.ReconcilerUtilsInternal; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.ErrorStatusUpdateControl; @@ -90,7 +90,7 @@ protected Deployment desired( StandaloneDependentTestCustomResource primary, Context context) { Deployment deployment = - ReconcilerUtils.loadYaml( + ReconcilerUtilsInternal.loadYaml( Deployment.class, StandaloneDependentResourceIT.class, "/io/javaoperatorsdk/operator/nginx-deployment.yaml"); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/statefulsetdesiredsanitizer/StatefulSetDesiredSanitizerDependentResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/statefulsetdesiredsanitizer/StatefulSetDesiredSanitizerDependentResource.java index e86c772cda..e4bcaac460 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/statefulsetdesiredsanitizer/StatefulSetDesiredSanitizerDependentResource.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/statefulsetdesiredsanitizer/StatefulSetDesiredSanitizerDependentResource.java @@ -17,7 +17,7 @@ import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; import io.fabric8.kubernetes.api.model.apps.StatefulSet; -import io.javaoperatorsdk.operator.ReconcilerUtils; +import io.javaoperatorsdk.operator.ReconcilerUtilsInternal; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDKubernetesDependentResource; @@ -32,7 +32,7 @@ protected StatefulSet desired( StatefulSetDesiredSanitizerCustomResource primary, Context context) { var template = - ReconcilerUtils.loadYaml( + ReconcilerUtilsInternal.loadYaml( StatefulSet.class, getClass(), "/io/javaoperatorsdk/operator/statefulset.yaml"); template.setMetadata( new ObjectMetaBuilder() diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/complexdependent/dependent/BaseService.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/complexdependent/dependent/BaseService.java index 06abcc0889..7a0d50debf 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/complexdependent/dependent/BaseService.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/complexdependent/dependent/BaseService.java @@ -19,7 +19,7 @@ import io.fabric8.kubernetes.api.model.Service; import io.fabric8.kubernetes.api.model.ServiceBuilder; -import io.javaoperatorsdk.operator.ReconcilerUtils; +import io.javaoperatorsdk.operator.ReconcilerUtilsInternal; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.workflow.complexdependent.ComplexWorkflowCustomResource; @@ -33,7 +33,7 @@ public BaseService(String component) { protected Service desired( ComplexWorkflowCustomResource primary, Context context) { var template = - ReconcilerUtils.loadYaml( + ReconcilerUtilsInternal.loadYaml( Service.class, getClass(), "/io/javaoperatorsdk/operator/workflow/complexdependent/service.yaml"); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/complexdependent/dependent/BaseStatefulSet.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/complexdependent/dependent/BaseStatefulSet.java index b0a7b60805..1e4aa73e80 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/complexdependent/dependent/BaseStatefulSet.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/complexdependent/dependent/BaseStatefulSet.java @@ -19,7 +19,7 @@ import io.fabric8.kubernetes.api.model.apps.StatefulSet; import io.fabric8.kubernetes.api.model.apps.StatefulSetBuilder; -import io.javaoperatorsdk.operator.ReconcilerUtils; +import io.javaoperatorsdk.operator.ReconcilerUtilsInternal; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.workflow.complexdependent.ComplexWorkflowCustomResource; @@ -32,7 +32,7 @@ public BaseStatefulSet(String component) { protected StatefulSet desired( ComplexWorkflowCustomResource primary, Context context) { var template = - ReconcilerUtils.loadYaml( + ReconcilerUtilsInternal.loadYaml( StatefulSet.class, getClass(), "/io/javaoperatorsdk/operator/workflow/complexdependent/statefulset.yaml"); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/workflowallfeature/DeploymentDependentResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/workflowallfeature/DeploymentDependentResource.java index b9aa595b76..e5c7f726f5 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/workflowallfeature/DeploymentDependentResource.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/workflow/workflowallfeature/DeploymentDependentResource.java @@ -16,7 +16,7 @@ package io.javaoperatorsdk.operator.workflow.workflowallfeature; import io.fabric8.kubernetes.api.model.apps.Deployment; -import io.javaoperatorsdk.operator.ReconcilerUtils; +import io.javaoperatorsdk.operator.ReconcilerUtilsInternal; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDNoGCKubernetesDependentResource; @@ -27,7 +27,7 @@ public class DeploymentDependentResource protected Deployment desired( WorkflowAllFeatureCustomResource primary, Context context) { Deployment deployment = - ReconcilerUtils.loadYaml( + ReconcilerUtilsInternal.loadYaml( Deployment.class, WorkflowAllFeatureIT.class, "/io/javaoperatorsdk/operator/nginx-deployment.yaml"); diff --git a/sample-operators/tomcat-operator/src/main/java/io/javaoperatorsdk/operator/sample/DeploymentDependentResource.java b/sample-operators/tomcat-operator/src/main/java/io/javaoperatorsdk/operator/sample/DeploymentDependentResource.java index 0347b726ac..c4a47069e2 100644 --- a/sample-operators/tomcat-operator/src/main/java/io/javaoperatorsdk/operator/sample/DeploymentDependentResource.java +++ b/sample-operators/tomcat-operator/src/main/java/io/javaoperatorsdk/operator/sample/DeploymentDependentResource.java @@ -18,7 +18,7 @@ import io.fabric8.kubernetes.api.model.ObjectMeta; import io.fabric8.kubernetes.api.model.apps.Deployment; import io.fabric8.kubernetes.api.model.apps.DeploymentBuilder; -import io.javaoperatorsdk.operator.ReconcilerUtils; +import io.javaoperatorsdk.operator.ReconcilerUtilsInternal; import io.javaoperatorsdk.operator.api.config.informer.Informer; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDKubernetesDependentResource; @@ -36,7 +36,7 @@ private static String tomcatImage(Tomcat tomcat) { @Override protected Deployment desired(Tomcat tomcat, Context context) { Deployment deployment = - ReconcilerUtils.loadYaml(Deployment.class, getClass(), "deployment.yaml"); + ReconcilerUtilsInternal.loadYaml(Deployment.class, getClass(), "deployment.yaml"); final ObjectMeta tomcatMetadata = tomcat.getMetadata(); final String tomcatName = tomcatMetadata.getName(); deployment = diff --git a/sample-operators/tomcat-operator/src/main/java/io/javaoperatorsdk/operator/sample/ServiceDependentResource.java b/sample-operators/tomcat-operator/src/main/java/io/javaoperatorsdk/operator/sample/ServiceDependentResource.java index 72f430528e..bcb0e80026 100644 --- a/sample-operators/tomcat-operator/src/main/java/io/javaoperatorsdk/operator/sample/ServiceDependentResource.java +++ b/sample-operators/tomcat-operator/src/main/java/io/javaoperatorsdk/operator/sample/ServiceDependentResource.java @@ -18,7 +18,7 @@ import io.fabric8.kubernetes.api.model.ObjectMeta; import io.fabric8.kubernetes.api.model.Service; import io.fabric8.kubernetes.api.model.ServiceBuilder; -import io.javaoperatorsdk.operator.ReconcilerUtils; +import io.javaoperatorsdk.operator.ReconcilerUtilsInternal; import io.javaoperatorsdk.operator.api.config.informer.Informer; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDKubernetesDependentResource; @@ -31,7 +31,8 @@ public class ServiceDependentResource extends CRUDKubernetesDependentResource context) { final ObjectMeta tomcatMetadata = tomcat.getMetadata(); - return new ServiceBuilder(ReconcilerUtils.loadYaml(Service.class, getClass(), "service.yaml")) + return new ServiceBuilder( + ReconcilerUtilsInternal.loadYaml(Service.class, getClass(), "service.yaml")) .editMetadata() .withName(tomcatMetadata.getName()) .withNamespace(tomcatMetadata.getNamespace()) diff --git a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/Utils.java b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/Utils.java index ab4ed8a337..ecfe66d329 100644 --- a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/Utils.java +++ b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/Utils.java @@ -21,7 +21,7 @@ import io.javaoperatorsdk.operator.sample.customresource.WebPage; import io.javaoperatorsdk.operator.sample.customresource.WebPageStatus; -import static io.javaoperatorsdk.operator.ReconcilerUtils.loadYaml; +import static io.javaoperatorsdk.operator.ReconcilerUtilsInternal.loadYaml; public class Utils { diff --git a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconciler.java b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconciler.java index 94b460474f..941a159542 100644 --- a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconciler.java +++ b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconciler.java @@ -27,7 +27,7 @@ import io.fabric8.kubernetes.api.model.*; import io.fabric8.kubernetes.api.model.apps.Deployment; import io.fabric8.kubernetes.api.model.networking.v1.Ingress; -import io.javaoperatorsdk.operator.ReconcilerUtils; +import io.javaoperatorsdk.operator.ReconcilerUtilsInternal; import io.javaoperatorsdk.operator.api.config.informer.InformerEventSourceConfiguration; import io.javaoperatorsdk.operator.api.reconciler.*; import io.javaoperatorsdk.operator.api.reconciler.Context; @@ -219,7 +219,8 @@ private boolean match(ConfigMap desiredHtmlConfigMap, ConfigMap existingConfigMa } private Service makeDesiredService(WebPage webPage, String ns, Deployment desiredDeployment) { - Service desiredService = ReconcilerUtils.loadYaml(Service.class, getClass(), "service.yaml"); + Service desiredService = + ReconcilerUtilsInternal.loadYaml(Service.class, getClass(), "service.yaml"); desiredService.getMetadata().setName(serviceName(webPage)); desiredService.getMetadata().setNamespace(ns); desiredService.getMetadata().setLabels(lowLevelLabel()); @@ -233,7 +234,7 @@ private Service makeDesiredService(WebPage webPage, String ns, Deployment desire private Deployment makeDesiredDeployment( WebPage webPage, String deploymentName, String ns, String configMapName) { Deployment desiredDeployment = - ReconcilerUtils.loadYaml(Deployment.class, getClass(), "deployment.yaml"); + ReconcilerUtilsInternal.loadYaml(Deployment.class, getClass(), "deployment.yaml"); desiredDeployment.getMetadata().setName(deploymentName); desiredDeployment.getMetadata().setNamespace(ns); desiredDeployment.getMetadata().setLabels(lowLevelLabel()); diff --git a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/dependentresource/DeploymentDependentResource.java b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/dependentresource/DeploymentDependentResource.java index 6d1f7cc911..e383633ab1 100644 --- a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/dependentresource/DeploymentDependentResource.java +++ b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/dependentresource/DeploymentDependentResource.java @@ -27,7 +27,7 @@ import io.javaoperatorsdk.operator.sample.Utils; import io.javaoperatorsdk.operator.sample.customresource.WebPage; -import static io.javaoperatorsdk.operator.ReconcilerUtils.loadYaml; +import static io.javaoperatorsdk.operator.ReconcilerUtilsInternal.loadYaml; import static io.javaoperatorsdk.operator.sample.Utils.configMapName; import static io.javaoperatorsdk.operator.sample.Utils.deploymentName; import static io.javaoperatorsdk.operator.sample.WebPageManagedDependentsReconciler.SELECTOR; diff --git a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/dependentresource/ServiceDependentResource.java b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/dependentresource/ServiceDependentResource.java index 3dbc784887..02204d415a 100644 --- a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/dependentresource/ServiceDependentResource.java +++ b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/dependentresource/ServiceDependentResource.java @@ -25,7 +25,7 @@ import io.javaoperatorsdk.operator.sample.Utils; import io.javaoperatorsdk.operator.sample.customresource.WebPage; -import static io.javaoperatorsdk.operator.ReconcilerUtils.loadYaml; +import static io.javaoperatorsdk.operator.ReconcilerUtilsInternal.loadYaml; import static io.javaoperatorsdk.operator.sample.Utils.deploymentName; import static io.javaoperatorsdk.operator.sample.Utils.serviceName; import static io.javaoperatorsdk.operator.sample.WebPageManagedDependentsReconciler.SELECTOR; From 845490b78a75456ab1cbdbcdace54581e5663071 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Thu, 25 Dec 2025 18:47:03 +0100 Subject: [PATCH 02/21] wip 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 | 16 ---------------- 1 file changed, 16 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 3f25490534..c32282af9f 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 @@ -96,22 +96,6 @@ private InformerEventSource( genericFilter = informerConfig.getGenericFilter(); } - public R updateAndCacheResource( - R resourceToUpdate, Context context, UnaryOperator updateMethod) { - ResourceID id = ResourceID.fromResource(resourceToUpdate); - if (log.isDebugEnabled()) { - log.debug("Update and cache: {}", id); - } - try { - temporaryResourceCache.startModifying(id); - var updated = updateMethod.apply(resourceToUpdate); - handleRecentResourceUpdate(id, updated, resourceToUpdate); - return updated; - } finally { - temporaryResourceCache.doneModifying(id); - } - } - @Override public void onAdd(R newResource) { if (log.isDebugEnabled()) { From 1c37bd8662491ee8b67af38772c6d10339c05bd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Thu, 25 Dec 2025 21:20:56 +0100 Subject: [PATCH 03/21] feat: ReconcileUtils for strongly consistent updates - alternative algortihm MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../controller/ControllerEventSource.java | 8 +- .../source/informer/EventFilterDetails.java | 70 +++++++++ .../source/informer/InformerEventSource.java | 11 ++ .../informer/ManagedInformerEventSource.java | 30 +++- .../informer/TemporaryResourceCache.java | 145 +++++++++--------- .../controller/ControllerEventSourceTest.java | 30 ++-- .../TemporaryPrimaryResourceCacheTest.java | 10 +- 7 files changed, 201 insertions(+), 103 deletions(-) create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/EventFilterDetails.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerEventSource.java index c323f4841d..a7a41525ce 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerEventSource.java @@ -81,7 +81,7 @@ public synchronized void start() { } } - public synchronized void eventReceived( + public synchronized void handleEvent( ResourceAction action, T resource, T oldResource, @@ -138,13 +138,13 @@ private boolean isAcceptedByFilters(ResourceAction action, T resource, T oldReso @Override public void onAdd(T resource) { var knownResourceVersion = temporaryResourceCache.onAddOrUpdateEvent(resource); - eventReceived(ResourceAction.ADDED, resource, null, null, knownResourceVersion); + handleEvent(ResourceAction.ADDED, resource, null, null, knownResourceVersion); } @Override public void onUpdate(T oldCustomResource, T newCustomResource) { var knownResourceVersion = temporaryResourceCache.onAddOrUpdateEvent(newCustomResource); - eventReceived( + handleEvent( ResourceAction.UPDATED, newCustomResource, oldCustomResource, null, knownResourceVersion); } @@ -153,7 +153,7 @@ public void onDelete(T resource, boolean deletedFinalStateUnknown) { temporaryResourceCache.onDeleteEvent(resource, deletedFinalStateUnknown); // delete event is quite special here, that requires special care, since we clean up caches on // delete event. - eventReceived(ResourceAction.DELETED, resource, null, deletedFinalStateUnknown, false); + handleEvent(ResourceAction.DELETED, resource, null, deletedFinalStateUnknown, false); } @Override diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/EventFilterDetails.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/EventFilterDetails.java new file mode 100644 index 0000000000..753aad7980 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/EventFilterDetails.java @@ -0,0 +1,70 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.processing.event.source.informer; + +import java.util.ArrayList; +import java.util.List; +import java.util.Optional; + +import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEvent; + +class EventFilterDetails { + + // initially should be created during event filtering update + + private int activeUpdates = 0; + // todo might be just one + private final List events = new ArrayList<>(); + private int lastUpdatedResourceVersion = -1; + + public int getActiveUpdates() { + return activeUpdates; + } + + public void increaseActiveUpdates() { + activeUpdates = activeUpdates + 1; + } + + public void decreaseActiveUpdates() { + activeUpdates = activeUpdates - 1; + } + + public void recordEvent(ResourceEvent event) { + events.add(event); + } + + public void setLastUpdatedResourceVersion(String version) { + var parsed = Integer.parseInt(version); + if (parsed > lastUpdatedResourceVersion) { + lastUpdatedResourceVersion = parsed; + } + } + + public Optional getLatestEventAfterLastUpdateEvent() { + if (events.isEmpty()) return Optional.empty(); + var latest = events.get(events.size() - 1); + if (Integer.parseInt(latest.getResource().orElseThrow().getMetadata().getResourceVersion()) + > lastUpdatedResourceVersion) { + return Optional.of(latest); + } else { + return Optional.empty(); + } + } + + public boolean isFilteringDone() { + return activeUpdates == 0; + } +} 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 c32282af9f..49b5f32d3e 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 @@ -32,6 +32,7 @@ import io.javaoperatorsdk.operator.processing.event.EventHandler; import io.javaoperatorsdk.operator.processing.event.ResourceID; import io.javaoperatorsdk.operator.processing.event.source.PrimaryToSecondaryMapper; +import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceAction; import static io.javaoperatorsdk.operator.api.reconciler.Constants.DEFAULT_COMPARABLE_RESOURCE_VERSION; @@ -136,6 +137,16 @@ public synchronized void onDelete(R resource, boolean b) { } } + @Override + public void handleEvent( + ResourceAction action, + R resource, + R oldResource, + Boolean deletedFinalStateUnknown, + boolean filterEvent) { + propagateEvent(resource); + } + @Override public synchronized void start() { super.start(); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java index 6f3d732967..962dd284b9 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java @@ -42,6 +42,8 @@ import io.javaoperatorsdk.operator.health.Status; import io.javaoperatorsdk.operator.processing.event.ResourceID; import io.javaoperatorsdk.operator.processing.event.source.*; +import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceAction; +import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceDeleteEvent; @SuppressWarnings("rawtypes") public abstract class ManagedInformerEventSource< @@ -104,16 +106,36 @@ public R eventFilteringUpdateAndCacheResource(R resourceToUpdate, UnaryOperator< if (log.isDebugEnabled()) { log.debug("Update and cache: {}", id); } + R updatedResource = null; try { temporaryResourceCache.startEventFilteringModify(id); - var updated = updateMethod.apply(resourceToUpdate); - handleRecentResourceUpdate(id, updated, resourceToUpdate); - return updated; + updatedResource = updateMethod.apply(resourceToUpdate); + handleRecentResourceUpdate(id, updatedResource, resourceToUpdate); + return updatedResource; } finally { - temporaryResourceCache.doneEventFilterModify(id); + var res = + temporaryResourceCache.doneEventFilterModify( + id, + updatedResource == null ? null : updatedResource.getMetadata().getResourceVersion()); + res.ifPresent( + r -> + handleEvent( + r.getAction(), + (R) r.getResource().orElseThrow(), + null, + !(r instanceof ResourceDeleteEvent) + || ((ResourceDeleteEvent) r).isDeletedFinalStateUnknown(), + false)); } } + public abstract void handleEvent( + ResourceAction action, + R resource, + R oldResource, + Boolean deletedFinalStateUnknown, + boolean filterEvent); + @SuppressWarnings("unchecked") @Override public synchronized void start() { 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 6b6d0c3d95..f8befa7771 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 @@ -15,11 +15,10 @@ */ package io.javaoperatorsdk.operator.processing.event.source.informer; +import java.util.HashMap; import java.util.Map; import java.util.Optional; -import java.util.Set; import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.locks.ReentrantLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -28,6 +27,9 @@ import io.javaoperatorsdk.operator.api.reconciler.ReconcileUtils; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource; import io.javaoperatorsdk.operator.processing.event.ResourceID; +import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceAction; +import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceDeleteEvent; +import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEvent; /** * Temporal cache is used to solve the problem for {@link KubernetesDependentResource} that is, when @@ -51,59 +53,70 @@ */ public class TemporaryResourceCache { + // TODO + // requirements: + // - concurrent updates + // - no blocking i/o related updates + // - support non-event filtering + // - handle delete event + private static final Logger log = LoggerFactory.getLogger(TemporaryResourceCache.class); private final Map cache = new ConcurrentHashMap<>(); private final boolean comparableResourceVersions; - private final Map activelyModifying = new ConcurrentHashMap<>(); - private final Set skipFiltering = ConcurrentHashMap.newKeySet(); private String latestResourceVersion; + private final Map activeUpdates = new HashMap<>(); + public TemporaryResourceCache(boolean comparableResourceVersions) { this.comparableResourceVersions = comparableResourceVersions; } - public void startEventFilteringModify(ResourceID id) { + public synchronized void startEventFilteringModify(ResourceID resourceID) { if (!comparableResourceVersions) { return; } - activelyModifying - .compute( - id, - (ignored, lock) -> { - if (lock != null) { - throw new IllegalStateException(); // concurrent modifications to the same resource - // not allowed - this could be relaxed if needed - } - return new ReentrantLock(); - }) - .lock(); + var ed = activeUpdates.computeIfAbsent(resourceID, id -> new EventFilterDetails()); + ed.increaseActiveUpdates(); } - public void doneEventFilterModify(ResourceID id) { + public synchronized Optional doneEventFilterModify( + ResourceID resourceID, String updatedResourceVersion) { if (!comparableResourceVersions) { - return; + return Optional.empty(); + } + var ed = activeUpdates.get(resourceID); + ed.decreaseActiveUpdates(); + if (updatedResourceVersion != null) { + ed.setLastUpdatedResourceVersion(updatedResourceVersion); + } + if (ed.getActiveUpdates() == 0) { + var latestEventAfterUpdate = ed.getLatestEventAfterLastUpdateEvent(); + if (latestEventAfterUpdate.isPresent()) { + activeUpdates.remove(resourceID); + } + return latestEventAfterUpdate; + } else { + return Optional.empty(); } - activelyModifying.computeIfPresent( - id, - (ignored, lock) -> { - lock.unlock(); - return null; - }); } public void onDeleteEvent(T resource, boolean unknownState) { - onEvent(resource, unknownState); + onEvent(resource, unknownState, true); } /** * @return true if the resourceVersion was already known and not skipped for event filtering */ public boolean onAddOrUpdateEvent(T resource) { - return onEvent(resource, false); + return onEvent(resource, false, false); } - private boolean onEvent(T resource, boolean unknownState) { + private synchronized boolean onEvent(T resource, boolean unknownState, boolean delete) { + if (!comparableResourceVersions) { + return false; + } + var resourceId = ResourceID.fromResource(resource); if (log.isDebugEnabled()) { log.debug( @@ -111,48 +124,37 @@ private boolean onEvent(T resource, boolean unknownState) { resourceId, resource.getMetadata().getResourceVersion()); } - ReentrantLock lock = activelyModifying.get(resourceId); - if (lock != null) { - log.trace("Lock for event filtering resource id: {}", resourceId); - // note that this is a special case of lock striping; event handling happens - // always on the same thread of the informer we lock only if the update is happening - // for the same resource (not any resource), and if the event comes from the current update - // this should be locked for a very short time, since that update request already send at this - // point. - lock.lock(); // wait for the modification to finish - lock.unlock(); // simply unlock as the event is guaranteed after the modification - log.trace("Unlock for event resource id: {}", resourceId); + if (!unknownState) { + latestResourceVersion = resource.getMetadata().getResourceVersion(); } - boolean[] filter = new boolean[1]; - synchronized (this) { - if (!unknownState) { - latestResourceVersion = resource.getMetadata().getResourceVersion(); + var cached = cache.get(resourceId); + boolean filterEvent = true; + int comp = 0; + if (cached != null) { + comp = ReconcileUtils.compareResourceVersions(resource, cached); + if (comp >= 0 || unknownState) { + cache.remove(resourceId); + // we propagate event only for our update or newer other can be discarded since we know we + // will receive + // additional event + filterEvent = false; } - cache.computeIfPresent( - resourceId, - (id, cached) -> { - boolean remove = unknownState; - if (!unknownState) { - int comp = ReconcileUtils.compareResourceVersions(resource, cached); - if (comp >= 0) { - remove = true; - } - if (comp < 0) { - filter[0] = true; - } else if (comp == 0) { - filter[0] = !skipFiltering.remove(resourceId); - } else { - skipFiltering.remove(resourceId); - } - } else { - skipFiltering.remove(resourceId); - } - if (remove) { - return null; - } - return cached; - }); - return filter[0]; + } + var ed = activeUpdates.get(resourceId); + if (ed != null) { + ed.recordEvent( + delete + ? new ResourceDeleteEvent(ResourceAction.DELETED, resourceId, resource, unknownState) + : new ResourceEvent( + ResourceAction.UPDATED, resourceId, resource)); // todo true action + if (ed.isFilteringDone() && ed.getLatestEventAfterLastUpdateEvent().isPresent()) { + activeUpdates.remove(resourceId); + return false; + } else { + return true; + } + } else { + return filterEvent; } } @@ -163,7 +165,6 @@ public synchronized void putResource(T newResource) { } var resourceId = ResourceID.fromResource(newResource); - skipFiltering.remove(resourceId); if (newResource.getMetadata().getResourceVersion() == null) { log.warn( @@ -201,18 +202,10 @@ public synchronized void putResource(T newResource) { newResource.getMetadata().getResourceVersion(), resourceId); cache.put(resourceId, newResource); - if (!isFilteringModification(resourceId)) { - log.debug("Add resource id to skipFiltering: {}", resourceId); - skipFiltering.add(resourceId); - } } } public synchronized Optional getResourceFromCache(ResourceID resourceID) { return Optional.ofNullable(cache.get(resourceID)); } - - private boolean isFilteringModification(ResourceID resourceId) { - return activelyModifying.containsKey(resourceId); - } } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerEventSourceTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerEventSourceTest.java index 0cfb66711a..baef2110df 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerEventSourceTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerEventSourceTest.java @@ -68,10 +68,10 @@ void skipsEventHandlingIfGenerationNotIncreased() { TestCustomResource oldCustomResource = TestUtils.testCustomResource(); oldCustomResource.getMetadata().setFinalizers(List.of(FINALIZER)); - source.eventReceived(ResourceAction.UPDATED, customResource, oldCustomResource, null, false); + source.handleEvent(ResourceAction.UPDATED, customResource, oldCustomResource, null, false); verify(eventHandler, times(1)).handleEvent(any()); - source.eventReceived(ResourceAction.UPDATED, customResource, customResource, null, false); + source.handleEvent(ResourceAction.UPDATED, customResource, customResource, null, false); verify(eventHandler, times(1)).handleEvent(any()); } @@ -79,12 +79,12 @@ void skipsEventHandlingIfGenerationNotIncreased() { void dontSkipEventHandlingIfMarkedForDeletion() { TestCustomResource customResource1 = TestUtils.testCustomResource(); - source.eventReceived(ResourceAction.UPDATED, customResource1, customResource1, null, false); + source.handleEvent(ResourceAction.UPDATED, customResource1, customResource1, null, false); verify(eventHandler, times(1)).handleEvent(any()); // mark for deletion customResource1.getMetadata().setDeletionTimestamp(LocalDateTime.now().toString()); - source.eventReceived(ResourceAction.UPDATED, customResource1, customResource1, null, false); + source.handleEvent(ResourceAction.UPDATED, customResource1, customResource1, null, false); verify(eventHandler, times(2)).handleEvent(any()); } @@ -92,11 +92,11 @@ void dontSkipEventHandlingIfMarkedForDeletion() { void normalExecutionIfGenerationChanges() { TestCustomResource customResource1 = TestUtils.testCustomResource(); - source.eventReceived(ResourceAction.UPDATED, customResource1, customResource1, null, false); + source.handleEvent(ResourceAction.UPDATED, customResource1, customResource1, null, false); verify(eventHandler, times(1)).handleEvent(any()); customResource1.getMetadata().setGeneration(2L); - source.eventReceived(ResourceAction.UPDATED, customResource1, customResource1, null, false); + source.handleEvent(ResourceAction.UPDATED, customResource1, customResource1, null, false); verify(eventHandler, times(2)).handleEvent(any()); } @@ -107,10 +107,10 @@ void handlesAllEventIfNotGenerationAware() { TestCustomResource customResource1 = TestUtils.testCustomResource(); - source.eventReceived(ResourceAction.UPDATED, customResource1, customResource1, null, false); + source.handleEvent(ResourceAction.UPDATED, customResource1, customResource1, null, false); verify(eventHandler, times(1)).handleEvent(any()); - source.eventReceived(ResourceAction.UPDATED, customResource1, customResource1, null, false); + source.handleEvent(ResourceAction.UPDATED, customResource1, customResource1, null, false); verify(eventHandler, times(2)).handleEvent(any()); } @@ -118,7 +118,7 @@ void handlesAllEventIfNotGenerationAware() { void eventWithNoGenerationProcessedIfNoFinalizer() { TestCustomResource customResource1 = TestUtils.testCustomResource(); - source.eventReceived(ResourceAction.UPDATED, customResource1, customResource1, null, false); + source.handleEvent(ResourceAction.UPDATED, customResource1, customResource1, null, false); verify(eventHandler, times(1)).handleEvent(any()); } @@ -127,7 +127,7 @@ void eventWithNoGenerationProcessedIfNoFinalizer() { void callsBroadcastsOnResourceEvents() { TestCustomResource customResource1 = TestUtils.testCustomResource(); - source.eventReceived(ResourceAction.UPDATED, customResource1, customResource1, null, false); + source.handleEvent(ResourceAction.UPDATED, customResource1, customResource1, null, false); verify(testController.getEventSourceManager(), times(1)) .broadcastOnResourceEvent( @@ -143,8 +143,8 @@ void filtersOutEventsOnAddAndUpdate() { source = new ControllerEventSource<>(new TestController(onAddFilter, onUpdatePredicate, null)); setUpSource(source, true, controllerConfig); - source.eventReceived(ResourceAction.ADDED, cr, null, null, false); - source.eventReceived(ResourceAction.UPDATED, cr, cr, null, false); + source.handleEvent(ResourceAction.ADDED, cr, null, null, false); + source.handleEvent(ResourceAction.UPDATED, cr, cr, null, false); verify(eventHandler, never()).handleEvent(any()); } @@ -156,9 +156,9 @@ void genericFilterFiltersOutAddUpdateAndDeleteEvents() { source = new ControllerEventSource<>(new TestController(null, null, res -> false)); setUpSource(source, true, controllerConfig); - source.eventReceived(ResourceAction.ADDED, cr, null, null, false); - source.eventReceived(ResourceAction.UPDATED, cr, cr, null, false); - source.eventReceived(ResourceAction.DELETED, cr, cr, true, false); + source.handleEvent(ResourceAction.ADDED, cr, null, null, false); + source.handleEvent(ResourceAction.UPDATED, cr, cr, null, false); + source.handleEvent(ResourceAction.DELETED, cr, cr, true, false); verify(eventHandler, never()).handleEvent(any()); } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryPrimaryResourceCacheTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryPrimaryResourceCacheTest.java index c665cf8396..23204086dc 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryPrimaryResourceCacheTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryPrimaryResourceCacheTest.java @@ -21,6 +21,7 @@ import java.util.concurrent.TimeUnit; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import io.fabric8.kubernetes.api.model.ConfigMap; @@ -118,6 +119,7 @@ void nonComparableResourceVersionsDisables() { .isEmpty(); } + @Disabled("todo") @Test void lockedEventBeforePut() throws Exception { var testResource = testResource(); @@ -130,7 +132,7 @@ void lockedEventBeforePut() throws Exception { temporaryResourceCache.putResource(testResource); assertThat(result.isDone()).isFalse(); - temporaryResourceCache.doneEventFilterModify(ResourceID.fromResource(testResource)); + temporaryResourceCache.doneEventFilterModify(ResourceID.fromResource(testResource), "3"); assertThat(result.get(10, TimeUnit.SECONDS)).isTrue(); } finally { ex.shutdownNow(); @@ -143,7 +145,7 @@ void putBeforeEvent() { // first ensure an event is not known var result = temporaryResourceCache.onAddOrUpdateEvent(testResource); - assertThat(result).isFalse(); + assertThat(result).isTrue(); var nextResource = testResource(); nextResource.getMetadata().setResourceVersion("3"); @@ -160,7 +162,7 @@ void putBeforeEventWithEventFiltering() { // first ensure an event is not known var result = temporaryResourceCache.onAddOrUpdateEvent(testResource); - assertThat(result).isFalse(); + assertThat(result).isTrue(); var nextResource = testResource(); nextResource.getMetadata().setResourceVersion("3"); @@ -168,7 +170,7 @@ void putBeforeEventWithEventFiltering() { temporaryResourceCache.startEventFilteringModify(resourceId); temporaryResourceCache.putResource(nextResource); - temporaryResourceCache.doneEventFilterModify(resourceId); + temporaryResourceCache.doneEventFilterModify(resourceId, "3"); // the result is false since the put was not part of event filtering update result = temporaryResourceCache.onAddOrUpdateEvent(nextResource); From 94585c9fef35b7b4d56998e34a6d29122739991b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Thu, 25 Dec 2025 23:36:34 +0100 Subject: [PATCH 04/21] wip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../event/source/informer/TemporaryResourceCache.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 f8befa7771..cc174fa69b 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 @@ -128,7 +128,7 @@ private synchronized boolean onEvent(T resource, boolean unknownState, boolean d latestResourceVersion = resource.getMetadata().getResourceVersion(); } var cached = cache.get(resourceId); - boolean filterEvent = true; + boolean filterEvent = false; int comp = 0; if (cached != null) { comp = ReconcileUtils.compareResourceVersions(resource, cached); @@ -138,6 +138,8 @@ private synchronized boolean onEvent(T resource, boolean unknownState, boolean d // will receive // additional event filterEvent = false; + } else { + filterEvent = true; } } var ed = activeUpdates.get(resourceId); From 3d4efaac14645e436753f5164a2004a2a78b1159 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Fri, 26 Dec 2025 09:48:27 +0100 Subject: [PATCH 05/21] wip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../source/informer/TemporaryPrimaryResourceCacheTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryPrimaryResourceCacheTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryPrimaryResourceCacheTest.java index 23204086dc..6f423b1cf5 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryPrimaryResourceCacheTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryPrimaryResourceCacheTest.java @@ -145,7 +145,7 @@ void putBeforeEvent() { // first ensure an event is not known var result = temporaryResourceCache.onAddOrUpdateEvent(testResource); - assertThat(result).isTrue(); + assertThat(result).isFalse(); var nextResource = testResource(); nextResource.getMetadata().setResourceVersion("3"); @@ -162,7 +162,7 @@ void putBeforeEventWithEventFiltering() { // first ensure an event is not known var result = temporaryResourceCache.onAddOrUpdateEvent(testResource); - assertThat(result).isTrue(); + assertThat(result).isFalse(); var nextResource = testResource(); nextResource.getMetadata().setResourceVersion("3"); From 586293c8a75d4e65da6ff08a284ab3fe46bb5df6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Sat, 27 Dec 2025 14:16:44 +0100 Subject: [PATCH 06/21] record only last event MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../source/informer/EventFilterDetails.java | 18 ++++++------------ .../informer/TemporaryResourceCache.java | 2 +- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/EventFilterDetails.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/EventFilterDetails.java index 753aad7980..1920c9cf3b 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/EventFilterDetails.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/EventFilterDetails.java @@ -15,19 +15,14 @@ */ package io.javaoperatorsdk.operator.processing.event.source.informer; -import java.util.ArrayList; -import java.util.List; import java.util.Optional; import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEvent; class EventFilterDetails { - // initially should be created during event filtering update - private int activeUpdates = 0; - // todo might be just one - private final List events = new ArrayList<>(); + private ResourceEvent lastEvent; private int lastUpdatedResourceVersion = -1; public int getActiveUpdates() { @@ -42,8 +37,8 @@ public void decreaseActiveUpdates() { activeUpdates = activeUpdates - 1; } - public void recordEvent(ResourceEvent event) { - events.add(event); + public void setLastEvent(ResourceEvent event) { + lastEvent = event; } public void setLastUpdatedResourceVersion(String version) { @@ -54,11 +49,10 @@ public void setLastUpdatedResourceVersion(String version) { } public Optional getLatestEventAfterLastUpdateEvent() { - if (events.isEmpty()) return Optional.empty(); - var latest = events.get(events.size() - 1); - if (Integer.parseInt(latest.getResource().orElseThrow().getMetadata().getResourceVersion()) + if (lastEvent == null) return Optional.empty(); + if (Integer.parseInt(lastEvent.getResource().orElseThrow().getMetadata().getResourceVersion()) > lastUpdatedResourceVersion) { - return Optional.of(latest); + return Optional.of(lastEvent); } else { return Optional.empty(); } 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 cc174fa69b..1f02b36a1b 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 @@ -144,7 +144,7 @@ private synchronized boolean onEvent(T resource, boolean unknownState, boolean d } var ed = activeUpdates.get(resourceId); if (ed != null) { - ed.recordEvent( + ed.setLastEvent( delete ? new ResourceDeleteEvent(ResourceAction.DELETED, resourceId, resource, unknownState) : new ResourceEvent( From b0829e9be49559b1fb09618a1ca960b9b8dd08f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Tue, 6 Jan 2026 18:41:13 +0100 Subject: [PATCH 07/21] Compare functions without validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../api/reconciler/ReconcileUtils.java | 63 +++++++++- .../informer/ManagedInformerEventSource.java | 5 +- .../informer/TemporaryResourceCache.java | 11 +- .../api/reconciler/ReconcileUtilsTest.java | 117 +++++++++++++++--- 4 files changed, 166 insertions(+), 30 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtils.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtils.java index eb8ba95737..670a5625b0 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtils.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtils.java @@ -921,19 +921,76 @@ public static

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

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

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

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

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

Note: This method does not validate that the input strings are numeric. For + * validated numeric comparison, use {@link #validateAndCompareResourceVersions(String, String)}. + * + * @param v1 the first resource version string + * @param v2 the second resource version string + * @return a negative integer if v1 is less than v2, zero if they are equal, or a positive integer + * if v1 is greater than v2 + * @see #validateAndCompareResourceVersions(String, String) + */ + public static int compareResourceVersions(String v1, String v2) { + int comparison = v1.length() - v2.length(); + if (comparison != 0) { + return comparison; + } + for (int i = 0; i < v2.length(); i++) { + int comp = v1.charAt(i) - v2.charAt(i); + if (comp != 0) { + return comp; + } + } + return 0; + } + /** * Compares two Kubernetes resource versions numerically. Kubernetes resource versions are * expected to be numeric strings that increase monotonically. This method assumes both versions @@ -945,7 +1002,7 @@ public static int compareResourceVersions(HasMetadata h1, HasMetadata h2) { * @throws NonComparableResourceVersionException if either resource version is empty, has leading * zeros, or contains non-numeric characters */ - public static int compareResourceVersions(String v1, String v2) { + public static int validateAndCompareResourceVersions(String v1, String v2) { int v1Length = validateResourceVersion(v1); int v2Length = validateResourceVersion(v2); int comparison = v1Length - v2Length; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java index 962dd284b9..76d73cc9ad 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java @@ -176,7 +176,10 @@ public Optional get(ResourceID resourceID) { Optional resource = temporaryResourceCache.getResourceFromCache(resourceID); if (comparableResourceVersions && resource.isPresent() - && res.filter(r -> ReconcileUtils.compareResourceVersions(r, resource.orElseThrow()) > 0) + && res.filter( + r -> + ReconcileUtils.validateAndCompareResourceVersions(r, resource.orElseThrow()) + > 0) .isEmpty()) { log.debug("Latest resource found in temporary cache for Resource ID: {}", resourceID); return resource; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryResourceCache.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryResourceCache.java index 1f02b36a1b..2c8934f014 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 @@ -53,13 +53,6 @@ */ public class TemporaryResourceCache { - // TODO - // requirements: - // - concurrent updates - // - no blocking i/o related updates - // - support non-event filtering - // - handle delete event - private static final Logger log = LoggerFactory.getLogger(TemporaryResourceCache.class); private final Map cache = new ConcurrentHashMap<>(); @@ -131,7 +124,7 @@ private synchronized boolean onEvent(T resource, boolean unknownState, boolean d boolean filterEvent = false; int comp = 0; if (cached != null) { - comp = ReconcileUtils.compareResourceVersions(resource, cached); + comp = ReconcileUtils.validateAndCompareResourceVersions(resource, cached); if (comp >= 0 || unknownState) { cache.remove(resourceId); // we propagate event only for our update or newer other can be discarded since we know we @@ -198,7 +191,7 @@ public synchronized void putResource(T newResource) { var cachedResource = getResourceFromCache(resourceId).orElse(null); if (cachedResource == null - || ReconcileUtils.compareResourceVersions(newResource, cachedResource) > 0) { + || ReconcileUtils.validateAndCompareResourceVersions(newResource, cachedResource) > 0) { log.debug( "Temporarily moving ahead to target version {} for resource id: {}", newResource.getMetadata().getResourceVersion(), diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtilsTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtilsTest.java index 5480eb081a..6ce2dfd061 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtilsTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtilsTest.java @@ -20,7 +20,10 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import static io.javaoperatorsdk.operator.api.reconciler.ReconcileUtils.compareResourceVersions; +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.fabric8.kubernetes.api.model.PodBuilder; + import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.*; @@ -29,30 +32,110 @@ class ReconcileUtilsTest { private static final Logger log = LoggerFactory.getLogger(ReconcileUtilsTest.class); @Test - public void compareResourceVersionsTest() { - assertThat(compareResourceVersions("11", "22")).isNegative(); - assertThat(compareResourceVersions("22", "11")).isPositive(); - assertThat(compareResourceVersions("1", "1")).isZero(); - assertThat(compareResourceVersions("11", "11")).isZero(); - assertThat(compareResourceVersions("123", "2")).isPositive(); - assertThat(compareResourceVersions("3", "211")).isNegative(); + public void validateAndCompareResourceVersionsTest() { + assertThat(ReconcileUtils.validateAndCompareResourceVersions("11", "22")).isNegative(); + assertThat(ReconcileUtils.validateAndCompareResourceVersions("22", "11")).isPositive(); + assertThat(ReconcileUtils.validateAndCompareResourceVersions("1", "1")).isZero(); + assertThat(ReconcileUtils.validateAndCompareResourceVersions("11", "11")).isZero(); + assertThat(ReconcileUtils.validateAndCompareResourceVersions("123", "2")).isPositive(); + assertThat(ReconcileUtils.validateAndCompareResourceVersions("3", "211")).isNegative(); assertThrows( - NonComparableResourceVersionException.class, () -> compareResourceVersions("aa", "22")); + NonComparableResourceVersionException.class, + () -> ReconcileUtils.validateAndCompareResourceVersions("aa", "22")); assertThrows( - NonComparableResourceVersionException.class, () -> compareResourceVersions("11", "ba")); + NonComparableResourceVersionException.class, + () -> ReconcileUtils.validateAndCompareResourceVersions("11", "ba")); assertThrows( - NonComparableResourceVersionException.class, () -> compareResourceVersions("", "22")); + NonComparableResourceVersionException.class, + () -> ReconcileUtils.validateAndCompareResourceVersions("", "22")); assertThrows( - NonComparableResourceVersionException.class, () -> compareResourceVersions("11", "")); + NonComparableResourceVersionException.class, + () -> ReconcileUtils.validateAndCompareResourceVersions("11", "")); assertThrows( - NonComparableResourceVersionException.class, () -> compareResourceVersions("01", "123")); + NonComparableResourceVersionException.class, + () -> ReconcileUtils.validateAndCompareResourceVersions("01", "123")); assertThrows( - NonComparableResourceVersionException.class, () -> compareResourceVersions("123", "01")); + NonComparableResourceVersionException.class, + () -> ReconcileUtils.validateAndCompareResourceVersions("123", "01")); assertThrows( - NonComparableResourceVersionException.class, () -> compareResourceVersions("3213", "123a")); + NonComparableResourceVersionException.class, + () -> ReconcileUtils.validateAndCompareResourceVersions("3213", "123a")); assertThrows( - NonComparableResourceVersionException.class, () -> compareResourceVersions("321", "123a")); + NonComparableResourceVersionException.class, + () -> ReconcileUtils.validateAndCompareResourceVersions("321", "123a")); + } + + @Test + public void compareResourceVersionsWithStrings() { + // Test equal versions + assertThat(ReconcileUtils.compareResourceVersions("1", "1")).isZero(); + assertThat(ReconcileUtils.compareResourceVersions("123", "123")).isZero(); + + // Test different lengths - shorter version is less than longer version + assertThat(ReconcileUtils.compareResourceVersions("1", "12")).isNegative(); + assertThat(ReconcileUtils.compareResourceVersions("12", "1")).isPositive(); + assertThat(ReconcileUtils.compareResourceVersions("99", "100")).isNegative(); + assertThat(ReconcileUtils.compareResourceVersions("100", "99")).isPositive(); + assertThat(ReconcileUtils.compareResourceVersions("9", "100")).isNegative(); + assertThat(ReconcileUtils.compareResourceVersions("100", "9")).isPositive(); + + // Test same length - lexicographic comparison + assertThat(ReconcileUtils.compareResourceVersions("1", "2")).isNegative(); + assertThat(ReconcileUtils.compareResourceVersions("2", "1")).isPositive(); + assertThat(ReconcileUtils.compareResourceVersions("11", "12")).isNegative(); + assertThat(ReconcileUtils.compareResourceVersions("12", "11")).isPositive(); + assertThat(ReconcileUtils.compareResourceVersions("99", "100")).isNegative(); + assertThat(ReconcileUtils.compareResourceVersions("100", "99")).isPositive(); + assertThat(ReconcileUtils.compareResourceVersions("123", "124")).isNegative(); + assertThat(ReconcileUtils.compareResourceVersions("124", "123")).isPositive(); + + // Test with non-numeric strings (algorithm should still work character-wise) + assertThat(ReconcileUtils.compareResourceVersions("a", "b")).isNegative(); + assertThat(ReconcileUtils.compareResourceVersions("b", "a")).isPositive(); + assertThat(ReconcileUtils.compareResourceVersions("abc", "abd")).isNegative(); + assertThat(ReconcileUtils.compareResourceVersions("abd", "abc")).isPositive(); + + // Test edge cases with larger numbers + assertThat(ReconcileUtils.compareResourceVersions("1234567890", "1234567891")).isNegative(); + assertThat(ReconcileUtils.compareResourceVersions("1234567891", "1234567890")).isPositive(); + } + + @Test + public void compareResourceVersionsWithHasMetadata() { + // Test equal versions + HasMetadata resource1 = createResourceWithVersion("123"); + HasMetadata resource2 = createResourceWithVersion("123"); + assertThat(ReconcileUtils.compareResourceVersions(resource1, resource2)).isZero(); + + // Test different lengths + resource1 = createResourceWithVersion("1"); + resource2 = createResourceWithVersion("12"); + assertThat(ReconcileUtils.compareResourceVersions(resource1, resource2)).isNegative(); + assertThat(ReconcileUtils.compareResourceVersions(resource2, resource1)).isPositive(); + + // Test same length, different values + resource1 = createResourceWithVersion("100"); + resource2 = createResourceWithVersion("200"); + assertThat(ReconcileUtils.compareResourceVersions(resource1, resource2)).isNegative(); + assertThat(ReconcileUtils.compareResourceVersions(resource2, resource1)).isPositive(); + + // Test realistic Kubernetes resource versions + resource1 = createResourceWithVersion("12345"); + resource2 = createResourceWithVersion("12346"); + assertThat(ReconcileUtils.compareResourceVersions(resource1, resource2)).isNegative(); + assertThat(ReconcileUtils.compareResourceVersions(resource2, resource1)).isPositive(); + } + + private HasMetadata createResourceWithVersion(String resourceVersion) { + return new PodBuilder() + .withMetadata( + new ObjectMetaBuilder() + .withName("test-pod") + .withNamespace("default") + .withResourceVersion(resourceVersion) + .build()) + .build(); } // naive performance test that compares the work case scenario for the parsing and non-parsing @@ -63,7 +146,7 @@ public void compareResourcePerformanceTest() { var execNum = 30000000; var startTime = System.currentTimeMillis(); for (int i = 0; i < execNum; i++) { - var res = compareResourceVersions("123456788", "123456789"); + var res = ReconcileUtils.compareResourceVersions("123456788", "123456789"); } var dur1 = System.currentTimeMillis() - startTime; log.info("Duration without parsing: {}", dur1); From 5d7d1a65956ed2259d63ae8faa344ebd845fecc1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Tue, 6 Jan 2026 19:38:01 +0100 Subject: [PATCH 08/21] wip 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/ReconcileUtilsTest.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtilsTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtilsTest.java index 6ce2dfd061..b4258487ca 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtilsTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtilsTest.java @@ -32,7 +32,7 @@ class ReconcileUtilsTest { private static final Logger log = LoggerFactory.getLogger(ReconcileUtilsTest.class); @Test - public void validateAndCompareResourceVersionsTest() { + void validateAndCompareResourceVersionsTest() { assertThat(ReconcileUtils.validateAndCompareResourceVersions("11", "22")).isNegative(); assertThat(ReconcileUtils.validateAndCompareResourceVersions("22", "11")).isPositive(); assertThat(ReconcileUtils.validateAndCompareResourceVersions("1", "1")).isZero(); @@ -102,7 +102,7 @@ public void compareResourceVersionsWithStrings() { } @Test - public void compareResourceVersionsWithHasMetadata() { + void compareResourceVersionsWithHasMetadata() { // Test equal versions HasMetadata resource1 = createResourceWithVersion("123"); HasMetadata resource2 = createResourceWithVersion("123"); @@ -146,13 +146,13 @@ public void compareResourcePerformanceTest() { var execNum = 30000000; var startTime = System.currentTimeMillis(); for (int i = 0; i < execNum; i++) { - var res = ReconcileUtils.compareResourceVersions("123456788", "123456789"); + var res = ReconcileUtils.compareResourceVersions("123456788" + i, "123456789" + i); } var dur1 = System.currentTimeMillis() - startTime; log.info("Duration without parsing: {}", dur1); startTime = System.currentTimeMillis(); for (int i = 0; i < execNum; i++) { - var res = Long.parseLong("123456788") > Long.parseLong("123456789"); + var res = Long.parseLong("123456788" + i) > Long.parseLong("123456789" + i); } var dur2 = System.currentTimeMillis() - startTime; log.info("Duration with parsing: {}", dur2); From 61e700f5752a3bdbe282ce67061af83205c3a7f3 Mon Sep 17 00:00:00 2001 From: Steve Hawkins Date: Thu, 8 Jan 2026 09:59:14 -0500 Subject: [PATCH 09/21] updating obsolete event handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../event/source/controller/ControllerEventSource.java | 5 +++-- .../event/source/informer/TemporaryResourceCache.java | 10 +++++----- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerEventSource.java index a7a41525ce..cf694af360 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerEventSource.java @@ -81,6 +81,7 @@ public synchronized void start() { } } + @Override public synchronized void handleEvent( ResourceAction action, T resource, @@ -137,8 +138,8 @@ private boolean isAcceptedByFilters(ResourceAction action, T resource, T oldReso @Override public void onAdd(T resource) { - var knownResourceVersion = temporaryResourceCache.onAddOrUpdateEvent(resource); - handleEvent(ResourceAction.ADDED, resource, null, null, knownResourceVersion); + var obsoleteResourceVersion = temporaryResourceCache.onAddOrUpdateEvent(resource); + handleEvent(ResourceAction.ADDED, resource, null, null, obsoleteResourceVersion); } @Override 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 2c8934f014..edbad61dd1 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 @@ -99,7 +99,7 @@ public void onDeleteEvent(T resource, boolean unknownState) { } /** - * @return true if the resourceVersion was already known and not skipped for event filtering + * @return true if the resourceVersion was obsolete */ public boolean onAddOrUpdateEvent(T resource) { return onEvent(resource, false, false); @@ -121,7 +121,7 @@ private synchronized boolean onEvent(T resource, boolean unknownState, boolean d latestResourceVersion = resource.getMetadata().getResourceVersion(); } var cached = cache.get(resourceId); - boolean filterEvent = false; + boolean obsoleteEvent = false; int comp = 0; if (cached != null) { comp = ReconcileUtils.validateAndCompareResourceVersions(resource, cached); @@ -130,9 +130,9 @@ private synchronized boolean onEvent(T resource, boolean unknownState, boolean d // we propagate event only for our update or newer other can be discarded since we know we // will receive // additional event - filterEvent = false; + obsoleteEvent = comp == 0; } else { - filterEvent = true; + obsoleteEvent = true; } } var ed = activeUpdates.get(resourceId); @@ -149,7 +149,7 @@ private synchronized boolean onEvent(T resource, boolean unknownState, boolean d return true; } } else { - return filterEvent; + return obsoleteEvent; } } From b5a42a219e3cedb4af90bc82c40e2070a341f5a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Thu, 8 Jan 2026 16:18:44 +0100 Subject: [PATCH 10/21] wip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../event/source/informer/TemporaryResourceCache.java | 2 +- .../operator/api/reconciler/ReconcileUtilsTest.java | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) 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 edbad61dd1..85455b4ce7 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 @@ -130,7 +130,7 @@ private synchronized boolean onEvent(T resource, boolean unknownState, boolean d // we propagate event only for our update or newer other can be discarded since we know we // will receive // additional event - obsoleteEvent = comp == 0; + obsoleteEvent = false; } else { obsoleteEvent = true; } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtilsTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtilsTest.java index b4258487ca..464a44e6e2 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtilsTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtilsTest.java @@ -162,6 +162,7 @@ public void compareResourcePerformanceTest() { @Test void retriesAddingFinalizerWithoutSSA() { + // todo } From 87cd5d82596ed7f3bb4bf7170e8cec763d51e686 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Fri, 9 Jan 2026 13:42:31 +0100 Subject: [PATCH 11/21] Change approach how to reschedule event for backwards compatible purposes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../api/reconciler/AbstractUpdateControl.java | 36 -- .../operator/api/reconciler/BaseControl.java | 7 + .../reconciler/ErrorStatusUpdateControl.java | 2 +- .../api/reconciler/ReconcileUtils.java | 403 ++---------------- .../api/reconciler/UpdateControl.java | 2 +- .../event/ReconciliationDispatcher.java | 34 +- .../event/ReconciliationDispatcherTest.java | 43 +- .../FilterPatchEventTestReconciler.java | 2 +- .../PatchResourceWithSSAReconciler.java | 2 +- 9 files changed, 78 insertions(+), 453 deletions(-) delete mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/AbstractUpdateControl.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/AbstractUpdateControl.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/AbstractUpdateControl.java deleted file mode 100644 index 881cd210b9..0000000000 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/AbstractUpdateControl.java +++ /dev/null @@ -1,36 +0,0 @@ -/* - * Copyright Java Operator SDK Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package io.javaoperatorsdk.operator.api.reconciler; - -public abstract class AbstractUpdateControl> extends BaseControl { - - private boolean filterPatchEvent = true; - - /** - * The event from resource primary updates are filtered by default, thus does not trigger the - * reconciliation. Setting this to false will turn the filtering off. - * - * @since 5.3.0 - */ - public T filterPatchEvent(boolean filterPatchEvent) { - this.filterPatchEvent = filterPatchEvent; - return (T) this; - } - - public boolean isFilterPatchEvent() { - return filterPatchEvent; - } -} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/BaseControl.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/BaseControl.java index 5087f4052a..8b3f75e383 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/BaseControl.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/BaseControl.java @@ -21,6 +21,8 @@ public abstract class BaseControl> { + public static final Long INSTANT_RESCHEDULE = 0L; + private Long scheduleDelay = null; public T rescheduleAfter(long delay) { @@ -37,6 +39,11 @@ public T rescheduleAfter(long delay, TimeUnit timeUnit) { return rescheduleAfter(timeUnit.toMillis(delay)); } + public T reschedule() { + this.scheduleDelay = INSTANT_RESCHEDULE; + return (T) this; + } + public Optional getScheduleDelay() { return Optional.ofNullable(scheduleDelay); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ErrorStatusUpdateControl.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ErrorStatusUpdateControl.java index d51cb72720..2636fea879 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ErrorStatusUpdateControl.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ErrorStatusUpdateControl.java @@ -21,7 +21,7 @@ import io.fabric8.kubernetes.api.model.HasMetadata; public class ErrorStatusUpdateControl

- extends AbstractUpdateControl> { + extends BaseControl> { private final P resource; private boolean noRetry = false; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtils.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtils.java index 670a5625b0..5ad6671705 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtils.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtils.java @@ -43,22 +43,6 @@ public class ReconcileUtils { private ReconcileUtils() {} - /** - * Server-Side Apply the resource and filters out the resulting event. This is a convenience - * method that calls {@link #serverSideApply(Context, HasMetadata, boolean)} with filterEvent set - * to true. - * - * @param context of reconciler - * @param resource fresh resource for server side apply - * @return updated resource - * @param resource type - * @see #serverSideApply(Context, HasMetadata, boolean) - */ - public static R serverSideApply( - Context context, R resource) { - return serverSideApply(context, resource, true); - } - /** * Updates the resource and caches the response if needed, thus making sure that next * reconciliation will contain to updated resource. Or more recent one if someone did an update @@ -71,13 +55,11 @@ public static R serverSideApply( * * @param context of reconciler * @param resource fresh resource for server side apply - * @param filterEvent if true the event from this update will be filtered out so won't trigger the - * reconciliation * @return updated resource * @param resource type */ public static R serverSideApply( - Context context, R resource, boolean filterEvent) { + Context context, R resource) { return resourcePatch( context, resource, @@ -90,24 +72,7 @@ public static R serverSideApply( .withForce(true) .withFieldManager(context.getControllerConfiguration().fieldManager()) .withPatchType(PatchType.SERVER_SIDE_APPLY) - .build()), - filterEvent); - } - - /** - * Server-Side Apply the resource status subresource and filters out the resulting event. This is - * a convenience method that calls {@link #serverSideApplyStatus(Context, HasMetadata, boolean)} - * with filterEvent set to true. - * - * @param context of reconciler - * @param resource fresh resource for server side apply - * @return updated resource - * @param resource type - * @see #serverSideApplyStatus(Context, HasMetadata, boolean) - */ - public static R serverSideApplyStatus( - Context context, R resource) { - return serverSideApplyStatus(context, resource, true); + .build())); } /** @@ -116,13 +81,11 @@ public static R serverSideApplyStatus( * * @param context of reconciler * @param resource fresh resource for server side apply - * @param filterEvent if true the event from this update will be filtered out so won't trigger the - * reconciliation * @return updated resource * @param resource type */ public static R serverSideApplyStatus( - Context context, R resource, boolean filterEvent) { + Context context, R resource) { return resourcePatch( context, resource, @@ -136,23 +99,7 @@ public static R serverSideApplyStatus( .withForce(true) .withFieldManager(context.getControllerConfiguration().fieldManager()) .withPatchType(PatchType.SERVER_SIDE_APPLY) - .build()), - filterEvent); - } - - /** - * Server-Side Apply the primary resource and filters out the resulting event. This is a - * convenience method that calls {@link #serverSideApplyPrimary(Context, HasMetadata, boolean)} - * with filterEvent set to true. - * - * @param context of reconciler - * @param resource primary resource for server side apply - * @return updated resource - * @param

primary resource type - * @see #serverSideApplyPrimary(Context, HasMetadata, boolean) - */ - public static

P serverSideApplyPrimary(Context

context, P resource) { - return serverSideApplyPrimary(context, resource, true); + .build())); } /** @@ -162,13 +109,10 @@ public static

P serverSideApplyPrimary(Context

contex * * @param context of reconciler * @param resource primary resource for server side apply - * @param filterEvent if true the event from this update will be filtered out so won't trigger the - * reconciliation * @return updated resource * @param

primary resource type */ - public static

P serverSideApplyPrimary( - Context

context, P resource, boolean filterEvent) { + public static

P serverSideApplyPrimary(Context

context, P resource) { return resourcePatch( resource, r -> @@ -181,24 +125,7 @@ public static

P serverSideApplyPrimary( .withFieldManager(context.getControllerConfiguration().fieldManager()) .withPatchType(PatchType.SERVER_SIDE_APPLY) .build()), - context.eventSourceRetriever().getControllerEventSource(), - filterEvent); - } - - /** - * Server-Side Apply the primary resource status subresource and filters out the resulting event. - * This is a convenience method that calls {@link #serverSideApplyPrimaryStatus(Context, - * HasMetadata, boolean)} with filterEvent set to true. - * - * @param context of reconciler - * @param resource primary resource for server side apply - * @return updated resource - * @param

primary resource type - * @see #serverSideApplyPrimaryStatus(Context, HasMetadata, boolean) - */ - public static

P serverSideApplyPrimaryStatus( - Context

context, P resource) { - return serverSideApplyPrimaryStatus(context, resource, true); + context.eventSourceRetriever().getControllerEventSource()); } /** @@ -207,13 +134,11 @@ public static

P serverSideApplyPrimaryStatus( * * @param context of reconciler * @param resource primary resource for server side apply - * @param filterEvent if true the event from this update will be filtered out so won't trigger the - * reconciliation * @return updated resource * @param

primary resource type */ public static

P serverSideApplyPrimaryStatus( - Context

context, P resource, boolean filterEvent) { + Context

context, P resource) { return resourcePatch( resource, r -> @@ -227,24 +152,7 @@ public static

P serverSideApplyPrimaryStatus( .withFieldManager(context.getControllerConfiguration().fieldManager()) .withPatchType(PatchType.SERVER_SIDE_APPLY) .build()), - context.eventSourceRetriever().getControllerEventSource(), - filterEvent); - } - - /** - * Updates the resource and filters out the resulting event. This is a convenience method that - * calls {@link #update(Context, HasMetadata, boolean)} with filterEvent set to true. Uses - * optimistic locking based on the resource version. - * - * @param context of reconciler - * @param resource resource to update - * @return updated resource - * @param resource type - * @see #update(Context, HasMetadata, boolean) - */ - public static R update( - Context context, R resource) { - return update(context, resource, true); + context.eventSourceRetriever().getControllerEventSource()); } /** @@ -253,31 +161,12 @@ public static R update( * * @param context of reconciler * @param resource resource to update - * @param filterEvent if true the event from this update will be filtered out so won't trigger the - * reconciliation * @return updated resource * @param resource type */ public static R update( - Context context, R resource, boolean filterEvent) { - return resourcePatch( - context, resource, r -> context.getClient().resource(r).update(), filterEvent); - } - - /** - * Updates the resource status subresource and filters out the resulting event. This is a - * convenience method that calls {@link #updateStatus(Context, HasMetadata, boolean)} with - * filterEvent set to true. - * - * @param context of reconciler - * @param resource resource to update - * @return updated resource - * @param resource type - * @see #updateStatus(Context, HasMetadata, boolean) - */ - public static R updateStatus( Context context, R resource) { - return updateStatus(context, resource, true); + return resourcePatch(context, resource, r -> context.getClient().resource(r).update()); } /** @@ -285,30 +174,12 @@ public static R updateStatus( * * @param context of reconciler * @param resource resource to update - * @param filterEvent if true the event from this update will be filtered out so won't trigger the - * reconciliation * @return updated resource * @param resource type */ public static R updateStatus( - Context context, R resource, boolean filterEvent) { - return resourcePatch( - context, resource, r -> context.getClient().resource(r).updateStatus(), filterEvent); - } - - /** - * Updates the primary resource and filters out the resulting event. This is a convenience method - * that calls {@link #updatePrimary(Context, HasMetadata, boolean)} with filterEvent set to true. - * - * @param context of reconciler - * @param resource primary resource to update - * @return updated resource - * @param resource type - * @see #updatePrimary(Context, HasMetadata, boolean) - */ - public static R updatePrimary( Context context, R resource) { - return updatePrimary(context, resource, true); + return resourcePatch(context, resource, r -> context.getClient().resource(r).updateStatus()); } /** @@ -317,34 +188,15 @@ public static R updatePrimary( * * @param context of reconciler * @param resource primary resource to update - * @param filterEvent if true the event from this update will be filtered out so won't trigger the - * reconciliation * @return updated resource * @param resource type */ public static R updatePrimary( - Context context, R resource, boolean filterEvent) { + Context context, R resource) { return resourcePatch( resource, r -> context.getClient().resource(r).update(), - context.eventSourceRetriever().getControllerEventSource(), - filterEvent); - } - - /** - * Updates the primary resource status subresource and filters out the resulting event. This is a - * convenience method that calls {@link #updatePrimaryStatus(Context, HasMetadata, boolean)} with - * filterEvent set to true. - * - * @param context of reconciler - * @param resource primary resource to update - * @return updated resource - * @param resource type - * @see #updatePrimaryStatus(Context, HasMetadata, boolean) - */ - public static R updatePrimaryStatus( - Context context, R resource) { - return updatePrimaryStatus(context, resource, true); + context.eventSourceRetriever().getControllerEventSource()); } /** @@ -353,35 +205,15 @@ public static R updatePrimaryStatus( * * @param context of reconciler * @param resource primary resource to update - * @param filterEvent if true the event from this update will be filtered out so won't trigger the - * reconciliation * @return updated resource * @param resource type */ public static R updatePrimaryStatus( - Context context, R resource, boolean filterEvent) { + Context context, R resource) { return resourcePatch( resource, r -> context.getClient().resource(r).updateStatus(), - context.eventSourceRetriever().getControllerEventSource(), - filterEvent); - } - - /** - * Applies a JSON Patch to the resource and filters out the resulting event. This is a convenience - * method that calls {@link #jsonPatch(Context, HasMetadata, UnaryOperator, boolean)} with - * filterEvent set to true. - * - * @param context of reconciler - * @param resource resource to patch - * @param unaryOperator function to modify the resource - * @return updated resource - * @param resource type - * @see #jsonPatch(Context, HasMetadata, UnaryOperator, boolean) - */ - public static R jsonPatch( - Context context, R resource, UnaryOperator unaryOperator) { - return jsonPatch(context, resource, unaryOperator, true); + context.eventSourceRetriever().getControllerEventSource()); } /** @@ -391,35 +223,13 @@ public static R jsonPatch( * @param context of reconciler * @param resource resource to patch * @param unaryOperator function to modify the resource - * @param filterEvent if true the event from this update will be filtered out so won't trigger the - * reconciliation * @return updated resource * @param resource type */ public static R jsonPatch( - Context context, - R resource, - UnaryOperator unaryOperator, - boolean filterEvent) { - return resourcePatch( - context, resource, r -> context.getClient().resource(r).edit(unaryOperator), filterEvent); - } - - /** - * Applies a JSON Patch to the resource status subresource and filters out the resulting event. - * This is a convenience method that calls {@link #jsonPatchStatus(Context, HasMetadata, - * UnaryOperator, boolean)} with filterEvent set to true. - * - * @param context of reconciler - * @param resource resource to patch - * @param unaryOperator function to modify the resource - * @return updated resource - * @param resource type - * @see #jsonPatchStatus(Context, HasMetadata, UnaryOperator, boolean) - */ - public static R jsonPatchStatus( Context context, R resource, UnaryOperator unaryOperator) { - return jsonPatchStatus(context, resource, unaryOperator, true); + return resourcePatch( + context, resource, r -> context.getClient().resource(r).edit(unaryOperator)); } /** @@ -429,38 +239,13 @@ public static R jsonPatchStatus( * @param context of reconciler * @param resource resource to patch * @param unaryOperator function to modify the resource - * @param filterEvent if true the event from this update will be filtered out so won't trigger the - * reconciliation * @return updated resource * @param resource type */ public static R jsonPatchStatus( - Context context, - R resource, - UnaryOperator unaryOperator, - boolean filterEvent) { - return resourcePatch( - context, - resource, - r -> context.getClient().resource(r).editStatus(unaryOperator), - filterEvent); - } - - /** - * Applies a JSON Patch to the primary resource and filters out the resulting event. This is a - * convenience method that calls {@link #jsonPatchPrimary(Context, HasMetadata, UnaryOperator, - * boolean)} with filterEvent set to true. - * - * @param context of reconciler - * @param resource primary resource to patch - * @param unaryOperator function to modify the resource - * @return updated resource - * @param resource type - * @see #jsonPatchPrimary(Context, HasMetadata, UnaryOperator, boolean) - */ - public static R jsonPatchPrimary( Context context, R resource, UnaryOperator unaryOperator) { - return jsonPatchPrimary(context, resource, unaryOperator, true); + return resourcePatch( + context, resource, r -> context.getClient().resource(r).editStatus(unaryOperator)); } /** @@ -470,38 +255,15 @@ public static R jsonPatchPrimary( * @param context of reconciler * @param resource primary resource to patch * @param unaryOperator function to modify the resource - * @param filterEvent if true the event from this update will be filtered out so won't trigger the - * reconciliation * @return updated resource * @param resource type */ public static R jsonPatchPrimary( - Context context, - R resource, - UnaryOperator unaryOperator, - boolean filterEvent) { + Context context, R resource, UnaryOperator unaryOperator) { return resourcePatch( resource, r -> context.getClient().resource(r).edit(unaryOperator), - context.eventSourceRetriever().getControllerEventSource(), - filterEvent); - } - - /** - * Applies a JSON Patch to the primary resource status subresource and filters out the resulting - * event. This is a convenience method that calls {@link #jsonPatchPrimaryStatus(Context, - * HasMetadata, UnaryOperator, boolean)} with filterEvent set to true. - * - * @param context of reconciler - * @param resource primary resource to patch - * @param unaryOperator function to modify the resource - * @return updated resource - * @param resource type - * @see #jsonPatchPrimaryStatus(Context, HasMetadata, UnaryOperator, boolean) - */ - public static R jsonPatchPrimaryStatus( - Context context, R resource, UnaryOperator unaryOperator) { - return jsonPatchPrimaryStatus(context, resource, unaryOperator, true); + context.eventSourceRetriever().getControllerEventSource()); } /** @@ -511,38 +273,15 @@ public static R jsonPatchPrimaryStatus( * @param context of reconciler * @param resource primary resource to patch * @param unaryOperator function to modify the resource - * @param filterEvent if true the event from this update will be filtered out so won't trigger the - * reconciliation * @return updated resource * @param resource type */ public static R jsonPatchPrimaryStatus( - Context context, - R resource, - UnaryOperator unaryOperator, - boolean filterEvent) { + Context context, R resource, UnaryOperator unaryOperator) { return resourcePatch( resource, r -> context.getClient().resource(r).editStatus(unaryOperator), - context.eventSourceRetriever().getControllerEventSource(), - filterEvent); - } - - /** - * Applies a JSON Merge Patch to the resource and filters out the resulting event. This is a - * convenience method that calls {@link #jsonMergePatch(Context, HasMetadata, boolean)} with - * filterEvent set to true. JSON Merge Patch (RFC 7386) is a simpler patching strategy compared to - * JSON Patch. - * - * @param context of reconciler - * @param resource resource to patch - * @return updated resource - * @param resource type - * @see #jsonMergePatch(Context, HasMetadata, boolean) - */ - public static R jsonMergePatch( - Context context, R resource) { - return jsonMergePatch(context, resource, true); + context.eventSourceRetriever().getControllerEventSource()); } /** @@ -551,31 +290,12 @@ public static R jsonMergePatch( * * @param context of reconciler * @param resource resource to patch - * @param filterEvent if true the event from this update will be filtered out so won't trigger the - * reconciliation * @return updated resource * @param resource type */ public static R jsonMergePatch( - Context context, R resource, boolean filterEvent) { - return resourcePatch( - context, resource, r -> context.getClient().resource(r).patch(), filterEvent); - } - - /** - * Applies a JSON Merge Patch to the resource status subresource and filters out the resulting - * event. This is a convenience method that calls {@link #jsonMergePatchStatus(Context, - * HasMetadata, boolean)} with filterEvent set to true. - * - * @param context of reconciler - * @param resource resource to patch - * @return updated resource - * @param resource type - * @see #jsonMergePatchStatus(Context, HasMetadata, boolean) - */ - public static R jsonMergePatchStatus( Context context, R resource) { - return jsonMergePatchStatus(context, resource, true); + return resourcePatch(context, resource, r -> context.getClient().resource(r).patch()); } /** @@ -584,31 +304,12 @@ public static R jsonMergePatchStatus( * * @param context of reconciler * @param resource resource to patch - * @param filterEvent if true the event from this update will be filtered out so won't trigger the - * reconciliation * @return updated resource * @param resource type */ public static R jsonMergePatchStatus( - Context context, R resource, boolean filterEvent) { - return resourcePatch( - context, resource, r -> context.getClient().resource(r).patchStatus(), filterEvent); - } - - /** - * Applies a JSON Merge Patch to the primary resource and filters out the resulting event. This is - * a convenience method that calls {@link #jsonMergePatchPrimary(Context, HasMetadata, boolean)} - * with filterEvent set to true. - * - * @param context of reconciler - * @param resource primary resource to patch - * @return updated resource - * @param resource type - * @see #jsonMergePatchPrimary(Context, HasMetadata, boolean) - */ - public static R jsonMergePatchPrimary( Context context, R resource) { - return jsonMergePatchPrimary(context, resource, true); + return resourcePatch(context, resource, r -> context.getClient().resource(r).patchStatus()); } /** @@ -616,72 +317,48 @@ public static R jsonMergePatchPrimary( * event source. * * @param context of reconciler - * @param resource primary resource to patch - * @param filterEvent if true the event from this update will be filtered out so won't trigger the - * reconciliation + * @param resource primary resource to patch reconciliation * @return updated resource * @param resource type */ public static R jsonMergePatchPrimary( - Context context, R resource, boolean filterEvent) { + Context context, R resource) { return resourcePatch( resource, r -> context.getClient().resource(r).patch(), - context.eventSourceRetriever().getControllerEventSource(), - filterEvent); + context.eventSourceRetriever().getControllerEventSource()); } /** * Applies a JSON Merge Patch to the primary resource status subresource and filters out the * resulting event. This is a convenience method that calls {@link - * #jsonMergePatchPrimaryStatus(Context, HasMetadata, boolean)} with filterEvent set to true. + * #jsonMergePatchPrimaryStatus(Context, HasMetadata)} with filterEvent set to true. * * @param context of reconciler * @param resource primary resource to patch * @return updated resource * @param resource type - * @see #jsonMergePatchPrimaryStatus(Context, HasMetadata, boolean) + * @see #jsonMergePatchPrimaryStatus(Context, HasMetadata) */ public static R jsonMergePatchPrimaryStatus( Context context, R resource) { - return jsonMergePatchPrimaryStatus(context, resource, true); - } - - /** - * Applies a JSON Merge Patch to the primary resource status subresource. Caches the response - * using the controller's event source. - * - * @param context of reconciler - * @param resource primary resource to patch - * @param filterEvent if true the event from this update will be filtered out so won't trigger the - * reconciliation - * @return updated resource - * @param resource type - */ - public static R jsonMergePatchPrimaryStatus( - Context context, R resource, boolean filterEvent) { - return resourcePatch( - resource, - r -> context.getClient().resource(r).patch(), - context.eventSourceRetriever().getControllerEventSource(), - filterEvent); + return jsonMergePatchPrimaryStatus(context, resource); } /** * Internal utility method to patch a resource and cache the result. Automatically discovers the * event source for the resource type and delegates to {@link #resourcePatch(HasMetadata, - * UnaryOperator, ManagedInformerEventSource, boolean)}. + * UnaryOperator, ManagedInformerEventSource)}. * * @param context of reconciler * @param resource resource to patch * @param updateOperation operation to perform (update, patch, edit, etc.) - * @param filterEvent if true the event from this update will be filtered out * @return updated resource * @param resource type * @throws IllegalStateException if no event source or multiple event sources are found */ public static R resourcePatch( - Context context, R resource, UnaryOperator updateOperation, boolean filterEvent) { + Context context, R resource, UnaryOperator updateOperation) { var esList = context.eventSourceRetriever().getEventSourcesFor(resource.getClass()); if (esList.isEmpty()) { @@ -695,7 +372,7 @@ public static R resourcePatch( } var es = esList.get(0); if (es instanceof ManagedInformerEventSource mes) { - return resourcePatch(resource, updateOperation, mes, filterEvent); + return resourcePatch(resource, updateOperation, mes); } else { throw new IllegalStateException( "Target event source must be a subclass off " @@ -711,19 +388,13 @@ public static R resourcePatch( * @param resource resource to patch * @param updateOperation operation to perform (update, patch, edit, etc.) * @param ies the managed informer event source to use for caching - * @param filterEvent if true the event from this update will be filtered out * @return updated resource * @param resource type */ @SuppressWarnings("unchecked") public static R resourcePatch( - R resource, - UnaryOperator updateOperation, - ManagedInformerEventSource ies, - boolean filterEvent) { - return filterEvent - ? (R) ies.eventFilteringUpdateAndCacheResource(resource, updateOperation) - : (R) ies.updateAndCacheResource(resource, updateOperation); + R resource, UnaryOperator updateOperation, ManagedInformerEventSource ies) { + return (R) ies.eventFilteringUpdateAndCacheResource(resource, updateOperation); } /** @@ -825,7 +496,7 @@ public static

P conflictRetryingPatch( if (!preCondition.test(resource)) { return resource; } - return jsonPatchPrimary(context, resource, resourceChangesOperator, false); + return jsonPatchPrimary(context, resource, resourceChangesOperator); } catch (KubernetesClientException e) { log.trace("Exception during patch for resource: {}", resource); retryIndex++; @@ -906,7 +577,7 @@ public static

P addFinalizerWithSSA( resource.setMetadata(objectMeta); resource.addFinalizer(finalizerName); - return serverSideApplyPrimary(context, resource, false); + return serverSideApplyPrimary(context, resource); } catch (InstantiationException | IllegalAccessException | InvocationTargetException diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/UpdateControl.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/UpdateControl.java index 48b1bf4b50..08611ff06f 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/UpdateControl.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/UpdateControl.java @@ -20,7 +20,7 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.client.CustomResource; -public class UpdateControl

extends AbstractUpdateControl> { +public class UpdateControl

extends BaseControl> { private final P resource; private final boolean patchResource; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java index 0a1ff7d429..73539a48cc 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java @@ -183,8 +183,7 @@ private PostExecutionControl

reconcileExecution( } if (updateControl.isPatchResource()) { - updatedCustomResource = - patchResource(context, toUpdate, originalResource, updateControl.isFilterPatchEvent()); + updatedCustomResource = patchResource(context, toUpdate, originalResource); if (!useSSA) { toUpdate .getMetadata() @@ -193,8 +192,7 @@ private PostExecutionControl

reconcileExecution( } if (updateControl.isPatchStatus()) { - customResourceFacade.patchStatus( - context, toUpdate, originalResource, updateControl.isFilterPatchEvent()); + customResourceFacade.patchStatus(context, toUpdate, originalResource); } return createPostExecutionControl(updatedCustomResource, updateControl, executionScope); } @@ -232,10 +230,7 @@ public boolean isLastAttempt() { try { updatedResource = customResourceFacade.patchStatus( - context, - errorStatusUpdateControl.getResource().orElseThrow(), - originalResource, - errorStatusUpdateControl.isFilterPatchEvent()); + context, errorStatusUpdateControl.getResource().orElseThrow(), originalResource); } catch (Exception ex) { int code = ex instanceof KubernetesClientException kcex ? kcex.getCode() : -1; Level exceptionLevel = Level.ERROR; @@ -341,7 +336,7 @@ private PostExecutionControl

handleCleanup( return postExecutionControl; } - private P patchResource(Context

context, P resource, P originalResource, boolean filterEvent) { + private P patchResource(Context

context, P resource, P originalResource) { if (log.isDebugEnabled()) { log.debug( "Updating resource: {} with version: {}; SSA: {}", @@ -356,7 +351,7 @@ private P patchResource(Context

context, P resource, P originalResource, bool // addFinalizer already prevents adding an already present finalizer so no need to check resource.addFinalizer(finalizerName); } - return customResourceFacade.patchResource(context, resource, originalResource, filterEvent); + return customResourceFacade.patchResource(context, resource, originalResource); } ControllerConfiguration

configuration() { @@ -383,8 +378,7 @@ public CustomResourceFacade(ControllerConfiguration configuration, Cloner clo this.cloner = cloner; } - public R patchResource( - Context context, R resource, R originalResource, boolean filterEvent) { + public R patchResource(Context context, R resource, R originalResource) { if (log.isDebugEnabled()) { log.debug( "Trying to replace resource {}, version: {}", @@ -392,29 +386,28 @@ public R patchResource( resource.getMetadata().getResourceVersion()); } if (useSSA) { - return ReconcileUtils.serverSideApplyPrimary(context, resource, filterEvent); + return ReconcileUtils.serverSideApplyPrimary(context, resource); } else { - return ReconcileUtils.jsonPatchPrimary( - context, originalResource, r -> resource, filterEvent); + return ReconcileUtils.jsonPatchPrimary(context, originalResource, r -> resource); } } - public R patchStatus(Context context, R resource, R originalResource, boolean filterEvent) { + public R patchStatus(Context context, R resource, R originalResource) { log.trace("Patching status for resource: {} with ssa: {}", resource, useSSA); if (useSSA) { var managedFields = resource.getMetadata().getManagedFields(); try { resource.getMetadata().setManagedFields(null); - return ReconcileUtils.serverSideApplyPrimaryStatus(context, resource, filterEvent); + return ReconcileUtils.serverSideApplyPrimaryStatus(context, resource); } finally { resource.getMetadata().setManagedFields(managedFields); } } else { - return editStatus(context, resource, originalResource, filterEvent); + return editStatus(context, resource, originalResource); } } - private R editStatus(Context context, R resource, R originalResource, boolean filterEvent) { + private R editStatus(Context context, R resource, R originalResource) { String resourceVersion = resource.getMetadata().getResourceVersion(); // the cached resource should not be changed in any circumstances // that can lead to all kinds of race conditions. @@ -428,8 +421,7 @@ private R editStatus(Context context, R resource, R originalResource, boolean r -> { ReconcilerUtilsInternal.setStatus(r, ReconcilerUtilsInternal.getStatus(resource)); return r; - }, - filterEvent); + }); } finally { // restore initial resource version clonedOriginal.getMetadata().setResourceVersion(resourceVersion); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java index 017e84dcfa..13673a72d5 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java @@ -186,15 +186,13 @@ void patchesBothResourceAndStatusIfFinalizerSet() { testCustomResource.addFinalizer(DEFAULT_FINALIZER); reconciler.reconcile = (r, c) -> UpdateControl.patchResourceAndStatus(testCustomResource); - when(customResourceFacade.patchResource(any(), eq(testCustomResource), any(), anyBoolean())) + when(customResourceFacade.patchResource(any(), eq(testCustomResource), any())) .thenReturn(testCustomResource); reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); - verify(customResourceFacade, times(1)) - .patchResource(any(), eq(testCustomResource), any(), eq(true)); - verify(customResourceFacade, times(1)) - .patchStatus(any(), eq(testCustomResource), any(), eq(true)); + verify(customResourceFacade, times(1)).patchResource(any(), eq(testCustomResource), any()); + verify(customResourceFacade, times(1)).patchStatus(any(), eq(testCustomResource), any()); } @Test @@ -205,9 +203,8 @@ void patchesStatus() { reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); - verify(customResourceFacade, times(1)) - .patchStatus(any(), eq(testCustomResource), any(), eq(true)); - verify(customResourceFacade, never()).patchResource(any(), any(), any(), eq(true)); + verify(customResourceFacade, times(1)).patchStatus(any(), eq(testCustomResource), any()); + verify(customResourceFacade, never()).patchResource(any(), any(), any()); } @Test @@ -283,7 +280,7 @@ void doesNotRemovesTheSetFinalizerIfTheDeleteNotMethodInstructsIt() { reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); assertEquals(1, testCustomResource.getMetadata().getFinalizers().size()); - verify(customResourceFacade, never()).patchResource(any(), any(), any(), eq(true)); + verify(customResourceFacade, never()).patchResource(any(), any(), any()); } @Test @@ -293,9 +290,8 @@ void doesNotUpdateTheResourceIfNoUpdateUpdateControlIfFinalizerSet() { reconciler.reconcile = (r, c) -> UpdateControl.noUpdate(); reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); - verify(customResourceFacade, never()).patchResource(any(), any(), any(), eq(true)); - verify(customResourceFacade, never()) - .patchStatus(any(), eq(testCustomResource), any(), eq(true)); + verify(customResourceFacade, never()).patchResource(any(), any(), any()); + verify(customResourceFacade, never()).patchStatus(any(), eq(testCustomResource), any()); } @Test @@ -322,7 +318,7 @@ void doesNotCallDeleteIfMarkedForDeletionButNotOurFinalizer() { reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); - verify(customResourceFacade, never()).patchResource(any(), any(), any(), eq(true)); + verify(customResourceFacade, never()).patchResource(any(), any(), any()); verify(reconciler, never()).cleanup(eq(testCustomResource), any()); } @@ -403,7 +399,7 @@ void doesNotUpdatesObservedGenerationIfStatusIsNotPatchedWhenUsingSSA() throws E CustomResourceFacade facade = mock(CustomResourceFacade.class); when(config.isGenerationAware()).thenReturn(true); when(reconciler.reconcile(any(), any())).thenReturn(UpdateControl.noUpdate()); - when(facade.patchStatus(any(), any(), any(), anyBoolean())).thenReturn(observedGenResource); + when(facade.patchStatus(any(), any(), any())).thenReturn(observedGenResource); var dispatcher = init(observedGenResource, reconciler, config, facade, true); PostExecutionControl control = @@ -421,12 +417,12 @@ void doesNotPatchObservedGenerationOnCustomResourcePatch() throws Exception { when(config.isGenerationAware()).thenReturn(true); when(reconciler.reconcile(any(), any())) .thenReturn(UpdateControl.patchResource(observedGenResource)); - when(facade.patchResource(any(), any(), any(), anyBoolean())).thenReturn(observedGenResource); + when(facade.patchResource(any(), any(), any())).thenReturn(observedGenResource); var dispatcher = init(observedGenResource, reconciler, config, facade, false); dispatcher.handleExecution(executionScopeWithCREvent(observedGenResource)); - verify(facade, never()).patchStatus(any(), any(), any(), anyBoolean()); + verify(facade, never()).patchStatus(any(), any(), any()); } @Test @@ -461,8 +457,7 @@ public boolean isLastAttempt() { false) .setResource(testCustomResource)); - verify(customResourceFacade, times(1)) - .patchStatus(any(), eq(testCustomResource), any(), eq(true)); + verify(customResourceFacade, times(1)).patchStatus(any(), eq(testCustomResource), any()); verify(reconciler, times(1)).updateErrorStatus(eq(testCustomResource), any(), any()); } @@ -483,8 +478,7 @@ void callErrorStatusHandlerEvenOnFirstError() { var postExecControl = reconciliationDispatcher.handleExecution( new ExecutionScope(null, null, false, false).setResource(testCustomResource)); - verify(customResourceFacade, times(1)) - .patchStatus(any(), eq(testCustomResource), any(), eq(true)); + verify(customResourceFacade, times(1)).patchStatus(any(), eq(testCustomResource), any()); verify(reconciler, times(1)).updateErrorStatus(eq(testCustomResource), any(), any()); assertThat(postExecControl.exceptionDuringExecution()).isTrue(); } @@ -507,8 +501,7 @@ void errorHandlerCanInstructNoRetryWithUpdate() { new ExecutionScope(null, null, false, false).setResource(testCustomResource)); verify(reconciler, times(1)).updateErrorStatus(eq(testCustomResource), any(), any()); - verify(customResourceFacade, times(1)) - .patchStatus(any(), eq(testCustomResource), any(), eq(true)); + verify(customResourceFacade, times(1)).patchStatus(any(), eq(testCustomResource), any()); assertThat(postExecControl.exceptionDuringExecution()).isFalse(); } @@ -530,8 +523,7 @@ void errorHandlerCanInstructNoRetryNoUpdate() { new ExecutionScope(null, null, false, false).setResource(testCustomResource)); verify(reconciler, times(1)).updateErrorStatus(eq(testCustomResource), any(), any()); - verify(customResourceFacade, times(0)) - .patchStatus(any(), eq(testCustomResource), any(), eq(true)); + verify(customResourceFacade, times(0)).patchStatus(any(), eq(testCustomResource), any()); assertThat(postExecControl.exceptionDuringExecution()).isFalse(); } @@ -547,8 +539,7 @@ void errorStatusHandlerCanPatchResource() { reconciliationDispatcher.handleExecution( new ExecutionScope(null, null, false, false).setResource(testCustomResource)); - verify(customResourceFacade, times(1)) - .patchStatus(any(), eq(testCustomResource), any(), eq(true)); + verify(customResourceFacade, times(1)).patchStatus(any(), eq(testCustomResource), any()); verify(reconciler, times(1)).updateErrorStatus(eq(testCustomResource), any(), any()); } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/filterpatchevent/FilterPatchEventTestReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/filterpatchevent/FilterPatchEventTestReconciler.java index 3af28f5aa3..0c4c5a1f8d 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/filterpatchevent/FilterPatchEventTestReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/filterpatchevent/FilterPatchEventTestReconciler.java @@ -42,7 +42,7 @@ public UpdateControl reconcile( resource.setStatus(new FilterPatchEventTestCustomResourceStatus()); resource.getStatus().setValue(UPDATED); - return UpdateControl.patchStatus(resource).filterPatchEvent(filterPatchEvent.get()); + return UpdateControl.patchStatus(resource).reschedule(); } public int getNumberOfExecutions() { diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/patchresourcewithssa/PatchResourceWithSSAReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/patchresourcewithssa/PatchResourceWithSSAReconciler.java index 78ece627dd..a252115b80 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/patchresourcewithssa/PatchResourceWithSSAReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/patchresourcewithssa/PatchResourceWithSSAReconciler.java @@ -42,7 +42,7 @@ public UpdateControl reconcile( res.setSpec(new PatchResourceWithSSASpec()); res.getSpec().setControllerManagedValue(ADDED_VALUE); // test assumes we will run this in the next reconciliation - return UpdateControl.patchResource(res).filterPatchEvent(false); + return UpdateControl.patchResource(res).reschedule(); } else { res.setStatus(new PatchResourceWithSSAStatus()); res.getStatus().setSuccessfullyReconciled(true); From d469b278e06b0aabc10dfd96a012a3c8155b2e76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Fri, 9 Jan 2026 14:06:29 +0100 Subject: [PATCH 12/21] Instant reschedule after finalizer added MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../processing/event/ReconciliationDispatcher.java | 3 ++- .../event/source/timer/TimerEventSource.java | 9 +++++++-- .../event/source/timer/TimerEventSourceTest.java | 10 ++++++++++ 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java index 73539a48cc..82d9a3ed21 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java @@ -141,7 +141,8 @@ private PostExecutionControl

handleReconcile( } else { updatedResource = ReconcileUtils.addFinalizer(context); } - return PostExecutionControl.onlyFinalizerAdded(updatedResource); + return PostExecutionControl.onlyFinalizerAdded(updatedResource) + .withReSchedule(BaseControl.INSTANT_RESCHEDULE); } else { try { return reconcileExecution(executionScope, resourceForExecution, originalResource, context); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/timer/TimerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/timer/TimerEventSource.java index 2530c661ab..eae9663fe6 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/timer/TimerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/timer/TimerEventSource.java @@ -25,6 +25,7 @@ import org.slf4j.LoggerFactory; import io.fabric8.kubernetes.api.model.HasMetadata; +import io.javaoperatorsdk.operator.api.reconciler.BaseControl; import io.javaoperatorsdk.operator.health.Status; import io.javaoperatorsdk.operator.processing.event.Event; import io.javaoperatorsdk.operator.processing.event.ResourceID; @@ -62,8 +63,12 @@ public void scheduleOnce(ResourceID resourceID, long delay) { cancelOnceSchedule(resourceID); } EventProducerTimeTask task = new EventProducerTimeTask(resourceID); - onceTasks.put(resourceID, task); - timer.schedule(task, delay); + if (delay == BaseControl.INSTANT_RESCHEDULE) { + task.run(); + } else { + onceTasks.put(resourceID, task); + timer.schedule(task, delay); + } } @Override diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/timer/TimerEventSourceTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/timer/TimerEventSourceTest.java index f444a5e2ba..3a4e1cb80d 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/timer/TimerEventSourceTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/timer/TimerEventSourceTest.java @@ -26,6 +26,7 @@ import org.junit.jupiter.api.Test; import io.javaoperatorsdk.operator.TestUtils; +import io.javaoperatorsdk.operator.api.reconciler.BaseControl; import io.javaoperatorsdk.operator.health.Status; import io.javaoperatorsdk.operator.processing.event.Event; import io.javaoperatorsdk.operator.processing.event.EventHandler; @@ -115,6 +116,15 @@ public void eventNotFiredIfStopped() { assertThat(source.getStatus()).isEqualTo(Status.UNHEALTHY); } + @Test + public void handlesInstanceReschedule() { + var resourceID = ResourceID.fromResource(TestUtils.testCustomResource()); + + source.scheduleOnce(resourceID, BaseControl.INSTANT_RESCHEDULE); + + assertThat(eventHandler.events).hasSize(1); + } + private void untilAsserted(ThrowingRunnable assertion) { untilAsserted(INITIAL_DELAY, PERIOD, assertion); } From ca2e47e29677a8d6e5ab843ad33955143469d9f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Fri, 9 Jan 2026 14:32:46 +0100 Subject: [PATCH 13/21] wip 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/BaseControl.java | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/BaseControl.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/BaseControl.java index 8b3f75e383..6ac46ee0a6 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/BaseControl.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/BaseControl.java @@ -25,20 +25,44 @@ public abstract class BaseControl> { private Long scheduleDelay = null; + /** + * Schedules a reconciliation to occur after the specified delay in milliseconds. + * + * @param delay the delay in milliseconds after which to reschedule + * @return this control instance for fluent chaining + */ public T rescheduleAfter(long delay) { rescheduleAfter(Duration.ofMillis(delay)); return (T) this; } + /** + * Schedules a reconciliation to occur after the specified delay. + * + * @param delay the {@link Duration} after which to reschedule + * @return this control instance for fluent chaining + */ public T rescheduleAfter(Duration delay) { this.scheduleDelay = delay.toMillis(); return (T) this; } + /** + * Schedules a reconciliation to occur after the specified delay using the given time unit. + * + * @param delay the delay value + * @param timeUnit the time unit of the delay + * @return this control instance for fluent chaining + */ public T rescheduleAfter(long delay, TimeUnit timeUnit) { return rescheduleAfter(timeUnit.toMillis(delay)); } + /** + * Schedules an instant reconciliation. The reconciliation will be triggered as soon as possible. + * + * @return this control instance for fluent chaining + */ public T reschedule() { this.scheduleDelay = INSTANT_RESCHEDULE; return (T) this; From c913167e0dbbbdc5e6a9df8c6afff3779bba7f2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Fri, 9 Jan 2026 15:06:39 +0100 Subject: [PATCH 14/21] fix integration test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../filterpatchevent/FilterPatchEventTestReconciler.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/filterpatchevent/FilterPatchEventTestReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/filterpatchevent/FilterPatchEventTestReconciler.java index 0c4c5a1f8d..4a15877916 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/filterpatchevent/FilterPatchEventTestReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/filterpatchevent/FilterPatchEventTestReconciler.java @@ -42,7 +42,11 @@ public UpdateControl reconcile( resource.setStatus(new FilterPatchEventTestCustomResourceStatus()); resource.getStatus().setValue(UPDATED); - return UpdateControl.patchStatus(resource).reschedule(); + var uc = UpdateControl.patchStatus(resource); + if (!filterPatchEvent.get()) { + uc = uc.reschedule(); + } + return uc; } public int getNumberOfExecutions() { From 89da920e9792cb25c64a893d2471056cb463a921 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Fri, 9 Jan 2026 15:09:55 +0100 Subject: [PATCH 15/21] Reconile utils unit tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../api/reconciler/ReconcileUtilsTest.java | 128 +++++++++++++++++- .../FilterPatchEventTestReconciler.java | 4 +- 2 files changed, 127 insertions(+), 5 deletions(-) diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtilsTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtilsTest.java index 464a44e6e2..fc29e774df 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtilsTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtilsTest.java @@ -15,6 +15,9 @@ */ package io.javaoperatorsdk.operator.api.reconciler; +import java.util.function.UnaryOperator; + +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.slf4j.Logger; @@ -23,13 +26,55 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; import io.fabric8.kubernetes.api.model.PodBuilder; +import io.fabric8.kubernetes.client.KubernetesClient; +import io.fabric8.kubernetes.client.KubernetesClientException; +import io.fabric8.kubernetes.client.dsl.MixedOperation; +import io.fabric8.kubernetes.client.dsl.Resource; +import io.javaoperatorsdk.operator.TestUtils; +import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; +import io.javaoperatorsdk.operator.processing.event.EventSourceRetriever; +import io.javaoperatorsdk.operator.processing.event.source.controller.ControllerEventSource; +import io.javaoperatorsdk.operator.sample.simple.TestCustomResource; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.*; class ReconcileUtilsTest { private static final Logger log = LoggerFactory.getLogger(ReconcileUtilsTest.class); + private static final String FINALIZER_NAME = "test.javaoperatorsdk.io/finalizer"; + + private Context context; + private KubernetesClient client; + private MixedOperation mixedOperation; + private Resource resourceOp; + private ControllerEventSource controllerEventSource; + private ControllerConfiguration controllerConfiguration; + + @BeforeEach + @SuppressWarnings("unchecked") + void setupMocks() { + context = mock(Context.class); + client = mock(KubernetesClient.class); + mixedOperation = mock(MixedOperation.class); + resourceOp = mock(Resource.class); + controllerEventSource = mock(ControllerEventSource.class); + controllerConfiguration = mock(ControllerConfiguration.class); + + var eventSourceRetriever = mock(EventSourceRetriever.class); + + when(context.getClient()).thenReturn(client); + when(context.eventSourceRetriever()).thenReturn(eventSourceRetriever); + when(context.getControllerConfiguration()).thenReturn(controllerConfiguration); + when(controllerConfiguration.getFinalizerName()).thenReturn(FINALIZER_NAME); + when(eventSourceRetriever.getControllerEventSource()).thenReturn(controllerEventSource); + + when(client.resources(TestCustomResource.class)).thenReturn(mixedOperation); + when(mixedOperation.inNamespace(any())).thenReturn(mixedOperation); + when(mixedOperation.withName(any())).thenReturn(resourceOp); + } @Test void validateAndCompareResourceVersionsTest() { @@ -162,17 +207,94 @@ public void compareResourcePerformanceTest() { @Test void retriesAddingFinalizerWithoutSSA() { + var resource = TestUtils.testCustomResource1(); + resource.getMetadata().setResourceVersion("1"); + + when(context.getPrimaryResource()).thenReturn(resource); + + // First call throws conflict, second succeeds + when(controllerEventSource.eventFilteringUpdateAndCacheResource( + any(), any(UnaryOperator.class))) + .thenThrow(new KubernetesClientException("Conflict", 409, null)) + .thenAnswer( + invocation -> { + var res = TestUtils.testCustomResource1(); + res.getMetadata().setResourceVersion("2"); + res.addFinalizer(FINALIZER_NAME); + return res; + }); + + // Return fresh resource on retry + when(resourceOp.get()).thenReturn(resource); + + var result = ReconcileUtils.addFinalizer(context, FINALIZER_NAME); - // todo + assertThat(result).isNotNull(); + assertThat(result.hasFinalizer(FINALIZER_NAME)).isTrue(); + verify(controllerEventSource, times(2)) + .eventFilteringUpdateAndCacheResource(any(), any(UnaryOperator.class)); + verify(resourceOp, times(1)).get(); } + // todo double check @Test void nullResourceIsGracefullyHandledOnFinalizerRemovalRetry() { - // todo + var resource = TestUtils.testCustomResource1(); + resource.getMetadata().setResourceVersion("1"); + resource.addFinalizer(FINALIZER_NAME); + + when(context.getPrimaryResource()).thenReturn(resource); + + // First call throws conflict + when(controllerEventSource.eventFilteringUpdateAndCacheResource( + any(), any(UnaryOperator.class))) + .thenThrow(new KubernetesClientException("Conflict", 409, null)); + + // Return null on retry (resource was deleted) + when(resourceOp.get()).thenReturn(null); + + // Should throw NullPointerException when resource is null after retry + assertThrows( + NullPointerException.class, () -> ReconcileUtils.removeFinalizer(context, FINALIZER_NAME)); + + verify(controllerEventSource, times(1)) + .eventFilteringUpdateAndCacheResource(any(), any(UnaryOperator.class)); + verify(resourceOp, times(1)).get(); } @Test void retriesFinalizerRemovalWithFreshResource() { - // todo + var originalResource = TestUtils.testCustomResource1(); + originalResource.getMetadata().setResourceVersion("1"); + originalResource.addFinalizer(FINALIZER_NAME); + + when(context.getPrimaryResource()).thenReturn(originalResource); + + // First call throws unprocessable (422), second succeeds + when(controllerEventSource.eventFilteringUpdateAndCacheResource( + any(), any(UnaryOperator.class))) + .thenThrow(new KubernetesClientException("Unprocessable", 422, null)) + .thenAnswer( + invocation -> { + var res = TestUtils.testCustomResource1(); + res.getMetadata().setResourceVersion("3"); + // finalizer should be removed + return res; + }); + + // Return fresh resource with newer version on retry + var freshResource = TestUtils.testCustomResource1(); + freshResource.getMetadata().setResourceVersion("2"); + freshResource.addFinalizer(FINALIZER_NAME); + when(resourceOp.get()).thenReturn(freshResource); + + var result = ReconcileUtils.removeFinalizer(context, FINALIZER_NAME); + + assertThat(result).isNotNull(); + assertThat(result.getMetadata().getResourceVersion()).isEqualTo("3"); + assertThat(result.hasFinalizer(FINALIZER_NAME)).isFalse(); + verify(controllerEventSource, times(2)) + .eventFilteringUpdateAndCacheResource(any(), any(UnaryOperator.class)); + verify(resourceOp, times(1)).get(); } } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/filterpatchevent/FilterPatchEventTestReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/filterpatchevent/FilterPatchEventTestReconciler.java index 4a15877916..e7599a2881 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/filterpatchevent/FilterPatchEventTestReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/filterpatchevent/FilterPatchEventTestReconciler.java @@ -42,9 +42,9 @@ public UpdateControl reconcile( resource.setStatus(new FilterPatchEventTestCustomResourceStatus()); resource.getStatus().setValue(UPDATED); - var uc = UpdateControl.patchStatus(resource); + var uc = UpdateControl.patchStatus(resource); if (!filterPatchEvent.get()) { - uc = uc.reschedule(); + uc = uc.reschedule(); } return uc; } From 8a8a353d464e4dda43a40ad0344dbb83829212c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Fri, 9 Jan 2026 15:26:17 +0100 Subject: [PATCH 16/21] unit tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../api/reconciler/ReconcileUtils.java | 2 +- .../api/reconciler/ReconcileUtilsTest.java | 49 +++++++++---------- 2 files changed, 24 insertions(+), 27 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtils.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtils.java index 5ad6671705..035df11d80 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtils.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtils.java @@ -467,7 +467,7 @@ public static

P removeFinalizer( r.removeFinalizer(finalizerName); return r; }, - r -> r.hasFinalizer(finalizerName)); + r -> r != null && r.hasFinalizer(finalizerName)); } /** diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtilsTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtilsTest.java index fc29e774df..1e714dc8d1 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtilsTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtilsTest.java @@ -183,28 +183,6 @@ private HasMetadata createResourceWithVersion(String resourceVersion) { .build(); } - // naive performance test that compares the work case scenario for the parsing and non-parsing - // variants - @Test - @Disabled - public void compareResourcePerformanceTest() { - var execNum = 30000000; - var startTime = System.currentTimeMillis(); - for (int i = 0; i < execNum; i++) { - var res = ReconcileUtils.compareResourceVersions("123456788" + i, "123456789" + i); - } - var dur1 = System.currentTimeMillis() - startTime; - log.info("Duration without parsing: {}", dur1); - startTime = System.currentTimeMillis(); - for (int i = 0; i < execNum; i++) { - var res = Long.parseLong("123456788" + i) > Long.parseLong("123456789" + i); - } - var dur2 = System.currentTimeMillis() - startTime; - log.info("Duration with parsing: {}", dur2); - - assertThat(dur1).isLessThan(dur2); - } - @Test void retriesAddingFinalizerWithoutSSA() { var resource = TestUtils.testCustomResource1(); @@ -236,7 +214,6 @@ void retriesAddingFinalizerWithoutSSA() { verify(resourceOp, times(1)).get(); } - // todo double check @Test void nullResourceIsGracefullyHandledOnFinalizerRemovalRetry() { var resource = TestUtils.testCustomResource1(); @@ -253,9 +230,7 @@ void nullResourceIsGracefullyHandledOnFinalizerRemovalRetry() { // Return null on retry (resource was deleted) when(resourceOp.get()).thenReturn(null); - // Should throw NullPointerException when resource is null after retry - assertThrows( - NullPointerException.class, () -> ReconcileUtils.removeFinalizer(context, FINALIZER_NAME)); + ReconcileUtils.removeFinalizer(context, FINALIZER_NAME); verify(controllerEventSource, times(1)) .eventFilteringUpdateAndCacheResource(any(), any(UnaryOperator.class)); @@ -297,4 +272,26 @@ void retriesFinalizerRemovalWithFreshResource() { .eventFilteringUpdateAndCacheResource(any(), any(UnaryOperator.class)); verify(resourceOp, times(1)).get(); } + + // naive performance test that compares the work case scenario for the parsing and non-parsing + // variants + @Test + @Disabled + public void compareResourcePerformanceTest() { + var execNum = 30000000; + var startTime = System.currentTimeMillis(); + for (int i = 0; i < execNum; i++) { + var res = ReconcileUtils.compareResourceVersions("123456788" + i, "123456789" + i); + } + var dur1 = System.currentTimeMillis() - startTime; + log.info("Duration without parsing: {}", dur1); + startTime = System.currentTimeMillis(); + for (int i = 0; i < execNum; i++) { + var res = Long.parseLong("123456788" + i) > Long.parseLong("123456789" + i); + } + var dur2 = System.currentTimeMillis() - startTime; + log.info("Duration with parsing: {}", dur2); + + assertThat(dur1).isLessThan(dur2); + } } From 8afa94ec83c75f3d8efb405fb181364143338f08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Fri, 9 Jan 2026 15:40:54 +0100 Subject: [PATCH 17/21] additional unit tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../api/reconciler/ReconcileUtils.java | 8 +- .../api/reconciler/ReconcileUtilsTest.java | 82 +++++++++++++++++++ 2 files changed, 89 insertions(+), 1 deletion(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtils.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtils.java index 035df11d80..6876fb0f8a 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtils.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtils.java @@ -467,7 +467,13 @@ public static

P removeFinalizer( r.removeFinalizer(finalizerName); return r; }, - r -> r != null && r.hasFinalizer(finalizerName)); + r -> { + if (r == null) { + log.warn("Cannot remove finalizer since resource not exists."); + return false; + } + return r.hasFinalizer(finalizerName); + }); } /** diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtilsTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtilsTest.java index 1e714dc8d1..51519c1e23 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtilsTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtilsTest.java @@ -183,6 +183,88 @@ private HasMetadata createResourceWithVersion(String resourceVersion) { .build(); } + @Test + void addsFinalizer() { + var resource = TestUtils.testCustomResource1(); + resource.getMetadata().setResourceVersion("1"); + + when(context.getPrimaryResource()).thenReturn(resource); + + // Mock successful finalizer addition + when(controllerEventSource.eventFilteringUpdateAndCacheResource( + any(), any(UnaryOperator.class))) + .thenAnswer( + invocation -> { + var res = TestUtils.testCustomResource1(); + res.getMetadata().setResourceVersion("2"); + res.addFinalizer(FINALIZER_NAME); + return res; + }); + + var result = ReconcileUtils.addFinalizer(context, FINALIZER_NAME); + + assertThat(result).isNotNull(); + assertThat(result.hasFinalizer(FINALIZER_NAME)).isTrue(); + assertThat(result.getMetadata().getResourceVersion()).isEqualTo("2"); + verify(controllerEventSource, times(1)) + .eventFilteringUpdateAndCacheResource(any(), any(UnaryOperator.class)); + } + + @Test + void addsFinalizerWithSSA() { + var resource = TestUtils.testCustomResource1(); + resource.getMetadata().setResourceVersion("1"); + + when(context.getPrimaryResource()).thenReturn(resource); + + // Mock successful SSA finalizer addition + when(controllerEventSource.eventFilteringUpdateAndCacheResource( + any(), any(UnaryOperator.class))) + .thenAnswer( + invocation -> { + var res = TestUtils.testCustomResource1(); + res.getMetadata().setResourceVersion("2"); + res.addFinalizer(FINALIZER_NAME); + return res; + }); + + var result = ReconcileUtils.addFinalizerWithSSA(context, FINALIZER_NAME); + + assertThat(result).isNotNull(); + assertThat(result.hasFinalizer(FINALIZER_NAME)).isTrue(); + assertThat(result.getMetadata().getResourceVersion()).isEqualTo("2"); + verify(controllerEventSource, times(1)) + .eventFilteringUpdateAndCacheResource(any(), any(UnaryOperator.class)); + } + + @Test + void removesFinalizer() { + var resource = TestUtils.testCustomResource1(); + resource.getMetadata().setResourceVersion("1"); + resource.addFinalizer(FINALIZER_NAME); + + when(context.getPrimaryResource()).thenReturn(resource); + + // Mock successful finalizer removal + when(controllerEventSource.eventFilteringUpdateAndCacheResource( + any(), any(UnaryOperator.class))) + .thenAnswer( + invocation -> { + var res = TestUtils.testCustomResource1(); + res.getMetadata().setResourceVersion("2"); + // finalizer is removed, so don't add it + return res; + }); + + var result = ReconcileUtils.removeFinalizer(context, FINALIZER_NAME); + + assertThat(result).isNotNull(); + assertThat(result.hasFinalizer(FINALIZER_NAME)).isFalse(); + assertThat(result.getMetadata().getResourceVersion()).isEqualTo("2"); + verify(controllerEventSource, times(1)) + .eventFilteringUpdateAndCacheResource(any(), any(UnaryOperator.class)); + } + @Test void retriesAddingFinalizerWithoutSSA() { var resource = TestUtils.testCustomResource1(); From c0007a8639812d905af988d4858f79fb12c2c648 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Fri, 9 Jan 2026 15:59:26 +0100 Subject: [PATCH 18/21] Unit tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../api/reconciler/ReconcileUtilsTest.java | 87 ++++++++++++++++++- .../mysql-schema/k8s/operator.yaml | 2 +- 2 files changed, 87 insertions(+), 2 deletions(-) diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtilsTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtilsTest.java index 51519c1e23..6d8c244c83 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtilsTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ReconcileUtilsTest.java @@ -15,6 +15,8 @@ */ package io.javaoperatorsdk.operator.api.reconciler; +import java.util.Collections; +import java.util.List; import java.util.function.UnaryOperator; import org.junit.jupiter.api.BeforeEach; @@ -33,7 +35,9 @@ import io.javaoperatorsdk.operator.TestUtils; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.processing.event.EventSourceRetriever; +import io.javaoperatorsdk.operator.processing.event.source.EventSource; import io.javaoperatorsdk.operator.processing.event.source.controller.ControllerEventSource; +import io.javaoperatorsdk.operator.processing.event.source.informer.ManagedInformerEventSource; import io.javaoperatorsdk.operator.sample.simple.TestCustomResource; import static org.assertj.core.api.Assertions.assertThat; @@ -112,7 +116,7 @@ void validateAndCompareResourceVersionsTest() { } @Test - public void compareResourceVersionsWithStrings() { + void compareResourceVersionsWithStrings() { // Test equal versions assertThat(ReconcileUtils.compareResourceVersions("1", "1")).isZero(); assertThat(ReconcileUtils.compareResourceVersions("123", "123")).isZero(); @@ -355,6 +359,87 @@ void retriesFinalizerRemovalWithFreshResource() { verify(resourceOp, times(1)).get(); } + @Test + void resourcePatchWithSingleEventSource() { + var resource = TestUtils.testCustomResource1(); + resource.getMetadata().setResourceVersion("1"); + + var updatedResource = TestUtils.testCustomResource1(); + updatedResource.getMetadata().setResourceVersion("2"); + + var eventSourceRetriever = mock(EventSourceRetriever.class); + var managedEventSource = mock(ManagedInformerEventSource.class); + + when(context.eventSourceRetriever()).thenReturn(eventSourceRetriever); + when(eventSourceRetriever.getEventSourcesFor(TestCustomResource.class)) + .thenReturn(List.of(managedEventSource)); + when(managedEventSource.eventFilteringUpdateAndCacheResource(any(), any(UnaryOperator.class))) + .thenReturn(updatedResource); + + var result = ReconcileUtils.resourcePatch(context, resource, UnaryOperator.identity()); + + assertThat(result).isNotNull(); + assertThat(result.getMetadata().getResourceVersion()).isEqualTo("2"); + verify(managedEventSource, times(1)) + .eventFilteringUpdateAndCacheResource(any(), any(UnaryOperator.class)); + } + + @Test + void resourcePatchThrowsWhenNoEventSourceFound() { + var resource = TestUtils.testCustomResource1(); + var eventSourceRetriever = mock(EventSourceRetriever.class); + + when(context.eventSourceRetriever()).thenReturn(eventSourceRetriever); + when(eventSourceRetriever.getEventSourcesFor(TestCustomResource.class)) + .thenReturn(Collections.emptyList()); + + var exception = + assertThrows( + IllegalStateException.class, + () -> ReconcileUtils.resourcePatch(context, resource, UnaryOperator.identity())); + + assertThat(exception.getMessage()).contains("No event source found for type"); + } + + @Test + void resourcePatchThrowsWhenMultipleEventSourcesFound() { + var resource = TestUtils.testCustomResource1(); + var eventSourceRetriever = mock(EventSourceRetriever.class); + var eventSource1 = mock(ManagedInformerEventSource.class); + var eventSource2 = mock(ManagedInformerEventSource.class); + + when(context.eventSourceRetriever()).thenReturn(eventSourceRetriever); + when(eventSourceRetriever.getEventSourcesFor(TestCustomResource.class)) + .thenReturn(List.of(eventSource1, eventSource2)); + + var exception = + assertThrows( + IllegalStateException.class, + () -> ReconcileUtils.resourcePatch(context, resource, UnaryOperator.identity())); + + assertThat(exception.getMessage()).contains("Multiple event sources found for"); + assertThat(exception.getMessage()).contains("please provide the target event source"); + } + + @Test + void resourcePatchThrowsWhenEventSourceIsNotManagedInformer() { + var resource = TestUtils.testCustomResource1(); + var eventSourceRetriever = mock(EventSourceRetriever.class); + var nonManagedEventSource = mock(EventSource.class); + + when(context.eventSourceRetriever()).thenReturn(eventSourceRetriever); + when(eventSourceRetriever.getEventSourcesFor(TestCustomResource.class)) + .thenReturn(List.of(nonManagedEventSource)); + + var exception = + assertThrows( + IllegalStateException.class, + () -> ReconcileUtils.resourcePatch(context, resource, UnaryOperator.identity())); + + assertThat(exception.getMessage()).contains("Target event source must be a subclass off"); + assertThat(exception.getMessage()).contains("ManagedInformerEventSource"); + } + // naive performance test that compares the work case scenario for the parsing and non-parsing // variants @Test diff --git a/sample-operators/mysql-schema/k8s/operator.yaml b/sample-operators/mysql-schema/k8s/operator.yaml index a6f1214e34..10543900e9 100644 --- a/sample-operators/mysql-schema/k8s/operator.yaml +++ b/sample-operators/mysql-schema/k8s/operator.yaml @@ -39,7 +39,7 @@ spec: serviceAccountName: mysql-schema-operator # specify the ServiceAccount under which's RBAC persmissions the operator will be executed under containers: - name: operator - image: mysql-schema-operator # TODO Change this to point to your pushed mysql-schema-operator image + image: mysql-schema-operator # Change this to point to your pushed mysql-schema-operator image imagePullPolicy: IfNotPresent ports: - containerPort: 80 From e9ad24edec90db9f7146a653386a48fbd8524c4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Fri, 9 Jan 2026 17:42:12 +0100 Subject: [PATCH 19/21] fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../event/source/informer/ManagedInformerEventSource.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java index 76d73cc9ad..8d95d58ddc 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java @@ -122,7 +122,7 @@ public R eventFilteringUpdateAndCacheResource(R resourceToUpdate, UnaryOperator< handleEvent( r.getAction(), (R) r.getResource().orElseThrow(), - null, + resourceToUpdate, !(r instanceof ResourceDeleteEvent) || ((ResourceDeleteEvent) r).isDeletedFinalStateUnknown(), false)); From dc80f180aabc3772156777b74068a1201c9f260b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Fri, 9 Jan 2026 18:02:04 +0100 Subject: [PATCH 20/21] wip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../informer/ManagedInformerEventSource.java | 26 +++++++------------ 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java index 8d95d58ddc..504573b256 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java @@ -85,17 +85,6 @@ public void changeNamespaces(Set namespaces) { } } - /** - * Updates the resource and makes sure that the response is available for the next reconciliation. - * It does not filter the event produced by this update. - */ - public R updateAndCacheResource(R resourceToUpdate, UnaryOperator updateMethod) { - ResourceID id = ResourceID.fromResource(resourceToUpdate); - var updated = updateMethod.apply(resourceToUpdate); - handleRecentResourceUpdate(id, updated, resourceToUpdate); - return updated; - } - /** * Updates the resource and makes sure that the response is available for the next reconciliation. * Also makes sure that the even produced by this update is filtered, thus does not trigger the @@ -106,23 +95,26 @@ public R eventFilteringUpdateAndCacheResource(R resourceToUpdate, UnaryOperator< if (log.isDebugEnabled()) { log.debug("Update and cache: {}", id); } - R updatedResource = null; + HasMetadata[] updatedResource = new HasMetadata[1]; try { temporaryResourceCache.startEventFilteringModify(id); - updatedResource = updateMethod.apply(resourceToUpdate); - handleRecentResourceUpdate(id, updatedResource, resourceToUpdate); - return updatedResource; + updatedResource[0] = updateMethod.apply(resourceToUpdate); + handleRecentResourceUpdate(id, (R) updatedResource[0], resourceToUpdate); + return (R) updatedResource[0]; } finally { var res = temporaryResourceCache.doneEventFilterModify( id, - updatedResource == null ? null : updatedResource.getMetadata().getResourceVersion()); + updatedResource[0] == null + ? null + : updatedResource[0].getMetadata().getResourceVersion()); + res.ifPresent( r -> handleEvent( r.getAction(), (R) r.getResource().orElseThrow(), - resourceToUpdate, + (R) r.getResource().orElseThrow(), // todo handle this nicer for updates? !(r instanceof ResourceDeleteEvent) || ((ResourceDeleteEvent) r).isDeletedFinalStateUnknown(), false)); From 9c9d44a1143165176beb63e2637875eb71bb81b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Fri, 9 Jan 2026 18:15:50 +0100 Subject: [PATCH 21/21] wip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../informer/ManagedInformerEventSource.java | 38 +++++++++++-------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java index 504573b256..d88a829f99 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java @@ -95,29 +95,35 @@ public R eventFilteringUpdateAndCacheResource(R resourceToUpdate, UnaryOperator< if (log.isDebugEnabled()) { log.debug("Update and cache: {}", id); } - HasMetadata[] updatedResource = new HasMetadata[1]; + R updatedResource = null; try { temporaryResourceCache.startEventFilteringModify(id); - updatedResource[0] = updateMethod.apply(resourceToUpdate); - handleRecentResourceUpdate(id, (R) updatedResource[0], resourceToUpdate); - return (R) updatedResource[0]; + updatedResource = updateMethod.apply(resourceToUpdate); + handleRecentResourceUpdate(id, updatedResource, resourceToUpdate); + return updatedResource; } finally { var res = temporaryResourceCache.doneEventFilterModify( id, - updatedResource[0] == null - ? null - : updatedResource[0].getMetadata().getResourceVersion()); - + updatedResource == null ? null : updatedResource.getMetadata().getResourceVersion()); + var updatedForLambda = updatedResource; res.ifPresent( - r -> - handleEvent( - r.getAction(), - (R) r.getResource().orElseThrow(), - (R) r.getResource().orElseThrow(), // todo handle this nicer for updates? - !(r instanceof ResourceDeleteEvent) - || ((ResourceDeleteEvent) r).isDeletedFinalStateUnknown(), - false)); + r -> { + R latestResource = (R) r.getResource().orElseThrow(); + // for update we need to have a historic resource, this might be improved to mimic more + // realistic scenario + R prevVersionOfResource = + updatedForLambda != null + ? updatedForLambda + : (r.getAction() == ResourceAction.UPDATED ? latestResource : null); + handleEvent( + r.getAction(), + latestResource, + prevVersionOfResource, + !(r instanceof ResourceDeleteEvent) + || ((ResourceDeleteEvent) r).isDeletedFinalStateUnknown(), + false); + }); } }