From 41eb66fbee95ac40d212700c1cc746ad3124ac60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Fri, 9 May 2025 09:07:08 +0200 Subject: [PATCH 1/9] improve: remove fabric8 daily build MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I think this never worked partially because the crd generator maven plugin is not released into snapshot repo. For now I would just remove this. Signed-off-by: Attila Mészáros --- .../fabric8-next-version-schedule.yml | 30 ------- .../event/ReconciliationDispatcher.java | 79 +++++++++++++------ .../event/ReconciliationDispatcherTest.java | 7 +- 3 files changed, 60 insertions(+), 56 deletions(-) delete mode 100644 .github/workflows/fabric8-next-version-schedule.yml diff --git a/.github/workflows/fabric8-next-version-schedule.yml b/.github/workflows/fabric8-next-version-schedule.yml deleted file mode 100644 index 64d2042135..0000000000 --- a/.github/workflows/fabric8-next-version-schedule.yml +++ /dev/null @@ -1,30 +0,0 @@ -name: Fabric8 Client Snapshot Build - -env: - MAVEN_ARGS: -V -ntp -e - -concurrency: - group: ${{ github.ref }}-${{ github.workflow }} - cancel-in-progress: true -on: - schedule: - # Run on end of the day - - cron: '0 0 * * *' - workflow_dispatch: -jobs: - check_format_and_unit_tests: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - with: - ref: 'fabric8-next-version' - - name: Set up Java and Maven - uses: actions/setup-java@v4 - with: - distribution: temurin - java-version: 17 - - name: Run unit tests - run: ./mvnw ${MAVEN_ARGS} clean install --file pom.xml - - build: - uses: ./.github/workflows/build.yml \ No newline at end of file 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 c4b161ef27..c701b37fbc 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 @@ -25,6 +25,7 @@ import io.javaoperatorsdk.operator.api.reconciler.RetryInfo; import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; import io.javaoperatorsdk.operator.processing.Controller; +import io.javaoperatorsdk.operator.processing.event.source.controller.ControllerEventSource; import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.*; @@ -60,7 +61,8 @@ public ReconciliationDispatcher(Controller

controller) { new CustomResourceFacade<>( controller.getCRClient(), controller.getConfiguration(), - controller.getConfiguration().getConfigurationService().getResourceCloner())); + controller.getConfiguration().getConfigurationService().getResourceCloner(), + controller.getEventSourceManager().getControllerEventSource())); } public PostExecutionControl

