From 2293e63365252c62070ccdebd1c12040b166965b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Thu, 28 May 2026 13:45:42 +0200 Subject: [PATCH 1/2] improve: extend SecondaryToPrimaryMapper so it also gets the old resource MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This might needed in some corner cases where might help with optimizations to which resource to trigger. Signed-off-by: Attila Mészáros --- .../content/en/docs/documentation/eventing.md | 19 ++++++++ .../source/SecondaryToPrimaryMapper.java | 43 ++++++++++++++++++- .../DefaultPrimaryToSecondaryIndex.java | 6 ++- .../source/informer/InformerEventSource.java | 10 ++--- .../informer/InformerEventSourceTest.java | 2 +- .../event/source/informer/MappersTest.java | 9 ++-- .../informer/PrimaryToSecondaryIndexTest.java | 2 +- 7 files changed, 76 insertions(+), 15 deletions(-) diff --git a/docs/content/en/docs/documentation/eventing.md b/docs/content/en/docs/documentation/eventing.md index 1fdb3795d7..8219288076 100644 --- a/docs/content/en/docs/documentation/eventing.md +++ b/docs/content/en/docs/documentation/eventing.md @@ -135,6 +135,25 @@ rare corner cases. Returning an empty set means that the mapper considered the s resource event as irrelevant and the SDK will thus not trigger a reconciliation of the primary resource in that situation. +`SecondaryToPrimaryMapper` exposes two methods: + +- `toPrimaryResourceIDs(R resource)` — the original mapper. Implementing it is sufficient for + the vast majority of use cases. +- `toPrimaryResourceIDs(R newResource, R oldResource)` — a variant that is the one actually + invoked by the SDK on every secondary event. Its default implementation delegates to the + single-argument method, so existing mappers keep working unchanged. + +Override the two-argument variant only in edge cases where the set of primary resources to +reconcile depends on what changed between the previous and the new version of the secondary +resource (e.g. a reference that moved from one primary to another, where both primaries need +to be reconciled). **Use it with caution:** `oldResource` is sourced from the informer cache and +is only populated for genuine update events observed while the controller is already running. +On controller startup the cache is empty, so the initial events received for resources that +already exist in the cluster are delivered as adds with `oldResource == null` — even if those +resources had been updated before the operator came up. `oldResource` is also `null` for delete +events and for events triggered through the primary-to-secondary index. Implementations must +therefore handle a `null` `oldResource` gracefully. + Adding a `SecondaryToPrimaryMapper` is typically sufficient when there is a one-to-many relationship between primary and secondary resources. The secondary resources can be mapped to its primary owner, and this is enough information to also get these secondary resources from the `Context` diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/SecondaryToPrimaryMapper.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/SecondaryToPrimaryMapper.java index 0c6126105c..6861b6592a 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/SecondaryToPrimaryMapper.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/SecondaryToPrimaryMapper.java @@ -26,9 +26,48 @@ */ @FunctionalInterface public interface SecondaryToPrimaryMapper { + /** - * @param resource - secondary - * @return set of primary resource IDs + * Maps a secondary resource to the set of primary resources that should be reconciled in + * response. Implementing this single-argument form is sufficient for the vast majority of use + * cases — prefer it unless you specifically need access to the previous version of the + * secondary resource (see {@link #toPrimaryResourceIDs(Object, Object)}). + * + * @param resource the secondary resource for which an event was received + * @return set of primary resource IDs to enqueue for reconciliation; an empty set means the + * event is irrelevant and no reconciliation is triggered */ Set toPrimaryResourceIDs(R resource); + + /** + * Variant invoked by the framework for every secondary resource event, providing both the new and + * the previous version of the resource (when available). The default implementation simply + * delegates to {@link #toPrimaryResourceIDs(Object)} and ignores {@code oldResource}, so existing + * mappers keep working unchanged. + * + *

Override this method only for edge cases where the set of primary resources to reconcile + * depends on what changed between the old and the new version of the secondary resource (for + * example, when a reference held by the secondary resource has moved from one primary to another + * and both primaries need to be reconciled). + * + *

Use with caution. {@code oldResource} is sourced from the informer cache + * and is therefore only populated for genuine update events observed while the controller is + * already running. In particular, when the controller starts up, the cache is empty and the + * initial events received for resources that already existed in the cluster are delivered as adds + * with {@code oldResource == null} (even if those resources had been updated previously). {@code + * oldResource} is also {@code null} for delete events and for events triggered through the + * primary-to-secondary index. + * + *

Implementations must therefore handle a {@code null} {@code oldResource} gracefully and not + * rely on it being present for correctness — overriding this method is intended for edge cases + * only. + * + * @param newResource the current version of the secondary resource + * @param oldResource the previous version of the secondary resource, or {@code null} if not + * available (see above) + * @return set of primary resource IDs to enqueue for reconciliation + */ + default Set toPrimaryResourceIDs(R newResource, R oldResource) { + return toPrimaryResourceIDs(newResource); + } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/DefaultPrimaryToSecondaryIndex.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/DefaultPrimaryToSecondaryIndex.java index 2b4f3814b3..c9a99aadb0 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/DefaultPrimaryToSecondaryIndex.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/DefaultPrimaryToSecondaryIndex.java @@ -33,7 +33,8 @@ public DefaultPrimaryToSecondaryIndex(SecondaryToPrimaryMapper secondaryToPri @Override public synchronized void onAddOrUpdate(R resource) { - Set primaryResources = secondaryToPrimaryMapper.toPrimaryResourceIDs(resource); + Set primaryResources = + secondaryToPrimaryMapper.toPrimaryResourceIDs(resource, null); primaryResources.forEach( primaryResource -> { var resourceSet = @@ -44,7 +45,8 @@ public synchronized void onAddOrUpdate(R resource) { @Override public synchronized void onDelete(R resource) { - Set primaryResources = secondaryToPrimaryMapper.toPrimaryResourceIDs(resource); + Set primaryResources = + secondaryToPrimaryMapper.toPrimaryResourceIDs(resource, null); primaryResources.forEach( primaryResource -> { var secondaryResources = index.get(primaryResource); 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 93d3eb5e80..f9c08bbb61 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 @@ -126,7 +126,7 @@ public synchronized void onDelete(R resource, boolean deletedFinalStateUnknown) primaryToSecondaryIndex.onDelete(resource); temporaryResourceCache.onDeleteEvent(resource, deletedFinalStateUnknown); if (acceptedByDeleteFilters(resource, deletedFinalStateUnknown)) { - propagateEvent(resource); + propagateEvent(resource, null); } }); } @@ -134,7 +134,7 @@ public synchronized void onDelete(R resource, boolean deletedFinalStateUnknown) @Override protected void handleEvent( ResourceAction action, R resource, R oldResource, Boolean deletedFinalStateUnknown) { - propagateEvent(resource); + propagateEvent(resource, oldResource); } @Override @@ -161,15 +161,15 @@ private synchronized void onAddOrUpdate(ResourceAction action, R newObject, R ol log.debug( "Propagating event for {}, resource with same version not result of a reconciliation.", action); - propagateEvent(newObject); + propagateEvent(newObject, oldObject); } else { log.debug("Event filtered out for operation: {}, resourceID: {}", action, resourceID); } } - private void propagateEvent(R object) { + private void propagateEvent(R resource, R oldResource) { var primaryResourceIdSet = - configuration().getSecondaryToPrimaryMapper().toPrimaryResourceIDs(object); + configuration().getSecondaryToPrimaryMapper().toPrimaryResourceIDs(resource, oldResource); if (primaryResourceIdSet.isEmpty()) { return; } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSourceTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSourceTest.java index fe78bd3147..6aa5493973 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSourceTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSourceTest.java @@ -87,7 +87,7 @@ void setup() { SecondaryToPrimaryMapper secondaryToPrimaryMapper = mock(SecondaryToPrimaryMapper.class); when(informerEventSourceConfiguration.getSecondaryToPrimaryMapper()) .thenReturn(secondaryToPrimaryMapper); - when(secondaryToPrimaryMapper.toPrimaryResourceIDs(any())) + when(secondaryToPrimaryMapper.toPrimaryResourceIDs(any(), any())) .thenReturn(Set.of(ResourceID.fromResource(testDeployment()))); when(informerEventSourceConfiguration.getInformerConfig()).thenReturn(informerConfig); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/MappersTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/MappersTest.java index 8d08c4e8a0..8b4a9fbff7 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/MappersTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/MappersTest.java @@ -40,7 +40,8 @@ void secondaryToPrimaryMapperFromOwnerReference() { var secondary = getConfigMap(primary); secondary.addOwnerReference(primary); - var res = Mappers.fromOwnerReferences(TestCustomResource.class).toPrimaryResourceIDs(secondary); + var res = + Mappers.fromOwnerReferences(TestCustomResource.class).toPrimaryResourceIDs(secondary, null); assertThat(res).contains(ResourceID.fromResource(primary)); } @@ -65,7 +66,7 @@ void secondaryToPrimaryMapperFromOwnerReferenceWhereGroupIdIsEmpty() { .build(); secondary.addOwnerReference(primary); - var res = Mappers.fromOwnerReferences(ConfigMap.class).toPrimaryResourceIDs(secondary); + var res = Mappers.fromOwnerReferences(ConfigMap.class).toPrimaryResourceIDs(secondary, null); assertThat(res).contains(ResourceID.fromResource(primary)); } @@ -79,7 +80,7 @@ void secondaryToPrimaryMapperFromOwnerReferenceFiltersByType() { var res = Mappers.fromOwnerReferences(TestCustomResourceOtherV1.class) - .toPrimaryResourceIDs(secondary); + .toPrimaryResourceIDs(secondary, null); assertThat(res).isEmpty(); } @@ -103,7 +104,7 @@ void fromOwnerReferenceIgnoresVersionFromApiVersion() { HasMetadata.getGroup(TestCustomResource.class) + "/v2", HasMetadata.getKind(TestCustomResource.class), false) - .toPrimaryResourceIDs(secondary); + .toPrimaryResourceIDs(secondary, null); assertThat(res).contains(ResourceID.fromResource(primary)); } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/PrimaryToSecondaryIndexTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/PrimaryToSecondaryIndexTest.java index 91bca3708c..9503085a03 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/PrimaryToSecondaryIndexTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/PrimaryToSecondaryIndexTest.java @@ -46,7 +46,7 @@ class PrimaryToSecondaryIndexTest { @BeforeEach void setup() { - when(secondaryToPrimaryMapperMock.toPrimaryResourceIDs(any())) + when(secondaryToPrimaryMapperMock.toPrimaryResourceIDs(any(), any())) .thenReturn(Set.of(primaryID1, primaryID2)); } From 2bfc29d449d40a70b3e08f4b16b91e7cce013b0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Thu, 28 May 2026 13:50:27 +0200 Subject: [PATCH 2/2] format MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../processing/event/source/SecondaryToPrimaryMapper.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/SecondaryToPrimaryMapper.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/SecondaryToPrimaryMapper.java index 6861b6592a..be38dde4a7 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/SecondaryToPrimaryMapper.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/SecondaryToPrimaryMapper.java @@ -30,12 +30,12 @@ public interface SecondaryToPrimaryMapper { /** * Maps a secondary resource to the set of primary resources that should be reconciled in * response. Implementing this single-argument form is sufficient for the vast majority of use - * cases — prefer it unless you specifically need access to the previous version of the - * secondary resource (see {@link #toPrimaryResourceIDs(Object, Object)}). + * cases — prefer it unless you specifically need access to the previous version of the secondary + * resource (see {@link #toPrimaryResourceIDs(Object, Object)}). * * @param resource the secondary resource for which an event was received - * @return set of primary resource IDs to enqueue for reconciliation; an empty set means the - * event is irrelevant and no reconciliation is triggered + * @return set of primary resource IDs to enqueue for reconciliation; an empty set means the event + * is irrelevant and no reconciliation is triggered */ Set toPrimaryResourceIDs(R resource);