diff --git a/docs/content/en/blog/news/read-after-write-consistency.md b/docs/content/en/blog/news/read-after-write-consistency.md index 30c072e1c2..493b065c61 100644 --- a/docs/content/en/blog/news/read-after-write-consistency.md +++ b/docs/content/en/blog/news/read-after-write-consistency.md @@ -179,6 +179,11 @@ From this point the idea of the algorithm is very simple: 2. When the informer propagates an event, check if its resource version is greater than or equal to the one in the TRC. If yes, evict the resource from the TRC. 3. When the controller reads a resource from cache, it checks the TRC first, then falls back to the Informer's cache. + +The actual filtering of events for our own writes is more nuanced than a simple +"evict on RV ≥ TRC version" rule — it is driven by a per-resource state machine +that tracks in-flight writes and the events received around them. See +[Filtering events for our own updates](#filtering-events-for-our-own-updates) below. ```mermaid @@ -221,13 +226,38 @@ sequenceDiagram When we update a resource, eventually the informer will propagate an event that would trigger a reconciliation. However, this is mostly not desired. Since we already have the up-to-date resource at that point, we would like to be notified only if the resource is changed after our change. -Therefore, in addition to caching the resource, we also filter out events that contain a resource -version older than or equal to our cached resource version. - -Note that the implementation of this is relatively complex, since while performing the update we want to record all the -events received in the meantime and decide whether to propagate them further once the update request is complete. -However, this way we significantly reduce the number of reconciliations, making the whole process much more efficient. +The framework runs a per-resource *event filter window* around each in-flight +write: it records the resource version returned by our update, buffers any +related events that arrive in the meantime, and at the end of the window +decides what (if anything) to surface to the reconciler. The rules: + +- **Pure own echo**: if the only events in the window are watch events whose + resource versions match our recorded own writes (and the action is `UPDATED`), + they are filtered out — the reconciler isn't bothered. +- **Foreign change in the window**: if a resource version arrived that was *not* + one of our own writes — e.g. a third party modified the resource between two + of our updates — the framework synthesizes a single `UPDATED` event covering + the whole window (`previousResource` = the resource just before the window, + `resource` = the latest known state). The reconciler is notified once, with a + faithful before/after picture, instead of receiving each underlying watch + event individually. +- **DELETE in the middle**: if the resource was deleted at some point during + the window, that DELETE participates in the synthesis. A trailing `DELETED` + is surfaced verbatim; a DELETE-then-recreate inside the window collapses to + an `UPDATED` from the deleted state to the recreated state. +- **Held foreign events**: a foreign event that arrives *before* the matching + own write echo is buffered until the write completes. This avoids + surfacing it as foreign only to immediately overwrite it with a synthesized + echo. +- **ReList**: events arriving while the informer is performing a relist are + tagged. Because a relist may have hidden events, the framework defaults to + surfacing such events to the reconciler rather than silently filtering + them — even when they would otherwise look like our own echoes. + +This way we significantly reduce the number of reconciliations, making the whole +process much more efficient, while preserving the invariant that any +foreign change reaches the reconciler. ### The case for instant reschedule diff --git a/docs/content/en/docs/documentation/reconciler.md b/docs/content/en/docs/documentation/reconciler.md index 0681fc2a23..f4e474da30 100644 --- a/docs/content/en/docs/documentation/reconciler.md +++ b/docs/content/en/docs/documentation/reconciler.md @@ -175,6 +175,23 @@ supports stronger guarantees, both for primary and secondary resources. If this that resource again. This feature also makes sure that the reconciliation is not triggered from the event from our writes. + The filter is implemented as a per-resource *event filter window* that opens + when an update starts and closes when it completes. Inside the window: + - Pure own echoes (watch events whose resource version matches one of our + recorded own writes) are dropped. + - Foreign events received during the window are merged with the surrounding + own writes into a single synthesized `UPDATED` event so the reconciler + gets a faithful before/after picture rather than each individual watch + event. These events are carefully crafted so they correspond to a real-life scenario, + and remain fully usable by filters. + - A `DELETED` arriving in the window is propagated; a delete-then-recreate + inside the window collapses into one synthesized `UPDATED` from the + deleted state to the recreated state. + - During an informer relist the filter degrades to "surface what we see": + events received while a relist is in progress are propagated even when + they would otherwise look like own echoes, since the relist may have + hidden events. + In order to benefit from these stronger guarantees, use [`ResourceOperations`](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperations.java) from the context of the reconciliation: 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 07d59e039a..3838bf36b5 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 @@ -31,8 +31,8 @@ import io.javaoperatorsdk.operator.processing.event.source.ResourceAction; import io.javaoperatorsdk.operator.processing.event.source.filter.OnDeleteFilter; import io.javaoperatorsdk.operator.processing.event.source.filter.OnUpdateFilter; +import io.javaoperatorsdk.operator.processing.event.source.informer.GenericResourceEvent; import io.javaoperatorsdk.operator.processing.event.source.informer.ManagedInformerEventSource; -import io.javaoperatorsdk.operator.processing.event.source.informer.TemporaryResourceCache.EventHandling; import static io.javaoperatorsdk.operator.ReconcilerUtilsInternal.handleKubernetesClientException; import static io.javaoperatorsdk.operator.processing.event.source.controller.InternalEventFilters.*; @@ -141,11 +141,22 @@ private void handleOnAddOrUpdate( ResourceAction action, T oldCustomResource, T newCustomResource) { var handling = temporaryResourceCache.onAddOrUpdateEvent(action, newCustomResource, oldCustomResource); - if (handling == EventHandling.NEW) { - handleEvent(action, newCustomResource, oldCustomResource, null); - } else if (log.isDebugEnabled()) { - log.debug("{} event propagation for action: {}", handling, action); - } + handling.ifPresentOrElse( + this::handleEvent, + () -> { + if (log.isDebugEnabled()) { + log.debug("Skipping/deferring event propagation for action: {}", action); + } + }); + } + + @SuppressWarnings("unchecked") + private void handleEvent(GenericResourceEvent r) { + handleEvent( + r.getAction(), + (T) r.getResource().orElseThrow(), + (T) r.getPreviousResource().orElse(null), + r.isLastStateUnknow()); } @Override @@ -154,10 +165,10 @@ public synchronized void onDelete(T resource, boolean deletedFinalStateUnknown) resource, ResourceAction.DELETED, () -> { - temporaryResourceCache.onDeleteEvent(resource, deletedFinalStateUnknown); + var res = temporaryResourceCache.onDeleteEvent(resource, deletedFinalStateUnknown); // delete event is quite special here, that requires special care, since we clean up // caches on delete event. - handleEvent(ResourceAction.DELETED, resource, null, deletedFinalStateUnknown); + res.ifPresent(this::handleEvent); }); } 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 deleted file mode 100644 index b747c69dff..0000000000 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/EventFilterDetails.java +++ /dev/null @@ -1,72 +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.processing.event.source.informer; - -import java.util.Optional; -import java.util.function.UnaryOperator; - -import io.fabric8.kubernetes.api.model.HasMetadata; -import io.javaoperatorsdk.operator.ReconcilerUtilsInternal; -import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEvent; - -class EventFilterDetails { - - private int activeUpdates = 0; - private ResourceEvent lastEvent; - private String lastOwnUpdatedResourceVersion; - - public void increaseActiveUpdates() { - activeUpdates = activeUpdates + 1; - } - - /** - * resourceVersion is needed for case when multiple parallel updates happening inside the - * controller to prevent race condition and send event from {@link - * ManagedInformerEventSource#eventFilteringUpdateAndCacheResource(HasMetadata, UnaryOperator)} - */ - public boolean decreaseActiveUpdates(String updatedResourceVersion) { - if (updatedResourceVersion != null - && (lastOwnUpdatedResourceVersion == null - || ReconcilerUtilsInternal.compareResourceVersions( - updatedResourceVersion, lastOwnUpdatedResourceVersion) - > 0)) { - lastOwnUpdatedResourceVersion = updatedResourceVersion; - } - - activeUpdates = activeUpdates - 1; - return activeUpdates == 0; - } - - public void setLastEvent(ResourceEvent event) { - lastEvent = event; - } - - public Optional getLatestEventAfterLastUpdateEvent() { - if (lastEvent != null - && (lastOwnUpdatedResourceVersion == null - || ReconcilerUtilsInternal.compareResourceVersions( - lastEvent.getResource().orElseThrow().getMetadata().getResourceVersion(), - lastOwnUpdatedResourceVersion) - > 0)) { - return Optional.of(lastEvent); - } - return Optional.empty(); - } - - public int getActiveUpdates() { - return activeUpdates; - } -} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/EventFilterSupport.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/EventFilterSupport.java new file mode 100644 index 0000000000..ce9a8efd2c --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/EventFilterSupport.java @@ -0,0 +1,128 @@ +/* + * 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.HashMap; +import java.util.Map; +import java.util.Optional; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import io.javaoperatorsdk.operator.processing.event.ResourceID; + +class EventFilterSupport { + + private static final Logger log = LoggerFactory.getLogger(EventFilterSupport.class); + + private final Map eventFilterWindows = new HashMap<>(); + private boolean ongoingReList = false; + + public synchronized void startEventFilteringModify(ResourceID resourceID) { + var existing = eventFilterWindows.get(resourceID); + var ed = + eventFilterWindows.computeIfAbsent(resourceID, id -> new EventFilterWindow(ongoingReList)); + ed.increaseActiveUpdates(); + log.debug( + "startEventFilteringModify: id={}, windowReused={}, ongoingReList={}", + resourceID, + existing != null, + ongoingReList); + } + + public synchronized Optional doneEventFilterModify(ResourceID resourceID) { + var ed = eventFilterWindows.get(resourceID); + if (ed == null) { + log.debug("doneEventFilterModify: no window for id={}", resourceID); + return Optional.empty(); + } + ed.decreaseActiveUpdates(); + log.debug("doneEventFilterModify: id={}", resourceID); + return check(ed, resourceID); + } + + public synchronized Optional processEvent( + ResourceID resourceId, GenericResourceEvent genericResourceEvent) { + var ed = eventFilterWindows.get(resourceId); + if (ed != null) { + log.debug( + "processEvent: buffering event in window. id={}, action={}, rv={}", + resourceId, + genericResourceEvent.getAction(), + genericResourceEvent + .getResource() + .map(r -> r.getMetadata().getResourceVersion()) + .orElse("?")); + ed.addRelatedEvent(genericResourceEvent); + return check(ed, resourceId); + } else { + log.debug( + "processEvent: no active window, surfacing directly. id={}, action={}", + resourceId, + genericResourceEvent.getAction()); + return Optional.of(genericResourceEvent); + } + } + + private Optional check( + EventFilterWindow eventFilterWindow, ResourceID resourceID) { + var res = eventFilterWindow.check(); + if (eventFilterWindow.canBeRemoved()) { + log.debug("Removing empty event filter window. id={}", resourceID); + eventFilterWindows.remove(resourceID); + } + return res; + } + + public synchronized void addToOwnResourceVersions(ResourceID resourceId, String resourceVersion) { + var window = eventFilterWindows.get(resourceId); + if (window != null) { + log.debug("Recording own resourceVersion. id={}, rv={}", resourceId, resourceVersion); + window.addToOwnResourceVersions(resourceVersion); + } else { + log.debug( + "addToOwnResourceVersions: no active window for id={}, rv={} (skipped)", + resourceId, + resourceVersion); + } + } + + public synchronized void handleGhostResourceRemoval(ResourceID resourceId) { + log.debug("Ghost resource removal: discarding event filter window. id={}", resourceId); + eventFilterWindows.remove(resourceId); + } + + // for testing purposes + synchronized Map getEventFilterWindows() { + return eventFilterWindows; + } + + public synchronized void setStartingReList() { + log.debug("ReList starting: tagging {} active window(s)", eventFilterWindows.size()); + ongoingReList = true; + eventFilterWindows.values().forEach(EventFilterWindow::setReListStarted); + } + + public synchronized void setRelistFinished() { + log.debug("ReList finished: clearing tag from {} active window(s)", eventFilterWindows.size()); + ongoingReList = false; + eventFilterWindows.values().forEach(EventFilterWindow::setReListFinished); + } + + public synchronized boolean isActiveUpdateFor(ResourceID resourceId) { + return eventFilterWindows.containsKey(resourceId); + } +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/EventFilterWindow.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/EventFilterWindow.java new file mode 100644 index 0000000000..b52a86362c --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/EventFilterWindow.java @@ -0,0 +1,241 @@ +/* + * 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.Optional; +import java.util.SortedMap; +import java.util.SortedSet; +import java.util.TreeMap; +import java.util.TreeSet; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import io.javaoperatorsdk.operator.processing.event.source.ResourceAction; + +/** + * Contains all the relevant information around the event filtering algorithms of a single + * resources. + * + *

How does it work: + * + *

    + *
  • we continuously process incoming events from the informer + *
  • we record the resource version of the updated resources for our own writes + *
+ * + *

The goal: + * + *

    + *
  • is to filter out events for which we are sure that results of our own updates. Note that + * updates can happen before our updates and after or between two updates since we don't + * require optimistic locking. + *
  • if we have to emit an event we should make it equivalent to a real life like event and + * should be as wide as possible + *
  • we receive events from informers, informers sometimes do relist. Meaning there might be + * events lost. But we have callback when that is going on. + *
  • we should emit events as soon as possible, thus for example we have two parallel updates, + * we see that we have an additional event before our first update received but recording + * already started. We should emit the synth event from this check method as soon as we + * received an event that has same resource version or newer as our resource. + *
