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..9bda71effa 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.*; @@ -84,7 +84,7 @@ protected synchronized void handleEvent( try { if (log.isDebugEnabled()) { log.debug("Event received with action: {}", action); - log.trace("Event Old resource: {},\n new resource: {}", oldResource, resource); + log.debug("Event Old resource: {},\n new resource: {}", oldResource, resource); } MDCUtils.addResourceInfo(resource); controller.getEventSourceManager().broadcastOnResourceEvent(action, resource, oldResource); @@ -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.getLastStateUnknow()); } @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..45f860c6ac --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/EventFilterSupport.java @@ -0,0 +1,89 @@ +/* + * 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 io.javaoperatorsdk.operator.processing.event.ResourceID; + +public class EventFilterSupport { + + private final Map eventFilterWindows = new HashMap<>(); + private boolean ongoingReList = false; + + public synchronized void startEventFilteringModify(ResourceID resourceID) { + var ed = + eventFilterWindows.computeIfAbsent(resourceID, id -> new EventFilterWindow(ongoingReList)); + ed.increaseActiveUpdates(); + } + + public synchronized Optional doneEventFilterModify(ResourceID resourceID) { + var ed = eventFilterWindows.get(resourceID); + if (ed == null) return Optional.empty(); + ed.decreaseActiveUpdates(); + return check(ed, resourceID); + } + + public synchronized Optional processEvent( + ResourceID resourceId, GenericResourceEvent genericResourceEvent) { + var ed = eventFilterWindows.get(resourceId); + if (ed != null) { + ed.addRelatedEvent(genericResourceEvent); + return check(ed, resourceId); + } else { + return Optional.of(genericResourceEvent); + } + } + + private Optional check( + EventFilterWindow eventFilterWindow, ResourceID resourceID) { + var res = eventFilterWindow.check(); + if (eventFilterWindow.canBeRemoved()) { + eventFilterWindows.remove(resourceID); + } + return res; + } + + public synchronized void addToOwnResourceVersions(ResourceID resourceId, String resourceVersion) { + Optional.ofNullable(eventFilterWindows.get(resourceId)) + .ifPresent(au -> au.addToOwnResourceVersions(resourceVersion)); + } + + public synchronized void handleGhostResourceRemoval(ResourceID resourceId) { + eventFilterWindows.remove(resourceId); + } + + // for testing purposes + synchronized Map getEventFilterWindows() { + return eventFilterWindows; + } + + public synchronized void setStartingReList() { + ongoingReList = true; + eventFilterWindows.values().forEach(EventFilterWindow::setReListStarted); + } + + public synchronized void setRelistFinished() { + 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..089ed4d47d --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/EventFilterWindow.java @@ -0,0 +1,214 @@ +/* + * 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 eventing and algorithms of a single resources. + */ +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; + } + + // Before we run this method + // - 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 + public synchronized Optional check() { + 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..472fd91c40 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 getLastStateUnknow() { + 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..ea2ab89c2d 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,8 +122,15 @@ 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; + } + if (resultEvent.orElseThrow().getAction() != ResourceAction.DELETED) { + log.warn("Non delete event received on onDelete handling. This should not happen."); + } primaryToSecondaryIndex.onDelete(resource); - temporaryResourceCache.onDeleteEvent(resource, deletedFinalStateUnknown); if (acceptedByDeleteFilters(resource, deletedFinalStateUnknown)) { propagateEvent(resource); } @@ -152,22 +158,26 @@ private synchronized void onAddOrUpdate(ResourceAction action, R newObject, R ol 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"); + if (resultEvent.isEmpty()) { + log.debug("Deferring event propagation"); } else if (eventAcceptedByFilter(action, newObject, oldObject)) { 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(); + handleEvent( + event.getAction(), + (R) event.getResource().orElseThrow(), + (R) event.getPreviousResource().orElse(null), + event.getLastStateUnknow()); } 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()) { 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..52697557c6 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< @@ -101,45 +100,19 @@ public R eventFilteringUpdateAndCacheResource(R resourceToUpdate, UnaryOperator< try { temporaryResourceCache.startEventFilteringModify(id); updatedResource = updateMethod.apply(resourceToUpdate); - log.debug("Resource update successful"); handleRecentResourceUpdate(id, updatedResource, resourceToUpdate); + log.debug("Caching resource update successful"); 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"); handleEvent( r.getAction(), - latestResource, - prevVersionOfResource, - (r instanceof ResourceDeleteEvent) - ? ((ResourceDeleteEvent) r).isDeletedFinalStateUnknown() - : null); + (R) r.getResource().orElseThrow(), + (R) r.getPreviousResource().orElse(null), + r.getLastStateUnknow()); }, () -> log.debug("No new event present after the filtering update")); } @@ -173,9 +146,16 @@ public synchronized void stop() { @Override public void onList(String resourceVersion, boolean remainedEmpty) { + temporaryResourceCache.setRelistFinished(resourceVersion); temporaryResourceCache.checkGhostResources(); } + // should be enabled when related feature added to fabric8 client + // @Override + // 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..0e6060f07e 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,53 @@ 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"); } 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: {}", 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; } } - 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. */ @@ -178,6 +144,17 @@ public synchronized void putResource(T newResource) { return; } + // also make sure that we're later than the existing temporary entry — compare + // against the temp cache directly; using managedInformerEventSource.get() here + // would fall back to the informer cache and skip the put when this resource's + // latest RV in informer is the SSA result already (or, more subtly, when + // namespace-level lastSyncResourceVersion is ahead due to OTHER resources), + // breaking read-cache-after-write consistency for byIndex/list lookups that + // run before the watch event for the new RV reaches the indexer. + var cachedResource = getResourceFromCache(resourceId).orElse(null); + eventFilteringSupport.addToOwnResourceVersions( + resourceId, newResource.getMetadata().getResourceVersion()); + var ns = newResource.getMetadata().getNamespace(); // this can happen when we dynamically change the followed namespace list if (!managedInformerEventSource.manager().isWatchingNamespace(ns)) { @@ -206,9 +183,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( @@ -231,7 +205,7 @@ 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() { log.debug("Checking for ghost resources."); var iterator = cache.entrySet().iterator(); while (iterator.hasNext()) { @@ -246,19 +220,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 +245,20 @@ synchronized boolean isEmpty() { } synchronized Map getResources() { - return Collections.unmodifiableMap(cache); + return Map.copyOf(cache); + } + + // for testing purposes + synchronized EventFilterSupport getEventFilterSupport() { + return eventFilteringSupport; + } + + public synchronized void setOngoingRelist(String lastKnownSyncVersion) { + eventFilteringSupport.setStartingReList(); + } + + public synchronized void setRelistFinished(String syncResourceVersions) { + // turned off until client support + // 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..39da208d57 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); @@ -196,71 +199,22 @@ void eventFilteringNewEventDuringUpdate() { await().untilAsserted(() -> expectHandleEvent(3, 2)); } - @Test - void eventFilteringMoreNewEventsDuringUpdate() { - 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)); - latch.countDown(); - - await().untilAsserted(() -> expectHandleEvent(4, 2)); - } - - @Test - void eventFilteringExceptionDuringUpdate() { - 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)); - latch.countDown(); - - expectHandleEvent(2, 1); - } - 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..7163234dc9 --- /dev/null +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/EventFilterSupportTest.java @@ -0,0 +1,599 @@ +/* + * 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)); + support.processEvent(RESOURCE_ID, updateEvent(FIRST_OWN_VERSION)); + + var res = support.doneEventFilterModify(RESOURCE_ID); + + assertThat(res).isEmpty(); + assertThat(support.isActiveUpdateFor(RESOURCE_ID)).isFalse(); + } + + @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..3dcd23aa51 --- /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.getLastStateUnknow()).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.getLastStateUnknow()).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.getLastStateUnknow()).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..43a7af5d1e 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 @@ -29,7 +29,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 +45,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 +55,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 +68,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 +110,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 +124,7 @@ void propagateEventAndRemoveResourceFromTempCacheIfResourceVersionMismatch() { } @Test - void genericFilterForEvents() { + void genericFilterRejectsAddUpdateAndDelete() { informerEventSource.setGenericFilter(r -> false); when(temporaryResourceCache.getResourceFromCache(any())).thenReturn(Optional.empty()); @@ -176,7 +136,7 @@ void genericFilterForEvents() { } @Test - void filtersOnAddEvents() { + void onAddFilterRejectsAdd() { informerEventSource.setOnAddFilter(r -> false); when(temporaryResourceCache.getResourceFromCache(any())).thenReturn(Optional.empty()); @@ -186,7 +146,7 @@ void filtersOnAddEvents() { } @Test - void filtersOnUpdateEvents() { + void onUpdateFilterRejectsUpdate() { informerEventSource.setOnUpdateFilter((r1, r2) -> false); when(temporaryResourceCache.getResourceFromCache(any())).thenReturn(Optional.empty()); @@ -196,7 +156,7 @@ void filtersOnUpdateEvents() { } @Test - void filtersOnDeleteEvents() { + void onDeleteFilterRejectsDelete() { informerEventSource.setOnDeleteFilter((r, b) -> false); when(temporaryResourceCache.getResourceFromCache(any())).thenReturn(Optional.empty()); @@ -206,293 +166,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(); - } - - @Test - void filterAddEventBeforeUpdate() { - withRealTemporaryResourceCache(); - - CountDownLatch latch = sendForEventFilteringUpdate(2); - informerEventSource.onAdd(deploymentWithResourceVersion(1)); - latch.countDown(); - - assertNoEventProduced(); + expectHandleUpdateEvent(2, 1); } @Test - void multipleCachingFilteringUpdates() { + 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); - 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); - - latch.countDown(); informerEventSource.onUpdate( deploymentWithResourceVersion(1), deploymentWithResourceVersion(2)); - informerEventSource.onUpdate( - deploymentWithResourceVersion(2), deploymentWithResourceVersion(3)); - latch2.countDown(); - assertNoEventProduced(); + expectHandleUpdateEvent(2, 1); } @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 +331,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 +409,165 @@ 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 resourceId = ResourceID.fromResource(resource); - var ghostResourceId = ResourceID.fromResource(ghostResource); + var tempCache = new TemporaryResourceCache<>(true, informerEventSource); + informerEventSource.setTemporalResourceCache(tempCache); - when(temporaryResourceCache.getResources()).thenReturn(Map.of(ghostResourceId, ghostResource)); - when(temporaryResourceCache.isEmpty()).thenReturn(false); + 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 mim = mock(InformerManager.class); - when(mim.keys()).thenReturn(Stream.of(resourceId)); - when(mim.contains(ghostResourceId)).thenReturn(false); - when(informerEventSource.manager()).thenReturn(mim); + tempCache.putResource(resource); + when(manager.lastSyncResourceVersion(any())).thenReturn("5"); - var result = informerEventSource.keys().toList(); + tempCache.checkGhostResources(); - assertThat(result).containsExactlyInAnyOrder(resourceId, ghostResourceId); + assertThat(tempCache.getResources()).containsKey(ResourceID.fromResource(resource)); + verify(eventHandlerMock, never()).handleEvent(any()); } - @Test - void keysDoesNotDuplicateExistingKeys() { - var resource = testDeployment(); - var newerResource = testDeployment(); - newerResource.getMetadata().setResourceVersion("5"); + private void assertNoEventProduced() { + await() + .pollDelay(Duration.ofMillis(70)) + .timeout(Duration.ofMillis(150)) + .untilAsserted(() -> verify(informerEventSource, never()).propagateEvent(any())); + } - var resourceId = ResourceID.fromResource(resource); + 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())); + } - when(temporaryResourceCache.getResources()).thenReturn(Map.of(resourceId, newerResource)); - when(temporaryResourceCache.isEmpty()).thenReturn(false); + 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..73757e385b 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.getLastStateUnknow()).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/readownupdates/ReadOwnUpdatesCustomResource.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/readownupdates/ReadOwnUpdatesCustomResource.java index 0d25bbfdd4..f12431c3ea 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/readownupdates/ReadOwnUpdatesCustomResource.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 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("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;