handleExecution(ExecutionScope

executionScope) { @@ -175,7 +177,7 @@ private PostExecutionControl

reconcileExecution( } if (updateControl.isPatchStatus()) { - customResourceFacade.patchStatus(toUpdate, originalResource); + updatedCustomResource = customResourceFacade.patchStatus(toUpdate, originalResource); } return createPostExecutionControl(updatedCustomResource, updateControl); } @@ -315,7 +317,7 @@ private P addFinalizerWithSSA(P originalResource) { objectMeta.setNamespace(originalResource.getMetadata().getNamespace()); resource.setMetadata(objectMeta); resource.addFinalizer(configuration().getFinalizerName()); - return customResourceFacade.patchResourceWithSSA(resource); + return customResourceFacade.patchResourceWithSSA(resource, originalResource); } catch (InstantiationException | IllegalAccessException | InvocationTargetException @@ -414,15 +416,30 @@ static class CustomResourceFacade { private final boolean useSSA; private final String fieldManager; private final Cloner cloner; + private final boolean previousAnnotationForDependentResourcesEventFiltering; + private final ControllerEventSource controllerEventSource; public CustomResourceFacade( MixedOperation, Resource> resourceOperation, ControllerConfiguration configuration, - Cloner cloner) { + Cloner cloner, + ControllerEventSource controllerEventSource) { this.resourceOperation = resourceOperation; this.useSSA = configuration.getConfigurationService().useSSAToPatchPrimaryResource(); this.fieldManager = configuration.fieldManager(); + this.previousAnnotationForDependentResourcesEventFiltering = + configuration + .getConfigurationService() + .previousAnnotationForDependentResourcesEventFiltering(); this.cloner = cloner; + this.controllerEventSource = controllerEventSource; + } + + private void cachePrimaryResource(R updatedResource, R previousVersionOfResource) { + if (previousAnnotationForDependentResourcesEventFiltering) { + controllerEventSource.handleRecentResourceUpdate( + ResourceID.fromResource(updatedResource), updatedResource, previousVersionOfResource); + } } public R getResource(String namespace, String name) { @@ -434,7 +451,9 @@ public R getResource(String namespace, String name) { } public R patchResourceWithoutSSA(R resource, R originalResource) { - return resource(originalResource).edit(r -> resource); + R updated = resource(originalResource).edit(r -> resource); + cachePrimaryResource(updated, originalResource); + return updated; } public R patchResource(R resource, R originalResource) { @@ -444,32 +463,43 @@ public R patchResource(R resource, R originalResource) { ResourceID.fromResource(resource), resource.getMetadata().getResourceVersion()); } + R updated; if (useSSA) { - return patchResourceWithSSA(resource); + return patchResourceWithSSA(resource, originalResource); } else { - return resource(originalResource).edit(r -> resource); + updated = resource(originalResource).edit(r -> resource); + cachePrimaryResource(updated, originalResource); + return updated; } } public R patchStatus(R resource, R originalResource) { log.trace("Patching status for resource: {} with ssa: {}", resource, useSSA); if (useSSA) { + R updated = null; 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()); + updated = + res.subresource("status") + .patch( + new PatchContext.Builder() + .withFieldManager(fieldManager) + .withForce(true) + .withPatchType(PatchType.SERVER_SIDE_APPLY) + .build()); + return updated; } finally { resource.getMetadata().setManagedFields(managedFields); + if (updated != null) { + cachePrimaryResource(updated, originalResource); + } } } else { - return editStatus(resource, originalResource); + R updated = editStatus(resource, originalResource); + cachePrimaryResource(updated, originalResource); + return updated; } } @@ -490,14 +520,17 @@ private R editStatus(R resource, R originalResource) { } } - public R patchResourceWithSSA(R resource) { - return resource(resource) - .patch( - new PatchContext.Builder() - .withFieldManager(fieldManager) - .withForce(true) - .withPatchType(PatchType.SERVER_SIDE_APPLY) - .build()); + public R patchResourceWithSSA(R resource, R originalResource) { + R updated = + resource(resource) + .patch( + new PatchContext.Builder() + .withFieldManager(fieldManager) + .withForce(true) + .withPatchType(PatchType.SERVER_SIDE_APPLY) + .build()); + cachePrimaryResource(updated, originalResource); + return updated; } private Resource resource(R resource) { 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 89f3655356..72c4db1ad9 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 @@ -144,7 +144,8 @@ void addFinalizerOnNewResource() { verify(reconciler, never()).reconcile(ArgumentMatchers.eq(testCustomResource), any()); verify(customResourceFacade, times(1)) .patchResourceWithSSA( - argThat(testCustomResource -> testCustomResource.hasFinalizer(DEFAULT_FINALIZER))); + argThat(testCustomResource -> testCustomResource.hasFinalizer(DEFAULT_FINALIZER)), + any()); } @Test @@ -357,13 +358,13 @@ void doesNotUpdateTheResourceIfNoUpdateUpdateControlIfFinalizerSet() { void addsFinalizerIfNotMarkedForDeletionAndEmptyCustomResourceReturned() { removeFinalizers(testCustomResource); reconciler.reconcile = (r, c) -> UpdateControl.noUpdate(); - when(customResourceFacade.patchResourceWithSSA(any())).thenReturn(testCustomResource); + when(customResourceFacade.patchResourceWithSSA(any(), any())).thenReturn(testCustomResource); var postExecControl = reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); verify(customResourceFacade, times(1)) - .patchResourceWithSSA(argThat(a -> !a.getMetadata().getFinalizers().isEmpty())); + .patchResourceWithSSA(argThat(a -> !a.getMetadata().getFinalizers().isEmpty()), any()); assertThat(postExecControl.updateIsStatusPatch()).isFalse(); assertThat(postExecControl.getUpdatedCustomResource()).isPresent(); } From 80495e198208cae51dcf3e394b416f26343d9c40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Fri, 9 May 2025 09:10:07 +0200 Subject: [PATCH 2/9] revert daily build removal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../fabric8-next-version-schedule.yml | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 .github/workflows/fabric8-next-version-schedule.yml diff --git a/.github/workflows/fabric8-next-version-schedule.yml b/.github/workflows/fabric8-next-version-schedule.yml new file mode 100644 index 0000000000..64d2042135 --- /dev/null +++ b/.github/workflows/fabric8-next-version-schedule.yml @@ -0,0 +1,30 @@ +name: Fabric8 Client Snapshot Build + +env: + MAVEN_ARGS: -V -ntp -e + +concurrency: + group: ${{ github.ref }}-${{ github.workflow }} + cancel-in-progress: true +on: + schedule: + # Run on end of the day + - cron: '0 0 * * *' + workflow_dispatch: +jobs: + check_format_and_unit_tests: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + ref: 'fabric8-next-version' + - name: Set up Java and Maven + uses: actions/setup-java@v4 + with: + distribution: temurin + java-version: 17 + - name: Run unit tests + run: ./mvnw ${MAVEN_ARGS} clean install --file pom.xml + + build: + uses: ./.github/workflows/build.yml \ No newline at end of file From 5d662b82655bf60e27c319074ea19a6082d472e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Fri, 9 May 2025 10:16:16 +0200 Subject: [PATCH 3/9] fix unit test 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/ControllerEventSourceTest.java | 6 ++++++ 1 file changed, 6 insertions(+) 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 6548bbddc7..d86ab29825 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 @@ -165,6 +165,9 @@ public TestController( reconciler, new TestConfiguration(true, onAddFilter, onUpdateFilter, genericFilter), MockKubernetesClient.client(TestCustomResource.class)); + + when(eventSourceManager.getControllerEventSource()) + .thenReturn(mock(ControllerEventSource.class)); } public TestController(boolean generationAware) { @@ -176,6 +179,9 @@ public TestController(boolean generationAware) { @Override public EventSourceManager getEventSourceManager() { + if (eventSourceManager == null) { + return super.getEventSourceManager(); + } return eventSourceManager; } From 2d6a2a1f7acd1de55289ab5d45521d6730f5465c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Fri, 9 May 2025 10:44:04 +0200 Subject: [PATCH 4/9] cache option MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../operator/api/config/ConfigurationService.java | 7 +++++-- .../api/config/ConfigurationServiceOverrider.java | 13 +++++++++++++ .../processing/event/ReconciliationDispatcher.java | 12 +++++++----- 3 files changed, 25 insertions(+), 7 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java index 3ffc913c5e..7d3cb08603 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java @@ -457,14 +457,17 @@ default boolean previousAnnotationForDependentResourcesEventFiltering() { * logic, and you want to further minimize the amount of work done / updates issued by the * operator. * - * @return if resource version should be parsed (as integer) * @since 4.5.0 - * @return if resource version should be parsed (as integer) + * @return if resourceVersion should be parsed (as integer) */ default boolean parseResourceVersionsForEventFilteringAndCaching() { return false; } + default boolean cacheUpdatedResourcesViaUpdateControl() { + return false; + } + /** * {@link io.javaoperatorsdk.operator.api.reconciler.UpdateControl} patch resource or status can * either use simple patches or SSA. Setting this to {@code true}, controllers will use SSA for diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java index f420be0fff..c14616d122 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java @@ -40,6 +40,7 @@ public class ConfigurationServiceOverrider { private Boolean parseResourceVersions; private Boolean useSSAToPatchPrimaryResource; private Boolean cloneSecondaryResourcesWhenGettingFromCache; + private Boolean cacheUpdatedResourcesViaUpdateControl; @SuppressWarnings("rawtypes") private DependentResourceFactory dependentResourceFactory; @@ -188,6 +189,11 @@ public ConfigurationServiceOverrider withCloneSecondaryResourcesWhenGettingFromC return this; } + public ConfigurationServiceOverrider withCacheUpdatedResourcesViaUpdateControl(boolean value) { + this.cacheUpdatedResourcesViaUpdateControl = value; + return this; + } + public ConfigurationService build() { return new BaseConfigurationService(original.getVersion(), cloner, client) { @Override @@ -328,6 +334,13 @@ public boolean cloneSecondaryResourcesWhenGettingFromCache() { cloneSecondaryResourcesWhenGettingFromCache, ConfigurationService::cloneSecondaryResourcesWhenGettingFromCache); } + + @Override + public boolean cacheUpdatedResourcesViaUpdateControl() { + return overriddenValueOrDefault( + cacheUpdatedResourcesViaUpdateControl, + ConfigurationService::cacheUpdatedResourcesViaUpdateControl); + } }; } } 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 c701b37fbc..51c3188112 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 @@ -416,7 +416,8 @@ static class CustomResourceFacade { private final boolean useSSA; private final String fieldManager; private final Cloner cloner; - private final boolean previousAnnotationForDependentResourcesEventFiltering; + private final boolean cacheUpdatedResources; + private final ControllerEventSource controllerEventSource; public CustomResourceFacade( @@ -427,16 +428,17 @@ public CustomResourceFacade( this.resourceOperation = resourceOperation; this.useSSA = configuration.getConfigurationService().useSSAToPatchPrimaryResource(); this.fieldManager = configuration.fieldManager(); - this.previousAnnotationForDependentResourcesEventFiltering = + this.cacheUpdatedResources = configuration - .getConfigurationService() - .previousAnnotationForDependentResourcesEventFiltering(); + .getConfigurationService() + .previousAnnotationForDependentResourcesEventFiltering() + && configuration.getConfigurationService().cacheUpdatedResourcesViaUpdateControl(); this.cloner = cloner; this.controllerEventSource = controllerEventSource; } private void cachePrimaryResource(R updatedResource, R previousVersionOfResource) { - if (previousAnnotationForDependentResourcesEventFiltering) { + if (cacheUpdatedResources) { controllerEventSource.handleRecentResourceUpdate( ResourceID.fromResource(updatedResource), updatedResource, previousVersionOfResource); } From 935637c5a5d0b327bcacbddfbb1135d4007852b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Fri, 9 May 2025 11:12:24 +0200 Subject: [PATCH 5/9] flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../api/config/ConfigurationService.java | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java index 7d3cb08603..d3bac23c6e 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java @@ -464,6 +464,24 @@ default boolean parseResourceVersionsForEventFilteringAndCaching() { return false; } + /** + * If you update the primary resource from the reconciler with or without {@link + * io.javaoperatorsdk.operator.api.reconciler.UpdateControl} it is not guaranteed that for the + * next reconciliation will receive the most up-to-date resource. This is a consequence of + * Informers design in the Operator pattern. + * + *

The framework can be smart about it and make such guarantees, but this can be done + * absolutely reliably only if it bends some rules of the Kubernetes API contract. Thus, if we + * parse the resourceVersion of a primary resource as an integer and compare it with the version + * of the resource in the cache. Note that this is a gray area, since with default setup Etcd + * guarantees such monotonically increasing resource versions. But since it is still bending the + * rules by default, this feature is turned off, and work only if {@link + * #parseResourceVersionsForEventFilteringAndCaching} is also set to true. + * + *

See also {@link io.javaoperatorsdk.operator.api.reconciler.PrimaryUpdateAndCacheUtils} + * + * @return if the framework should cache the updated resource for next reconciliation + */ default boolean cacheUpdatedResourcesViaUpdateControl() { return false; } From 08e1b3445234b344106038b0d36baca9317315fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Wed, 14 May 2025 10:16:40 +0200 Subject: [PATCH 6/9] improvements MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../api/config/ConfigurationService.java | 18 +++++++++++------- .../config/ConfigurationServiceOverrider.java | 7 ++++--- .../event/ReconciliationDispatcher.java | 16 +++++++++++++--- 3 files changed, 28 insertions(+), 13 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java index d3bac23c6e..6f50c6b5c2 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java @@ -465,24 +465,28 @@ default boolean parseResourceVersionsForEventFilteringAndCaching() { } /** - * If you update the primary resource from the reconciler with or without {@link - * io.javaoperatorsdk.operator.api.reconciler.UpdateControl} it is not guaranteed that for the - * next reconciliation will receive the most up-to-date resource. This is a consequence of - * Informers design in the Operator pattern. + * Guarantees that primary resource updated by {@link + * io.javaoperatorsdk.operator.api.reconciler.UpdateControl} will be available for next + * reconciliation. * - *

The framework can be smart about it and make such guarantees, but this can be done + *

If by default you update the primary resource from reconciler regardless if you use {@link + * io.javaoperatorsdk.operator.api.reconciler.UpdateControl} or directly the Kubernetes client, it + * is not guaranteed that for the next reconciliation will receive the most up-to-date resource. + * This is a consequence of how the informers work in Operator pattern. + * + *

However, the framework can be smart about it and make such guarantees, but this can be done * absolutely reliably only if it bends some rules of the Kubernetes API contract. Thus, if we * parse the resourceVersion of a primary resource as an integer and compare it with the version * of the resource in the cache. Note that this is a gray area, since with default setup Etcd * guarantees such monotonically increasing resource versions. But since it is still bending the - * rules by default, this feature is turned off, and work only if {@link + * rules by default, this feature is turned off, and requires {@link * #parseResourceVersionsForEventFilteringAndCaching} is also set to true. * *

See also {@link io.javaoperatorsdk.operator.api.reconciler.PrimaryUpdateAndCacheUtils} * * @return if the framework should cache the updated resource for next reconciliation */ - default boolean cacheUpdatedResourcesViaUpdateControl() { + default boolean guaranteeUpdatedPrimaryIsAvailableForNextReconciliation() { return false; } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java index c14616d122..407d801fc1 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java @@ -189,7 +189,8 @@ public ConfigurationServiceOverrider withCloneSecondaryResourcesWhenGettingFromC return this; } - public ConfigurationServiceOverrider withCacheUpdatedResourcesViaUpdateControl(boolean value) { + public ConfigurationServiceOverrider withGuaranteeUpdatedPrimaryIsAvailableForNextReconciliation( + boolean value) { this.cacheUpdatedResourcesViaUpdateControl = value; return this; } @@ -336,10 +337,10 @@ public boolean cloneSecondaryResourcesWhenGettingFromCache() { } @Override - public boolean cacheUpdatedResourcesViaUpdateControl() { + public boolean guaranteeUpdatedPrimaryIsAvailableForNextReconciliation() { return overriddenValueOrDefault( cacheUpdatedResourcesViaUpdateControl, - ConfigurationService::cacheUpdatedResourcesViaUpdateControl); + ConfigurationService::guaranteeUpdatedPrimaryIsAvailableForNextReconciliation); } }; } 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 51c3188112..fbff2ebab6 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 @@ -430,11 +430,21 @@ public CustomResourceFacade( this.fieldManager = configuration.fieldManager(); this.cacheUpdatedResources = configuration - .getConfigurationService() - .previousAnnotationForDependentResourcesEventFiltering() - && configuration.getConfigurationService().cacheUpdatedResourcesViaUpdateControl(); + .getConfigurationService() + .guaranteeUpdatedPrimaryIsAvailableForNextReconciliation(); + this.cloner = cloner; this.controllerEventSource = controllerEventSource; + + if (cacheUpdatedResources + && !configuration + .getConfigurationService() + .previousAnnotationForDependentResourcesEventFiltering()) { + throw new OperatorException( + "guaranteeUpdatedPrimaryIsAvailableForNextReconciliation is set to true, but" + + " previousAnnotationForDependentResourcesEventFiltering is set to false, set this" + + " flag also to true if you want to use this feature"); + } } private void cachePrimaryResource(R updatedResource, R previousVersionOfResource) { From c1775a9036fd2d1c75d71a8c488e65aa18ad0c17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Wed, 14 May 2025 10:49:04 +0200 Subject: [PATCH 7/9] Integration test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- ...dateControlPrimaryCacheCustomResource.java | 14 +++++ .../UpdateControlPrimaryCacheIT.java | 52 ++++++++++++++++ .../UpdateControlPrimaryCacheSpec.java | 14 +++++ .../UpdateControlPrimaryCacheStatus.java | 15 +++++ .../UpdateControlPrimaryReconciler.java | 62 +++++++++++++++++++ 5 files changed, 157 insertions(+) create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/updatecontrol/UpdateControlPrimaryCacheCustomResource.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/updatecontrol/UpdateControlPrimaryCacheIT.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/updatecontrol/UpdateControlPrimaryCacheSpec.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/updatecontrol/UpdateControlPrimaryCacheStatus.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/updatecontrol/UpdateControlPrimaryReconciler.java diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/updatecontrol/UpdateControlPrimaryCacheCustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/updatecontrol/UpdateControlPrimaryCacheCustomResource.java new file mode 100644 index 0000000000..d40e30bc0d --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/updatecontrol/UpdateControlPrimaryCacheCustomResource.java @@ -0,0 +1,14 @@ +package io.javaoperatorsdk.operator.baseapi.statuscache.updatecontrol; + +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("ucpc") +public class UpdateControlPrimaryCacheCustomResource + extends CustomResource + implements Namespaced {} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/updatecontrol/UpdateControlPrimaryCacheIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/updatecontrol/UpdateControlPrimaryCacheIT.java new file mode 100644 index 0000000000..2249f70a31 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/updatecontrol/UpdateControlPrimaryCacheIT.java @@ -0,0 +1,52 @@ +package io.javaoperatorsdk.operator.baseapi.statuscache.updatecontrol; + +import java.time.Duration; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + +public class UpdateControlPrimaryCacheIT { + + public static final String TEST_1 = "test1"; + + @RegisterExtension + LocallyRunOperatorExtension extension = + LocallyRunOperatorExtension.builder() + .withReconciler(UpdateControlPrimaryReconciler.class) + .withConfigurationService( + o -> + o.withParseResourceVersions(true) + .withGuaranteeUpdatedPrimaryIsAvailableForNextReconciliation(true)) + .build(); + + @Test + void testStatusAlwaysUpToDate() { + var reconciler = extension.getReconcilerOfType(UpdateControlPrimaryReconciler.class); + + extension.create(testResource()); + + // the reconciliation is periodically triggered, the status values should be increasing + // monotonically + await() + .pollDelay(Duration.ofSeconds(1)) + .pollInterval(Duration.ofMillis(30)) + .untilAsserted( + () -> { + assertThat(reconciler.errorPresent).isFalse(); + assertThat(reconciler.latestValue).isGreaterThan(10); + }); + } + + UpdateControlPrimaryCacheCustomResource testResource() { + var res = new UpdateControlPrimaryCacheCustomResource(); + res.setMetadata(new ObjectMetaBuilder().withName(TEST_1).build()); + res.setSpec(new UpdateControlPrimaryCacheSpec()); + return res; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/updatecontrol/UpdateControlPrimaryCacheSpec.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/updatecontrol/UpdateControlPrimaryCacheSpec.java new file mode 100644 index 0000000000..556d1253da --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/updatecontrol/UpdateControlPrimaryCacheSpec.java @@ -0,0 +1,14 @@ +package io.javaoperatorsdk.operator.baseapi.statuscache.updatecontrol; + +public class UpdateControlPrimaryCacheSpec { + + private int counter = 0; + + public int getCounter() { + return counter; + } + + public void setCounter(int counter) { + this.counter = counter; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/updatecontrol/UpdateControlPrimaryCacheStatus.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/updatecontrol/UpdateControlPrimaryCacheStatus.java new file mode 100644 index 0000000000..031fd2d4ed --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/updatecontrol/UpdateControlPrimaryCacheStatus.java @@ -0,0 +1,15 @@ +package io.javaoperatorsdk.operator.baseapi.statuscache.updatecontrol; + +public class UpdateControlPrimaryCacheStatus { + + private Integer value = 0; + + public Integer getValue() { + return value; + } + + public UpdateControlPrimaryCacheStatus setValue(Integer value) { + this.value = value; + return this; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/updatecontrol/UpdateControlPrimaryReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/updatecontrol/UpdateControlPrimaryReconciler.java new file mode 100644 index 0000000000..8996f3f8cb --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/updatecontrol/UpdateControlPrimaryReconciler.java @@ -0,0 +1,62 @@ +package io.javaoperatorsdk.operator.baseapi.statuscache.updatecontrol; + +import java.util.List; + +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.EventSourceContext; +import io.javaoperatorsdk.operator.api.reconciler.Reconciler; +import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; +import io.javaoperatorsdk.operator.baseapi.statuscache.PeriodicTriggerEventSource; +import io.javaoperatorsdk.operator.processing.event.source.EventSource; + +@ControllerConfiguration +public class UpdateControlPrimaryReconciler + implements Reconciler { + + public volatile int latestValue = 0; + public volatile boolean errorPresent = false; + + @Override + public UpdateControl reconcile( + UpdateControlPrimaryCacheCustomResource resource, + Context context) { + + if (resource.getStatus() != null && resource.getStatus().getValue() != latestValue) { + errorPresent = true; + throw new IllegalStateException( + "status is not up to date. Latest value: " + + latestValue + + " status values: " + + resource.getStatus().getValue()); + } + + var freshCopy = createFreshCopy(resource); + + latestValue = resource.getStatus() == null ? 1 : resource.getStatus().getValue() + 1; + freshCopy.getStatus().setValue(latestValue); + + return UpdateControl.patchStatus(freshCopy); + } + + @Override + public List> prepareEventSources( + EventSourceContext context) { + // periodic event triggering for testing purposes + return List.of(new PeriodicTriggerEventSource<>(context.getPrimaryCache())); + } + + private UpdateControlPrimaryCacheCustomResource createFreshCopy( + UpdateControlPrimaryCacheCustomResource resource) { + var res = new UpdateControlPrimaryCacheCustomResource(); + res.setMetadata( + new ObjectMetaBuilder() + .withName(resource.getMetadata().getName()) + .withNamespace(resource.getMetadata().getNamespace()) + .build()); + res.setStatus(new UpdateControlPrimaryCacheStatus()); + + return res; + } +} From bc2cf5bd9fb170a529862f20a55c92345b0e724d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Wed, 14 May 2025 10:56:37 +0200 Subject: [PATCH 8/9] fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../operator/processing/event/ReconciliationDispatcher.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 fbff2ebab6..4b92ae9db9 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 @@ -439,7 +439,7 @@ public CustomResourceFacade( if (cacheUpdatedResources && !configuration .getConfigurationService() - .previousAnnotationForDependentResourcesEventFiltering()) { + .parseResourceVersionsForEventFilteringAndCaching()) { throw new OperatorException( "guaranteeUpdatedPrimaryIsAvailableForNextReconciliation is set to true, but" + " previousAnnotationForDependentResourcesEventFiltering is set to false, set this" From 0d49575d17f4bc937ac0db297396486759730546 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Wed, 14 May 2025 12:59:38 +0200 Subject: [PATCH 9/9] docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../en/docs/documentation/reconciler.md | 43 ++++++++++++++++--- 1 file changed, 37 insertions(+), 6 deletions(-) diff --git a/docs/content/en/docs/documentation/reconciler.md b/docs/content/en/docs/documentation/reconciler.md index b9ede8aa95..b2bda9a4c4 100644 --- a/docs/content/en/docs/documentation/reconciler.md +++ b/docs/content/en/docs/documentation/reconciler.md @@ -188,10 +188,39 @@ These utility methods come in two flavors: #### Using internal cache -In almost all cases for this purpose, you can use internal caches: +In almost all cases for this purpose, you can use internal caches. You can turn on this functionality for +`UpdateControl`, thus all updates for `UpdateControl` will be guaranteed to be present in the next reconciliation: ```java - @Override + Operator operator = new Operator(o -> o + .withParseResourceVersions(true) + .withGuaranteeUpdatedPrimaryIsAvailableForNextReconciliation(true)); +``` + +With this setup the following trivial code will cache the resource for the next reconciliation: + +```java + +@Override +public UpdateControl reconcile( + UpdateControlPrimaryCacheCustomResource resource, + Context context) { + + // your logic omitted + var freshCopy = createFreshCopy(resource); // create fresh copy for SSA + freshCopy.getStatus().setValue(statusWithState()); + + return UpdateControl.patchStatus(freshCopy); +} +``` +See related [integration test](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/updatecontrol/UpdateControlPrimaryCacheCustomResource.java). + +If you need to do update of the resource during the reconciliation you can just use `PrimaryUpdateAndCacheUtils`. +(The code below is equivalent to the one above.) +Note that this utility does not require to set `withGuaranteeUpdatedPrimaryIsAvailableForNextReconciliation` to true. + +```java +@Override public UpdateControl reconcile( StatusPatchCacheCustomResource resource, Context context) { @@ -207,10 +236,12 @@ public UpdateControl reconcile( } ``` -In the background `PrimaryUpdateAndCacheUtils.ssaPatchAndCacheStatus` puts the result of the update into an internal -cache and will make sure that the next reconciliation will contain the most recent version of the resource. Note that it -is not necessarily the version of the resource you got as response from the update, it can be newer since other parties -can do additional updates meanwhile, but if not explicitly modified, it will contain the up-to-date status. +In the background `PrimaryUpdateAndCacheUtils.ssaPatchAndCacheStatus` and also the mechanism behind `UpdateControl.patchStatus` +puts the result of the update into an internal cache and will make sure that the next reconciliation will contain the +most recent version of the resource. +Note that it is not necessarily the version of the resource you got as a response +from the update, it can be more recent since other parties can do additional updates meanwhile. +But if not explicitly modified by another party, it will contain the up-to-date status. See related integration test [here](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/statuscache/internal).