+ */ +class EventFilterWindow { + + private static final Logger log = LoggerFactory.getLogger(EventFilterWindow.class); + + private final SortedMap relatedEvents = new TreeMap<>(); + private final SortedSet ownResourceVersions = new TreeSet<>(); + private boolean reListOnGoing; + private int activeUpdates = 0; + + public EventFilterWindow(boolean reListOnGoing) { + this.reListOnGoing = reListOnGoing; + } + + public synchronized Optional check() { + String beforeState = log.isDebugEnabled() ? snapshotState() : null; + Optional result = doCheck(); + if (log.isDebugEnabled()) { + log.debug( + "check() input state: {} → outcome: {} → state after: {}", + beforeState, + result.map(GenericResourceEvent::toString).orElse("empty"), + snapshotState()); + } + return result; + } + + private String snapshotState() { + return String.format( + "relatedEvents=%s, ownResourceVersions=%s, activeUpdates=%d, reListOnGoing=%s", + relatedEvents.keySet(), ownResourceVersions, activeUpdates, reListOnGoing); + } + + private Optional doCheck() { + if (relatedEvents.isEmpty()) { + return Optional.empty(); + } + if (activeUpdates == 0 && ownResourceVersions.isEmpty()) { + return eventForRangeAndClear(relatedEvents, ownResourceVersions); + } + if (ownResourceVersions.isEmpty() + && getFirstRelatedEvent().getAction().equals(ResourceAction.DELETED)) { + return eventForRangeAndClear(relatedEvents, ownResourceVersions); + } + + var lastEventVersion = relatedEvents.lastKey(); + var numberOwnUpdatesSelected = 0; + long lastOwnVersion = -1; + for (long ownVersion : ownResourceVersions) { + if (ownVersion <= lastEventVersion) { + numberOwnUpdatesSelected++; + lastOwnVersion = ownVersion; + } else { + break; + } + } + if (numberOwnUpdatesSelected > 0) { + if (numberOwnUpdatesSelected == ownResourceVersions.size() && activeUpdates == 0) { + return eventForRangeAndClear(relatedEvents, ownResourceVersions); + } else { + if (numberOwnUpdatesSelected < ownResourceVersions.size()) { + return eventForRangeAndClear( + relatedEvents.headMap(ownResourceVersions.tailSet(lastOwnVersion + 1).first()), + ownResourceVersions.headSet(lastOwnVersion + 1)); + } else + return eventForRangeAndClear( + relatedEvents.headMap(lastOwnVersion + 1), + ownResourceVersions.headSet(lastOwnVersion + 1)); + } + } + return Optional.empty(); + } + + // it has responsibility to clear those ranges and emit event if needed + Optional eventForRangeAndClear( + SortedMap events, SortedSet ownResourceVersions) { + if (events.isEmpty()) { + return Optional.empty(); + } + var isAnyEventFromReList = + events.values().stream().anyMatch(GenericResourceEvent::isPartOfReList); + + var first = getFirstRelatedEvent(events); + if (events.size() > 1 && first.getAction() == ResourceAction.DELETED) { + events.remove(events.firstKey()); + first = getFirstRelatedEvent(events); + } + + if (events.keySet().equals(ownResourceVersions) && !isAnyEventFromReList) { + GenericResourceEvent res = null; + var lastEvent = getLastRelatedEvent(events); + if (lastEvent.getAction() == ResourceAction.DELETED) { + res = lastEvent; + } + events.clear(); + ownResourceVersions.clear(); + return Optional.ofNullable(res); + } + + if (events.size() == 1) { + ownResourceVersions.clear(); + var res = Optional.of(events.values().iterator().next()); + events.clear(); + return res; + } + var lastEvent = getLastRelatedEvent(events); + if (lastEvent.getAction() == ResourceAction.DELETED) { + events.clear(); + ownResourceVersions.clear(); + return Optional.of(lastEvent); + } + + var res = + Optional.of( + new GenericResourceEvent( + ResourceAction.UPDATED, + lastEvent.getResource().orElseThrow(), + first.getPreviousResource().isEmpty() + ? first.getResource().orElseThrow() + : first.getPreviousResource().orElseThrow(), + null)); + events.clear(); + ownResourceVersions.clear(); + return res; + } + + private GenericResourceEvent getFirstRelatedEvent() { + return getFirstRelatedEvent(relatedEvents); + } + + private GenericResourceEvent getFirstRelatedEvent(SortedMap subMap) { + return subMap.values().iterator().next(); + } + + private GenericResourceEvent getLastRelatedEvent(SortedMap subMap) { + return subMap.get(subMap.lastKey()); + } + + private GenericResourceEvent getLastRelatedEvent() { + return getLastRelatedEvent(relatedEvents); + } + + public synchronized boolean canBeRemoved() { + if (activeUpdates == 0 && ownResourceVersions.isEmpty() && relatedEvents.isEmpty()) { + return true; + } + return false; + } + + public synchronized void addToOwnResourceVersions(String resourceVersion) { + ownResourceVersions.add(Long.parseLong(resourceVersion)); + } + + public synchronized void addRelatedEvent(GenericResourceEvent event) { + if (reListOnGoing) { + event.setPartOfReList(true); + } + + relatedEvents.put( + Long.parseLong(event.getResource().orElseThrow().getMetadata().getResourceVersion()), + event); + } + + public synchronized void setReListStarted() { + reListOnGoing = true; + } + + public synchronized void setReListFinished() { + reListOnGoing = false; + } + + public synchronized void increaseActiveUpdates() { + activeUpdates++; + } + + public synchronized void decreaseActiveUpdates() { + activeUpdates--; + } + + synchronized SortedMap getRelatedEvents() { + return relatedEvents; + } + + synchronized SortedSet getOwnResourceVersions() { + return ownResourceVersions; + } +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ExtendedResourceEvent.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/GenericResourceEvent.java similarity index 72% rename from operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ExtendedResourceEvent.java rename to operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/GenericResourceEvent.java index 5d30d1b0e1..0d5a935a5f 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ExtendedResourceEvent.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/GenericResourceEvent.java @@ -24,26 +24,33 @@ import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEvent; /** Used only for resource event filtering. */ -public class ExtendedResourceEvent extends ResourceEvent { +public class GenericResourceEvent extends ResourceEvent { private final HasMetadata previousResource; + private final Boolean lastStateUnknow; + private boolean partOfReList = false; - public ExtendedResourceEvent( + public GenericResourceEvent( ResourceAction action, - ResourceID resourceID, HasMetadata latestResource, - HasMetadata previousResource) { - super(action, resourceID, latestResource); + HasMetadata previousResource, + Boolean lastStateUnknow) { + super(action, ResourceID.fromResource(latestResource), latestResource); this.previousResource = previousResource; + this.lastStateUnknow = lastStateUnknow; } public Optional getPreviousResource() { return Optional.ofNullable(previousResource); } + public Boolean isLastStateUnknow() { + return lastStateUnknow; + } + @Override public String toString() { - return "ExtendedResourceEvent{" + return "GenericResourceEvent{" + getPreviousResource() .map(r -> "previousResourceVersion=" + r.getMetadata().getResourceVersion()) .orElse("") @@ -61,7 +68,7 @@ public String toString() { public boolean equals(Object o) { if (o == null || getClass() != o.getClass()) return false; if (!super.equals(o)) return false; - ExtendedResourceEvent that = (ExtendedResourceEvent) o; + GenericResourceEvent that = (GenericResourceEvent) o; return Objects.equals(previousResource, that.previousResource); } @@ -69,4 +76,16 @@ public boolean equals(Object o) { public int hashCode() { return Objects.hash(super.hashCode(), previousResource); } + + public long getResourceVersion() { + return Long.parseLong(getResource().orElseThrow().getMetadata().getResourceVersion()); + } + + public boolean isPartOfReList() { + return partOfReList; + } + + public void setPartOfReList(boolean partOfReList) { + this.partOfReList = partOfReList; + } } 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..c425a4d413 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 @@ -33,7 +33,6 @@ import io.javaoperatorsdk.operator.processing.event.ResourceID; import io.javaoperatorsdk.operator.processing.event.source.PrimaryToSecondaryMapper; import io.javaoperatorsdk.operator.processing.event.source.ResourceAction; -import io.javaoperatorsdk.operator.processing.event.source.informer.TemporaryResourceCache.EventHandling; /** * Wraps informer(s) so they are connected to the eventing system of the framework. Note that since @@ -123,9 +122,14 @@ public synchronized void onDelete(R resource, boolean deletedFinalStateUnknown) log.debug( "On delete event received. deletedFinalStateUnknown: {}", deletedFinalStateUnknown); } + var resultEvent = + temporaryResourceCache.onDeleteEvent(resource, deletedFinalStateUnknown); + if (resultEvent.isEmpty()) { + return; + } primaryToSecondaryIndex.onDelete(resource); - temporaryResourceCache.onDeleteEvent(resource, deletedFinalStateUnknown); - if (acceptedByDeleteFilters(resource, deletedFinalStateUnknown)) { + if (eventAcceptedByFilter( + ResourceAction.DELETED, resource, null, deletedFinalStateUnknown)) { propagateEvent(resource); } }); @@ -134,6 +138,34 @@ public synchronized void onDelete(R resource, boolean deletedFinalStateUnknown) @Override protected void handleEvent( ResourceAction action, R resource, R oldResource, Boolean deletedFinalStateUnknown) { + // Called from ManagedInformerEventSource#eventFilteringUpdateAndCacheResource after the temp + // cache decided to surface a (possibly synthesized) event. The user-level filters + // (onAdd/onUpdate/onDelete/genericFilter) still apply, so this path mirrors the direct + // onAdd/onUpdate/onDelete watch paths. The index is updated for DELETED regardless of the + // filter outcome — the resource is really gone, so leaving a tombstone in the index would + // make getSecondaryResources keep returning a stale entry. + if (action == ResourceAction.DELETED) { + log.debug( + "handleEvent: removing from primaryToSecondaryIndex. id={}", + ResourceID.fromResource(resource)); + primaryToSecondaryIndex.onDelete(resource); + } + if (!eventAcceptedByFilter(action, resource, oldResource, deletedFinalStateUnknown)) { + if (log.isDebugEnabled()) { + log.debug( + "handleEvent: event rejected by user filter, not propagating. id={}, action={}", + ResourceID.fromResource(resource), + action); + } + return; + } + if (log.isDebugEnabled()) { + log.debug( + "handleEvent: propagating event. id={}, action={}, rv={}", + ResourceID.fromResource(resource), + action, + resource.getMetadata().getResourceVersion()); + } propagateEvent(resource); } @@ -148,26 +180,27 @@ public synchronized void start() { manager().list().forEach(primaryToSecondaryIndex::onAddOrUpdate); } + @SuppressWarnings("unchecked") private synchronized void onAddOrUpdate(ResourceAction action, R newObject, R oldObject) { primaryToSecondaryIndex.onAddOrUpdate(newObject); var resourceID = ResourceID.fromResource(newObject); - var eventHandling = temporaryResourceCache.onAddOrUpdateEvent(action, newObject, oldObject); + var resultEvent = temporaryResourceCache.onAddOrUpdateEvent(action, newObject, oldObject); - if (eventHandling != EventHandling.NEW) { - log.debug( - "{} event propagation", eventHandling == EventHandling.DEFER ? "Deferring" : "Skipping"); - } else if (eventAcceptedByFilter(action, newObject, oldObject)) { + if (resultEvent.isEmpty()) { + log.debug("Deferring event propagation"); + } else if (eventAcceptedByFilter(action, newObject, oldObject, null)) { log.debug( - "Propagating event for {}, resource with same version not result of a reconciliation.", + "Propagating event for {}, resource with same version not result of a our update.", action); - propagateEvent(newObject); + var event = resultEvent.get(); + propagateEvent((R) event.getResource().orElseThrow()); } else { log.debug("Event filtered out for operation: {}, resourceID: {}", action, resourceID); } } - private void propagateEvent(R object) { + protected void propagateEvent(R object) { var primaryResourceIdSet = configuration().getSecondaryToPrimaryMapper().toPrimaryResourceIDs(object); if (primaryResourceIdSet.isEmpty()) { @@ -238,19 +271,17 @@ public boolean allowsNamespaceChanges() { return configuration().followControllerNamespaceChanges(); } - private boolean eventAcceptedByFilter(ResourceAction action, R newObject, R oldObject) { + private boolean eventAcceptedByFilter( + ResourceAction action, R newObject, R oldObject, Boolean deletedFinalStateUnknown) { if (genericFilter != null && !genericFilter.accept(newObject)) { return false; } - if (action == ResourceAction.ADDED) { - return onAddFilter == null || onAddFilter.accept(newObject); - } else { - return onUpdateFilter == null || onUpdateFilter.accept(newObject, oldObject); - } - } - - private boolean acceptedByDeleteFilters(R resource, boolean b) { - return (onDeleteFilter == null || onDeleteFilter.accept(resource, b)) - && (genericFilter == null || genericFilter.accept(resource)); + return switch (action) { + case ADDED -> onAddFilter == null || onAddFilter.accept(newObject); + case UPDATED -> onUpdateFilter == null || onUpdateFilter.accept(newObject, oldObject); + case DELETED -> + onDeleteFilter == null + || onDeleteFilter.accept(newObject, Boolean.TRUE.equals(deletedFinalStateUnknown)); + }; } } 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 f021101229..608c638916 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 @@ -46,7 +46,6 @@ import io.javaoperatorsdk.operator.processing.event.ResourceID; import io.javaoperatorsdk.operator.processing.event.source.*; import io.javaoperatorsdk.operator.processing.event.source.ResourceAction; -import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceDeleteEvent; @SuppressWarnings("rawtypes") public abstract class ManagedInformerEventSource< @@ -96,52 +95,35 @@ public void changeNamespaces(Set namespaces) { @SuppressWarnings("unchecked") public R eventFilteringUpdateAndCacheResource(R resourceToUpdate, UnaryOperator updateMethod) { ResourceID id = ResourceID.fromResource(resourceToUpdate); - log.debug("Starting event filtering and caching update"); + log.debug("Starting event filtering and caching update for id={}", id); R updatedResource = null; try { temporaryResourceCache.startEventFilteringModify(id); updatedResource = updateMethod.apply(resourceToUpdate); - log.debug("Resource update successful"); handleRecentResourceUpdate(id, updatedResource, resourceToUpdate); + log.debug( + "Caching resource update successful. id={}, rv={}", + id, + updatedResource.getMetadata().getResourceVersion()); return updatedResource; } finally { - var res = - temporaryResourceCache.doneEventFilterModify( - id, - updatedResource == null ? null : updatedResource.getMetadata().getResourceVersion()); - var updatedForLambda = updatedResource; + var res = temporaryResourceCache.doneEventFilterModify(id); res.ifPresentOrElse( r -> { - R latestResource = (R) r.getResource().orElseThrow(); - // as previous resource version we use the one from successful update, since - // we process new event here only if that is more recent then the event from our update. - // Note that this is equivalent with the scenario when an informer watch connection - // would reconnect and loose some events in between. - // If that update was not successful we still record the previous version from the - // actual event in the ExtendedResourceEvent. - R extendedResourcePrevVersion = - (r instanceof ExtendedResourceEvent) - ? (R) ((ExtendedResourceEvent) r).getPreviousResource().orElse(null) - : null; - R prevVersionOfResource = - updatedForLambda != null ? updatedForLambda : extendedResourcePrevVersion; - if (log.isDebugEnabled()) { - log.debug( - "Previous resource version: {} resource from update present: {}" - + " extendedPrevResource present: {}", - prevVersionOfResource.getMetadata().getResourceVersion(), - updatedForLambda != null, - extendedResourcePrevVersion != null); - } + log.debug( + "Propagating not own event after filtering update. id={}, action={}, rv={}", + id, + r.getAction(), + r.getResource() + .map(rr -> rr.getMetadata().getResourceVersion()) + .orElse("[not set]")); handleEvent( r.getAction(), - latestResource, - prevVersionOfResource, - (r instanceof ResourceDeleteEvent) - ? ((ResourceDeleteEvent) r).isDeletedFinalStateUnknown() - : null); + (R) r.getResource().orElseThrow(), + (R) r.getPreviousResource().orElse(null), + r.isLastStateUnknow()); }, - () -> log.debug("No new event present after the filtering update")); + () -> log.debug("No new event present after the filtering update. id={}", id)); } } @@ -173,9 +155,17 @@ public synchronized void stop() { @Override public void onList(String resourceVersion, boolean remainedEmpty) { + // re-list supported by fabric8 client https://github.com/fabric8io/kubernetes-client/pull/7899 + // temporaryResourceCache.setRelistFinished(resourceVersion); temporaryResourceCache.checkGhostResources(); } + // @Override (enable when + // re-list supported by fabric8 client https://github.com/fabric8io/kubernetes-client/pull/7899 + // public void onBeforeList(String lastSyncResourceVersion) { + // temporaryResourceCache.setOngoingRelist(lastSyncResourceVersion); + // } + @Override public void handleRecentResourceUpdate( ResourceID resourceID, R resource, R previousVersionOfResource) { 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 405f52cc8d..12e338b2b9 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,8 +15,6 @@ */ package io.javaoperatorsdk.operator.processing.event.source.informer; -import java.util.Collections; -import java.util.HashMap; import java.util.Map; import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; @@ -29,8 +27,6 @@ import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource; import io.javaoperatorsdk.operator.processing.event.ResourceID; import io.javaoperatorsdk.operator.processing.event.source.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 @@ -50,24 +46,27 @@ * *

If comparable resource versions are disabled, then this cache is effectively disabled. * + *

Some principles to realize with the current filtering algorithm: + * + *

    + *
  • We propagate events only if we received an event that has the same resourceVersion or newer + * than resource version from update + *
  • The propagated event should correspond to a possible real world scenario - considering also + * ones that could happen if the Informer does a re-list. + *
+ * * @param resource to cache. */ public class TemporaryResourceCache { private static final Logger log = LoggerFactory.getLogger(TemporaryResourceCache.class); - private final Map cache = new ConcurrentHashMap<>(); - private final Map activeUpdates = new HashMap<>(); private final boolean comparableResourceVersions; + private final Map cache = new ConcurrentHashMap<>(); + private final EventFilterSupport eventFilteringSupport = new EventFilterSupport(); private final ManagedInformerEventSource managedInformerEventSource; - public enum EventHandling { - DEFER, - OBSOLETE, - NEW - } - public TemporaryResourceCache( boolean comparableResourceVersions, ManagedInformerEventSource managedInformerEventSource) { @@ -79,86 +78,63 @@ public synchronized void startEventFilteringModify(ResourceID resourceID) { if (!comparableResourceVersions) { return; } - var ed = activeUpdates.computeIfAbsent(resourceID, id -> new EventFilterDetails()); - ed.increaseActiveUpdates(); + eventFilteringSupport.startEventFilteringModify(resourceID); } - public synchronized Optional doneEventFilterModify( - ResourceID resourceID, String updatedResourceVersion) { + public synchronized Optional doneEventFilterModify(ResourceID resourceID) { if (!comparableResourceVersions) { return Optional.empty(); } - var ed = activeUpdates.get(resourceID); - if (ed == null || !ed.decreaseActiveUpdates(updatedResourceVersion)) { - log.debug( - "Active updates {} for resource id: {}", - ed != null ? ed.getActiveUpdates() : 0, - resourceID); - return Optional.empty(); - } - activeUpdates.remove(resourceID); - var res = ed.getLatestEventAfterLastUpdateEvent(); - log.debug( - "Zero active updates for resource id: {}; event after update event: {}; updated resource" - + " version: {}", - resourceID, - res.isPresent(), - updatedResourceVersion); - return res; + return eventFilteringSupport.doneEventFilterModify(resourceID); } - public void onDeleteEvent(T resource, boolean unknownState) { - onEvent(ResourceAction.DELETED, resource, null, unknownState, true); + public Optional onDeleteEvent(T resource, boolean unknownState) { + return onEvent(ResourceAction.DELETED, resource, null, unknownState); } - public EventHandling onAddOrUpdateEvent( + public Optional onAddOrUpdateEvent( ResourceAction action, T resource, T prevResourceVersion) { - return onEvent(action, resource, prevResourceVersion, false, false); + return onEvent(action, resource, prevResourceVersion, null); } - private synchronized EventHandling onEvent( - ResourceAction action, - T resource, - T prevResourceVersion, - boolean unknownState, - boolean delete) { + private synchronized Optional onEvent( + ResourceAction action, T resource, T prevResourceVersion, Boolean unknownState) { + GenericResourceEvent actualEvent = + toGenericResourceEvent(action, resource, prevResourceVersion, unknownState); if (!comparableResourceVersions) { - return EventHandling.NEW; + return Optional.of(actualEvent); } - var resourceId = ResourceID.fromResource(resource); - if (log.isDebugEnabled()) { - log.debug("Processing event"); - } + log.debug( + "Processing event in temp cache. id={}, action={}, rv={}, unknownState={}", + resourceId, + action, + resource.getMetadata().getResourceVersion(), + unknownState); var cached = cache.get(resourceId); - EventHandling result = EventHandling.NEW; if (cached != null) { int comp = ReconcilerUtilsInternal.compareResourceVersions(resource, cached); - if (comp >= 0 || unknownState) { + if (comp >= 0 || Boolean.TRUE.equals(unknownState)) { log.debug( - "Removing resource from temp cache. comparison: {} unknown state: {}", + "Removing resource from temp cache. id={}, comparison={}, unknownState={}", + resourceId, comp, 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 - result = comp == 0 ? EventHandling.OBSOLETE : EventHandling.NEW; } else { - result = EventHandling.OBSOLETE; + log.debug( + "Keeping temp cache entry; event rv {} is older than cached rv {}. id={}", + resource.getMetadata().getResourceVersion(), + cached.getMetadata().getResourceVersion(), + resourceId); } } - var ed = activeUpdates.get(resourceId); - if (ed != null && result != EventHandling.OBSOLETE) { - log.debug("Setting last event for id: {} delete: {}", resourceId, delete); - ed.setLastEvent( - delete - ? new ResourceDeleteEvent(ResourceAction.DELETED, resourceId, resource, unknownState) - : new ExtendedResourceEvent(action, resourceId, resource, prevResourceVersion)); - return EventHandling.DEFER; - } else { - return result; - } + return eventFilteringSupport.processEvent(resourceId, actualEvent); + } + + static GenericResourceEvent toGenericResourceEvent( + ResourceAction action, T resource, T prevResourceVersion, Boolean unknownState) { + return new GenericResourceEvent(action, resource, prevResourceVersion, unknownState); } /** put the item into the cache if it's for a later state than what has already been observed. */ @@ -188,6 +164,10 @@ public synchronized void putResource(T newResource) { return; } + var cachedResource = getResourceFromCache(resourceId).orElse(null); + eventFilteringSupport.addToOwnResourceVersions( + resourceId, newResource.getMetadata().getResourceVersion()); + // check against the latestResourceVersion processed by the TemporaryResourceCache // If the resource is older, then we can safely ignore. // @@ -206,9 +186,6 @@ public synchronized void putResource(T newResource) { return; } - // also make sure that we're later than the existing temporary entry - var cachedResource = getResourceFromCache(resourceId).orElse(null); - if (cachedResource == null || ReconcilerUtilsInternal.compareResourceVersions(newResource, cachedResource) > 0) { log.debug( @@ -216,6 +193,12 @@ public synchronized void putResource(T newResource) { newResource.getMetadata().getResourceVersion(), resourceId); cache.put(resourceId, newResource); + } else { + log.debug( + "Skipping temp cache put; new rv {} is not later than cached rv {}. id={}", + newResource.getMetadata().getResourceVersion(), + cachedResource.getMetadata().getResourceVersion(), + resourceId); } } @@ -231,7 +214,11 @@ private String getLastSyncResourceVersion(String namespace) { * explicitly add resources to this cache. Those are cleaned up by this check, which is triggered * by the informer's onList callback. */ - public void checkGhostResources() { + public synchronized void checkGhostResources() { + if (!comparableResourceVersions) { + return; + } + log.debug("Checking for ghost resources."); var iterator = cache.entrySet().iterator(); while (iterator.hasNext()) { @@ -246,19 +233,18 @@ public void checkGhostResources() { e.getKey(), ns); iterator.remove(); + eventFilteringSupport.handleGhostResourceRemoval(e.getKey()); continue; } if ((ReconcilerUtilsInternal.compareResourceVersions( e.getValue().getMetadata().getResourceVersion(), getLastSyncResourceVersion(ns)) < 0) // making sure we have the situation where resource is missing from the cache - && managedInformerEventSource - .manager() - .get(ResourceID.fromResource(e.getValue())) - .isEmpty()) { + && managedInformerEventSource.manager().get(e.getKey()).isEmpty()) { + log.debug("Removing ghost resource with ID: {}", e.getKey()); iterator.remove(); + eventFilteringSupport.handleGhostResourceRemoval(e.getKey()); managedInformerEventSource.handleEvent(ResourceAction.DELETED, e.getValue(), null, true); - log.debug("Removing ghost resource with ID: {}", e.getKey()); } } } @@ -272,6 +258,18 @@ synchronized boolean isEmpty() { } synchronized Map getResources() { - return Collections.unmodifiableMap(cache); + return Map.copyOf(cache); + } + + EventFilterSupport getEventFilterSupport() { + return eventFilteringSupport; + } + + public void setOngoingRelist(String lastKnownSyncVersion) { + eventFilteringSupport.setStartingReList(); + } + + public void setRelistFinished(String syncResourceVersions) { + eventFilteringSupport.setRelistFinished(); } } 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 4528fa8a83..f8cb54f68e 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 @@ -17,12 +17,12 @@ import java.time.LocalDateTime; import java.util.List; +import java.util.Optional; import java.util.concurrent.CountDownLatch; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import io.fabric8.kubernetes.client.KubernetesClientException; import io.javaoperatorsdk.operator.MockKubernetesClient; import io.javaoperatorsdk.operator.ReconcilerUtilsInternal; import io.javaoperatorsdk.operator.TestUtils; @@ -44,7 +44,6 @@ import io.javaoperatorsdk.operator.sample.simple.TestCustomResource; import static io.javaoperatorsdk.operator.processing.event.source.EventFilterTestUtils.withResourceVersion; -import static org.assertj.core.api.Assertions.assertThat; import static org.awaitility.Awaitility.await; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.*; @@ -90,7 +89,6 @@ void dontSkipEventHandlingIfMarkedForDeletion() { source.handleEvent(ResourceAction.UPDATED, customResource1, customResource1, null); verify(eventHandler, times(1)).handleEvent(any()); - // mark for deletion customResource1.getMetadata().setDeletionTimestamp(LocalDateTime.now().toString()); source.handleEvent(ResourceAction.UPDATED, customResource1, customResource1, null); verify(eventHandler, times(2)).handleEvent(any()); @@ -172,9 +170,13 @@ void genericFilterFiltersOutAddUpdateAndDeleteEvents() { } @Test - void testEventFilteringBasicScenario() throws InterruptedException { + void ownUpdateEchoIsFilteredOutByEventFilter() throws InterruptedException { + // End-to-end smoke for the event-filter wiring on the controller path: an event for our + // own write must not propagate. Detail-level filter scenarios are covered in + // EventingDetailTest / EventFilterSupportTest. source = spy(new ControllerEventSource<>(new TestController(null, null, null))); setUpSource(source, true, controllerConfig); + doReturn(Optional.empty()).when(source).get(any()); var latch = sendForEventFilteringUpdate(2); source.onUpdate(testResourceWithVersion(1), testResourceWithVersion(2)); @@ -185,7 +187,8 @@ void testEventFilteringBasicScenario() throws InterruptedException { } @Test - void eventFilteringNewEventDuringUpdate() { + void foreignUpdateDuringFilteringPropagatesAsUpdate() { + // An external event during the filter window must surface (not be filtered as own). source = spy(new ControllerEventSource<>(new TestController(null, null, null))); setUpSource(source, true, controllerConfig); @@ -197,70 +200,56 @@ void eventFilteringNewEventDuringUpdate() { } @Test - void eventFilteringMoreNewEventsDuringUpdate() { + void deleteEventDuringFilteringPropagatesAsDelete() { + // A DELETE arriving during the filter window must surface — the resource has gone, + // so the filter must not silence it just because our own write is still tracking RVs. source = spy(new ControllerEventSource<>(new TestController(null, null, null))); setUpSource(source, true, controllerConfig); var latch = sendForEventFilteringUpdate(2); - source.onUpdate(testResourceWithVersion(2), testResourceWithVersion(3)); - source.onUpdate(testResourceWithVersion(3), testResourceWithVersion(4)); + source.onDelete(testResourceWithVersion(3), false); latch.countDown(); - await().untilAsserted(() -> expectHandleEvent(4, 2)); + await() + .untilAsserted( + () -> { + verify(eventHandler, atLeastOnce()).handleEvent(any()); + verify(source, atLeastOnce()) + .handleEvent(eq(ResourceAction.DELETED), any(), any(), any()); + }); } @Test - void eventFilteringExceptionDuringUpdate() { + void multipleForeignEventsDuringFilteringMergeIntoSingleEvent() { + // Several external events during one filter window collapse into a single + // synthesized event spanning prev → latest seen. source = spy(new ControllerEventSource<>(new TestController(null, null, null))); setUpSource(source, true, controllerConfig); - var latch = - EventFilterTestUtils.sendForEventFilteringUpdate( - source, - TestUtils.testCustomResource1(), - r -> { - throw new KubernetesClientException("fake"); - }); - source.onUpdate(testResourceWithVersion(1), testResourceWithVersion(2)); + var latch = sendForEventFilteringUpdate(2); + source.onUpdate(testResourceWithVersion(2), testResourceWithVersion(3)); + source.onUpdate(testResourceWithVersion(3), testResourceWithVersion(4)); latch.countDown(); - expectHandleEvent(2, 1); + await().untilAsserted(() -> expectHandleEvent(4, 2)); } private void expectHandleEvent(int newResourceVersion, int oldResourceVersion) { - await() - .untilAsserted( - () -> { - verify(eventHandler, times(1)).handleEvent(any()); - verify(source, times(1)) - .handleEvent( - eq(ResourceAction.UPDATED), - argThat( - r -> { - assertThat(r.getMetadata().getResourceVersion()) - .isEqualTo("" + newResourceVersion); - return true; - }), - argThat( - r -> { - assertThat(r.getMetadata().getResourceVersion()) - .isEqualTo("" + oldResourceVersion); - return true; - }), - isNull()); - }); + verify(eventHandler, times(1)).handleEvent(any()); + verify(source, times(1)) + .handleEvent( + eq(ResourceAction.UPDATED), + argThat(r -> ("" + newResourceVersion).equals(r.getMetadata().getResourceVersion())), + argThat(r -> ("" + oldResourceVersion).equals(r.getMetadata().getResourceVersion())), + any()); } private TestCustomResource testResourceWithVersion(int v) { return withResourceVersion(TestUtils.testCustomResource1(), v); } - private CountDownLatch sendForEventFilteringUpdate(int v) { - return sendForEventFilteringUpdate(TestUtils.testCustomResource1(), v); - } - - private CountDownLatch sendForEventFilteringUpdate( - TestCustomResource testResource, int resourceVersion) { + private CountDownLatch sendForEventFilteringUpdate(int resourceVersion) { + var testResource = TestUtils.testCustomResource1(); return EventFilterTestUtils.sendForEventFilteringUpdate( source, testResource, r -> withResourceVersion(testResource, resourceVersion)); } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/EventFilterSupportTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/EventFilterSupportTest.java new file mode 100644 index 0000000000..06775d809e --- /dev/null +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/EventFilterSupportTest.java @@ -0,0 +1,618 @@ +/* + * 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 org.junit.jupiter.api.Test; + +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.javaoperatorsdk.operator.processing.event.ResourceID; + +import static io.javaoperatorsdk.operator.processing.event.source.ResourceAction.ADDED; +import static io.javaoperatorsdk.operator.processing.event.source.ResourceAction.DELETED; +import static io.javaoperatorsdk.operator.processing.event.source.ResourceAction.UPDATED; +import static org.assertj.core.api.Assertions.assertThat; + +class EventFilterSupportTest { + + static final Long FIRST_OWN_VERSION = 5L; + static final ResourceID RESOURCE_ID = new ResourceID("id1", "default"); + static final ResourceID OTHER_RESOURCE_ID = new ResourceID("id2", "default"); + + EventFilterSupport support = new EventFilterSupport(); + + @Test + void startEventFilteringCreatesEventingDetail() { + support.startEventFilteringModify(RESOURCE_ID); + + assertThat(support.isActiveUpdateFor(RESOURCE_ID)).isTrue(); + assertThat(support.getEventFilterWindows()).containsOnlyKeys(RESOURCE_ID); + } + + @Test + void startEventFilteringTwiceReusesEventingDetail() { + support.startEventFilteringModify(RESOURCE_ID); + var first = support.getEventFilterWindows().get(RESOURCE_ID); + + support.startEventFilteringModify(RESOURCE_ID); + var second = support.getEventFilterWindows().get(RESOURCE_ID); + + assertThat(second).isSameAs(first); + } + + @Test + void doneEventFilterModifyEmptyWhenNoEventingDetail() { + assertThat(support.doneEventFilterModify(RESOURCE_ID)).isEmpty(); + } + + @Test + void doneEventFilterModifyRemovesDetailWhenRemovable() { + support.startEventFilteringModify(RESOURCE_ID); + support.addToOwnResourceVersions(RESOURCE_ID, s(FIRST_OWN_VERSION)); + assertThat(support.processEvent(RESOURCE_ID, updateEvent(FIRST_OWN_VERSION))).isEmpty(); + + var res = support.doneEventFilterModify(RESOURCE_ID); + + assertThat(res).isEmpty(); + assertThat(support.isActiveUpdateFor(RESOURCE_ID)).isFalse(); + } + + @Test + void scenarioWithSurroundingEvent() { + assertThat(support.processEvent(RESOURCE_ID, updateEvent(FIRST_OWN_VERSION - 1))) + .hasValueSatisfying(e -> assertThat(e.getAction()).isEqualTo(UPDATED)); + + support.startEventFilteringModify(RESOURCE_ID); + support.addToOwnResourceVersions(RESOURCE_ID, s(FIRST_OWN_VERSION)); + assertThat(support.processEvent(RESOURCE_ID, updateEvent(FIRST_OWN_VERSION))).isEmpty(); + assertThat(support.processEvent(RESOURCE_ID, updateEvent(FIRST_OWN_VERSION + 1))).isEmpty(); + + var res = support.doneEventFilterModify(RESOURCE_ID); + + assertThat(res).hasValueSatisfying(e -> assertThat(e.getAction()).isEqualTo(UPDATED)); + assertThat(support.isActiveUpdateFor(RESOURCE_ID)).isFalse(); + + assertThat(support.processEvent(RESOURCE_ID, updateEvent(FIRST_OWN_VERSION + 2))) + .hasValueSatisfying(e -> assertThat(e.getAction()).isEqualTo(UPDATED)); + } + + @Test + void processEventPropagatesWhenNoEventingDetail() { + var event = updateEvent(FIRST_OWN_VERSION); + + var res = support.processEvent(RESOURCE_ID, event); + + assertThat(res).contains(event); + } + + @Test + void processEventHoldsOwnEcho() { + support.startEventFilteringModify(RESOURCE_ID); + support.addToOwnResourceVersions(RESOURCE_ID, s(FIRST_OWN_VERSION)); + + var res = support.processEvent(RESOURCE_ID, updateEvent(FIRST_OWN_VERSION)); + + assertThat(res).isEmpty(); + } + + @Test + void processEventEmitsSynthForForeignEvent() { + support.startEventFilteringModify(RESOURCE_ID); + support.addToOwnResourceVersions(RESOURCE_ID, s(FIRST_OWN_VERSION)); + support.processEvent(RESOURCE_ID, updateEvent(FIRST_OWN_VERSION - 1)); + + var res = support.processEvent(RESOURCE_ID, updateEvent(FIRST_OWN_VERSION)); + + assertThat(res).hasValueSatisfying(e -> assertThat(e.getAction()).isEqualTo(UPDATED)); + } + + @Test + void processEventEmitsAddedForeignVerbatim() { + support.startEventFilteringModify(RESOURCE_ID); + support.addToOwnResourceVersions(RESOURCE_ID, s(FIRST_OWN_VERSION)); + var addEvent = addEvent(FIRST_OWN_VERSION); + var updateEvent = addEvent(FIRST_OWN_VERSION + 1); + support.processEvent(RESOURCE_ID, addEvent); + + var res = support.processEvent(RESOURCE_ID, updateEvent); + assertThat(res).isEmpty(); + + res = support.doneEventFilterModify(RESOURCE_ID); + + assertThat(res).contains(addEvent); + } + + @Test + void addToOwnResourceVersionsIsNoOpWithoutEventingDetail() { + support.addToOwnResourceVersions(RESOURCE_ID, s(FIRST_OWN_VERSION)); + + assertThat(support.isActiveUpdateFor(RESOURCE_ID)).isFalse(); + } + + @Test + void handleGhostResourceRemovalDropsWindow() { + support.startEventFilteringModify(RESOURCE_ID); + + support.handleGhostResourceRemoval(RESOURCE_ID); + + assertThat(support.isActiveUpdateFor(RESOURCE_ID)).isFalse(); + } + + @Test + void handleGhostResourceRemovalIsNoOpForUnknownResource() { + support.startEventFilteringModify(RESOURCE_ID); + + support.handleGhostResourceRemoval(OTHER_RESOURCE_ID); + + assertThat(support.isActiveUpdateFor(RESOURCE_ID)).isTrue(); + assertThat(support.isActiveUpdateFor(OTHER_RESOURCE_ID)).isFalse(); + } + + @Test + void fullLifecycleOwnWriteOnlyEmitsNothingAndCleansUp() { + support.startEventFilteringModify(RESOURCE_ID); + support.addToOwnResourceVersions(RESOURCE_ID, s(FIRST_OWN_VERSION)); + assertThat(support.processEvent(RESOURCE_ID, updateEvent(FIRST_OWN_VERSION))).isEmpty(); + + var res = support.doneEventFilterModify(RESOURCE_ID); + + assertThat(res).isEmpty(); + assertThat(support.isActiveUpdateFor(RESOURCE_ID)).isFalse(); + } + + @Test + void fullLifecycleForeignBeforeOwnEchoEmitsSynth() { + support.startEventFilteringModify(RESOURCE_ID); + support.addToOwnResourceVersions(RESOURCE_ID, s(FIRST_OWN_VERSION)); + + var foreign = updateEvent(FIRST_OWN_VERSION - 1); + assertThat(support.processEvent(RESOURCE_ID, foreign)).isEmpty(); + + // catch-up emit triggered by the own echo arriving after the prior emit + assertThat(support.processEvent(RESOURCE_ID, updateEvent(FIRST_OWN_VERSION))).isPresent(); + assertThat(support.doneEventFilterModify(RESOURCE_ID)).isEmpty(); + assertThat(support.isActiveUpdateFor(RESOURCE_ID)).isFalse(); + } + + @Test + void oneOwnVersionNoEvent() { + support.startEventFilteringModify(RESOURCE_ID); + support.addToOwnResourceVersions(RESOURCE_ID, s(FIRST_OWN_VERSION)); + + assertThat(support.doneEventFilterModify(RESOURCE_ID)).isEmpty(); + // own RV recorded but no echo arrived yet → window stays + assertThat(support.isActiveUpdateFor(RESOURCE_ID)).isTrue(); + assertThat(support.getEventFilterWindows().get(RESOURCE_ID).getOwnResourceVersions()) + .containsExactly(FIRST_OWN_VERSION); + } + + @Test + void oneOwnVersionEventReceivedEventForIt() { + support.startEventFilteringModify(RESOURCE_ID); + support.addToOwnResourceVersions(RESOURCE_ID, s(FIRST_OWN_VERSION)); + + assertThat(support.processEvent(RESOURCE_ID, updateEvent(FIRST_OWN_VERSION))).isEmpty(); + assertThat(support.isActiveUpdateFor(RESOURCE_ID)).isTrue(); + + assertThat(support.doneEventFilterModify(RESOURCE_ID)).isEmpty(); + assertThat(support.isActiveUpdateFor(RESOURCE_ID)).isFalse(); + } + + @Test + void receivedAsFirstAddEventReturnTheSameEventIfThatIsOnlyRelevant() { + support.startEventFilteringModify(RESOURCE_ID); + support.addToOwnResourceVersions(RESOURCE_ID, s(FIRST_OWN_VERSION)); + + assertThat(support.processEvent(RESOURCE_ID, addEvent(FIRST_OWN_VERSION))).isEmpty(); + } + + @Test + void oneOwnVersionAdditionalEventReceivedBeforeIt() { + support.startEventFilteringModify(RESOURCE_ID); + support.addToOwnResourceVersions(RESOURCE_ID, s(FIRST_OWN_VERSION)); + + assertThat(support.processEvent(RESOURCE_ID, updateEvent(FIRST_OWN_VERSION - 1))).isEmpty(); + assertThat(support.processEvent(RESOURCE_ID, updateEvent(FIRST_OWN_VERSION))).isPresent(); + assertThat(support.isActiveUpdateFor(RESOURCE_ID)).isTrue(); + + assertThat(support.doneEventFilterModify(RESOURCE_ID)).isEmpty(); + assertThat(support.isActiveUpdateFor(RESOURCE_ID)).isFalse(); + } + + @Test + void twoOwnVersionEventReceivedEventOnlyForFirstThenForSecond() { + support.startEventFilteringModify(RESOURCE_ID); + support.addToOwnResourceVersions(RESOURCE_ID, s(FIRST_OWN_VERSION)); + support.startEventFilteringModify(RESOURCE_ID); + support.addToOwnResourceVersions(RESOURCE_ID, s(FIRST_OWN_VERSION + 1)); + + assertThat(support.processEvent(RESOURCE_ID, updateEvent(FIRST_OWN_VERSION))).isEmpty(); + var window = support.getEventFilterWindows().get(RESOURCE_ID); + assertThat(window.getRelatedEvents()).isEmpty(); + assertThat(window.getOwnResourceVersions()).containsExactly(FIRST_OWN_VERSION + 1); + + assertThat(support.doneEventFilterModify(RESOURCE_ID)).isEmpty(); + assertThat(support.doneEventFilterModify(RESOURCE_ID)).isEmpty(); + assertThat(support.isActiveUpdateFor(RESOURCE_ID)).isTrue(); + + assertThat(support.processEvent(RESOURCE_ID, updateEvent(FIRST_OWN_VERSION + 1))).isEmpty(); + assertThat(support.isActiveUpdateFor(RESOURCE_ID)).isFalse(); + } + + @Test + void twoOwnVersionEventReceivedOne() { + support.startEventFilteringModify(RESOURCE_ID); + support.addToOwnResourceVersions(RESOURCE_ID, s(FIRST_OWN_VERSION)); + support.startEventFilteringModify(RESOURCE_ID); + support.addToOwnResourceVersions(RESOURCE_ID, s(FIRST_OWN_VERSION + 1)); + + assertThat(support.processEvent(RESOURCE_ID, updateEvent(FIRST_OWN_VERSION))).isEmpty(); + + var window = support.getEventFilterWindows().get(RESOURCE_ID); + assertThat(window.getRelatedEvents()).isEmpty(); + assertThat(window.getOwnResourceVersions()).containsExactly(FIRST_OWN_VERSION + 1); + + support.doneEventFilterModify(RESOURCE_ID); + support.doneEventFilterModify(RESOURCE_ID); + assertThat(support.isActiveUpdateFor(RESOURCE_ID)).isTrue(); + } + + @Test + void receivedAddEventAfterOurUpdate() { + support.startEventFilteringModify(RESOURCE_ID); + support.addToOwnResourceVersions(RESOURCE_ID, s(FIRST_OWN_VERSION)); + + assertThat(support.processEvent(RESOURCE_ID, addEvent(FIRST_OWN_VERSION + 1))).isEmpty(); + assertThat(support.doneEventFilterModify(RESOURCE_ID)) + .hasValueSatisfying(e -> assertThat(e.getAction()).isEqualTo(ADDED)); + assertThat(support.isActiveUpdateFor(RESOURCE_ID)).isFalse(); + } + + @Test + void receivedAddEventAfterOurUpdateDone() { + support.startEventFilteringModify(RESOURCE_ID); + support.addToOwnResourceVersions(RESOURCE_ID, s(FIRST_OWN_VERSION)); + assertThat(support.doneEventFilterModify(RESOURCE_ID)).isEmpty(); + + // Window remains because own=[5] is non-empty. Late ADDED arrives after done. + assertThat(support.processEvent(RESOURCE_ID, addEvent(FIRST_OWN_VERSION + 1))) + .hasValueSatisfying(e -> assertThat(e.getAction()).isEqualTo(ADDED)); + assertThat(support.isActiveUpdateFor(RESOURCE_ID)).isFalse(); + } + + @Test + void canBeRemovedIfNoActiveUpdatesOnly() { + support.startEventFilteringModify(RESOURCE_ID); + assertThat(support.processEvent(RESOURCE_ID, updateEvent(FIRST_OWN_VERSION))).isEmpty(); + + assertThat(support.doneEventFilterModify(RESOURCE_ID)) + .hasValueSatisfying(e -> assertThat(e.getAction()).isEqualTo(UPDATED)); + } + + @Test + void propagateEventIfNoOwnResourceAndNoActiveUpdate() { + support.startEventFilteringModify(RESOURCE_ID); + support.doneEventFilterModify(RESOURCE_ID); + // After the done call, active=0 and own is empty → window removed. + assertThat(support.isActiveUpdateFor(RESOURCE_ID)).isFalse(); + + // A subsequent event has no window → propagated verbatim. + var event = updateEvent(FIRST_OWN_VERSION); + assertThat(support.processEvent(RESOURCE_ID, event)).contains(event); + } + + @Test + void receiveEventAfterEventForOwnUpdate() { + support.startEventFilteringModify(RESOURCE_ID); + support.addToOwnResourceVersions(RESOURCE_ID, s(FIRST_OWN_VERSION + 1)); + + assertThat(support.processEvent(RESOURCE_ID, updateEvent(FIRST_OWN_VERSION))).isEmpty(); + assertThat(support.processEvent(RESOURCE_ID, updateEvent(FIRST_OWN_VERSION + 1))) + .hasValueSatisfying(e -> assertThat(e.getAction()).isEqualTo(UPDATED)); + assertThat(support.processEvent(RESOURCE_ID, updateEvent(FIRST_OWN_VERSION + 2))).isEmpty(); + + assertThat(support.doneEventFilterModify(RESOURCE_ID)) + .hasValueSatisfying(e -> assertThat(e.getAction()).isEqualTo(UPDATED)); + assertThat(support.isActiveUpdateFor(RESOURCE_ID)).isFalse(); + } + + @Test + void doNotIncludeAfterEventForFirstOwnUpdateIfOtherOwnUpdateIsActive() { + support.startEventFilteringModify(RESOURCE_ID); + support.addToOwnResourceVersions(RESOURCE_ID, s(FIRST_OWN_VERSION + 1)); + support.startEventFilteringModify(RESOURCE_ID); + + assertThat(support.processEvent(RESOURCE_ID, updateEvent(FIRST_OWN_VERSION))).isEmpty(); + assertThat(support.processEvent(RESOURCE_ID, updateEvent(FIRST_OWN_VERSION + 1))).isPresent(); + assertThat(support.processEvent(RESOURCE_ID, updateEvent(FIRST_OWN_VERSION + 2))).isEmpty(); + + support.doneEventFilterModify(RESOURCE_ID); + support.addToOwnResourceVersions(RESOURCE_ID, s(FIRST_OWN_VERSION + 2)); + support.doneEventFilterModify(RESOURCE_ID); + + assertThat(support.isActiveUpdateFor(RESOURCE_ID)).isFalse(); + } + + @Test + void assertMultipleUpdatesAndIntermediateEventBetween() { + support.startEventFilteringModify(RESOURCE_ID); + support.addToOwnResourceVersions(RESOURCE_ID, s(FIRST_OWN_VERSION)); + support.startEventFilteringModify(RESOURCE_ID); + support.addToOwnResourceVersions(RESOURCE_ID, s(FIRST_OWN_VERSION + 2)); + + assertThat(support.processEvent(RESOURCE_ID, updateEvent(FIRST_OWN_VERSION))).isEmpty(); + assertThat(support.processEvent(RESOURCE_ID, updateEvent(FIRST_OWN_VERSION + 1))).isEmpty(); + assertThat(support.processEvent(RESOURCE_ID, updateEvent(FIRST_OWN_VERSION + 2))) + .hasValueSatisfying(e -> assertThat(e.getAction()).isEqualTo(UPDATED)); + + support.doneEventFilterModify(RESOURCE_ID); + support.doneEventFilterModify(RESOURCE_ID); + assertThat(support.isActiveUpdateFor(RESOURCE_ID)).isFalse(); + } + + @Test + void receiveIntermediateBetweenTwoOwnUpdates() { + support.startEventFilteringModify(RESOURCE_ID); + support.addToOwnResourceVersions(RESOURCE_ID, s(FIRST_OWN_VERSION)); + support.startEventFilteringModify(RESOURCE_ID); + support.addToOwnResourceVersions(RESOURCE_ID, s(FIRST_OWN_VERSION + 2)); + + assertThat(support.processEvent(RESOURCE_ID, updateEvent(FIRST_OWN_VERSION))).isEmpty(); + assertThat(support.processEvent(RESOURCE_ID, updateEvent(FIRST_OWN_VERSION + 1))).isEmpty(); + + assertThat(support.isActiveUpdateFor(RESOURCE_ID)).isTrue(); + + assertThat(support.doneEventFilterModify(RESOURCE_ID)).isEmpty(); + assertThat(support.isActiveUpdateFor(RESOURCE_ID)).isTrue(); + + assertThat(support.processEvent(RESOURCE_ID, updateEvent(FIRST_OWN_VERSION + 2))) + .hasValueSatisfying(e -> assertThat(e.getAction()).isEqualTo(UPDATED)); + assertThat(support.doneEventFilterModify(RESOURCE_ID)).isEmpty(); + assertThat(support.isActiveUpdateFor(RESOURCE_ID)).isFalse(); + } + + @Test + void deleteEventAsLastEvent_simpleCase() { + support.startEventFilteringModify(RESOURCE_ID); + support.addToOwnResourceVersions(RESOURCE_ID, s(FIRST_OWN_VERSION)); + + assertThat(support.processEvent(RESOURCE_ID, deleteEvent(FIRST_OWN_VERSION))) + .hasValueSatisfying(e -> assertThat(e.getAction()).isEqualTo(DELETED)); + assertThat(support.isActiveUpdateFor(RESOURCE_ID)).isTrue(); + + assertThat(support.doneEventFilterModify(RESOURCE_ID)).isEmpty(); + assertThat(support.isActiveUpdateFor(RESOURCE_ID)).isFalse(); + } + + @Test + void deleteEventBeforeOurUpdate() { + support.startEventFilteringModify(RESOURCE_ID); + support.addToOwnResourceVersions(RESOURCE_ID, s(FIRST_OWN_VERSION)); + + assertThat(support.processEvent(RESOURCE_ID, deleteEvent(FIRST_OWN_VERSION - 1))).isEmpty(); + assertThat(support.processEvent(RESOURCE_ID, addEvent(FIRST_OWN_VERSION))).isEmpty(); + + assertThat(support.doneEventFilterModify(RESOURCE_ID)).isEmpty(); + assertThat(support.isActiveUpdateFor(RESOURCE_ID)).isFalse(); + } + + @Test + void deleteEventOnMiddleOfOwnUpdate() { + support.startEventFilteringModify(RESOURCE_ID); + support.addToOwnResourceVersions(RESOURCE_ID, s(FIRST_OWN_VERSION + 2)); + + assertThat(support.processEvent(RESOURCE_ID, updateEvent(FIRST_OWN_VERSION))).isEmpty(); + assertThat(support.processEvent(RESOURCE_ID, deleteEvent(FIRST_OWN_VERSION + 1))).isEmpty(); + assertThat(support.processEvent(RESOURCE_ID, addEvent(FIRST_OWN_VERSION + 2))) + .hasValueSatisfying(e -> assertThat(e.getAction()).isEqualTo(UPDATED)); + + assertThat(support.doneEventFilterModify(RESOURCE_ID)).isEmpty(); + assertThat(support.isActiveUpdateFor(RESOURCE_ID)).isFalse(); + } + + @Test + void deleteEventAsAdditionalEventAfterOwnUpdates() { + support.startEventFilteringModify(RESOURCE_ID); + support.addToOwnResourceVersions(RESOURCE_ID, s(FIRST_OWN_VERSION)); + + assertThat(support.processEvent(RESOURCE_ID, updateEvent(FIRST_OWN_VERSION))).isEmpty(); + assertThat(support.processEvent(RESOURCE_ID, deleteEvent(FIRST_OWN_VERSION + 1))) + .hasValueSatisfying(e -> assertThat(e.getAction()).isEqualTo(DELETED)); + + assertThat(support.doneEventFilterModify(RESOURCE_ID)).isEmpty(); + assertThat(support.isActiveUpdateFor(RESOURCE_ID)).isFalse(); + } + + @Test + void additionalDeleteEvent() { + support.startEventFilteringModify(RESOURCE_ID); + support.addToOwnResourceVersions(RESOURCE_ID, s(FIRST_OWN_VERSION + 1)); + + assertThat(support.processEvent(RESOURCE_ID, updateEvent(FIRST_OWN_VERSION))).isEmpty(); + assertThat(support.processEvent(RESOURCE_ID, updateEvent(FIRST_OWN_VERSION + 1))) + .hasValueSatisfying(e -> assertThat(e.getAction()).isEqualTo(UPDATED)); + assertThat(support.processEvent(RESOURCE_ID, deleteEvent(FIRST_OWN_VERSION + 2))) + .hasValueSatisfying(e -> assertThat(e.getAction()).isEqualTo(DELETED)); + + assertThat(support.processEvent(RESOURCE_ID, addEvent(FIRST_OWN_VERSION + 3))); + assertThat(support.doneEventFilterModify(RESOURCE_ID)) + .hasValueSatisfying(e -> assertThat(e.getAction()).isEqualTo(ADDED)); + + assertThat(support.isActiveUpdateFor(RESOURCE_ID)).isFalse(); + } + + @Test + void deleteEventInMiddleTwoUpdates() { + support.startEventFilteringModify(RESOURCE_ID); + support.addToOwnResourceVersions(RESOURCE_ID, s(FIRST_OWN_VERSION)); + + assertThat(support.processEvent(RESOURCE_ID, updateEvent(FIRST_OWN_VERSION))).isEmpty(); + assertThat(support.processEvent(RESOURCE_ID, deleteEvent(FIRST_OWN_VERSION + 1))) + .hasValueSatisfying(e -> assertThat(e.getAction()).isEqualTo(DELETED)); + + support.startEventFilteringModify(RESOURCE_ID); + support.addToOwnResourceVersions(RESOURCE_ID, s(FIRST_OWN_VERSION + 2)); + assertThat(support.processEvent(RESOURCE_ID, addEvent(FIRST_OWN_VERSION + 2))).isEmpty(); + + assertThat(support.doneEventFilterModify(RESOURCE_ID)).isEmpty(); + assertThat(support.doneEventFilterModify(RESOURCE_ID)).isEmpty(); + + assertThat(support.isActiveUpdateFor(RESOURCE_ID)).isFalse(); + } + + @Test + void deleteEventAfterTwoUpdates() { + support.startEventFilteringModify(RESOURCE_ID); + support.addToOwnResourceVersions(RESOURCE_ID, s(FIRST_OWN_VERSION)); + support.startEventFilteringModify(RESOURCE_ID); + support.addToOwnResourceVersions(RESOURCE_ID, s(FIRST_OWN_VERSION + 1)); + + assertThat(support.processEvent(RESOURCE_ID, updateEvent(FIRST_OWN_VERSION))).isEmpty(); + assertThat(support.processEvent(RESOURCE_ID, updateEvent(FIRST_OWN_VERSION + 1))).isEmpty(); + assertThat(support.processEvent(RESOURCE_ID, deleteEvent(FIRST_OWN_VERSION + 2))) + .hasValueSatisfying(e -> assertThat(e.getAction()).isEqualTo(DELETED)); + + support.doneEventFilterModify(RESOURCE_ID); + assertThat(support.doneEventFilterModify(RESOURCE_ID)).isEmpty(); + + assertThat(support.isActiveUpdateFor(RESOURCE_ID)).isFalse(); + } + + @Test + void reListBeforeUpdateStarted() { + support.startEventFilteringModify(RESOURCE_ID); + support.addToOwnResourceVersions(RESOURCE_ID, s(FIRST_OWN_VERSION)); + support.setStartingReList(); + assertThat(support.processEvent(RESOURCE_ID, updateEvent(FIRST_OWN_VERSION))) + .hasValueSatisfying(e -> assertThat(e.getAction()).isEqualTo(UPDATED)); + support.setRelistFinished(); + + assertThat(support.doneEventFilterModify(RESOURCE_ID)).isEmpty(); + + assertThat(support.isActiveUpdateFor(RESOURCE_ID)).isFalse(); + } + + @Test + void reListHappensAfterUpdate() { + support.startEventFilteringModify(RESOURCE_ID); + support.addToOwnResourceVersions(RESOURCE_ID, s(FIRST_OWN_VERSION)); + assertThat(support.processEvent(RESOURCE_ID, updateEvent(FIRST_OWN_VERSION))).isEmpty(); + support.setStartingReList(); + assertThat(support.processEvent(RESOURCE_ID, updateEvent(FIRST_OWN_VERSION + 1))).isEmpty(); + support.setRelistFinished(); + + assertThat(support.doneEventFilterModify(RESOURCE_ID)) + .hasValueSatisfying(e -> assertThat(e.getAction()).isEqualTo(UPDATED)); + assertThat(support.isActiveUpdateFor(RESOURCE_ID)).isFalse(); + } + + @Test + void reListBetweenTwoUpdates() { + support.startEventFilteringModify(RESOURCE_ID); + support.addToOwnResourceVersions(RESOURCE_ID, s(FIRST_OWN_VERSION)); + assertThat(support.processEvent(RESOURCE_ID, updateEvent(FIRST_OWN_VERSION))).isEmpty(); + + support.startEventFilteringModify(RESOURCE_ID); + support.addToOwnResourceVersions(RESOURCE_ID, s(FIRST_OWN_VERSION + 1)); + support.setStartingReList(); + assertThat(support.processEvent(RESOURCE_ID, updateEvent(FIRST_OWN_VERSION + 1))) + .hasValueSatisfying(e -> assertThat(e.getAction()).isEqualTo(UPDATED)); + support.setRelistFinished(); + + support.doneEventFilterModify(RESOURCE_ID); + support.doneEventFilterModify(RESOURCE_ID); + assertThat(support.isActiveUpdateFor(RESOURCE_ID)).isFalse(); + } + + // -------- ghost resource removal in combination with active windows / events -------- + + @Test + void ghostRemovalDuringActiveUpdateClearsWindow() { + // A ghost cleanup arriving while an own write is in flight wipes the window + // outright (current semantics — see EventFilterSupport.handleGhostResourceRemoval). + support.startEventFilteringModify(RESOURCE_ID); + support.addToOwnResourceVersions(RESOURCE_ID, s(FIRST_OWN_VERSION)); + support.processEvent(RESOURCE_ID, updateEvent(FIRST_OWN_VERSION - 1)); + assertThat(support.isActiveUpdateFor(RESOURCE_ID)).isTrue(); + + support.handleGhostResourceRemoval(RESOURCE_ID); + + assertThat(support.isActiveUpdateFor(RESOURCE_ID)).isFalse(); + // Subsequent events for this resource have no window → propagate verbatim. + var follow = updateEvent(FIRST_OWN_VERSION + 5); + assertThat(support.processEvent(RESOURCE_ID, follow)).contains(follow); + } + + @Test + void ghostRemovalAfterEventsHaveBeenHeldDropsThem() { + // Held foreign events that haven't yet been emitted are discarded by ghost removal. + support.startEventFilteringModify(RESOURCE_ID); + support.addToOwnResourceVersions(RESOURCE_ID, s(FIRST_OWN_VERSION)); + assertThat(support.processEvent(RESOURCE_ID, updateEvent(FIRST_OWN_VERSION - 2))).isEmpty(); + assertThat(support.processEvent(RESOURCE_ID, updateEvent(FIRST_OWN_VERSION - 1))).isEmpty(); + var window = support.getEventFilterWindows().get(RESOURCE_ID); + assertThat(window.getRelatedEvents()).isNotEmpty(); + + support.handleGhostResourceRemoval(RESOURCE_ID); + + assertThat(support.isActiveUpdateFor(RESOURCE_ID)).isFalse(); + } + + @Test + void ghostRemovalDuringReListAffectsOnlyTargetResource() { + // Ghost removal targeting one resource doesn't disturb a parallel reList window + // for another resource. + support.startEventFilteringModify(RESOURCE_ID); + support.addToOwnResourceVersions(RESOURCE_ID, s(FIRST_OWN_VERSION)); + support.startEventFilteringModify(OTHER_RESOURCE_ID); + support.setStartingReList(); + + support.handleGhostResourceRemoval(RESOURCE_ID); + + assertThat(support.isActiveUpdateFor(RESOURCE_ID)).isFalse(); + assertThat(support.isActiveUpdateFor(OTHER_RESOURCE_ID)).isTrue(); + } + + // -------- end of replicated tests -------- + + GenericResourceEvent updateEvent(long version) { + return new GenericResourceEvent( + UPDATED, testResource(version), testResource(version - 1), null); + } + + GenericResourceEvent addEvent(long version) { + return new GenericResourceEvent(ADDED, testResource(version), null, null); + } + + GenericResourceEvent deleteEvent(long version) { + return new GenericResourceEvent(DELETED, testResource(version), null, true); + } + + ConfigMap testResource(long version) { + var cm = new ConfigMap(); + cm.setMetadata( + new ObjectMetaBuilder() + .withName(RESOURCE_ID.getName()) + .withNamespace(RESOURCE_ID.getNamespace().orElseThrow()) + .withResourceVersion(Long.toString(version)) + .build()); + return cm; + } + + private String s(long l) { + return Long.toString(l); + } +} diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/EventFilterWindowTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/EventFilterWindowTest.java new file mode 100644 index 0000000000..5fbfcefbba --- /dev/null +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/EventFilterWindowTest.java @@ -0,0 +1,636 @@ +/* + * 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 org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Test; + +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.javaoperatorsdk.operator.processing.event.ResourceID; + +import static io.javaoperatorsdk.operator.processing.event.source.ResourceAction.ADDED; +import static io.javaoperatorsdk.operator.processing.event.source.ResourceAction.DELETED; +import static io.javaoperatorsdk.operator.processing.event.source.ResourceAction.UPDATED; +import static org.assertj.core.api.Assertions.assertThat; + +class EventFilterWindowTest { + + static final Long FIRST_OWN_VERSION = 5L; + + static final ResourceID RESOURCE_ID = new ResourceID("id1", "default"); + + EventFilterWindow eventFilterWindow = new EventFilterWindow(false); + + // todo ensure real call scenarios + + @Test + void oneOwnVersionNoEvent() { + eventFilterWindow.increaseActiveUpdates(); + eventFilterWindow.addToOwnResourceVersions(s(FIRST_OWN_VERSION)); + + assertThat(eventFilterWindow.check()).isEmpty(); + assertThat(eventFilterWindow.canBeRemoved()).isFalse(); + eventFilterWindow.decreaseActiveUpdates(); + assertThat(eventFilterWindow.canBeRemoved()).isFalse(); + assertThat(eventFilterWindow.getOwnResourceVersions()).containsExactly(FIRST_OWN_VERSION); + } + + @Test + void oneOwnVersionEventReceivedEventForIt() { + eventFilterWindow.increaseActiveUpdates(); + eventFilterWindow.addToOwnResourceVersions(s(FIRST_OWN_VERSION)); + eventFilterWindow.addRelatedEvent(updateEvent(FIRST_OWN_VERSION)); + + // check also cleans up the current since we received event for our own resource + assertThat(eventFilterWindow.check()).isEmpty(); + assertThat(eventFilterWindow.canBeRemoved()).isFalse(); + + eventFilterWindow.decreaseActiveUpdates(); + assertThat(eventFilterWindow.canBeRemoved()).isTrue(); + } + + @Test + void receivedAsFirstAddEventReturnTheSameEventIfThatIsOnlyRelevant() { + eventFilterWindow.increaseActiveUpdates(); + eventFilterWindow.addToOwnResourceVersions(s(FIRST_OWN_VERSION)); + eventFilterWindow.addRelatedEvent(addEvent(FIRST_OWN_VERSION)); + + assertThat(eventFilterWindow.check()).isEmpty(); + } + + @Test + void oneOwnVersionAdditionalEventReceivedBeforeIt() { + eventFilterWindow.increaseActiveUpdates(); + eventFilterWindow.addToOwnResourceVersions(s(FIRST_OWN_VERSION)); + eventFilterWindow.addRelatedEvent(updateEvent(FIRST_OWN_VERSION - 1)); + eventFilterWindow.addRelatedEvent(updateEvent(FIRST_OWN_VERSION)); + + assertThat(eventFilterWindow.check()).isPresent(); + // check also cleans up the current state, so call is not idempotent + assertThat(eventFilterWindow.check()).isEmpty(); + assertThat(eventFilterWindow.canBeRemoved()).isFalse(); + + eventFilterWindow.decreaseActiveUpdates(); + assertThat(eventFilterWindow.canBeRemoved()).isTrue(); + } + + @Test + void twoOwnVersionEventReceivedEventOnlyForFirstThenForSecond() { + eventFilterWindow.increaseActiveUpdates(); + eventFilterWindow.addToOwnResourceVersions(s(FIRST_OWN_VERSION)); + eventFilterWindow.increaseActiveUpdates(); + eventFilterWindow.addToOwnResourceVersions(s(FIRST_OWN_VERSION + 1)); + + eventFilterWindow.addRelatedEvent(updateEvent(FIRST_OWN_VERSION)); + + // check also cleans up the current since we received event for our own resource + assertThat(eventFilterWindow.check()).isEmpty(); + + assertThat(eventFilterWindow.getRelatedEvents()).isEmpty(); + assertThat(eventFilterWindow.getOwnResourceVersions()) + .containsExactlyInAnyOrder(FIRST_OWN_VERSION + 1); + + eventFilterWindow.decreaseActiveUpdates(); + eventFilterWindow.decreaseActiveUpdates(); + assertThat(eventFilterWindow.canBeRemoved()).isFalse(); + + eventFilterWindow.addRelatedEvent(updateEvent(FIRST_OWN_VERSION + 1)); + assertThat(eventFilterWindow.check()).isEmpty(); + assertThat(eventFilterWindow.canBeRemoved()).isTrue(); + assertEmptyState(); + } + + @Test + void twoOwnVersionEventReceivedOne() { + eventFilterWindow.increaseActiveUpdates(); + eventFilterWindow.addToOwnResourceVersions(s(FIRST_OWN_VERSION)); + eventFilterWindow.increaseActiveUpdates(); + eventFilterWindow.addToOwnResourceVersions(s(FIRST_OWN_VERSION + 1)); + + eventFilterWindow.addRelatedEvent(updateEvent(FIRST_OWN_VERSION)); + + // check also cleans up the current since we received event for our own resource + assertThat(eventFilterWindow.check()).isEmpty(); + + assertThat(eventFilterWindow.getRelatedEvents()).isEmpty(); + assertThat(eventFilterWindow.getOwnResourceVersions()) + .containsExactlyInAnyOrder(FIRST_OWN_VERSION + 1); + + eventFilterWindow.decreaseActiveUpdates(); + eventFilterWindow.decreaseActiveUpdates(); + assertThat(eventFilterWindow.canBeRemoved()).isFalse(); + } + + @Test + void receivedAddEventAfterOurUpdate() { + eventFilterWindow.increaseActiveUpdates(); + eventFilterWindow.addToOwnResourceVersions(s(FIRST_OWN_VERSION)); + eventFilterWindow.addRelatedEvent(addEvent(FIRST_OWN_VERSION + 1)); + eventFilterWindow.decreaseActiveUpdates(); + + assertThat(eventFilterWindow.check()) + .hasValueSatisfying(e -> assertAddEvent(e, FIRST_OWN_VERSION + 1)); + + assertThat(eventFilterWindow.check()).isEmpty(); + assertThat(eventFilterWindow.canBeRemoved()).isTrue(); + assertEmptyState(); + } + + @Test + void receivedAddEventAfterOurUpdateDone() { + eventFilterWindow.increaseActiveUpdates(); + eventFilterWindow.addToOwnResourceVersions(s(FIRST_OWN_VERSION)); + eventFilterWindow.decreaseActiveUpdates(); + eventFilterWindow.addRelatedEvent(addEvent(FIRST_OWN_VERSION + 1)); + assertThat(eventFilterWindow.check()) + .hasValueSatisfying(e -> assertAddEvent(e, FIRST_OWN_VERSION + 1)); + + assertThat(eventFilterWindow.check()).isEmpty(); + assertThat(eventFilterWindow.canBeRemoved()).isTrue(); + assertEmptyState(); + } + + @Test + void canBeRemovedIfNoActiveUpdatesOnly() { + eventFilterWindow.increaseActiveUpdates(); + eventFilterWindow.addRelatedEvent(updateEvent(FIRST_OWN_VERSION)); + assertThat(eventFilterWindow.check()).isEmpty(); + eventFilterWindow.decreaseActiveUpdates(); + + assertThat(eventFilterWindow.check()) + .hasValueSatisfying(e -> assertUpdateEvent(e, FIRST_OWN_VERSION)); + } + + @Test + void propagateEventIfNoOwnResourceAndNoActiveUpdate() { + eventFilterWindow.increaseActiveUpdates(); + eventFilterWindow.addRelatedEvent(updateEvent(FIRST_OWN_VERSION)); + eventFilterWindow.decreaseActiveUpdates(); + + assertThat(eventFilterWindow.check()) + .hasValueSatisfying(e -> assertUpdateEvent(e, FIRST_OWN_VERSION)); + assertThat(eventFilterWindow.canBeRemoved()).isTrue(); + assertEmptyState(); + } + + @Test + void receiveEventAfterEventForOwnUpdate() { + eventFilterWindow.increaseActiveUpdates(); + eventFilterWindow.addToOwnResourceVersions(s(FIRST_OWN_VERSION + 1)); + + eventFilterWindow.addRelatedEvent(updateEvent(FIRST_OWN_VERSION)); + eventFilterWindow.addRelatedEvent(updateEvent(FIRST_OWN_VERSION + 1)); + eventFilterWindow.addRelatedEvent(updateEvent(FIRST_OWN_VERSION + 2)); + + eventFilterWindow.decreaseActiveUpdates(); + + assertThat(eventFilterWindow.check()) + .hasValueSatisfying( + e -> assertUpdateEvent(e, FIRST_OWN_VERSION + 2, FIRST_OWN_VERSION - 1)); + + assertThat(eventFilterWindow.check()).isEmpty(); + + assertThat(eventFilterWindow.canBeRemoved()).isTrue(); + assertEmptyState(); + } + + @Test + void doNotIncludeAfterEventForFirstOwnUpdateIfOtherOwnUpdateIsActive() { + eventFilterWindow.increaseActiveUpdates(); + eventFilterWindow.addToOwnResourceVersions(s(FIRST_OWN_VERSION + 1)); + + eventFilterWindow.increaseActiveUpdates(); + + eventFilterWindow.addRelatedEvent(updateEvent(FIRST_OWN_VERSION)); + eventFilterWindow.addRelatedEvent(updateEvent(FIRST_OWN_VERSION + 1)); + eventFilterWindow.addRelatedEvent(updateEvent(FIRST_OWN_VERSION + 2)); + // We do not expect the update (+2) to be added here to the first check since + // other parallel update is going on. + assertThat(eventFilterWindow.check()) + .hasValueSatisfying( + e -> assertUpdateEvent(e, FIRST_OWN_VERSION + 1, FIRST_OWN_VERSION - 1)); + + eventFilterWindow.decreaseActiveUpdates(); + + assertThat(eventFilterWindow.getRelatedEvents()).isNotEmpty(); + eventFilterWindow.addToOwnResourceVersions(s(FIRST_OWN_VERSION + 2)); + + assertThat(eventFilterWindow.check()).isEmpty(); + + eventFilterWindow.decreaseActiveUpdates(); + + assertThat(eventFilterWindow.canBeRemoved()).isTrue(); + assertEmptyState(); + } + + @Test + void assertMultipleUpdatesAndIntermediateEventBetween() { + eventFilterWindow.increaseActiveUpdates(); + eventFilterWindow.addToOwnResourceVersions(s(FIRST_OWN_VERSION)); + eventFilterWindow.increaseActiveUpdates(); + eventFilterWindow.addToOwnResourceVersions(s(FIRST_OWN_VERSION + 2)); + + eventFilterWindow.addRelatedEvent(updateEvent(FIRST_OWN_VERSION)); + eventFilterWindow.addRelatedEvent(updateEvent(FIRST_OWN_VERSION + 1)); + eventFilterWindow.addRelatedEvent(updateEvent(FIRST_OWN_VERSION + 2)); + + assertThat(eventFilterWindow.check()) + .hasValueSatisfying( + e -> assertUpdateEvent(e, FIRST_OWN_VERSION + 2, FIRST_OWN_VERSION - 1)); + assertThat(eventFilterWindow.check()).isEmpty(); + + eventFilterWindow.decreaseActiveUpdates(); + eventFilterWindow.decreaseActiveUpdates(); + assertThat(eventFilterWindow.canBeRemoved()).isTrue(); + assertEmptyState(); + } + + @Test + void receiveIntermediateBetweenTwoOwnUpdates() { + eventFilterWindow.increaseActiveUpdates(); + eventFilterWindow.addToOwnResourceVersions(s(FIRST_OWN_VERSION)); + eventFilterWindow.increaseActiveUpdates(); + eventFilterWindow.addToOwnResourceVersions(s(FIRST_OWN_VERSION + 2)); + + eventFilterWindow.addRelatedEvent(updateEvent(FIRST_OWN_VERSION)); + eventFilterWindow.addRelatedEvent(updateEvent(FIRST_OWN_VERSION + 1)); + + assertThat(eventFilterWindow.check()) + .hasValueSatisfying( + e -> assertUpdateEvent(e, FIRST_OWN_VERSION + 1, FIRST_OWN_VERSION - 1)); + assertThat(eventFilterWindow.canBeRemoved()).isFalse(); + + eventFilterWindow.decreaseActiveUpdates(); + assertThat(eventFilterWindow.canBeRemoved()).isFalse(); + assertThat(eventFilterWindow.getRelatedEvents()).isEmpty(); + assertThat(eventFilterWindow.getOwnResourceVersions()).containsExactly(FIRST_OWN_VERSION + 2); + + eventFilterWindow.addRelatedEvent(updateEvent(FIRST_OWN_VERSION + 2)); + assertThat(eventFilterWindow.check()).isEmpty(); + + eventFilterWindow.decreaseActiveUpdates(); + assertThat(eventFilterWindow.canBeRemoved()).isTrue(); + assertEmptyState(); + } + + @Test + void deleteEventAsLastEvent_simpleCase() { + eventFilterWindow.increaseActiveUpdates(); + eventFilterWindow.addToOwnResourceVersions(s(FIRST_OWN_VERSION)); + eventFilterWindow.addRelatedEvent(deleteEvent(FIRST_OWN_VERSION)); + assertThat(eventFilterWindow.check()).hasValueSatisfying(this::assertDeleteEvent); + assertThat(eventFilterWindow.canBeRemoved()).isFalse(); + + eventFilterWindow.decreaseActiveUpdates(); + assertThat(eventFilterWindow.canBeRemoved()).isTrue(); + assertEmptyState(); + } + + @Test + void deleteEventBeforeOurUpdate() { + eventFilterWindow.increaseActiveUpdates(); + eventFilterWindow.addToOwnResourceVersions(s(FIRST_OWN_VERSION)); + eventFilterWindow.addRelatedEvent(deleteEvent(FIRST_OWN_VERSION - 1)); + eventFilterWindow.addRelatedEvent(addEvent(FIRST_OWN_VERSION)); + + // check also cleans up the current since we received event for our own resource + assertThat(eventFilterWindow.check()).isEmpty(); + + eventFilterWindow.decreaseActiveUpdates(); + assertThat(eventFilterWindow.canBeRemoved()).isTrue(); + assertEmptyState(); + } + + @Test + void deleteEventOnMiddleOfOwnUpdate() { + eventFilterWindow.increaseActiveUpdates(); + eventFilterWindow.addToOwnResourceVersions(s(FIRST_OWN_VERSION + 2)); + eventFilterWindow.addRelatedEvent(updateEvent(FIRST_OWN_VERSION)); + eventFilterWindow.addRelatedEvent(deleteEvent(FIRST_OWN_VERSION + 1)); + eventFilterWindow.addRelatedEvent(addEvent(FIRST_OWN_VERSION + 2)); + + // it is questionable in this particular case we should propagate last Add or Update event. + // check also cleans up the current since we received event for our own resource + assertThat(eventFilterWindow.check()) + .hasValueSatisfying( + e -> assertUpdateEvent(e, FIRST_OWN_VERSION + 2, FIRST_OWN_VERSION - 1)); + eventFilterWindow.decreaseActiveUpdates(); + assertThat(eventFilterWindow.canBeRemoved()).isTrue(); + } + + @Test + void deleteEventAsAdditionalEventAfterOwnUpdates() { + eventFilterWindow.increaseActiveUpdates(); + eventFilterWindow.addToOwnResourceVersions(s(FIRST_OWN_VERSION)); + eventFilterWindow.addRelatedEvent(updateEvent(FIRST_OWN_VERSION)); + eventFilterWindow.addRelatedEvent(deleteEvent(FIRST_OWN_VERSION + 1)); + + // check also cleans up the current since we received event for our own resource + assertThat(eventFilterWindow.check()).isEmpty(); + + assertThat(eventFilterWindow.canBeRemoved()).isFalse(); + eventFilterWindow.decreaseActiveUpdates(); + assertThat(eventFilterWindow.check()) + .hasValueSatisfying(e -> assertDeleteEvent(e, FIRST_OWN_VERSION + 1)); + assertThat(eventFilterWindow.canBeRemoved()).isTrue(); + } + + @Test + void additionalDeleteEvent() { + eventFilterWindow.increaseActiveUpdates(); + eventFilterWindow.addToOwnResourceVersions(s(FIRST_OWN_VERSION + 1)); + eventFilterWindow.addRelatedEvent(updateEvent(FIRST_OWN_VERSION)); + eventFilterWindow.addRelatedEvent(updateEvent(FIRST_OWN_VERSION + 1)); + eventFilterWindow.addRelatedEvent(deleteEvent(FIRST_OWN_VERSION + 2)); + + assertThat(eventFilterWindow.check()) + .hasValueSatisfying( + e -> assertUpdateEvent(e, FIRST_OWN_VERSION + 1, FIRST_OWN_VERSION - 1)); + + eventFilterWindow.decreaseActiveUpdates(); + + assertThat(eventFilterWindow.canBeRemoved()).isFalse(); + assertThat(eventFilterWindow.check()) + .hasValueSatisfying(e -> assertDeleteEvent(e, FIRST_OWN_VERSION + 2)); + + assertThat(eventFilterWindow.canBeRemoved()).isTrue(); + } + + @Test + void additionalEventAndDeleteEvent() { + eventFilterWindow.increaseActiveUpdates(); + eventFilterWindow.addToOwnResourceVersions(s(FIRST_OWN_VERSION)); + eventFilterWindow.addRelatedEvent(updateEvent(FIRST_OWN_VERSION)); + + eventFilterWindow.addRelatedEvent(updateEvent(FIRST_OWN_VERSION + 1)); + eventFilterWindow.addRelatedEvent(deleteEvent(FIRST_OWN_VERSION + 2)); + + assertThat(eventFilterWindow.check()).isEmpty(); + + eventFilterWindow.decreaseActiveUpdates(); + assertThat(eventFilterWindow.check()) + .hasValueSatisfying(e -> assertDeleteEvent(e, FIRST_OWN_VERSION + 2)); + assertThat(eventFilterWindow.canBeRemoved()).isTrue(); + } + + @Test + @Disabled("should be part of event filter support") + void additionalEventAndDeleteEventNoUpdate() { + eventFilterWindow.increaseActiveUpdates(); + eventFilterWindow.addToOwnResourceVersions(s(FIRST_OWN_VERSION)); + eventFilterWindow.addRelatedEvent(updateEvent(FIRST_OWN_VERSION)); + eventFilterWindow.addRelatedEvent(updateEvent(FIRST_OWN_VERSION + 1)); + eventFilterWindow.addRelatedEvent(deleteEvent(FIRST_OWN_VERSION + 2)); + + assertThat(eventFilterWindow.check()) + .hasValueSatisfying(e -> assertDeleteEvent(e, FIRST_OWN_VERSION + 2)); + assertThat(eventFilterWindow.check()).isEmpty(); + + assertEmptyState(); + eventFilterWindow.decreaseActiveUpdates(); + + assertThat(eventFilterWindow.canBeRemoved()).isTrue(); + } + + @Test + void deleteEventInMiddleTwoUpdates() { + eventFilterWindow.increaseActiveUpdates(); + eventFilterWindow.addToOwnResourceVersions(s(FIRST_OWN_VERSION)); + eventFilterWindow.addRelatedEvent(updateEvent(FIRST_OWN_VERSION)); + assertThat(eventFilterWindow.check()).isEmpty(); + + eventFilterWindow.addRelatedEvent(deleteEvent(FIRST_OWN_VERSION + 1)); + + eventFilterWindow + .increaseActiveUpdates(); // started new update delete event should not be included in first + + assertThat(eventFilterWindow.check()) + .hasValueSatisfying(e -> assertDeleteEvent(e, FIRST_OWN_VERSION + 1)); + assertEmptyState(); + + eventFilterWindow.addToOwnResourceVersions(s(FIRST_OWN_VERSION + 2)); + eventFilterWindow.addRelatedEvent(addEvent(FIRST_OWN_VERSION + 2)); + // delete event should be skipped in these cases and taking directly the last event + assertThat(eventFilterWindow.check()).isEmpty(); + + eventFilterWindow.decreaseActiveUpdates(); + + assertEmptyState(); + eventFilterWindow.decreaseActiveUpdates(); + assertThat(eventFilterWindow.canBeRemoved()).isTrue(); + } + + @Test + void deleteEventAfterTwoUpdates() { + eventFilterWindow.increaseActiveUpdates(); + eventFilterWindow.addToOwnResourceVersions(s(FIRST_OWN_VERSION)); + + eventFilterWindow.increaseActiveUpdates(); + eventFilterWindow.addToOwnResourceVersions(s(FIRST_OWN_VERSION + 1)); + + eventFilterWindow.addRelatedEvent(updateEvent(FIRST_OWN_VERSION)); + eventFilterWindow.addRelatedEvent(updateEvent(FIRST_OWN_VERSION + 1)); + + eventFilterWindow.addRelatedEvent(deleteEvent(FIRST_OWN_VERSION + 2)); + + assertThat(eventFilterWindow.check()).isEmpty(); + + eventFilterWindow.decreaseActiveUpdates(); + eventFilterWindow.decreaseActiveUpdates(); + assertThat(eventFilterWindow.check()) + .hasValueSatisfying(e -> assertDeleteEvent(e, FIRST_OWN_VERSION + 2)); + + assertEmptyState(); + assertThat(eventFilterWindow.canBeRemoved()).isTrue(); + } + + @Test + void deleteEventAfterTwoUpdatesFinished() { + eventFilterWindow.increaseActiveUpdates(); + eventFilterWindow.addToOwnResourceVersions(s(FIRST_OWN_VERSION)); + + eventFilterWindow.increaseActiveUpdates(); + eventFilterWindow.addToOwnResourceVersions(s(FIRST_OWN_VERSION + 1)); + + eventFilterWindow.addRelatedEvent(updateEvent(FIRST_OWN_VERSION)); + eventFilterWindow.addRelatedEvent(updateEvent(FIRST_OWN_VERSION + 1)); + + eventFilterWindow.addRelatedEvent(deleteEvent(FIRST_OWN_VERSION + 2)); + + eventFilterWindow.decreaseActiveUpdates(); + eventFilterWindow.decreaseActiveUpdates(); + assertThat(eventFilterWindow.check()) + .hasValueSatisfying(e -> assertDeleteEvent(e, FIRST_OWN_VERSION + 2)); + + assertEmptyState(); + assertThat(eventFilterWindow.canBeRemoved()).isTrue(); + } + + // if there is a re-list other events / changes might have arrived before re-list was done, + // so we always assume that there was an additional event there + @Test + void reListBeforeUpdateStarted() { + eventFilterWindow.increaseActiveUpdates(); + eventFilterWindow.addToOwnResourceVersions(s(FIRST_OWN_VERSION)); + eventFilterWindow.setReListStarted(); + eventFilterWindow.addRelatedEvent(updateEvent(FIRST_OWN_VERSION)); + eventFilterWindow.setReListFinished(); + + assertThat(eventFilterWindow.check()) + .hasValueSatisfying(e -> assertUpdateEvent(e, FIRST_OWN_VERSION)); + + eventFilterWindow.decreaseActiveUpdates(); + assertEmptyState(); + assertThat(eventFilterWindow.canBeRemoved()).isTrue(); + } + + @Test + void reListHappensAfterUpdate() { + eventFilterWindow.increaseActiveUpdates(); + eventFilterWindow.addToOwnResourceVersions(s(FIRST_OWN_VERSION)); + eventFilterWindow.addRelatedEvent(updateEvent(FIRST_OWN_VERSION)); + eventFilterWindow.setReListStarted(); + eventFilterWindow.addRelatedEvent(updateEvent(FIRST_OWN_VERSION + 1)); + eventFilterWindow.setReListFinished(); + + assertThat(eventFilterWindow.check()).isEmpty(); + eventFilterWindow.decreaseActiveUpdates(); + + assertThat(eventFilterWindow.check()) + .hasValueSatisfying(e -> assertUpdateEvent(e, FIRST_OWN_VERSION + 1)); + assertEmptyState(); + assertThat(eventFilterWindow.canBeRemoved()).isTrue(); + } + + @Test + void reListBetweenTwoUpdates() { + eventFilterWindow.increaseActiveUpdates(); + eventFilterWindow.addToOwnResourceVersions(s(FIRST_OWN_VERSION)); + eventFilterWindow.addRelatedEvent(updateEvent(FIRST_OWN_VERSION)); + + eventFilterWindow.increaseActiveUpdates(); + eventFilterWindow.addToOwnResourceVersions(s(FIRST_OWN_VERSION + 1)); + eventFilterWindow.setReListStarted(); + eventFilterWindow.addRelatedEvent(updateEvent(FIRST_OWN_VERSION + 1)); + eventFilterWindow.setReListFinished(); + + // this should be the case regardless of re-list + assertThat(eventFilterWindow.check()) + .hasValueSatisfying( + e -> assertUpdateEvent(e, FIRST_OWN_VERSION + 1, FIRST_OWN_VERSION - 1)); + + eventFilterWindow.decreaseActiveUpdates(); + eventFilterWindow.decreaseActiveUpdates(); + assertEmptyState(); + assertThat(eventFilterWindow.canBeRemoved()).isTrue(); + } + + @Test + void combinedCaseWithEarlyEvent() { + // Scenario: an own write is in flight (RV recorded), a foreign event with a + // lower RV arrives, then the write completes (active → 0) but no echo for + // our own RV ever arrives. The held foreign event must surface — otherwise + // the window wedges (canRemoved stays false because relatedEvents is not + // empty) and the reconciler never learns about the foreign change. + eventFilterWindow.increaseActiveUpdates(); + eventFilterWindow.addToOwnResourceVersions(s(FIRST_OWN_VERSION)); + eventFilterWindow.addRelatedEvent(updateEvent(FIRST_OWN_VERSION - 2)); + + // Held while the write is in flight. + assertThat(eventFilterWindow.check()).isEmpty(); + + // Write completes, no echo for own=[FIRST_OWN_VERSION] ever arrived. + eventFilterWindow.decreaseActiveUpdates(); + + eventFilterWindow.setReListStarted(); + eventFilterWindow.addRelatedEvent(updateEvent(FIRST_OWN_VERSION)); + // The foreign event must surface now. + eventFilterWindow.setReListFinished(); + assertThat(eventFilterWindow.check()) + .hasValueSatisfying(e -> assertUpdateEvent(e, FIRST_OWN_VERSION, FIRST_OWN_VERSION - 3)); + + assertEmptyState(); + assertThat(eventFilterWindow.canBeRemoved()).isTrue(); + } + + void assertUpdateEvent(GenericResourceEvent event, Long resourceVersion) { + assertUpdateEvent(event, resourceVersion, resourceVersion - 1); + } + + void assertUpdateEvent( + GenericResourceEvent event, Long resourceVersion, Long previousResourceVersion) { + assertThat(event.getAction()).isEqualTo(UPDATED); + assertThat(event.getResource().orElseThrow().getMetadata().getResourceVersion()) + .isEqualTo(s(resourceVersion)); + assertThat(event.getPreviousResource().orElseThrow().getMetadata().getResourceVersion()) + .isEqualTo(s(previousResourceVersion)); + assertThat(event.isLastStateUnknow()).isNull(); + } + + void assertAddEvent(GenericResourceEvent event, Long resourceVersion) { + assertThat(event.getAction()).isEqualTo(ADDED); + assertThat(event.getResource().orElseThrow().getMetadata().getResourceVersion()) + .isEqualTo(s(resourceVersion)); + assertThat(event.getPreviousResource()).isEmpty(); + assertThat(event.isLastStateUnknow()).isNull(); + } + + void assertDeleteEvent(GenericResourceEvent event) { + assertDeleteEvent(event, FIRST_OWN_VERSION); + } + + void assertDeleteEvent(GenericResourceEvent event, Long resourceVersion) { + assertThat(event.getAction()).isEqualTo(DELETED); + assertThat(event.getResource().orElseThrow().getMetadata().getResourceVersion()) + .isEqualTo(s(resourceVersion)); + assertThat(event.getPreviousResource()).isEmpty(); + assertThat(event.isLastStateUnknow()).isTrue(); + } + + GenericResourceEvent updateEvent(long version) { + return new GenericResourceEvent( + UPDATED, testResource(version), testResource(version - 1), null); + } + + GenericResourceEvent addEvent(long version) { + return new GenericResourceEvent(ADDED, testResource(version), null, null); + } + + GenericResourceEvent deleteEvent(long version) { + return new GenericResourceEvent(DELETED, testResource(version), null, true); + } + + ConfigMap testResource(Long version) { + var cm = new ConfigMap(); + cm.setMetadata( + new ObjectMetaBuilder() + .withName(RESOURCE_ID.getName()) + .withNamespace(RESOURCE_ID.getNamespace().orElseThrow()) + .withResourceVersion(version.toString()) + .build()); + return cm; + } + + private void assertEmptyState() { + assertThat(eventFilterWindow.getRelatedEvents()).isEmpty(); + assertThat(eventFilterWindow.getOwnResourceVersions()).isEmpty(); + } + + private String s(long l) { + return Long.toString(l); + } +} 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..6e5de525f7 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 @@ -15,6 +15,7 @@ */ package io.javaoperatorsdk.operator.processing.event.source.informer; +import java.lang.reflect.Field; import java.time.Duration; import java.util.HashMap; import java.util.List; @@ -29,7 +30,6 @@ 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.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.KubernetesClientException; import io.javaoperatorsdk.operator.MockKubernetesClient; @@ -46,7 +46,6 @@ import io.javaoperatorsdk.operator.processing.event.source.EventFilterTestUtils; import io.javaoperatorsdk.operator.processing.event.source.ResourceAction; import io.javaoperatorsdk.operator.processing.event.source.SecondaryToPrimaryMapper; -import io.javaoperatorsdk.operator.processing.event.source.informer.TemporaryResourceCache.EventHandling; import io.javaoperatorsdk.operator.sample.simple.TestCustomResource; import static io.javaoperatorsdk.operator.api.reconciler.Constants.DEFAULT_NAMESPACES_SET; @@ -57,7 +56,6 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.ArgumentMatchers.isNull; import static org.mockito.ArgumentMatchers.nullable; import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.mock; @@ -71,7 +69,7 @@ class InformerEventSourceTest { private static final String PREV_RESOURCE_VERSION = "0"; - private static final String DEFAULT_RESOURCE_VERSION = "1"; + private static final String DEFAULT_RESOURCE_VERSION = "2"; private InformerEventSource informerEventSource; private final KubernetesClient clientMock = MockKubernetesClient.client(Deployment.class); @@ -113,44 +111,7 @@ public synchronized void start() {} } @Test - void skipsEventPropagation() { - when(temporaryResourceCache.getResourceFromCache(any())) - .thenReturn(Optional.of(testDeployment())); - - when(temporaryResourceCache.onAddOrUpdateEvent(any(), any(), any())) - .thenReturn(EventHandling.OBSOLETE); - - informerEventSource.onAdd(testDeployment()); - informerEventSource.onUpdate(testDeployment(), testDeployment()); - - verify(eventHandlerMock, never()).handleEvent(any()); - } - - @Test - void processEventPropagationWithoutAnnotation() { - when(temporaryResourceCache.onAddOrUpdateEvent(any(), any(), any())) - .thenReturn(EventHandling.NEW); - informerEventSource.onUpdate(testDeployment(), testDeployment()); - - verify(eventHandlerMock, times(1)).handleEvent(any()); - } - - @Test - void processEventPropagationWithIncorrectAnnotation() { - when(temporaryResourceCache.onAddOrUpdateEvent(any(), any(), any())) - .thenReturn(EventHandling.NEW); - informerEventSource.onAdd( - new DeploymentBuilder(testDeployment()) - .editMetadata() - .addToAnnotations(InformerEventSource.PREVIOUS_ANNOTATION_KEY, "invalid") - .endMetadata() - .build()); - - verify(eventHandlerMock, times(1)).handleEvent(any()); - } - - @Test - void propagateEventAndRemoveResourceFromTempCacheIfResourceVersionMismatch() { + void propagatesEventAndEvictsTempCacheOnVersionMismatch() { withRealTemporaryResourceCache(); Deployment cachedDeployment = testDeployment(); @@ -164,7 +125,7 @@ void propagateEventAndRemoveResourceFromTempCacheIfResourceVersionMismatch() { } @Test - void genericFilterForEvents() { + void genericFilterRejectsAddUpdateAndDelete() { informerEventSource.setGenericFilter(r -> false); when(temporaryResourceCache.getResourceFromCache(any())).thenReturn(Optional.empty()); @@ -176,7 +137,7 @@ void genericFilterForEvents() { } @Test - void filtersOnAddEvents() { + void onAddFilterRejectsAdd() { informerEventSource.setOnAddFilter(r -> false); when(temporaryResourceCache.getResourceFromCache(any())).thenReturn(Optional.empty()); @@ -186,7 +147,7 @@ void filtersOnAddEvents() { } @Test - void filtersOnUpdateEvents() { + void onUpdateFilterRejectsUpdate() { informerEventSource.setOnUpdateFilter((r1, r2) -> false); when(temporaryResourceCache.getResourceFromCache(any())).thenReturn(Optional.empty()); @@ -196,7 +157,7 @@ void filtersOnUpdateEvents() { } @Test - void filtersOnDeleteEvents() { + void onDeleteFilterRejectsDelete() { informerEventSource.setOnDeleteFilter((r, b) -> false); when(temporaryResourceCache.getResourceFromCache(any())).thenReturn(Optional.empty()); @@ -206,293 +167,87 @@ void filtersOnDeleteEvents() { } @Test - void handlesPrevResourceVersionForUpdate() { - withRealTemporaryResourceCache(); - - CountDownLatch latch = sendForEventFilteringUpdate(2); - informerEventSource.onUpdate( - deploymentWithResourceVersion(2), deploymentWithResourceVersion(3)); - latch.countDown(); - - expectHandleEvent(3, 2); - } - - @Test - void handlesPrevResourceVersionForUpdateInCaseOfException() { - withRealTemporaryResourceCache(); + void deletePropagatesWhenTempCacheEmitsDelete() { + var resource = testDeployment(); + when(temporaryResourceCache.onDeleteEvent(resource, false)) + .thenReturn( + Optional.of(new GenericResourceEvent(ResourceAction.DELETED, resource, null, false))); - CountDownLatch latch = - EventFilterTestUtils.sendForEventFilteringUpdate( - informerEventSource, - testDeployment(), - r -> { - throw new KubernetesClientException("fake"); - }); - informerEventSource.onUpdate( - deploymentWithResourceVersion(1), deploymentWithResourceVersion(2)); - latch.countDown(); + informerEventSource.onDelete(resource, false); - expectHandleEvent(2, 1); + verify(eventHandlerMock, times(1)).handleEvent(any()); } @Test - void handlesPrevResourceVersionForUpdateInCaseOfMultipleUpdates() { - withRealTemporaryResourceCache(); + void deleteSwallowsWhenTempCacheReturnsEmpty() { + var resource = testDeployment(); + when(temporaryResourceCache.onDeleteEvent(resource, false)).thenReturn(Optional.empty()); - var deployment = testDeployment(); - CountDownLatch latch = sendForEventFilteringUpdate(deployment, 2); - informerEventSource.onUpdate( - withResourceVersion(testDeployment(), 2), withResourceVersion(testDeployment(), 3)); - informerEventSource.onUpdate( - withResourceVersion(testDeployment(), 3), withResourceVersion(testDeployment(), 4)); - latch.countDown(); + informerEventSource.onDelete(resource, false); - expectHandleEvent(4, 2); + verify(eventHandlerMock, never()).handleEvent(any()); } @Test - void doesNotPropagateEventIfReceivedBeforeUpdate() { + void failingUpdate_propagatesEventReceivedDuringWindow() { + // Filter window opens, an event arrives, the update method throws. The event must + // still surface as a synthesized propagation. withRealTemporaryResourceCache(); - CountDownLatch latch = sendForEventFilteringUpdate(2); + CountDownLatch latch = sendForExceptionThrowingUpdate(); informerEventSource.onUpdate( deploymentWithResourceVersion(1), deploymentWithResourceVersion(2)); latch.countDown(); - assertNoEventProduced(); + expectHandleUpdateEvent(2, 1); } @Test - void filterAddEventBeforeUpdate() { + void failingUpdate_doesNotPopulateTempCache() { + // putResource is only called from handleRecentResourceUpdate, which never runs when + // updateMethod throws. The temp cache must therefore stay empty. withRealTemporaryResourceCache(); - CountDownLatch latch = sendForEventFilteringUpdate(2); - informerEventSource.onAdd(deploymentWithResourceVersion(1)); - latch.countDown(); - - assertNoEventProduced(); - } - - @Test - void multipleCachingFilteringUpdates() { - withRealTemporaryResourceCache(); - CountDownLatch latch = sendForEventFilteringUpdate(2); - CountDownLatch latch2 = - sendForEventFilteringUpdate(withResourceVersion(testDeployment(), 2), 3); - + CountDownLatch latch = sendForExceptionThrowingUpdate(); informerEventSource.onUpdate( deploymentWithResourceVersion(1), deploymentWithResourceVersion(2)); latch.countDown(); - latch2.countDown(); - informerEventSource.onUpdate( - deploymentWithResourceVersion(2), deploymentWithResourceVersion(3)); - assertNoEventProduced(); + expectHandleUpdateEvent(2, 1); + assertThat(temporaryResourceCache.getResources()).isEmpty(); } @Test - void multipleCachingFilteringUpdates_variant2() { + void eventReceivedAfterFailedUpdate_isPropagatedNormally() { + // After the exception unwinds and the filter window closes, subsequent events must + // propagate via the regular non-filtered path. withRealTemporaryResourceCache(); - CountDownLatch latch = sendForEventFilteringUpdate(2); - CountDownLatch latch2 = - sendForEventFilteringUpdate(withResourceVersion(testDeployment(), 2), 3); - - informerEventSource.onUpdate( - deploymentWithResourceVersion(1), deploymentWithResourceVersion(2)); + CountDownLatch latch = sendForExceptionThrowingUpdate(); latch.countDown(); - informerEventSource.onUpdate( - deploymentWithResourceVersion(2), deploymentWithResourceVersion(3)); - latch2.countDown(); - - assertNoEventProduced(); - } - - @Test - void multipleCachingFilteringUpdates_variant3() { - withRealTemporaryResourceCache(); - - CountDownLatch latch = sendForEventFilteringUpdate(2); - CountDownLatch latch2 = - sendForEventFilteringUpdate(withResourceVersion(testDeployment(), 2), 3); + var deployment = deploymentWithResourceVersion(2); - latch.countDown(); - informerEventSource.onUpdate( - deploymentWithResourceVersion(1), deploymentWithResourceVersion(2)); - informerEventSource.onUpdate( - deploymentWithResourceVersion(2), deploymentWithResourceVersion(3)); - latch2.countDown(); + informerEventSource.onUpdate(deploymentWithResourceVersion(1), deployment); - assertNoEventProduced(); + expectPropagateEvent(deployment); } @Test - void multipleCachingFilteringUpdates_variant4() { + void ownUpdateEventIsDeferredDuringActiveFilter() { + // Sanity check that the InformerEventSource end-to-end pipeline (informer → temp cache + // → filter support → propagateEvent) suppresses an event for our own write that arrives + // before the filter closes. Detail-level cases live in EventFilterWindowTest / + // EventFilterSupportTest. withRealTemporaryResourceCache(); CountDownLatch latch = sendForEventFilteringUpdate(2); - CountDownLatch latch2 = - sendForEventFilteringUpdate(withResourceVersion(testDeployment(), 2), 3); - informerEventSource.onUpdate( deploymentWithResourceVersion(1), deploymentWithResourceVersion(2)); - informerEventSource.onUpdate( - deploymentWithResourceVersion(2), deploymentWithResourceVersion(3)); latch.countDown(); - latch2.countDown(); assertNoEventProduced(); } - @Test - void ghostCheckRemovesCachedResourceDuringFilteringUpdate() { - var mes = mock(ManagedInformerEventSource.class); - var mim = mock(InformerManager.class); - when(mes.manager()).thenReturn(mim); - when(mim.isWatchingNamespace(any())).thenReturn(true); - when(mim.lastSyncResourceVersion(any())).thenReturn("1"); - when(mim.get(any())).thenReturn(Optional.empty()); - - temporaryResourceCache = spy(new TemporaryResourceCache<>(true, mes)); - informerEventSource.setTemporalResourceCache(temporaryResourceCache); - - // put resource in cache and start a filtering update - var deployment = deploymentWithResourceVersion(2); - temporaryResourceCache.putResource(deployment); - var resourceId = ResourceID.fromResource(deployment); - temporaryResourceCache.startEventFilteringModify(resourceId); - - // advance sync version so ghost check considers the cached resource outdated - when(mim.lastSyncResourceVersion(any())).thenReturn("3"); - - // ghost check should remove the cached resource - temporaryResourceCache.checkGhostResources(); - assertThat(temporaryResourceCache.getResourceFromCache(resourceId)).isEmpty(); - - // complete the filtering update - the resource should not reappear - temporaryResourceCache.doneEventFilterModify(resourceId, "2"); - assertThat(temporaryResourceCache.getResourceFromCache(resourceId)).isEmpty(); - } - - @Test - void ghostCheckRunsConcurrentlyWithPutResource() { - var mes = mock(ManagedInformerEventSource.class); - var mim = mock(InformerManager.class); - when(mes.manager()).thenReturn(mim); - when(mim.isWatchingNamespace(any())).thenReturn(true); - when(mim.lastSyncResourceVersion(any())).thenReturn("1"); - when(mim.get(any())).thenReturn(Optional.empty()); - - temporaryResourceCache = spy(new TemporaryResourceCache<>(true, mes)); - informerEventSource.setTemporalResourceCache(temporaryResourceCache); - - // put a resource that will become a ghost - var deployment = deploymentWithResourceVersion(2); - temporaryResourceCache.putResource(deployment); - - // advance sync version so ghost check removes it - when(mim.lastSyncResourceVersion(any())).thenReturn("3"); - - temporaryResourceCache.checkGhostResources(); - assertThat(temporaryResourceCache.getResourceFromCache(ResourceID.fromResource(deployment))) - .isEmpty(); - - // now put a newer resource - should succeed even after ghost removal - var newerDeployment = deploymentWithResourceVersion(4); - temporaryResourceCache.putResource(newerDeployment); - assertThat( - temporaryResourceCache.getResourceFromCache(ResourceID.fromResource(newerDeployment))) - .isPresent(); - } - - @Test - void filteringUpdateAndGhostCheckWithNamespaceChange() { - var mes = mock(ManagedInformerEventSource.class); - var mim = mock(InformerManager.class); - when(mes.manager()).thenReturn(mim); - when(mim.isWatchingNamespace(any())).thenReturn(true); - when(mim.lastSyncResourceVersion(any())).thenReturn("1"); - when(mim.get(any())).thenReturn(Optional.empty()); - - temporaryResourceCache = spy(new TemporaryResourceCache<>(true, mes)); - informerEventSource.setTemporalResourceCache(temporaryResourceCache); - - // start filtering update and put resource - var deployment = deploymentWithResourceVersion(2); - var resourceId = ResourceID.fromResource(deployment); - temporaryResourceCache.startEventFilteringModify(resourceId); - temporaryResourceCache.putResource(deployment); - - // namespace becomes unwatched - ghost check should clean up - when(mim.isWatchingNamespace(any())).thenReturn(false); - - temporaryResourceCache.checkGhostResources(); - assertThat(temporaryResourceCache.getResourceFromCache(resourceId)).isEmpty(); - - // complete the filtering update - var doneResult = temporaryResourceCache.doneEventFilterModify(resourceId, "2"); - // resource was already cleaned by ghost check, so no deferred event - assertThat(doneResult).isEmpty(); - - // put should be rejected since namespace is no longer watched - temporaryResourceCache.putResource(deploymentWithResourceVersion(3)); - assertThat(temporaryResourceCache.getResourceFromCache(resourceId)).isEmpty(); - } - - private void assertNoEventProduced() { - await() - .pollDelay(Duration.ofMillis(50)) - .timeout(Duration.ofMillis(51)) - .untilAsserted( - () -> verify(informerEventSource, never()).handleEvent(any(), any(), any(), any())); - } - - private void expectHandleEvent(int newResourceVersion, int oldResourceVersion) { - await() - .untilAsserted( - () -> { - verify(informerEventSource, times(1)) - .handleEvent( - eq(ResourceAction.UPDATED), - argThat( - newResource -> { - assertThat(newResource.getMetadata().getResourceVersion()) - .isEqualTo("" + newResourceVersion); - return true; - }), - argThat( - newResource -> { - assertThat(newResource.getMetadata().getResourceVersion()) - .isEqualTo("" + oldResourceVersion); - return true; - }), - isNull()); - }); - } - - private CountDownLatch sendForEventFilteringUpdate(int resourceVersion) { - return sendForEventFilteringUpdate(testDeployment(), resourceVersion); - } - - private CountDownLatch sendForEventFilteringUpdate(Deployment deployment, int resourceVersion) { - return EventFilterTestUtils.sendForEventFilteringUpdate( - informerEventSource, deployment, r -> withResourceVersion(deployment, resourceVersion)); - } - - private void withRealTemporaryResourceCache() { - var mes = mock(ManagedInformerEventSource.class); - var mim = mock(InformerManager.class); - when(mes.manager()).thenReturn(mim); - when(mim.lastSyncResourceVersion(any())).thenReturn("1"); - - temporaryResourceCache = spy(new TemporaryResourceCache<>(true, mes)); - informerEventSource.setTemporalResourceCache(temporaryResourceCache); - } - - Deployment deploymentWithResourceVersion(int resourceVersion) { - return withResourceVersion(testDeployment(), resourceVersion); - } - @Test void informerStoppedHandlerShouldBeCalledWhenInformerStops() { final var exception = new RuntimeException("Informer stopped exceptionally!"); @@ -577,23 +332,40 @@ void listKeepsResourceWhenNotInTempCache() { } @Test - void listReplacesOnlyMatchingResources() { - var dep1 = testDeployment(); - var dep2 = testDeployment(); - dep2.getMetadata().setName("other"); - var newerDep1 = testDeployment(); - newerDep1.getMetadata().setResourceVersion("5"); + void listKeepsResourceWhenTempCacheHasOlderVersion() { + var original = testDeployment(); + original.getMetadata().setResourceVersion("5"); + var olderTemp = testDeployment(); + olderTemp.getMetadata().setResourceVersion("3"); when(temporaryResourceCache.getResources()) - .thenReturn(new HashMap<>(Map.of(ResourceID.fromResource(dep1), newerDep1))); + .thenReturn(new HashMap<>(Map.of(ResourceID.fromResource(original), olderTemp))); - var informerManager = mock(InformerManager.class); - when(informerManager.list(nullable(String.class))).thenReturn(Stream.of(dep1, dep2)); - when(informerEventSource.manager()).thenReturn(informerManager); + var mim = mock(InformerManager.class); + when(mim.list(nullable(String.class))).thenReturn(Stream.of(original)); + when(informerEventSource.manager()).thenReturn(mim); + + var result = informerEventSource.list(null, r -> true).toList(); + + assertThat(result).containsExactly(original); + } + + @Test + void listAddsGhostResources() { + var resource = testDeployment(); + var ghostResource = testDeployment(); + ghostResource.getMetadata().setName("ghost"); + + when(temporaryResourceCache.getResources()) + .thenReturn(new HashMap<>(Map.of(ResourceID.fromResource(ghostResource), ghostResource))); + + var mim = mock(InformerManager.class); + when(mim.list(nullable(String.class))).thenReturn(Stream.of(resource)); + when(informerEventSource.manager()).thenReturn(mim); var result = informerEventSource.list(null, r -> true).toList(); - assertThat(result).containsExactlyInAnyOrder(newerDep1, dep2); + assertThat(result).containsExactlyInAnyOrder(resource, ghostResource); } @Test @@ -638,106 +410,402 @@ void byIndexStreamSkipsNewerTempCacheResourceWhenIndexedValueChanged() { } @Test - void listKeepsResourceWhenTempCacheHasOlderVersion() { - var original = testDeployment(); - original.getMetadata().setResourceVersion("5"); - var olderTemp = testDeployment(); - olderTemp.getMetadata().setResourceVersion("3"); + void keysIncludeGhostResourceKeys() { + var resource = testDeployment(); + var ghostResource = testDeployment(); + ghostResource.getMetadata().setName("ghost"); - when(temporaryResourceCache.getResources()) - .thenReturn(new HashMap<>(Map.of(ResourceID.fromResource(original), olderTemp))); + var resourceId = ResourceID.fromResource(resource); + var ghostResourceId = ResourceID.fromResource(ghostResource); + + when(temporaryResourceCache.getResources()).thenReturn(Map.of(ghostResourceId, ghostResource)); + when(temporaryResourceCache.isEmpty()).thenReturn(false); var mim = mock(InformerManager.class); - when(mim.list(nullable(String.class))).thenReturn(Stream.of(original)); + when(mim.keys()).thenReturn(Stream.of(resourceId)); + when(mim.contains(ghostResourceId)).thenReturn(false); when(informerEventSource.manager()).thenReturn(mim); - var result = informerEventSource.list(null, r -> true).toList(); + var result = informerEventSource.keys().toList(); - assertThat(result).containsExactly(original); + assertThat(result).containsExactlyInAnyOrder(resourceId, ghostResourceId); } @Test - void byIndexStreamKeepsResourceWhenTempCacheHasOlderVersion() { - var original = testDeployment(); - original.getMetadata().setResourceVersion("5"); - var olderTemp = testDeployment(); - olderTemp.getMetadata().setResourceVersion("3"); + void keysDoNotDuplicateExistingKeys() { + var resource = testDeployment(); + var newerResource = testDeployment(); + newerResource.getMetadata().setResourceVersion("5"); - when(temporaryResourceCache.getResources()) - .thenReturn(new HashMap<>(Map.of(ResourceID.fromResource(original), olderTemp))); + var resourceId = ResourceID.fromResource(resource); + + when(temporaryResourceCache.getResources()).thenReturn(Map.of(resourceId, newerResource)); + when(temporaryResourceCache.isEmpty()).thenReturn(false); var mim = mock(InformerManager.class); - when(mim.byIndexStream(any(), any())).thenReturn(Stream.of(original)); + when(mim.keys()).thenReturn(Stream.of(resourceId)); + when(mim.contains(resourceId)).thenReturn(true); when(informerEventSource.manager()).thenReturn(mim); - informerEventSource.addIndexers(Map.of("idx", d -> List.of("key"))); - var result = informerEventSource.byIndexStream("idx", "key").toList(); + var result = informerEventSource.keys().toList(); - assertThat(result).containsExactly(original); + assertThat(result).containsExactly(resourceId); } @Test - void listAddsGhostResources() { - var resource = testDeployment(); - var ghostResource = testDeployment(); - ghostResource.getMetadata().setName("ghost"); + void checkGhostResourcesPropagatesDeleteForMissingTempCacheEntry() { + // A resource lingers in the temp cache after our write but the informer never + // observed it (e.g. the resource was deleted before the watch caught up). + // checkGhostResources should remove it and surface a synthetic DELETE event + // so the reconciler is notified. + var ghost = testDeployment(); + ghost.getMetadata().setNamespace("default"); + ghost.getMetadata().setResourceVersion("3"); - when(temporaryResourceCache.getResources()) - .thenReturn(new HashMap<>(Map.of(ResourceID.fromResource(ghostResource), ghostResource))); + var tempCache = new TemporaryResourceCache<>(true, informerEventSource); + informerEventSource.setTemporalResourceCache(tempCache); - var mim = mock(InformerManager.class); - when(mim.list(nullable(String.class))).thenReturn(Stream.of(resource)); - when(informerEventSource.manager()).thenReturn(mim); + var manager = mock(InformerManager.class); + when(manager.isWatchingNamespace(any())).thenReturn(true); + when(manager.lastSyncResourceVersion(any())).thenReturn("1"); + when(manager.get(any())).thenReturn(Optional.empty()); + when(informerEventSource.manager()).thenReturn(manager); - var result = informerEventSource.list(null, r -> true).toList(); + tempCache.putResource(ghost); + assertThat(tempCache.getResources()).containsKey(ResourceID.fromResource(ghost)); - assertThat(result).containsExactlyInAnyOrder(resource, ghostResource); + // Informer's last-sync moves past the temp cache entry's RV and the resource + // is missing from the informer's cache → it qualifies as a ghost. + when(manager.lastSyncResourceVersion(any())).thenReturn("5"); + + tempCache.checkGhostResources(); + + assertThat(tempCache.getResources()).isEmpty(); + verify(eventHandlerMock, times(1)).handleEvent(any()); } @Test - void keysIncludesGhostResourceKeys() { + void checkGhostResourcesKeepsResourcePresentInInformerCache() { + // Same setup as the ghost test, but the informer's cache still has the + // resource — it is NOT a ghost; the temp cache entry should be left alone + // and no DELETE should propagate. var resource = testDeployment(); - var ghostResource = testDeployment(); - ghostResource.getMetadata().setName("ghost"); + resource.getMetadata().setNamespace("default"); + resource.getMetadata().setResourceVersion("3"); + + var tempCache = new TemporaryResourceCache<>(true, informerEventSource); + informerEventSource.setTemporalResourceCache(tempCache); + + var manager = mock(InformerManager.class); + when(manager.isWatchingNamespace(any())).thenReturn(true); + when(manager.lastSyncResourceVersion(any())).thenReturn("1"); + when(manager.get(any())).thenReturn(Optional.of(resource)); + when(informerEventSource.manager()).thenReturn(manager); + + tempCache.putResource(resource); + when(manager.lastSyncResourceVersion(any())).thenReturn("5"); + + tempCache.checkGhostResources(); + + assertThat(tempCache.getResources()).containsKey(ResourceID.fromResource(resource)); + verify(eventHandlerMock, never()).handleEvent(any()); + } + + @Test + void ghostCleanupDiscardsOrphanFilterWindow() { + // We did an own write, recorded its rv into the filter window, but the informer never + // delivered a watch event for it (resource deleted before the watch caught up). + // Ghost cleanup must drop both the temp cache entry AND the orphan filter window; + // otherwise the window leaks ownResourceVersions forever and a future event for a + // recreated resource at the same id would be wrongly filtered. + var ghost = testDeployment(); + ghost.getMetadata().setNamespace("default"); + ghost.getMetadata().setResourceVersion("3"); + var resourceId = ResourceID.fromResource(ghost); + + var tempCache = new TemporaryResourceCache<>(true, informerEventSource); + informerEventSource.setTemporalResourceCache(tempCache); + + var manager = mock(InformerManager.class); + when(manager.isWatchingNamespace(any())).thenReturn(true); + when(manager.lastSyncResourceVersion(any())).thenReturn("1"); + when(manager.get(any())).thenReturn(Optional.empty()); + when(informerEventSource.manager()).thenReturn(manager); + + // Open a filter window for an in-flight write, then close it (our update returned but + // the watch event never arrived). + tempCache.startEventFilteringModify(resourceId); + tempCache.putResource(ghost); + tempCache.doneEventFilterModify(resourceId); + assertThat(tempCache.getEventFilterSupport().isActiveUpdateFor(resourceId)) + .as("filter window must persist while ownResourceVersions is non-empty") + .isTrue(); + + when(manager.lastSyncResourceVersion(any())).thenReturn("5"); + + tempCache.checkGhostResources(); + + assertThat(tempCache.getResources()).isEmpty(); + assertThat(tempCache.getEventFilterSupport().isActiveUpdateFor(resourceId)) + .as("orphan filter window must be discarded by ghost cleanup") + .isFalse(); + verify(eventHandlerMock, times(1)).handleEvent(any()); + } + + @Test + void ghostCleanupSyntheticDeleteRespectsOnDeleteFilter() throws Exception { + // The synthetic DELETE produced by ghost cleanup flows through handleEvent and must + // honor the user's onDeleteFilter — same semantics as a real watch DELETE. The temp + // cache is still drained and the index still drops the entry; only propagation is + // skipped. + var indexMock = injectIndexMock(); + var ghost = testDeployment(); + ghost.getMetadata().setNamespace("default"); + ghost.getMetadata().setResourceVersion("3"); + var tempCache = new TemporaryResourceCache<>(true, informerEventSource); + informerEventSource.setTemporalResourceCache(tempCache); + informerEventSource.setOnDeleteFilter((r, b) -> false); + + var manager = mock(InformerManager.class); + when(manager.isWatchingNamespace(any())).thenReturn(true); + when(manager.lastSyncResourceVersion(any())).thenReturn("1"); + when(manager.get(any())).thenReturn(Optional.empty()); + when(informerEventSource.manager()).thenReturn(manager); + + tempCache.putResource(ghost); + when(manager.lastSyncResourceVersion(any())).thenReturn("5"); + + tempCache.checkGhostResources(); + + assertThat(tempCache.getResources()).isEmpty(); + verify(indexMock, times(1)).onDelete(ghost); + verify(eventHandlerMock, never()).handleEvent(any()); + } + + @Test + void ghostCleanupRetainsActiveFilterWindowWhenResourcePresentInInformer() { + // Mirror of checkGhostResourcesKeepsResourcePresentInInformerCache, but with an active + // filter window: if the resource is still in the informer cache, the temp entry stays + // AND the filter window must stay too — the in-flight write echo is still expected. + var resource = testDeployment(); + resource.getMetadata().setNamespace("default"); + resource.getMetadata().setResourceVersion("3"); var resourceId = ResourceID.fromResource(resource); - var ghostResourceId = ResourceID.fromResource(ghostResource); - when(temporaryResourceCache.getResources()).thenReturn(Map.of(ghostResourceId, ghostResource)); - when(temporaryResourceCache.isEmpty()).thenReturn(false); + var tempCache = new TemporaryResourceCache<>(true, informerEventSource); + informerEventSource.setTemporalResourceCache(tempCache); - var mim = mock(InformerManager.class); - when(mim.keys()).thenReturn(Stream.of(resourceId)); - when(mim.contains(ghostResourceId)).thenReturn(false); - when(informerEventSource.manager()).thenReturn(mim); + var manager = mock(InformerManager.class); + when(manager.isWatchingNamespace(any())).thenReturn(true); + when(manager.lastSyncResourceVersion(any())).thenReturn("1"); + when(manager.get(any())).thenReturn(Optional.of(resource)); + when(informerEventSource.manager()).thenReturn(manager); - var result = informerEventSource.keys().toList(); + tempCache.startEventFilteringModify(resourceId); + tempCache.putResource(resource); + tempCache.doneEventFilterModify(resourceId); - assertThat(result).containsExactlyInAnyOrder(resourceId, ghostResourceId); + when(manager.lastSyncResourceVersion(any())).thenReturn("5"); + tempCache.checkGhostResources(); + + assertThat(tempCache.getResources()).containsKey(resourceId); + assertThat(tempCache.getEventFilterSupport().isActiveUpdateFor(resourceId)) + .as("non-ghost: filter window must be preserved") + .isTrue(); + verify(eventHandlerMock, never()).handleEvent(any()); } @Test - void keysDoesNotDuplicateExistingKeys() { + void foreignUpdateDuringOwnUpdateIsPropagated() { + // Sanity check that an external update arriving while our write is in flight + // is surfaced to the reconciler — it isn't an own echo, so the filter must + // let it through. + withRealTemporaryResourceCache(); + + CountDownLatch latch = sendForEventFilteringUpdate(2); + informerEventSource.onUpdate( + deploymentWithResourceVersion(2), deploymentWithResourceVersion(3)); + latch.countDown(); + + expectHandleUpdateEvent(3, 2); + } + + @Test + void deleteEventDuringOwnUpdateIsPropagated() { + // A DELETE arriving while our write is in flight must surface — the + // resource has gone, so the filter should not silence it just because our + // own write is still tracking RVs. + withRealTemporaryResourceCache(); + + CountDownLatch latch = sendForEventFilteringUpdate(2); + informerEventSource.onDelete(deploymentWithResourceVersion(3), false); + latch.countDown(); + + await() + .atMost(Duration.ofSeconds(1)) + .untilAsserted(() -> verify(eventHandlerMock, atLeastOnce()).handleEvent(any())); + } + + @Test + void handleEventUpdatesIndexWhenDeletePropagatesFromTempCache() throws Exception { + // handleEvent is invoked from ManagedInformerEventSource#eventFilteringUpdateAndCacheResource + // only after the temp cache decided to surface the event. For a DELETE that means the resource + // is really gone and the secondary→primary index must drop it; otherwise stale entries linger + // and getSecondaryResources keeps returning a tombstone. + var indexMock = injectIndexMock(); var resource = testDeployment(); - var newerResource = testDeployment(); - newerResource.getMetadata().setResourceVersion("5"); - var resourceId = ResourceID.fromResource(resource); + informerEventSource.handleEvent(ResourceAction.DELETED, resource, null, false); - when(temporaryResourceCache.getResources()).thenReturn(Map.of(resourceId, newerResource)); - when(temporaryResourceCache.isEmpty()).thenReturn(false); + verify(indexMock, times(1)).onDelete(resource); + verify(indexMock, never()).onAddOrUpdate(any()); + verify(eventHandlerMock, times(1)).handleEvent(any()); + } + + @Test + void handleEventDoesNotTouchIndexForNonDeleteAction() throws Exception { + // The onAdd/onUpdate path maintains the index in onAddOrUpdate(); handleEvent must not + // double-update it for non-DELETE actions, otherwise we'd index resources twice. + var indexMock = injectIndexMock(); + + informerEventSource.handleEvent( + ResourceAction.UPDATED, testDeployment(), testDeployment(), null); + + verify(indexMock, never()).onDelete(any()); + verify(indexMock, never()).onAddOrUpdate(any()); + verify(eventHandlerMock, times(1)).handleEvent(any()); + } + + @Test + void handleEventRespectsOnDeleteFilter() throws Exception { + // The temp-cache pipeline must honor user-level filters: if onDeleteFilter rejects, the + // synthesized DELETE must not be surfaced. The index, however, is still updated because the + // resource is really gone — same semantics as the direct onDelete watch path. + var indexMock = injectIndexMock(); + informerEventSource.setOnDeleteFilter((r, b) -> false); + var resource = testDeployment(); + + informerEventSource.handleEvent(ResourceAction.DELETED, resource, null, false); + + verify(indexMock, times(1)).onDelete(resource); + verify(eventHandlerMock, never()).handleEvent(any()); + } + + @Test + void handleEventRespectsOnUpdateFilter() throws Exception { + var indexMock = injectIndexMock(); + informerEventSource.setOnUpdateFilter((n, o) -> false); + + informerEventSource.handleEvent( + ResourceAction.UPDATED, testDeployment(), testDeployment(), null); + + verify(indexMock, never()).onDelete(any()); + verify(eventHandlerMock, never()).handleEvent(any()); + } + + @Test + void handleEventRespectsOnAddFilter() throws Exception { + var indexMock = injectIndexMock(); + informerEventSource.setOnAddFilter(r -> false); + informerEventSource.handleEvent(ResourceAction.ADDED, testDeployment(), null, null); + + verify(indexMock, never()).onDelete(any()); + verify(eventHandlerMock, never()).handleEvent(any()); + } + + @Test + void handleEventRespectsGenericFilter() throws Exception { + // The generic filter applies regardless of action and short-circuits per-action filters. + // For DELETE the index is still updated (resource really gone), but no event is propagated + // for any action. + var indexMock = injectIndexMock(); + informerEventSource.setGenericFilter(r -> false); + var resource = testDeployment(); + + informerEventSource.handleEvent(ResourceAction.DELETED, resource, null, true); + informerEventSource.handleEvent(ResourceAction.UPDATED, resource, resource, null); + informerEventSource.handleEvent(ResourceAction.ADDED, resource, null, null); + + verify(indexMock, times(1)).onDelete(resource); + verify(eventHandlerMock, never()).handleEvent(any()); + } + + private PrimaryToSecondaryIndex injectIndexMock() throws Exception { + @SuppressWarnings("unchecked") + PrimaryToSecondaryIndex indexMock = mock(PrimaryToSecondaryIndex.class); + Field field = InformerEventSource.class.getDeclaredField("primaryToSecondaryIndex"); + field.setAccessible(true); + field.set(informerEventSource, indexMock); + return indexMock; + } + + private void assertNoEventProduced() { + await() + .pollDelay(Duration.ofMillis(70)) + .timeout(Duration.ofMillis(150)) + .untilAsserted(() -> verify(informerEventSource, never()).propagateEvent(any())); + } + + private void expectPropagateEvent(Deployment newResourceVersion) { + await() + .atMost(Duration.ofSeconds(1)) + .untilAsserted( + () -> verify(informerEventSource, times(1)).propagateEvent(newResourceVersion)); + } + + private void expectHandleUpdateEvent(int newResourceVersion, int oldResourceVersion) { + await() + .atMost(Duration.ofSeconds(1)) + .untilAsserted( + () -> + verify(informerEventSource, times(1)) + .handleEvent( + eq(ResourceAction.UPDATED), + argThat( + r -> + ("" + newResourceVersion) + .equals(r.getMetadata().getResourceVersion())), + argThat( + r -> + ("" + oldResourceVersion) + .equals(r.getMetadata().getResourceVersion())), + any())); + } + + private CountDownLatch sendForEventFilteringUpdate(int resourceVersion) { + return EventFilterTestUtils.sendForEventFilteringUpdate( + informerEventSource, + testDeployment(), + r -> withResourceVersion(testDeployment(), resourceVersion)); + } + + private CountDownLatch sendForExceptionThrowingUpdate() { + return EventFilterTestUtils.sendForEventFilteringUpdate( + informerEventSource, + testDeployment(), + r -> { + throw new KubernetesClientException("fake"); + }); + } + + private void withRealTemporaryResourceCache() { + var mes = mock(ManagedInformerEventSource.class); var mim = mock(InformerManager.class); - when(mim.keys()).thenReturn(Stream.of(resourceId)); - when(mim.contains(resourceId)).thenReturn(true); - when(informerEventSource.manager()).thenReturn(mim); + when(mes.manager()).thenReturn(mim); + when(mim.isWatchingNamespace(any())).thenReturn(true); + when(mim.lastSyncResourceVersion(any())).thenReturn("1"); - var result = informerEventSource.keys().toList(); + temporaryResourceCache = spy(new TemporaryResourceCache<>(true, mes)); + informerEventSource.setTemporalResourceCache(temporaryResourceCache); + } - assertThat(result).containsExactly(resourceId); + private Deployment deploymentWithResourceVersion(int resourceVersion) { + return withResourceVersion(testDeployment(), resourceVersion); } - Deployment testDeployment() { + private Deployment testDeployment() { Deployment deployment = new Deployment(); deployment.setMetadata(new ObjectMeta()); deployment.getMetadata().setResourceVersion(DEFAULT_RESOURCE_VERSION); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryResourceCacheTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryResourceCacheTest.java index 9a58b83f88..948a8a330c 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryResourceCacheTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryResourceCacheTest.java @@ -16,6 +16,7 @@ package io.javaoperatorsdk.operator.processing.event.source.informer; import java.util.Map; +import java.util.Optional; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -25,7 +26,6 @@ import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; import io.javaoperatorsdk.operator.processing.event.ResourceID; import io.javaoperatorsdk.operator.processing.event.source.ResourceAction; -import io.javaoperatorsdk.operator.processing.event.source.informer.TemporaryResourceCache.EventHandling; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -100,7 +100,7 @@ void addOperationNotAddsTheResourceIfInformerCacheNotEmpty() { var testResource = testResource(); temporaryResourceCache.putResource(testResource); - + when(managedInformerEventSource.get(any())).thenReturn(Optional.of(testResource)); temporaryResourceCache.putResource( new ConfigMapBuilder(testResource) .editMetadata() @@ -155,37 +155,13 @@ void eventReceivedDuringFiltering() { .isEmpty(); var doneRes = - temporaryResourceCache.doneEventFilterModify(ResourceID.fromResource(testResource), "2"); + temporaryResourceCache.doneEventFilterModify(ResourceID.fromResource(testResource)); assertThat(doneRes).isEmpty(); assertThat(temporaryResourceCache.getResourceFromCache(ResourceID.fromResource(testResource))) .isEmpty(); } - @Test - void newerEventDuringFiltering() { - var testResource = testResource(); - - temporaryResourceCache.startEventFilteringModify(ResourceID.fromResource(testResource)); - - temporaryResourceCache.putResource(testResource); - assertThat(temporaryResourceCache.getResourceFromCache(ResourceID.fromResource(testResource))) - .isPresent(); - - var testResource2 = testResource(); - testResource2.getMetadata().setResourceVersion("3"); - temporaryResourceCache.onAddOrUpdateEvent(ResourceAction.UPDATED, testResource2, testResource); - assertThat(temporaryResourceCache.getResourceFromCache(ResourceID.fromResource(testResource))) - .isEmpty(); - - var doneRes = - temporaryResourceCache.doneEventFilterModify(ResourceID.fromResource(testResource), "2"); - - assertThat(doneRes).isPresent(); - assertThat(temporaryResourceCache.getResourceFromCache(ResourceID.fromResource(testResource))) - .isEmpty(); - } - @Test void eventAfterFiltering() { var testResource = testResource(); @@ -197,7 +173,7 @@ void eventAfterFiltering() { .isPresent(); var doneRes = - temporaryResourceCache.doneEventFilterModify(ResourceID.fromResource(testResource), "2"); + temporaryResourceCache.doneEventFilterModify(ResourceID.fromResource(testResource)); assertThat(doneRes).isEmpty(); assertThat(temporaryResourceCache.getResourceFromCache(ResourceID.fromResource(testResource))) @@ -208,23 +184,6 @@ void eventAfterFiltering() { .isEmpty(); } - @Test - void putBeforeEvent() { - var testResource = testResource(); - - // first ensure an event is not known - var result = - temporaryResourceCache.onAddOrUpdateEvent(ResourceAction.ADDED, testResource, null); - assertThat(result).isEqualTo(EventHandling.NEW); - - var nextResource = testResource(); - nextResource.getMetadata().setResourceVersion("3"); - temporaryResourceCache.putResource(nextResource); - - result = temporaryResourceCache.onAddOrUpdateEvent(ResourceAction.UPDATED, nextResource, null); - assertThat(result).isEqualTo(EventHandling.OBSOLETE); - } - @Test void putBeforeEventWithEventFiltering() { var testResource = testResource(); @@ -232,7 +191,7 @@ void putBeforeEventWithEventFiltering() { // first ensure an event is not known var result = temporaryResourceCache.onAddOrUpdateEvent(ResourceAction.ADDED, testResource, null); - assertThat(result).isEqualTo(EventHandling.NEW); + assertThat(result).isPresent(); latestSyncVersion = RESOURCE_VERSION; var nextResource = testResource(); @@ -241,11 +200,11 @@ void putBeforeEventWithEventFiltering() { temporaryResourceCache.startEventFilteringModify(resourceId); temporaryResourceCache.putResource(nextResource); - temporaryResourceCache.doneEventFilterModify(resourceId, "3"); + temporaryResourceCache.doneEventFilterModify(resourceId); latestSyncVersion = "3"; result = temporaryResourceCache.onAddOrUpdateEvent(ResourceAction.UPDATED, nextResource, null); - assertThat(result).isEqualTo(EventHandling.OBSOLETE); + assertThat(result).isEmpty(); } @Test @@ -255,7 +214,14 @@ void putAfterEventWithEventFilteringNoPost() { // first ensure an event is not known var result = temporaryResourceCache.onAddOrUpdateEvent(ResourceAction.ADDED, testResource, null); - assertThat(result).isEqualTo(EventHandling.NEW); + assertThat(result) + .hasValueSatisfying( + v -> { + assertThat(v.getAction()).isEqualTo(ResourceAction.ADDED); + assertThat(v.getPreviousResource()).isEmpty(); + assertThat(v.getResource()).contains(testResource); + assertThat(v.isLastStateUnknow()).isNull(); + }); var nextResource = testResource(); nextResource.getMetadata().setResourceVersion("3"); @@ -265,10 +231,10 @@ void putAfterEventWithEventFilteringNoPost() { result = temporaryResourceCache.onAddOrUpdateEvent( ResourceAction.UPDATED, nextResource, testResource); - // the result is deferred - assertThat(result).isEqualTo(EventHandling.DEFER); + assertThat(result).isEmpty(); + temporaryResourceCache.putResource(nextResource); - var postEvent = temporaryResourceCache.doneEventFilterModify(resourceId, "3"); + var postEvent = temporaryResourceCache.doneEventFilterModify(resourceId); // there is no post event because the done call claimed responsibility for rv 3 assertTrue(postEvent.isEmpty()); @@ -280,20 +246,45 @@ void putAfterEventWithEventFilteringWithPost() { var resourceId = ResourceID.fromResource(testResource); temporaryResourceCache.startEventFilteringModify(resourceId); - // this should be a corner case - watch had a hard reset since the start of the + // this should be a corner case - watch had a hard reset since the start // of the update operation, such that 4 rv event is seen prior to the update // completing with the 3 rv. var nextResource = testResource(); nextResource.getMetadata().setResourceVersion("4"); var result = temporaryResourceCache.onAddOrUpdateEvent(ResourceAction.ADDED, nextResource, null); - assertThat(result).isEqualTo(EventHandling.DEFER); + assertThat(result).isEmpty(); - var postEvent = temporaryResourceCache.doneEventFilterModify(resourceId, "3"); + var postEvent = temporaryResourceCache.doneEventFilterModify(resourceId); assertTrue(postEvent.isPresent()); } + @Test + void intermediateEventPropagatedWhenNoActiveUpdate() { + // Cache holds a newer version from a prior own write; no active filter is in progress. + // An older event arriving used to be OBSOLETE; now it must be propagated as INTERMEDIATE + // so callers can react to changes that happened between read and write. + var olderEvent = testResource(); + var newer = testResource(); + newer.getMetadata().setResourceVersion("3"); + + temporaryResourceCache.putResource(newer); + assertThat(temporaryResourceCache.getResourceFromCache(ResourceID.fromResource(olderEvent))) + .isPresent(); + + var result = + temporaryResourceCache.onAddOrUpdateEvent(ResourceAction.UPDATED, olderEvent, null); + + assertThat(result) + .hasValueSatisfying( + e -> { + assertThat(e.getResource().orElseThrow()).isEqualTo(olderEvent); + assertThat(e.getPreviousResource()).isNotPresent(); + assertThat(e.getAction()).isEqualTo(ResourceAction.UPDATED); + }); + } + @Test void rapidDeletion() { var testResource = testResource(); @@ -372,6 +363,25 @@ void doNotCacheResourceOnPutIfNamespaceIsNotFollowedAnymore() { assertThat(temporaryResourceCache.getResourceFromCache(ResourceID.fromResource(tr))).isEmpty(); } + @Test + void unknownStateDeleteEvictsTempCacheEvenWhenOlder() { + var newer = + new ConfigMapBuilder(testResource()) + .editMetadata() + .withResourceVersion("5") + .endMetadata() + .build(); + temporaryResourceCache.putResource(newer); + + var olderUnknownState = testResource(); + var result = temporaryResourceCache.onDeleteEvent(olderUnknownState, true); + + assertThat(result).isPresent(); + assertThat(result.get().getAction()).isEqualTo(ResourceAction.DELETED); + assertThat(temporaryResourceCache.getResourceFromCache(ResourceID.fromResource(newer))) + .isEmpty(); + } + private ConfigMap propagateTestResourceToCache() { var testResource = testResource(); temporaryResourceCache.putResource(testResource); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/deletionduringstatusupdate/DeletionDuringStatusUpdateCustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/deletionduringstatusupdate/DeletionDuringStatusUpdateCustomResource.java new file mode 100644 index 0000000000..60d09f82f8 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/deletionduringstatusupdate/DeletionDuringStatusUpdateCustomResource.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.readcacheafterwrite.deletionduringstatusupdate; + +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("ddsu") +public class DeletionDuringStatusUpdateCustomResource + extends CustomResource implements Namespaced {} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/deletionduringstatusupdate/DeletionDuringStatusUpdateIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/deletionduringstatusupdate/DeletionDuringStatusUpdateIT.java new file mode 100644 index 0000000000..3012c9538c --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/deletionduringstatusupdate/DeletionDuringStatusUpdateIT.java @@ -0,0 +1,107 @@ +/* + * 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.readcacheafterwrite.deletionduringstatusupdate; + +import java.time.Duration; +import java.util.Collections; +import java.util.concurrent.TimeUnit; + +import org.junit.jupiter.api.AfterEach; +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; + +/** + * Regression test for: deletion event dropped when resource is deleted concurrently with a status + * update. + */ +class DeletionDuringStatusUpdateIT { + + static final String RESOURCE_NAME = "test-resource"; + + @RegisterExtension + LocallyRunOperatorExtension extension = + LocallyRunOperatorExtension.builder() + .withReconciler(new DeletionDuringStatusUpdateReconciler()) + .build(); + + @AfterEach + void forceCleanup() { + // If the test failed, remove the finalizer so the resource can be deleted + var res = extension.get(DeletionDuringStatusUpdateCustomResource.class, RESOURCE_NAME); + if (res != null) { + res.getMetadata().setFinalizers(Collections.emptyList()); + extension.replace(res); + extension.delete(res); + } + } + + @Test + void deletionDuringStatusUpdateTriggersCleanup() throws InterruptedException { + var reconciler = extension.getReconcilerOfType(DeletionDuringStatusUpdateReconciler.class); + + extension.create(testResource()); + + // Wait until the reconciler is inside the update operation (active-update window is open) + assertThat(reconciler.patchStartedLatch.await(30, TimeUnit.SECONDS)) + .as("reconciler should enter the patch update operation") + .isTrue(); + + // Issue delete — K8s sets deletionTimestamp while the active-update window is open + extension.delete(testResource()); + + // Wait for deletionTimestamp to be confirmed on the resource in K8s + await() + .atMost(Duration.ofSeconds(30)) + .until( + () -> { + var res = + extension.get(DeletionDuringStatusUpdateCustomResource.class, RESOURCE_NAME); + return res != null && res.isMarkedForDeletion(); + }); + + // Signal the reconciler to proceed with the actual PATCH. K8s will merge deletionTimestamp + // into the response - the deletion event (lower RV) is now deferred and will be dropped + // without the fix. + reconciler.deleteConfirmedLatch.countDown(); + + // cleanup() must be called — the deletion must not be silently lost + assertThat(reconciler.cleanupCalledLatch.await(30, TimeUnit.SECONDS)) + .as("cleanup() must be called after the status update that races with the delete") + .isTrue(); + + // Resource must eventually disappear (finalizer removed) + await() + .atMost(Duration.ofSeconds(30)) + .untilAsserted( + () -> + assertThat( + extension.get( + DeletionDuringStatusUpdateCustomResource.class, RESOURCE_NAME)) + .isNull()); + } + + DeletionDuringStatusUpdateCustomResource testResource() { + var resource = new DeletionDuringStatusUpdateCustomResource(); + resource.setMetadata(new ObjectMetaBuilder().withName(RESOURCE_NAME).build()); + return resource; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/deletionduringstatusupdate/DeletionDuringStatusUpdateReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/deletionduringstatusupdate/DeletionDuringStatusUpdateReconciler.java new file mode 100644 index 0000000000..feb0509e72 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/deletionduringstatusupdate/DeletionDuringStatusUpdateReconciler.java @@ -0,0 +1,79 @@ +/* + * 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.readcacheafterwrite.deletionduringstatusupdate; + +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; + +import io.javaoperatorsdk.operator.api.reconciler.Cleaner; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.DeleteControl; +import io.javaoperatorsdk.operator.api.reconciler.Reconciler; +import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; + +@ControllerConfiguration +public class DeletionDuringStatusUpdateReconciler + implements Reconciler, + Cleaner { + + final CountDownLatch patchStartedLatch = new CountDownLatch(1); + final CountDownLatch deleteConfirmedLatch = new CountDownLatch(1); + final CountDownLatch cleanupCalledLatch = new CountDownLatch(1); + + @Override + public UpdateControl reconcile( + DeletionDuringStatusUpdateCustomResource resource, + Context context) + throws InterruptedException { + if (resource.isMarkedForDeletion()) { + return UpdateControl.noUpdate(); + } + + var status = new DeletionDuringStatusUpdateStatus(); + status.setReady(true); + resource.setStatus(status); + + context + .resourceOperations() + .resourcePatch( + resource, + r -> { + patchStartedLatch.countDown(); + try { + if (!deleteConfirmedLatch.await(30, TimeUnit.SECONDS)) { + throw new RuntimeException("Timed out waiting for delete confirmation"); + } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new RuntimeException(e); + } + r.getMetadata().setResourceVersion(null); + return context.getClient().resource(r).patchStatus(); + }, + context.eventSourceRetriever().getControllerEventSource()); + + return UpdateControl.noUpdate(); + } + + @Override + public DeleteControl cleanup( + DeletionDuringStatusUpdateCustomResource resource, + Context context) { + cleanupCalledLatch.countDown(); + return DeleteControl.defaultDelete(); + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/deletionduringstatusupdate/DeletionDuringStatusUpdateStatus.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/deletionduringstatusupdate/DeletionDuringStatusUpdateStatus.java new file mode 100644 index 0000000000..c7acedce20 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/deletionduringstatusupdate/DeletionDuringStatusUpdateStatus.java @@ -0,0 +1,29 @@ +/* + * 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.readcacheafterwrite.deletionduringstatusupdate; + +public class DeletionDuringStatusUpdateStatus { + + private boolean ready; + + public boolean isReady() { + return ready; + } + + public void setReady(boolean ready) { + this.ready = ready; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/externalsecondaryupdate/ExternalSecondaryUpdateCustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/externalsecondaryupdate/ExternalSecondaryUpdateCustomResource.java new file mode 100644 index 0000000000..dd28ca9254 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/externalsecondaryupdate/ExternalSecondaryUpdateCustomResource.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.readcacheafterwrite.externalsecondaryupdate; + +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("esu") +public class ExternalSecondaryUpdateCustomResource + extends CustomResource implements Namespaced {} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/externalsecondaryupdate/ExternalSecondaryUpdateIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/externalsecondaryupdate/ExternalSecondaryUpdateIT.java new file mode 100644 index 0000000000..04b1565654 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/externalsecondaryupdate/ExternalSecondaryUpdateIT.java @@ -0,0 +1,110 @@ +/* + * 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.readcacheafterwrite.externalsecondaryupdate; + +import java.time.Duration; +import java.util.HashMap; +import java.util.concurrent.TimeUnit; + +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 io.javaoperatorsdk.operator.baseapi.readcacheafterwrite.externalsecondaryupdate.ExternalSecondaryUpdateReconciler.EXTERNAL_LABEL_KEY; +import static io.javaoperatorsdk.operator.baseapi.readcacheafterwrite.externalsecondaryupdate.ExternalSecondaryUpdateReconciler.EXTERNAL_LABEL_VALUE; +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + +/** + * Verifies that when a secondary resource (a ConfigMap owned by the primary) is modified externally + * between two caching+filtering updates from the controller, the external change is NOT silently + * absorbed: a later reconciliation must observe it through the merged temp/informer cache. + */ +class ExternalSecondaryUpdateIT { + + static final String RESOURCE_NAME = "test-resource"; + + ExternalSecondaryUpdateReconciler reconciler = new ExternalSecondaryUpdateReconciler(); + + @RegisterExtension + LocallyRunOperatorExtension operator = + LocallyRunOperatorExtension.builder().withReconciler(reconciler).build(); + + @Test + void externalUpdateOnSecondaryDuringFilteringUpdatePropagates() throws InterruptedException { + operator.create(testResource()); + + // wait for the reconciler to enter the first reconciliation and create the secondary CM + assertThat(reconciler.firstReconcileEntered.await(30, TimeUnit.SECONDS)) + .as("reconciler must enter first reconciliation") + .isTrue(); + + // a third party adds a label to the secondary CM while we are mid-reconcile + var cm = + operator + .getKubernetesClient() + .configMaps() + .inNamespace(operator.getNamespace()) + .withName(RESOURCE_NAME) + .get(); + assertThat(cm).as("secondary CM created by reconciler").isNotNull(); + var labels = new HashMap(); + if (cm.getMetadata().getLabels() != null) { + labels.putAll(cm.getMetadata().getLabels()); + } + labels.put(EXTERNAL_LABEL_KEY, EXTERNAL_LABEL_VALUE); + cm.getMetadata().setLabels(labels); + operator.getKubernetesClient().resource(cm).inNamespace(operator.getNamespace()).replace(); + + // signal the reconciler to issue the second caching+filtering SSA + reconciler.externalUpdateApplied.countDown(); + + // a later reconciliation, triggered by the external label event, must see the label + // through the cache (informer + temp cache merge). + await() + .atMost(Duration.ofSeconds(30)) + .untilAsserted( + () -> { + assertThat(reconciler.numberOfExecutions.get()) + .as("external CM update must trigger a fresh reconciliation") + .isGreaterThanOrEqualTo(2); + assertThat(reconciler.externalLabelSeenInLaterReconciliation.get()) + .as("a later reconciliation must observe the externally-applied label") + .isTrue(); + }); + + // the second SSA from the reconciler did go through and was captured + assertThat(reconciler.rvAfterCachingFilteringUpdate.get()).isNotNull(); + var finalCm = + operator + .getKubernetesClient() + .configMaps() + .inNamespace(operator.getNamespace()) + .withName(RESOURCE_NAME) + .get(); + assertThat(finalCm.getMetadata().getLabels()) + .as("external label preserved on the secondary after the SSA") + .containsEntry(EXTERNAL_LABEL_KEY, EXTERNAL_LABEL_VALUE); + } + + ExternalSecondaryUpdateCustomResource testResource() { + var r = new ExternalSecondaryUpdateCustomResource(); + r.setMetadata(new ObjectMetaBuilder().withName(RESOURCE_NAME).build()); + return r; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/externalsecondaryupdate/ExternalSecondaryUpdateReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/externalsecondaryupdate/ExternalSecondaryUpdateReconciler.java new file mode 100644 index 0000000000..0dac8cae33 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/externalsecondaryupdate/ExternalSecondaryUpdateReconciler.java @@ -0,0 +1,123 @@ +/* + * 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.readcacheafterwrite.externalsecondaryupdate; + +import java.util.List; +import java.util.Map; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; + +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.api.config.informer.InformerEventSourceConfiguration; +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.processing.event.source.EventSource; +import io.javaoperatorsdk.operator.processing.event.source.informer.InformerEventSource; + +@ControllerConfiguration +public class ExternalSecondaryUpdateReconciler + implements Reconciler { + + static final String CM_DATA_KEY = "managed-by"; + static final String CM_DATA_VALUE = "operator"; + static final String EXTERNAL_LABEL_KEY = "externally-set"; + static final String EXTERNAL_LABEL_VALUE = "yes"; + + final AtomicInteger numberOfExecutions = new AtomicInteger(); + final CountDownLatch firstReconcileEntered = new CountDownLatch(1); + final CountDownLatch externalUpdateApplied = new CountDownLatch(1); + // Whether a later reconciliation (after the external label appeared) actually saw the label + // through the informer/temp cache. + final AtomicBoolean externalLabelSeenInLaterReconciliation = new AtomicBoolean(); + final AtomicReference rvAfterCachingFilteringUpdate = new AtomicReference<>(); + + private InformerEventSource + configMapEventSource; + + @Override + public UpdateControl reconcile( + ExternalSecondaryUpdateCustomResource resource, + Context context) + throws InterruptedException { + int execution = numberOfExecutions.incrementAndGet(); + + if (execution == 1) { + // first reconciliation: create the secondary CM via SSA, then ask the test to apply + // an external metadata change BEFORE we issue our second SSA on it. + context.resourceOperations().serverSideApply(prepareCM(resource, 1), configMapEventSource); + + firstReconcileEntered.countDown(); + if (!externalUpdateApplied.await(30, TimeUnit.SECONDS)) { + throw new RuntimeException("timed out waiting for external CM update"); + } + + // Second SSA on the secondary, with DIFFERENT data so it actually mutates the resource + // and bumps rv beyond the external label change. Without distinct data the SSA would be + // idempotent and return the rv produced by the external update — which would then be + // recorded as our own and incorrectly filter out the external event. + var updated = + context + .resourceOperations() + .serverSideApply(prepareCM(resource, 2), configMapEventSource); + rvAfterCachingFilteringUpdate.set(updated.getMetadata().getResourceVersion()); + } else { + // any subsequent reconciliation must be able to see the external label through the + // informer cache (merged with the temp cache). + var cached = context.getSecondaryResource(ConfigMap.class).orElse(null); + if (cached != null + && cached.getMetadata().getLabels() != null + && EXTERNAL_LABEL_VALUE.equals( + cached.getMetadata().getLabels().get(EXTERNAL_LABEL_KEY))) { + externalLabelSeenInLaterReconciliation.set(true); + } + } + return UpdateControl.noUpdate(); + } + + @Override + public List> prepareEventSources( + EventSourceContext context) { + configMapEventSource = + new InformerEventSource<>( + InformerEventSourceConfiguration.from( + ConfigMap.class, ExternalSecondaryUpdateCustomResource.class) + .build(), + context); + return List.of(configMapEventSource); + } + + private static ConfigMap prepareCM(ExternalSecondaryUpdateCustomResource p, int iteration) { + var cm = + new ConfigMapBuilder() + .withMetadata( + new ObjectMetaBuilder() + .withName(p.getMetadata().getName()) + .withNamespace(p.getMetadata().getNamespace()) + .build()) + .withData(Map.of(CM_DATA_KEY, CM_DATA_VALUE, "iteration", "" + iteration)) + .build(); + cm.addOwnerReference(p); + return cm; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/externalsecondaryupdate/ExternalSecondaryUpdateStatus.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/externalsecondaryupdate/ExternalSecondaryUpdateStatus.java new file mode 100644 index 0000000000..70c555f7aa --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/externalsecondaryupdate/ExternalSecondaryUpdateStatus.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.readcacheafterwrite.externalsecondaryupdate; + +public class ExternalSecondaryUpdateStatus { + + private Integer reconciliations; + + public Integer getReconciliations() { + return reconciliations; + } + + public ExternalSecondaryUpdateStatus setReconciliations(Integer reconciliations) { + this.reconciliations = reconciliations; + return this; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/externalupdateduringownupdate/ExternalUpdateDuringOwnUpdateCustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/externalupdateduringownupdate/ExternalUpdateDuringOwnUpdateCustomResource.java new file mode 100644 index 0000000000..3621c72c8f --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/externalupdateduringownupdate/ExternalUpdateDuringOwnUpdateCustomResource.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.readcacheafterwrite.externalupdateduringownupdate; + +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("eudou") +public class ExternalUpdateDuringOwnUpdateCustomResource + extends CustomResource implements Namespaced {} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/externalupdateduringownupdate/ExternalUpdateDuringOwnUpdateIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/externalupdateduringownupdate/ExternalUpdateDuringOwnUpdateIT.java new file mode 100644 index 0000000000..ed3330358e --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/externalupdateduringownupdate/ExternalUpdateDuringOwnUpdateIT.java @@ -0,0 +1,88 @@ +/* + * 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.readcacheafterwrite.externalupdateduringownupdate; + +import java.time.Duration; +import java.util.HashMap; +import java.util.concurrent.TimeUnit; + +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 io.javaoperatorsdk.operator.baseapi.readcacheafterwrite.externalupdateduringownupdate.ExternalUpdateDuringOwnUpdateReconciler.EXTERNAL_LABEL_KEY; +import static io.javaoperatorsdk.operator.baseapi.readcacheafterwrite.externalupdateduringownupdate.ExternalUpdateDuringOwnUpdateReconciler.EXTERNAL_LABEL_VALUE; +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + +/** + * Verifies that an external update arriving while the controller's own filter window is open is NOT + * mistakenly filtered. The third-party event must propagate as a fresh reconciliation in which the + * reconciler observes the externally-applied change. + */ +class ExternalUpdateDuringOwnUpdateIT { + + static final String RESOURCE_NAME = "test-resource"; + + ExternalUpdateDuringOwnUpdateReconciler reconciler = + new ExternalUpdateDuringOwnUpdateReconciler(); + + @RegisterExtension + LocallyRunOperatorExtension extension = + LocallyRunOperatorExtension.builder().withReconciler(reconciler).build(); + + @Test + void externalUpdateDuringOwnUpdateTriggersFreshReconciliation() throws InterruptedException { + extension.create(testResource()); + + assertThat(reconciler.updateStartedLatch.await(30, TimeUnit.SECONDS)) + .as("reconciler should enter the patch update operation") + .isTrue(); + + // external party modifies a label while our filter window is still open + var current = extension.get(ExternalUpdateDuringOwnUpdateCustomResource.class, RESOURCE_NAME); + var labels = new HashMap(); + if (current.getMetadata().getLabels() != null) { + labels.putAll(current.getMetadata().getLabels()); + } + labels.put(EXTERNAL_LABEL_KEY, EXTERNAL_LABEL_VALUE); + current.getMetadata().setLabels(labels); + extension.replace(current); + + // signal reconciler to complete its own status update + reconciler.externalUpdateDoneLatch.countDown(); + + // the external update event must NOT be silently absorbed by the filter window; + // a fresh reconciliation must observe the external label. + await() + .atMost(Duration.ofSeconds(30)) + .untilAsserted( + () -> { + assertThat(reconciler.numberOfExecutions.get()).isGreaterThanOrEqualTo(2); + assertThat(reconciler.externalLabelSeenInLaterReconciliation.get()) + .as("a later reconciliation must observe the externally-applied label") + .isTrue(); + }); + } + + ExternalUpdateDuringOwnUpdateCustomResource testResource() { + var r = new ExternalUpdateDuringOwnUpdateCustomResource(); + r.setMetadata(new ObjectMetaBuilder().withName(RESOURCE_NAME).build()); + return r; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/externalupdateduringownupdate/ExternalUpdateDuringOwnUpdateReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/externalupdateduringownupdate/ExternalUpdateDuringOwnUpdateReconciler.java new file mode 100644 index 0000000000..e5c956dc4e --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/externalupdateduringownupdate/ExternalUpdateDuringOwnUpdateReconciler.java @@ -0,0 +1,80 @@ +/* + * 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.readcacheafterwrite.externalupdateduringownupdate; + +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +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; + +@ControllerConfiguration(generationAwareEventProcessing = false) +public class ExternalUpdateDuringOwnUpdateReconciler + implements Reconciler { + + static final String EXTERNAL_LABEL_KEY = "externally-set"; + static final String EXTERNAL_LABEL_VALUE = "yes"; + static final String STATUS_VALUE = "ready"; + + final AtomicInteger numberOfExecutions = new AtomicInteger(); + final CountDownLatch updateStartedLatch = new CountDownLatch(1); + final CountDownLatch externalUpdateDoneLatch = new CountDownLatch(1); + final AtomicBoolean externalLabelSeenInLaterReconciliation = new AtomicBoolean(); + + @Override + public UpdateControl reconcile( + ExternalUpdateDuringOwnUpdateCustomResource resource, + Context context) { + int execution = numberOfExecutions.incrementAndGet(); + + if (execution == 1) { + var status = new ExternalUpdateDuringOwnUpdateStatus().setValue(STATUS_VALUE); + resource.setStatus(status); + + // wrap our own status update in resourcePatch with a hook that lets the test + // perform an external metadata update WHILE our filter window is still open. + context + .resourceOperations() + .resourcePatch( + resource, + r -> { + updateStartedLatch.countDown(); + try { + if (!externalUpdateDoneLatch.await(30, TimeUnit.SECONDS)) { + throw new RuntimeException("timed out waiting for external update"); + } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new RuntimeException(e); + } + // server-side state moved due to the external label change; drop our stale rv + r.getMetadata().setResourceVersion(null); + return context.getClient().resource(r).patchStatus(); + }, + context.eventSourceRetriever().getControllerEventSource()); + } else { + var labels = resource.getMetadata().getLabels(); + if (labels != null && EXTERNAL_LABEL_VALUE.equals(labels.get(EXTERNAL_LABEL_KEY))) { + externalLabelSeenInLaterReconciliation.set(true); + } + } + return UpdateControl.noUpdate(); + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/externalupdateduringownupdate/ExternalUpdateDuringOwnUpdateStatus.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/externalupdateduringownupdate/ExternalUpdateDuringOwnUpdateStatus.java new file mode 100644 index 0000000000..b059a6ee5e --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/externalupdateduringownupdate/ExternalUpdateDuringOwnUpdateStatus.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.readcacheafterwrite.externalupdateduringownupdate; + +public class ExternalUpdateDuringOwnUpdateStatus { + + private String value; + + public String getValue() { + return value; + } + + public ExternalUpdateDuringOwnUpdateStatus setValue(String value) { + this.value = value; + return this; + } +} 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/readcacheafterwrite/filterpatchevent/FilterPatchEventIT.java similarity index 98% rename from operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/filterpatchevent/FilterPatchEventIT.java rename to operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/filterpatchevent/FilterPatchEventIT.java index 6f27925e21..398cdcf864 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/filterpatchevent/FilterPatchEventIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/filterpatchevent/FilterPatchEventIT.java @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package io.javaoperatorsdk.operator.baseapi.filterpatchevent; +package io.javaoperatorsdk.operator.baseapi.readcacheafterwrite.filterpatchevent; import java.time.Duration; 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/readcacheafterwrite/filterpatchevent/FilterPatchEventTestCustomResource.java similarity index 93% rename from operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/filterpatchevent/FilterPatchEventTestCustomResource.java rename to operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/filterpatchevent/FilterPatchEventTestCustomResource.java index 7f8b4838de..f228c0caf4 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/filterpatchevent/FilterPatchEventTestCustomResource.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/filterpatchevent/FilterPatchEventTestCustomResource.java @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package io.javaoperatorsdk.operator.baseapi.filterpatchevent; +package io.javaoperatorsdk.operator.baseapi.readcacheafterwrite.filterpatchevent; import io.fabric8.kubernetes.api.model.Namespaced; import io.fabric8.kubernetes.client.CustomResource; diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/filterpatchevent/FilterPatchEventTestCustomResourceStatus.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/filterpatchevent/FilterPatchEventTestCustomResourceStatus.java similarity index 91% rename from operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/filterpatchevent/FilterPatchEventTestCustomResourceStatus.java rename to operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/filterpatchevent/FilterPatchEventTestCustomResourceStatus.java index 1c7aeafadd..b1828f0241 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/filterpatchevent/FilterPatchEventTestCustomResourceStatus.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/filterpatchevent/FilterPatchEventTestCustomResourceStatus.java @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package io.javaoperatorsdk.operator.baseapi.filterpatchevent; +package io.javaoperatorsdk.operator.baseapi.readcacheafterwrite.filterpatchevent; public class FilterPatchEventTestCustomResourceStatus { 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/readcacheafterwrite/filterpatchevent/FilterPatchEventTestReconciler.java similarity index 91% rename from operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/filterpatchevent/FilterPatchEventTestReconciler.java rename to operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/filterpatchevent/FilterPatchEventTestReconciler.java index e7599a2881..1f19015dcd 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/filterpatchevent/FilterPatchEventTestReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/filterpatchevent/FilterPatchEventTestReconciler.java @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package io.javaoperatorsdk.operator.baseapi.filterpatchevent; +package io.javaoperatorsdk.operator.baseapi.readcacheafterwrite.filterpatchevent; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; @@ -23,7 +23,7 @@ import io.javaoperatorsdk.operator.api.reconciler.Reconciler; import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; -import static io.javaoperatorsdk.operator.baseapi.filterpatchevent.FilterPatchEventIT.UPDATED; +import static io.javaoperatorsdk.operator.baseapi.readcacheafterwrite.filterpatchevent.FilterPatchEventIT.UPDATED; @ControllerConfiguration(generationAwareEventProcessing = false) public class FilterPatchEventTestReconciler diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/cachingfilteringupdate/CachingFilteringUpdateCustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/onrelistfilter/OnRelistFilterCustomResource.java similarity index 80% rename from operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/cachingfilteringupdate/CachingFilteringUpdateCustomResource.java rename to operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/onrelistfilter/OnRelistFilterCustomResource.java index 0d25bbfdd4..b36fd47d8d 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/cachingfilteringupdate/CachingFilteringUpdateCustomResource.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/onrelistfilter/OnRelistFilterCustomResource.java @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package io.javaoperatorsdk.operator.baseapi.cachingfilteringupdate; +package io.javaoperatorsdk.operator.baseapi.readcacheafterwrite.onrelistfilter; import io.fabric8.kubernetes.api.model.Namespaced; import io.fabric8.kubernetes.client.CustomResource; @@ -23,6 +23,6 @@ @Group("sample.javaoperatorsdk") @Version("v1") -@ShortNames("cfu") -public class CachingFilteringUpdateCustomResource - extends CustomResource implements Namespaced {} +@ShortNames("orf") +public class OnRelistFilterCustomResource extends CustomResource + implements Namespaced {} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/onrelistfilter/OnRelistFilterIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/onrelistfilter/OnRelistFilterIT.java new file mode 100644 index 0000000000..df8d7c2591 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/onrelistfilter/OnRelistFilterIT.java @@ -0,0 +1,109 @@ +/* + * 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.readcacheafterwrite.onrelistfilter; + +import java.time.Duration; + +import org.junit.jupiter.api.Disabled; +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; + +/** + * Verifies the re-list aware filtering of own writes on a secondary resource: + * + *
    + *
  • no re-list — own write is filtered, no extra reconciliation + *
  • re-list around the whole update window — own write is propagated + *
  • re-list completes BEFORE the update — own write is filtered + *
  • re-list starts WHILE the update window is open — own write is propagated + *
+ */ +@Disabled("enable when fabric8 supports relist") +class OnRelistFilterIT { + + static final String RESOURCE_NAME = "test-resource"; + + OnRelistFilterReconciler reconciler = new OnRelistFilterReconciler(); + + @RegisterExtension + LocallyRunOperatorExtension operator = + LocallyRunOperatorExtension.builder().withReconciler(reconciler).build(); + + @Test + void ownSecondaryWriteIsFilteredWithoutRelist() { + reconciler.setMode(OnRelistFilterReconciler.Mode.NO_RELIST); + operator.create(testResource()); + + assertOnlyInitialReconciliation(); + } + + @Test + void ownSecondaryWriteIsPropagatedWhenRelistWrapsTheUpdate() { + reconciler.setMode(OnRelistFilterReconciler.Mode.RELIST_AROUND_UPDATE); + operator.create(testResource()); + + assertExtraReconciliationTriggered(); + } + + @Test + void ownSecondaryWriteIsFilteredWhenRelistCompletesBeforeTheUpdate() { + reconciler.setMode(OnRelistFilterReconciler.Mode.RELIST_COMPLETES_BEFORE_UPDATE); + operator.create(testResource()); + + assertOnlyInitialReconciliation(); + } + + @Test + void ownSecondaryWriteIsPropagatedWhenRelistStartsDuringTheUpdate() { + reconciler.setMode(OnRelistFilterReconciler.Mode.RELIST_STARTS_DURING_UPDATE); + operator.create(testResource()); + + assertExtraReconciliationTriggered(); + } + + private void assertExtraReconciliationTriggered() { + await() + .atMost(Duration.ofSeconds(30)) + .untilAsserted( + () -> + assertThat(reconciler.getNumberOfExecutions()) + .as("watch event during re-list must trigger a fresh reconciliation") + .isGreaterThanOrEqualTo(2)); + } + + private void assertOnlyInitialReconciliation() { + await() + .pollDelay(Duration.ofSeconds(3)) + .atMost(Duration.ofSeconds(10)) + .untilAsserted( + () -> + assertThat(reconciler.getNumberOfExecutions()) + .as("own write must be filtered, no extra reconciliation expected") + .isEqualTo(1)); + } + + private OnRelistFilterCustomResource testResource() { + var r = new OnRelistFilterCustomResource(); + r.setMetadata(new ObjectMetaBuilder().withName(RESOURCE_NAME).build()); + return r; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/onrelistfilter/OnRelistFilterReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/onrelistfilter/OnRelistFilterReconciler.java new file mode 100644 index 0000000000..287141e4d1 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/onrelistfilter/OnRelistFilterReconciler.java @@ -0,0 +1,171 @@ +/* + * 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.readcacheafterwrite.onrelistfilter; + +import java.util.List; +import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; + +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.fabric8.kubernetes.api.model.ConfigMapBuilder; +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.fabric8.kubernetes.client.dsl.base.PatchContext; +import io.fabric8.kubernetes.client.dsl.base.PatchType; +import io.javaoperatorsdk.operator.api.config.informer.InformerEventSourceConfiguration; +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.processing.event.source.EventSource; +import io.javaoperatorsdk.operator.processing.event.source.informer.InformerEventSource; + +@ControllerConfiguration +public class OnRelistFilterReconciler implements Reconciler { + + public enum Mode { + /** No re-list around the update — the resulting watch event must be filtered as own write. */ + NO_RELIST, + /** + * onBeforeList → SSA → onList. The whole own-write window happens during a re-list. The watch + * event must be propagated, since intermediate events may have been lost. + */ + RELIST_AROUND_UPDATE, + /** + * onBeforeList → onList → SSA. The re-list completed cleanly before our update started, so the + * resulting watch event must be filtered as own write. + */ + RELIST_COMPLETES_BEFORE_UPDATE, + /** + * Update window opens → onBeforeList → SSA → onList. The re-list starts while our update is + * already in progress; the existing event-filter window must flip into "do not absorb own + * writes" mode and propagate the watch event. + */ + RELIST_STARTS_DURING_UPDATE + } + + static final String CM_DATA_KEY = "iteration"; + + private final AtomicInteger numberOfExecutions = new AtomicInteger(); + private final AtomicReference mode = new AtomicReference<>(Mode.NO_RELIST); + + private RelistAwareInformerEventSource + configMapEventSource; + + @Override + public UpdateControl reconcile( + OnRelistFilterCustomResource resource, Context context) { + int execution = numberOfExecutions.incrementAndGet(); + + if (execution == 1) { + var cm = prepareConfigMap(resource); + switch (mode.get()) { + case NO_RELIST -> context.resourceOperations().serverSideApply(cm, configMapEventSource); + case RELIST_AROUND_UPDATE -> { + configMapEventSource.simulateOnBeforeList(); + context.resourceOperations().serverSideApply(cm, configMapEventSource); + configMapEventSource.simulateOnList(); + } + case RELIST_COMPLETES_BEFORE_UPDATE -> { + configMapEventSource.simulateOnBeforeList(); + configMapEventSource.simulateOnList(); + context.resourceOperations().serverSideApply(cm, configMapEventSource); + } + case RELIST_STARTS_DURING_UPDATE -> { + // Drive the event-filtering update path manually so we can fire onBeforeList AFTER the + // window has been opened by startEventFilteringModify but BEFORE the SSA hits the API. + var fieldManager = context.getControllerConfiguration().fieldManager(); + configMapEventSource.eventFilteringUpdateAndCacheResource( + cm, + r -> { + configMapEventSource.simulateOnBeforeList(); + return context + .getClient() + .resource(r) + .patch( + new PatchContext.Builder() + .withForce(true) + .withFieldManager(fieldManager) + .withPatchType(PatchType.SERVER_SIDE_APPLY) + .build()); + }); + configMapEventSource.simulateOnList(); + } + } + } + + return UpdateControl.noUpdate(); + } + + @Override + public List> prepareEventSources( + EventSourceContext context) { + configMapEventSource = + new RelistAwareInformerEventSource<>( + InformerEventSourceConfiguration.from( + ConfigMap.class, OnRelistFilterCustomResource.class) + .build(), + context); + return List.of(configMapEventSource); + } + + public int getNumberOfExecutions() { + return numberOfExecutions.get(); + } + + public void setMode(Mode value) { + mode.set(value); + } + + private static ConfigMap prepareConfigMap(OnRelistFilterCustomResource p) { + var cm = + new ConfigMapBuilder() + .withMetadata( + new ObjectMetaBuilder() + .withName(p.getMetadata().getName()) + .withNamespace(p.getMetadata().getNamespace()) + .build()) + .withData(Map.of(CM_DATA_KEY, "1")) + .build(); + cm.addOwnerReference(p); + return cm; + } + + /** + * Subclass exposing the {@code onBeforeList}/{@code onList} callbacks so a test can simulate the + * informer's re-list lifecycle around an own write, without relying on actual watch + * disconnections. + */ + static class RelistAwareInformerEventSource + extends InformerEventSource { + + RelistAwareInformerEventSource( + InformerEventSourceConfiguration configuration, EventSourceContext

context) { + super(configuration, context); + } + + void simulateOnBeforeList() { + // uncomment when fabric8 supports re-list + // onBeforeList(null); + } + + void simulateOnList() { + onList(null, false); + } + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/ownsecondaryupdate/OwnSecondaryUpdateCustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/ownsecondaryupdate/OwnSecondaryUpdateCustomResource.java new file mode 100644 index 0000000000..5dae77d86b --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/ownsecondaryupdate/OwnSecondaryUpdateCustomResource.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.readcacheafterwrite.ownsecondaryupdate; + +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("osu") +public class OwnSecondaryUpdateCustomResource extends CustomResource + implements Namespaced {} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/ownsecondaryupdate/OwnSecondaryUpdateIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/ownsecondaryupdate/OwnSecondaryUpdateIT.java new file mode 100644 index 0000000000..dfa5b899fe --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/ownsecondaryupdate/OwnSecondaryUpdateIT.java @@ -0,0 +1,82 @@ +/* + * 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.readcacheafterwrite.ownsecondaryupdate; + +import java.time.Duration; + +import org.junit.jupiter.api.Disabled; +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; + +/** + * Verifies that when the controller updates a secondary resource through the read-cache-after-write + * path (here: {@code context.resourceOperations().serverSideApply}), the resulting watch events on + * the secondary are filtered and do NOT trigger additional reconciliations. Counterpart to {@code + * ExternalSecondaryUpdateIT}, which asserts the opposite for third-party updates. + */ +@Disabled("enable if re-list notification supported by fabric8 client") +class OwnSecondaryUpdateIT { + + static final String RESOURCE_NAME = "test-resource"; + + OwnSecondaryUpdateReconciler reconciler = new OwnSecondaryUpdateReconciler(); + + @RegisterExtension + LocallyRunOperatorExtension operator = + LocallyRunOperatorExtension.builder().withReconciler(reconciler).build(); + + @Test + void ownUpdateOnSecondaryDoesNotTriggerReconciliation() { + operator.create(testResource()); + + // Wait for the first reconciliation to have run all of its SSAs (the secondary CM exists + // and carries the data of the last SSA iteration). + await() + .atMost(Duration.ofSeconds(30)) + .untilAsserted( + () -> { + var cm = + operator + .getKubernetesClient() + .configMaps() + .inNamespace(operator.getNamespace()) + .withName(RESOURCE_NAME) + .get(); + assertThat(cm).isNotNull(); + assertThat(cm.getData()) + .containsEntry("iteration", "" + OwnSecondaryUpdateReconciler.OWN_SSA_COUNT); + }); + + // Give any spurious own-write events time to reach the controller. The filter must absorb + // them, so the reconciliation count must stay at 1 (the one triggered by the create). + await() + .pollDelay(Duration.ofSeconds(2)) + .atMost(Duration.ofSeconds(3)) + .untilAsserted(() -> assertThat(reconciler.numberOfExecutions.get()).isEqualTo(1)); + } + + OwnSecondaryUpdateCustomResource testResource() { + var r = new OwnSecondaryUpdateCustomResource(); + r.setMetadata(new ObjectMetaBuilder().withName(RESOURCE_NAME).build()); + return r; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/ownsecondaryupdate/OwnSecondaryUpdateReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/ownsecondaryupdate/OwnSecondaryUpdateReconciler.java new file mode 100644 index 0000000000..f0ac28b955 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/ownsecondaryupdate/OwnSecondaryUpdateReconciler.java @@ -0,0 +1,83 @@ +/* + * 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.readcacheafterwrite.ownsecondaryupdate; + +import java.util.List; +import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; + +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.api.config.informer.InformerEventSourceConfiguration; +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.processing.event.source.EventSource; +import io.javaoperatorsdk.operator.processing.event.source.informer.InformerEventSource; + +@ControllerConfiguration(generationAwareEventProcessing = false) +public class OwnSecondaryUpdateReconciler implements Reconciler { + + static final int OWN_SSA_COUNT = 3; + + final AtomicInteger numberOfExecutions = new AtomicInteger(); + + private InformerEventSource configMapEventSource; + + @Override + public UpdateControl reconcile( + OwnSecondaryUpdateCustomResource resource, + Context context) { + numberOfExecutions.incrementAndGet(); + + // Issue several SSA writes on the secondary, each with distinct data so the resource + // version actually advances. With the read-cache-after-write filter in place, none of the + // resulting watch events should trigger a fresh reconciliation. + for (int i = 1; i <= OWN_SSA_COUNT; i++) { + context.resourceOperations().serverSideApply(prepareCM(resource, i), configMapEventSource); + } + return UpdateControl.noUpdate(); + } + + @Override + public List> prepareEventSources( + EventSourceContext context) { + configMapEventSource = + new InformerEventSource<>( + InformerEventSourceConfiguration.from( + ConfigMap.class, OwnSecondaryUpdateCustomResource.class) + .build(), + context); + return List.of(configMapEventSource); + } + + private static ConfigMap prepareCM(OwnSecondaryUpdateCustomResource p, int iteration) { + var cm = + new ConfigMapBuilder() + .withMetadata( + new ObjectMetaBuilder() + .withName(p.getMetadata().getName()) + .withNamespace(p.getMetadata().getNamespace()) + .build()) + .withData(Map.of("iteration", "" + iteration)) + .build(); + cm.addOwnerReference(p); + return cm; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/readownupdates/ReadOwnUpdatesCustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/readownupdates/ReadOwnUpdatesCustomResource.java new file mode 100644 index 0000000000..f12431c3ea --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/readownupdates/ReadOwnUpdatesCustomResource.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.readcacheafterwrite.readownupdates; + +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("rou") +public class ReadOwnUpdatesCustomResource extends CustomResource + implements Namespaced {} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/cachingfilteringupdate/CachingFilteringUpdateIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/readownupdates/ReadOwnUpdatesIT.java similarity index 77% rename from operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/cachingfilteringupdate/CachingFilteringUpdateIT.java rename to operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/readownupdates/ReadOwnUpdatesIT.java index c62c8ca186..0fe2e79102 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/cachingfilteringupdate/CachingFilteringUpdateIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/readownupdates/ReadOwnUpdatesIT.java @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package io.javaoperatorsdk.operator.baseapi.cachingfilteringupdate; +package io.javaoperatorsdk.operator.baseapi.readcacheafterwrite.readownupdates; import java.time.Duration; @@ -26,10 +26,10 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.awaitility.Awaitility.await; -class CachingFilteringUpdateIT { +class ReadOwnUpdatesIT { public static final int RESOURCE_NUMBER = 250; - CachingFilteringUpdateReconciler reconciler = new CachingFilteringUpdateReconciler(); + ReadOwnUpdatesReconciler reconciler = new ReadOwnUpdatesReconciler(); @RegisterExtension LocallyRunOperatorExtension operator = @@ -52,26 +52,25 @@ void testResourceAccessAfterUpdate() { // Use a single representative resource to detect that updates have completed. var res = operator.get( - CachingFilteringUpdateCustomResource.class, - "resource" + (RESOURCE_NUMBER - 1)); + ReadOwnUpdatesCustomResource.class, "resource" + (RESOURCE_NUMBER - 1)); return res != null && res.getStatus() != null && Boolean.TRUE.equals(res.getStatus().getUpdated()); }); - if (operator.getReconcilerOfType(CachingFilteringUpdateReconciler.class).isIssueFound()) { + if (operator.getReconcilerOfType(ReadOwnUpdatesReconciler.class).isIssueFound()) { throw new IllegalStateException("Error already found."); } for (int i = 0; i < RESOURCE_NUMBER; i++) { - var res = operator.get(CachingFilteringUpdateCustomResource.class, "resource" + i); + var res = operator.get(ReadOwnUpdatesCustomResource.class, "resource" + i); assertThat(res.getStatus()).isNotNull(); assertThat(res.getStatus().getUpdated()).isTrue(); } } - public CachingFilteringUpdateCustomResource createCustomResource(int i) { - CachingFilteringUpdateCustomResource resource = new CachingFilteringUpdateCustomResource(); + public ReadOwnUpdatesCustomResource createCustomResource(int i) { + ReadOwnUpdatesCustomResource resource = new ReadOwnUpdatesCustomResource(); resource.setMetadata( new ObjectMetaBuilder() .withName("resource" + i) diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/cachingfilteringupdate/CachingFilteringUpdateReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/readownupdates/ReadOwnUpdatesReconciler.java similarity index 83% rename from operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/cachingfilteringupdate/CachingFilteringUpdateReconciler.java rename to operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/readownupdates/ReadOwnUpdatesReconciler.java index c8fc206106..545916d7f2 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/cachingfilteringupdate/CachingFilteringUpdateReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/readownupdates/ReadOwnUpdatesReconciler.java @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package io.javaoperatorsdk.operator.baseapi.cachingfilteringupdate; +package io.javaoperatorsdk.operator.baseapi.readcacheafterwrite.readownupdates; import java.util.List; import java.util.Map; @@ -33,18 +33,16 @@ import io.javaoperatorsdk.operator.processing.event.source.informer.InformerEventSource; @ControllerConfiguration -public class CachingFilteringUpdateReconciler - implements Reconciler { +public class ReadOwnUpdatesReconciler implements Reconciler { public static final String RESOURCE_VERSION_INDEX = "resourceVersionIndex"; private final AtomicBoolean issueFound = new AtomicBoolean(false); - private InformerEventSource configMapEventSource; + private InformerEventSource configMapEventSource; @Override - public UpdateControl reconcile( - CachingFilteringUpdateCustomResource resource, - Context context) { + public UpdateControl reconcile( + ReadOwnUpdatesCustomResource resource, Context context) { try { var updated = context.resourceOperations().serverSideApply(prepareCM(resource, 1)); var cachedCM = context.getSecondaryResource(ConfigMap.class); @@ -104,7 +102,7 @@ private void checkListContainsCM(ConfigMap updated) { } } - private static ConfigMap prepareCM(CachingFilteringUpdateCustomResource p, int num) { + private static ConfigMap prepareCM(ReadOwnUpdatesCustomResource p, int num) { var cm = new ConfigMapBuilder() .withMetadata( @@ -119,12 +117,12 @@ private static ConfigMap prepareCM(CachingFilteringUpdateCustomResource p, int n } @Override - public List> prepareEventSources( - EventSourceContext context) { + public List> prepareEventSources( + EventSourceContext context) { configMapEventSource = new InformerEventSource<>( InformerEventSourceConfiguration.from( - ConfigMap.class, CachingFilteringUpdateCustomResource.class) + ConfigMap.class, ReadOwnUpdatesCustomResource.class) .build(), context); configMapEventSource.addIndexers( @@ -132,10 +130,10 @@ public List> prepareEventSo return List.of(configMapEventSource); } - private void ensureStatusExists(CachingFilteringUpdateCustomResource resource) { - CachingFilteringUpdateStatus status = resource.getStatus(); + private void ensureStatusExists(ReadOwnUpdatesCustomResource resource) { + ReadOwnUpdatesStatus status = resource.getStatus(); if (status == null) { - status = new CachingFilteringUpdateStatus(); + status = new ReadOwnUpdatesStatus(); resource.setStatus(status); } } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/cachingfilteringupdate/CachingFilteringUpdateStatus.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/readownupdates/ReadOwnUpdatesStatus.java similarity index 86% rename from operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/cachingfilteringupdate/CachingFilteringUpdateStatus.java rename to operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/readownupdates/ReadOwnUpdatesStatus.java index 80b6c4ba54..7c5e6d3c8d 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/cachingfilteringupdate/CachingFilteringUpdateStatus.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/readcacheafterwrite/readownupdates/ReadOwnUpdatesStatus.java @@ -13,9 +13,9 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package io.javaoperatorsdk.operator.baseapi.cachingfilteringupdate; +package io.javaoperatorsdk.operator.baseapi.readcacheafterwrite.readownupdates; -public class CachingFilteringUpdateStatus { +public class ReadOwnUpdatesStatus { private Boolean updated;