From 1380caaf3e39f4f8b35b711f15825e603409a218 Mon Sep 17 00:00:00 2001 From: Steve Hawkins <shawkins@redhat.com> Date: Mon, 11 Sep 2023 08:12:04 -0400 Subject: [PATCH 1/4] refinements mentioned on #2012 provides two options - to control if the annotation is used (to omit events that come too quickly) - to parse the resource version (to keep the cache up-to-date and omit events if they come too slowly) Signed-off-by: Steve Hawkins <shawkins@redhat.com> --- .../api/config/ConfigurationService.java | 27 ++++++++++++ .../config/ConfigurationServiceOverrider.java | 28 ++++++++++++ .../informer/InformerConfiguration.java | 4 ++ .../KubernetesDependentResource.java | 18 +++++--- .../ControllerResourceEventSource.java | 4 +- .../source/informer/InformerEventSource.java | 31 +++++++++++-- .../informer/ManagedInformerEventSource.java | 12 +++--- .../informer/TemporaryResourceCache.java | 43 +++++++++++++++---- .../informer/InformerEventSourceTest.java | 2 +- .../informer/TemporaryResourceCacheTest.java | 38 +++++++++++++--- ...pdateInformerEventSourceEventFilterIT.java | 38 ++++++---------- .../PreviousAnnotationDisabledIT.java | 38 ++++++++++++++++ 12 files changed, 226 insertions(+), 57 deletions(-) create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/PreviousAnnotationDisabledIT.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java index 86efb457c5..ac9f6430df 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java @@ -364,4 +364,31 @@ default Set<Class<? extends HasMetadata>> defaultNonSSAResource() { return Set.of(ConfigMap.class, Secret.class); } + /** + * If an annotation should be used so that the operator sdk can detect events from its own updates + * of dependent resources and then filter them. + * <p> + * Disable this if you want to react to your own dependent resource updates + * + * @since 4.5.0 + */ + default boolean previousAnnotationForDependentResources() { + return true; + } + + /** + * If the event logic should parse the resourceVersion to determine the ordering of events. This + * is typically not needed. + * <p> + * Disabled by default as Kubernetes does not support, and discourages, this interpretation of + * resourceVersions. Enable only if your api server event processing seems to lag the operator + * logic and you want to further minimize the the amount of work done / updates issued by the + * operator. + * + * @since 4.5.0 + */ + default boolean parseResourceVersions() { + return false; + } + } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java index 15418aed5e..6b4e2a71fb 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java @@ -37,6 +37,8 @@ public class ConfigurationServiceOverrider { private ResourceClassResolver resourceClassResolver; private Boolean ssaBasedCreateUpdateMatchForDependentResources; private Set<Class<? extends HasMetadata>> defaultNonSSAResource; + private Boolean previousAnnotationForDependentResources; + private Boolean parseResourceVersions; ConfigurationServiceOverrider(ConfigurationService original) { this.original = original; @@ -158,6 +160,18 @@ public ConfigurationServiceOverrider withDefaultNonSSAResource( return this; } + public ConfigurationServiceOverrider withPreviousAnnotationForDependentResources( + boolean value) { + this.previousAnnotationForDependentResources = value; + return this; + } + + public ConfigurationServiceOverrider wihtParseResourceVersions( + boolean value) { + this.parseResourceVersions = value; + return this; + } + public ConfigurationService build() { return new BaseConfigurationService(original.getVersion(), cloner, client) { @Override @@ -270,6 +284,20 @@ public Set<Class<? extends HasMetadata>> defaultNonSSAResource() { return defaultNonSSAResource != null ? defaultNonSSAResource : super.defaultNonSSAResource(); } + + @Override + public boolean previousAnnotationForDependentResources() { + return previousAnnotationForDependentResources != null + ? previousAnnotationForDependentResources + : super.previousAnnotationForDependentResources(); + } + + @Override + public boolean parseResourceVersions() { + return parseResourceVersions != null + ? parseResourceVersions + : super.parseResourceVersions(); + } }; } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java index 1f40677ee7..4b0007c96e 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java @@ -62,6 +62,7 @@ public SecondaryToPrimaryMapper<R> getSecondaryToPrimaryMapper() { return secondaryToPrimaryMapper; } + @Override public Optional<OnDeleteFilter<? super R>> onDeleteFilter() { return Optional.ofNullable(onDeleteFilter); } @@ -95,12 +96,15 @@ public <P extends HasMetadata> PrimaryToSecondaryMapper<P> getPrimaryToSecondary */ SecondaryToPrimaryMapper<R> getSecondaryToPrimaryMapper(); + @Override Optional<OnAddFilter<? super R>> onAddFilter(); + @Override Optional<OnUpdateFilter<? super R>> onUpdateFilter(); Optional<OnDeleteFilter<? super R>> onDeleteFilter(); + @Override Optional<GenericFilter<? super R>> genericFilter(); <P extends HasMetadata> PrimaryToSecondaryMapper<P> getPrimaryToSecondaryMapper(); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java index 22ec5a48f9..657bff53fb 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java @@ -113,7 +113,7 @@ public R create(R desired, P primary, Context<P> context) { desired.getMetadata().setResourceVersion("1"); } } - addMetadata(false, null, desired, primary); + addMetadata(false, null, desired, primary, context); sanitizeDesired(desired, null, primary, context); final var resource = prepare(desired, primary, "Creating"); return useSSA(context) @@ -130,7 +130,7 @@ public R update(R actual, R desired, P primary, Context<P> context) { actual.getMetadata().getResourceVersion()); } R updatedResource; - addMetadata(false, actual, desired, primary); + addMetadata(false, actual, desired, primary, context); sanitizeDesired(desired, actual, primary, context); if (useSSA(context)) { updatedResource = prepare(desired, primary, "Updating") @@ -163,7 +163,7 @@ public Result<R> match(R actualResource, R desired, P primary, Context<P> contex public Result<R> match(R actualResource, R desired, P primary, ResourceUpdaterMatcher<R> matcher, Context<P> context) { final boolean matches; - addMetadata(true, actualResource, desired, primary); + addMetadata(true, actualResource, desired, primary, context); if (useSSA(context)) { matches = SSABasedGenericKubernetesResourceMatcher.getInstance() .matches(actualResource, desired, context); @@ -173,8 +173,9 @@ public Result<R> match(R actualResource, R desired, P primary, ResourceUpdaterMa return Result.computed(matches, desired); } - protected void addMetadata(boolean forMatch, R actualResource, final R target, P primary) { - if (forMatch) { // keep the current + protected void addMetadata(boolean forMatch, R actualResource, final R target, P primary, + Context<P> context) { + if (forMatch) { // keep the current previous annotation String actual = actualResource.getMetadata().getAnnotations() .get(InformerEventSource.PREVIOUS_ANNOTATION_KEY); Map<String, String> annotations = target.getMetadata().getAnnotations(); @@ -183,7 +184,7 @@ protected void addMetadata(boolean forMatch, R actualResource, final R target, P } else { annotations.remove(InformerEventSource.PREVIOUS_ANNOTATION_KEY); } - } else { // set a new one + } else if (usePreviousAnnotation(context)) { // set a new one eventSource().orElseThrow().addPreviousAnnotation( Optional.ofNullable(actualResource).map(r -> r.getMetadata().getResourceVersion()) .orElse(null), @@ -208,6 +209,11 @@ protected boolean useSSA(Context<P> context) { .ssaBasedCreateUpdateMatchForDependentResources()); } + private boolean usePreviousAnnotation(Context<P> context) { + return context.getControllerConfiguration().getConfigurationService() + .previousAnnotationForDependentResources(); + } + @Override protected void handleDelete(P primary, R secondary, Context<P> context) { if (secondary != null) { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerResourceEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerResourceEventSource.java index da2e517376..f7b51bd572 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerResourceEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerResourceEventSource.java @@ -32,12 +32,12 @@ public class ControllerResourceEventSource<T extends HasMetadata> @SuppressWarnings({"unchecked", "rawtypes"}) public ControllerResourceEventSource(Controller<T> controller) { - super(controller.getCRClient(), controller.getConfiguration()); + super(controller.getCRClient(), controller.getConfiguration(), false); this.controller = controller; final var config = controller.getConfiguration(); OnUpdateFilter internalOnUpdateFilter = - (OnUpdateFilter<T>) onUpdateFinalizerNeededAndApplied(controller.useFinalizer(), + onUpdateFinalizerNeededAndApplied(controller.useFinalizer(), config.getFinalizerName()) .or(onUpdateGenerationAware(config.isGenerationAware())) .or(onUpdateMarkedForDeletion()); 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 d154ad201b..8c2e1bb76a 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 @@ -69,6 +69,8 @@ public class InformerEventSource<R extends HasMetadata, P extends HasMetadata> extends ManagedInformerEventSource<R, P, InformerConfiguration<R>> implements ResourceEventHandler<R> { + private static final int MAX_RESOURCE_VERSIONS = 256; + public static String PREVIOUS_ANNOTATION_KEY = "javaoperatorsdk.io/previous"; private static final Logger log = LoggerFactory.getLogger(InformerEventSource.class); @@ -78,14 +80,31 @@ public class InformerEventSource<R extends HasMetadata, P extends HasMetadata> private final PrimaryToSecondaryMapper<P> primaryToSecondaryMapper; private Map<String, Function<R, List<String>>> indexerBuffer = new HashMap<>(); private final String id = UUID.randomUUID().toString(); + private final Set<String> knownResourceVersions; public InformerEventSource( InformerConfiguration<R> configuration, EventSourceContext<P> context) { - this(configuration, context.getClient()); + this(configuration, context.getClient(), + context.getControllerConfiguration().getConfigurationService().parseResourceVersions()); } public InformerEventSource(InformerConfiguration<R> configuration, KubernetesClient client) { - super(client.resources(configuration.getResourceClass()), configuration); + this(configuration, client, false); + } + + public InformerEventSource(InformerConfiguration<R> configuration, KubernetesClient client, + boolean parseResourceVersions) { + super(client.resources(configuration.getResourceClass()), configuration, parseResourceVersions); + if (parseResourceVersions) { + knownResourceVersions = Collections.newSetFromMap(new LinkedHashMap<String, Boolean>() { + @Override + protected boolean removeEldestEntry(java.util.Map.Entry<String, Boolean> eldest) { + return size() >= MAX_RESOURCE_VERSIONS; + } + }); + } else { + knownResourceVersions = null; + } // If there is a primary to secondary mapper there is no need for primary to secondary index. primaryToSecondaryMapper = configuration.getPrimaryToSecondaryMapper(); @@ -169,6 +188,10 @@ private synchronized void onAddOrUpdate(Operation operation, R newObject, R oldO } private boolean canSkipEvent(R newObject, R oldObject, ResourceID resourceID) { + if (knownResourceVersions != null + && knownResourceVersions.contains(newObject.getMetadata().getResourceVersion())) { + return true; + } var res = temporaryResourceCache.getResourceFromCache(resourceID); if (res.isEmpty()) { return isEventKnownFromAnnotation(newObject, oldObject); @@ -262,6 +285,9 @@ public synchronized void handleRecentResourceCreate(ResourceID resourceID, R res private void handleRecentCreateOrUpdate(Operation operation, R newResource, R oldResource) { primaryToSecondaryIndex.onAddOrUpdate(newResource); + if (knownResourceVersions != null) { + knownResourceVersions.add(newResource.getMetadata().getResourceVersion()); + } temporaryResourceCache.putResource(newResource, Optional.ofNullable(oldResource) .map(r -> r.getMetadata().getResourceVersion()).orElse(null)); } @@ -275,7 +301,6 @@ public boolean allowsNamespaceChanges() { return configuration().followControllerNamespaceChanges(); } - private boolean eventAcceptedByFilter(Operation operation, R newObject, R oldObject) { if (genericFilter != null && !genericFilter.accept(newObject)) { return false; 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 6ec6cd7f6e..5dff2be51b 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java @@ -42,26 +42,27 @@ public abstract class ManagedInformerEventSource<R extends HasMetadata, P extend protected MixedOperation<R, KubernetesResourceList<R>, Resource<R>> client; protected ManagedInformerEventSource( - MixedOperation<R, KubernetesResourceList<R>, Resource<R>> client, C configuration) { + MixedOperation<R, KubernetesResourceList<R>, Resource<R>> client, C configuration, + boolean parseResourceVersions) { super(configuration.getResourceClass()); this.client = client; - temporaryResourceCache = new TemporaryResourceCache<>(this); + temporaryResourceCache = new TemporaryResourceCache<>(this, parseResourceVersions); this.cache = new InformerManager<>(client, configuration, this); } @Override public void onAdd(R resource) { - temporaryResourceCache.removeResourceFromCache(resource); + temporaryResourceCache.onEvent(resource, false); } @Override public void onUpdate(R oldObj, R newObj) { - temporaryResourceCache.removeResourceFromCache(newObj); + temporaryResourceCache.onEvent(newObj, false); } @Override public void onDelete(R obj, boolean deletedFinalStateUnknown) { - temporaryResourceCache.removeResourceFromCache(obj); + temporaryResourceCache.onEvent(obj, deletedFinalStateUnknown); } protected InformerManager<R, C> manager() { @@ -127,6 +128,7 @@ void setTemporalResourceCache(TemporaryResourceCache<R> temporaryResourceCache) this.temporaryResourceCache = temporaryResourceCache; } + @Override public void addIndexers(Map<String, Function<R, List<String>>> indexers) { cache.addIndexers(indexers); } 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 233b409f3f..b72060cb54 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 @@ -8,6 +8,7 @@ import org.slf4j.LoggerFactory; import io.fabric8.kubernetes.api.model.HasMetadata; +import io.javaoperatorsdk.operator.api.config.informer.InformerConfiguration; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource; import io.javaoperatorsdk.operator.processing.event.ResourceID; @@ -36,13 +37,18 @@ public class TemporaryResourceCache<T extends HasMetadata> { private final Map<ResourceID, T> cache = new ConcurrentHashMap<>(); private final ManagedInformerEventSource<T, ?, ?> managedInformerEventSource; + private final boolean parseResourceVersions; - public TemporaryResourceCache(ManagedInformerEventSource<T, ?, ?> managedInformerEventSource) { + public TemporaryResourceCache(ManagedInformerEventSource<T, ?, ?> managedInformerEventSource, + boolean parseResourceVersions) { this.managedInformerEventSource = managedInformerEventSource; + this.parseResourceVersions = parseResourceVersions; } - public synchronized Optional<T> removeResourceFromCache(T resource) { - return Optional.ofNullable(cache.remove(ResourceID.fromResource(resource))); + public synchronized void onEvent(T resource, boolean unknownState) { + cache.computeIfPresent(ResourceID.fromResource(resource), + (id, cached) -> (unknownState || !isLaterResourceVersion(id, cached, resource)) ? null + : cached); } public synchronized void putAddedResource(T newResource) { @@ -61,18 +67,37 @@ public synchronized void putResource(T newResource, String previousResourceVersi .orElse(managedInformerEventSource.get(resourceId).orElse(null)); if ((previousResourceVersion == null && cachedResource == null) - || (cachedResource != null && previousResourceVersion != null - && cachedResource.getMetadata().getResourceVersion() - .equals(previousResourceVersion))) { + || (cachedResource != null + && (cachedResource.getMetadata().getResourceVersion().equals(previousResourceVersion)) + || isLaterResourceVersion(resourceId, newResource, cachedResource))) { log.debug( "Temporarily moving ahead to target version {} for resource id: {}", newResource.getMetadata().getResourceVersion(), resourceId); putToCache(newResource, resourceId); - } else { - if (cache.remove(resourceId) != null) { - log.debug("Removed an obsolete resource from cache for id: {}", resourceId); + } else if (cache.remove(resourceId) != null) { + log.debug("Removed an obsolete resource from cache for id: {}", resourceId); + } + } + + /** + * @return true if {@link InformerConfiguration#parseResourceVersions()} is enabled and the + * resourceVersion of newResource is numerically greater than cachedResource, otherwise + * false + */ + private boolean isLaterResourceVersion(ResourceID resourceId, T newResource, T cachedResource) { + try { + if (parseResourceVersions + && Long.compare(Long.parseLong(newResource.getMetadata().getResourceVersion()), + Long.parseLong(cachedResource.getMetadata().getResourceVersion())) > 0) { + return true; } + } catch (NumberFormatException e) { + log.debug( + "Could not compare resourceVersions {} and {} for {}", + newResource.getMetadata().getResourceVersion(), + cachedResource.getMetadata().getResourceVersion(), resourceId); } + return false; } private void putToCache(T resource, ResourceID resourceID) { 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 7acecc7099..0881cccaf2 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 @@ -119,7 +119,7 @@ void propagateEventAndRemoveResourceFromTempCacheIfResourceVersionMismatch() { informerEventSource.onUpdate(cachedDeployment, testDeployment()); verify(eventHandlerMock, times(1)).handleEvent(any()); - verify(temporaryResourceCacheMock, times(1)).removeResourceFromCache(any()); + verify(temporaryResourceCacheMock, times(1)).onEvent(testDeployment(), false); } @Test 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 4d5bdf0dfd..c021e3d5b1 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 @@ -3,9 +3,11 @@ import java.util.Map; import java.util.Optional; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; 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.processing.event.ResourceID; @@ -16,12 +18,16 @@ class TemporaryResourceCacheTest { - public static final String RESOURCE_VERSION = "1"; + public static final String RESOURCE_VERSION = "2"; @SuppressWarnings("unchecked") - private final InformerEventSource<ConfigMap, ?> informerEventSource = - mock(InformerEventSource.class); - private final TemporaryResourceCache<ConfigMap> temporaryResourceCache = - new TemporaryResourceCache<>(informerEventSource); + private InformerEventSource<ConfigMap, ?> informerEventSource; + private TemporaryResourceCache<ConfigMap> temporaryResourceCache; + + @BeforeEach + void setup() { + informerEventSource = mock(InformerEventSource.class); + temporaryResourceCache = new TemporaryResourceCache<>(informerEventSource, false); + } @Test void updateAddsTheResourceIntoCacheIfTheInformerHasThePreviousResourceVersion() { @@ -75,7 +81,27 @@ void addOperationNotAddsTheResourceIfInformerCacheNotEmpty() { void removesResourceFromCache() { ConfigMap testResource = propagateTestResourceToCache(); - temporaryResourceCache.removeResourceFromCache(testResource()); + temporaryResourceCache.onEvent(testResource(), false); + + assertThat(temporaryResourceCache.getResourceFromCache(ResourceID.fromResource(testResource))) + .isNotPresent(); + } + + @Test + void resourceVersionParsing() { + this.temporaryResourceCache = new TemporaryResourceCache<>(informerEventSource, true); + + ConfigMap testResource = propagateTestResourceToCache(); + + // an event with a newer version will not remove + temporaryResourceCache.onEvent(new ConfigMapBuilder(testResource()).editMetadata() + .withResourceVersion("1").endMetadata().build(), false); + + assertThat(temporaryResourceCache.getResourceFromCache(ResourceID.fromResource(testResource))) + .isPresent(); + + // anything else will remove + temporaryResourceCache.onEvent(testResource(), false); assertThat(temporaryResourceCache.getResourceFromCache(ResourceID.fromResource(testResource))) .isNotPresent(); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/CreateUpdateInformerEventSourceEventFilterIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/CreateUpdateInformerEventSourceEventFilterIT.java index f319509c47..0bd4767776 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/CreateUpdateInformerEventSourceEventFilterIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/CreateUpdateInformerEventSourceEventFilterIT.java @@ -26,28 +26,12 @@ class CreateUpdateInformerEventSourceEventFilterIT { @Test void updateEventNotReceivedAfterCreateOrUpdate() { - CreateUpdateEventFilterTestCustomResource resource = prepareTestResource(); + CreateUpdateEventFilterTestCustomResource resource = + CreateUpdateInformerEventSourceEventFilterIT.prepareTestResource(); var createdResource = operator.create(resource); - await() - .atMost(Duration.ofSeconds(1)) - .until(() -> { - var cm = operator.get(ConfigMap.class, createdResource.getMetadata().getName()); - if (cm == null) { - return false; - } - return cm.getData() - .get(CONFIG_MAP_TEST_DATA_KEY) - .equals(createdResource.getSpec().getValue()); - }); - - assertThat( - ((CreateUpdateEventFilterTestReconciler) operator.getFirstReconciler()) - .getNumberOfExecutions()) - .isEqualTo(1); // this should be 1 usually but sometimes event is received - // faster than added resource added to the cache - + assertData(operator, createdResource, 1); CreateUpdateEventFilterTestCustomResource actualCreatedResource = operator.get(CreateUpdateEventFilterTestCustomResource.class, @@ -55,26 +39,30 @@ void updateEventNotReceivedAfterCreateOrUpdate() { actualCreatedResource.getSpec().setValue("2"); operator.replace(actualCreatedResource); + assertData(operator, actualCreatedResource, 2); + } - await().atMost(Duration.ofSeconds(1)) + static void assertData(LocallyRunOperatorExtension operator, + CreateUpdateEventFilterTestCustomResource resource, int executions) { + await() + .atMost(Duration.ofSeconds(1)) .until(() -> { - var cm = operator.get(ConfigMap.class, createdResource.getMetadata().getName()); + var cm = operator.get(ConfigMap.class, resource.getMetadata().getName()); if (cm == null) { return false; } return cm.getData() .get(CONFIG_MAP_TEST_DATA_KEY) - .equals(actualCreatedResource.getSpec().getValue()); + .equals(resource.getSpec().getValue()); }); assertThat( ((CreateUpdateEventFilterTestReconciler) operator.getFirstReconciler()) .getNumberOfExecutions()) - // same as for previous assert (usually this should be 2) - .isEqualTo(2); + .isEqualTo(executions); } - private CreateUpdateEventFilterTestCustomResource prepareTestResource() { + static CreateUpdateEventFilterTestCustomResource prepareTestResource() { CreateUpdateEventFilterTestCustomResource resource = new CreateUpdateEventFilterTestCustomResource(); resource.setMetadata(new ObjectMeta()); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/PreviousAnnotationDisabledIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/PreviousAnnotationDisabledIT.java new file mode 100644 index 0000000000..31b84ce1ad --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/PreviousAnnotationDisabledIT.java @@ -0,0 +1,38 @@ +package io.javaoperatorsdk.operator; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; +import io.javaoperatorsdk.operator.sample.createupdateeventfilter.CreateUpdateEventFilterTestCustomResource; +import io.javaoperatorsdk.operator.sample.createupdateeventfilter.CreateUpdateEventFilterTestReconciler; + +class PreviousAnnotationDisabledIT { + + @RegisterExtension + LocallyRunOperatorExtension operator = + LocallyRunOperatorExtension.builder() + .withReconciler(new CreateUpdateEventFilterTestReconciler()) + .withConfigurationService( + overrider -> overrider.withPreviousAnnotationForDependentResources(false)) + .build(); + + @Test + void updateEventReceivedAfterCreateOrUpdate() { + CreateUpdateEventFilterTestCustomResource resource = + CreateUpdateInformerEventSourceEventFilterIT.prepareTestResource(); + var createdResource = + operator.create(resource); + + CreateUpdateInformerEventSourceEventFilterIT.assertData(operator, createdResource, 2); + + CreateUpdateEventFilterTestCustomResource actualCreatedResource = + operator.get(CreateUpdateEventFilterTestCustomResource.class, + resource.getMetadata().getName()); + actualCreatedResource.getSpec().setValue("2"); + operator.replace(actualCreatedResource); + + CreateUpdateInformerEventSourceEventFilterIT.assertData(operator, actualCreatedResource, 4); + } + +} From 4c84f9f2baffe50979b9007dd53fcc37ebe89f2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= <csviri@gmail.com> Date: Thu, 21 Sep 2023 13:35:12 +0200 Subject: [PATCH 2/4] minor improvements MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros <csviri@gmail.com> --- .../event/source/informer/InformerEventSource.java | 10 +++++----- .../event/source/informer/TemporaryResourceCache.java | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java index 8c2e1bb76a..6f41a29038 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java @@ -96,11 +96,11 @@ public InformerEventSource(InformerConfiguration<R> configuration, KubernetesCli boolean parseResourceVersions) { super(client.resources(configuration.getResourceClass()), configuration, parseResourceVersions); if (parseResourceVersions) { - knownResourceVersions = Collections.newSetFromMap(new LinkedHashMap<String, Boolean>() { - @Override - protected boolean removeEldestEntry(java.util.Map.Entry<String, Boolean> eldest) { - return size() >= MAX_RESOURCE_VERSIONS; - } + knownResourceVersions = Collections.newSetFromMap(new LinkedHashMap<>() { + @Override + protected boolean removeEldestEntry(java.util.Map.Entry<String, Boolean> eldest) { + return size() >= MAX_RESOURCE_VERSIONS; + } }); } else { knownResourceVersions = null; 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 b72060cb54..42490fc169 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 @@ -87,8 +87,8 @@ public synchronized void putResource(T newResource, String previousResourceVersi private boolean isLaterResourceVersion(ResourceID resourceId, T newResource, T cachedResource) { try { if (parseResourceVersions - && Long.compare(Long.parseLong(newResource.getMetadata().getResourceVersion()), - Long.parseLong(cachedResource.getMetadata().getResourceVersion())) > 0) { + && Long.parseLong(newResource.getMetadata().getResourceVersion()) > + Long.parseLong(cachedResource.getMetadata().getResourceVersion())) { return true; } } catch (NumberFormatException e) { From 05d7951a31d73f17ad30fbbe01371ffb343c0511 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= <csviri@gmail.com> Date: Thu, 21 Sep 2023 13:36:32 +0200 Subject: [PATCH 3/4] format MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros <csviri@gmail.com> --- .../event/source/informer/InformerEventSource.java | 8 ++++---- .../event/source/informer/TemporaryResourceCache.java | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java index 6f41a29038..c9351f87bb 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 @@ -97,10 +97,10 @@ public InformerEventSource(InformerConfiguration<R> configuration, KubernetesCli super(client.resources(configuration.getResourceClass()), configuration, parseResourceVersions); if (parseResourceVersions) { knownResourceVersions = Collections.newSetFromMap(new LinkedHashMap<>() { - @Override - protected boolean removeEldestEntry(java.util.Map.Entry<String, Boolean> eldest) { - return size() >= MAX_RESOURCE_VERSIONS; - } + @Override + protected boolean removeEldestEntry(java.util.Map.Entry<String, Boolean> eldest) { + return size() >= MAX_RESOURCE_VERSIONS; + } }); } else { knownResourceVersions = null; 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 42490fc169..8c5c0092d3 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 @@ -87,8 +87,8 @@ public synchronized void putResource(T newResource, String previousResourceVersi private boolean isLaterResourceVersion(ResourceID resourceId, T newResource, T cachedResource) { try { if (parseResourceVersions - && Long.parseLong(newResource.getMetadata().getResourceVersion()) > - Long.parseLong(cachedResource.getMetadata().getResourceVersion())) { + && Long.parseLong(newResource.getMetadata().getResourceVersion()) > Long + .parseLong(cachedResource.getMetadata().getResourceVersion())) { return true; } } catch (NumberFormatException e) { From 8f00c937ddf9a2e123a0c62bcd92350bfaed814c Mon Sep 17 00:00:00 2001 From: Steve Hawkins <shawkins@redhat.com> Date: Thu, 21 Sep 2023 09:07:48 -0400 Subject: [PATCH 4/4] refinements based upon review feedback moving handling of known resource versions, and making the config names more descriptive Signed-off-by: Steve Hawkins <shawkins@redhat.com> --- .../api/config/ConfigurationService.java | 12 +++++----- .../config/ConfigurationServiceOverrider.java | 8 +++---- .../KubernetesDependentResource.java | 2 +- .../source/informer/InformerEventSource.java | 23 +++---------------- .../informer/TemporaryResourceCache.java | 23 +++++++++++++++++++ .../informer/TemporaryResourceCacheTest.java | 5 +++- ...pdateInformerEventSourceEventFilterIT.java | 14 +++++------ .../PreviousAnnotationDisabledIT.java | 4 ++-- 8 files changed, 50 insertions(+), 41 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java index ac9f6430df..53bfc75df9 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java @@ -365,20 +365,20 @@ default Set<Class<? extends HasMetadata>> defaultNonSSAResource() { } /** - * If an annotation should be used so that the operator sdk can detect events from its own updates - * of dependent resources and then filter them. + * If a javaoperatorsdk.io/previous annotation should be used so that the operator sdk can detect + * events from its own updates of dependent resources and then filter them. * <p> * Disable this if you want to react to your own dependent resource updates * * @since 4.5.0 */ - default boolean previousAnnotationForDependentResources() { + default boolean previousAnnotationForDependentResourcesEventFiltering() { return true; } /** - * If the event logic should parse the resourceVersion to determine the ordering of events. This - * is typically not needed. + * If the event logic should parse the resourceVersion to determine the ordering of dependent + * resource events. This is typically not needed. * <p> * Disabled by default as Kubernetes does not support, and discourages, this interpretation of * resourceVersions. Enable only if your api server event processing seems to lag the operator @@ -387,7 +387,7 @@ default boolean previousAnnotationForDependentResources() { * * @since 4.5.0 */ - default boolean parseResourceVersions() { + default boolean parseResourceVersionsForEventFilteringAndCaching() { return false; } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java index 6b4e2a71fb..2a2f6964c1 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java @@ -286,17 +286,17 @@ public Set<Class<? extends HasMetadata>> defaultNonSSAResource() { } @Override - public boolean previousAnnotationForDependentResources() { + public boolean previousAnnotationForDependentResourcesEventFiltering() { return previousAnnotationForDependentResources != null ? previousAnnotationForDependentResources - : super.previousAnnotationForDependentResources(); + : super.previousAnnotationForDependentResourcesEventFiltering(); } @Override - public boolean parseResourceVersions() { + public boolean parseResourceVersionsForEventFilteringAndCaching() { return parseResourceVersions != null ? parseResourceVersions - : super.parseResourceVersions(); + : super.parseResourceVersionsForEventFilteringAndCaching(); } }; } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java index 657bff53fb..f81f98f12c 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java @@ -211,7 +211,7 @@ protected boolean useSSA(Context<P> context) { private boolean usePreviousAnnotation(Context<P> context) { return context.getControllerConfiguration().getConfigurationService() - .previousAnnotationForDependentResources(); + .previousAnnotationForDependentResourcesEventFiltering(); } @Override diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java index c9351f87bb..968ccc27b9 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 @@ -69,8 +69,6 @@ public class InformerEventSource<R extends HasMetadata, P extends HasMetadata> extends ManagedInformerEventSource<R, P, InformerConfiguration<R>> implements ResourceEventHandler<R> { - private static final int MAX_RESOURCE_VERSIONS = 256; - public static String PREVIOUS_ANNOTATION_KEY = "javaoperatorsdk.io/previous"; private static final Logger log = LoggerFactory.getLogger(InformerEventSource.class); @@ -80,12 +78,12 @@ public class InformerEventSource<R extends HasMetadata, P extends HasMetadata> private final PrimaryToSecondaryMapper<P> primaryToSecondaryMapper; private Map<String, Function<R, List<String>>> indexerBuffer = new HashMap<>(); private final String id = UUID.randomUUID().toString(); - private final Set<String> knownResourceVersions; public InformerEventSource( InformerConfiguration<R> configuration, EventSourceContext<P> context) { this(configuration, context.getClient(), - context.getControllerConfiguration().getConfigurationService().parseResourceVersions()); + context.getControllerConfiguration().getConfigurationService() + .parseResourceVersionsForEventFilteringAndCaching()); } public InformerEventSource(InformerConfiguration<R> configuration, KubernetesClient client) { @@ -95,17 +93,6 @@ public InformerEventSource(InformerConfiguration<R> configuration, KubernetesCli public InformerEventSource(InformerConfiguration<R> configuration, KubernetesClient client, boolean parseResourceVersions) { super(client.resources(configuration.getResourceClass()), configuration, parseResourceVersions); - if (parseResourceVersions) { - knownResourceVersions = Collections.newSetFromMap(new LinkedHashMap<>() { - @Override - protected boolean removeEldestEntry(java.util.Map.Entry<String, Boolean> eldest) { - return size() >= MAX_RESOURCE_VERSIONS; - } - }); - } else { - knownResourceVersions = null; - } - // If there is a primary to secondary mapper there is no need for primary to secondary index. primaryToSecondaryMapper = configuration.getPrimaryToSecondaryMapper(); if (primaryToSecondaryMapper == null) { @@ -188,8 +175,7 @@ private synchronized void onAddOrUpdate(Operation operation, R newObject, R oldO } private boolean canSkipEvent(R newObject, R oldObject, ResourceID resourceID) { - if (knownResourceVersions != null - && knownResourceVersions.contains(newObject.getMetadata().getResourceVersion())) { + if (temporaryResourceCache.isKnownResourceVersion(newObject)) { return true; } var res = temporaryResourceCache.getResourceFromCache(resourceID); @@ -285,9 +271,6 @@ public synchronized void handleRecentResourceCreate(ResourceID resourceID, R res private void handleRecentCreateOrUpdate(Operation operation, R newResource, R oldResource) { primaryToSecondaryIndex.onAddOrUpdate(newResource); - if (knownResourceVersions != null) { - knownResourceVersions.add(newResource.getMetadata().getResourceVersion()); - } temporaryResourceCache.putResource(newResource, Optional.ofNullable(oldResource) .map(r -> r.getMetadata().getResourceVersion()).orElse(null)); } 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 8c5c0092d3..fd9a8ad565 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 @@ -1,7 +1,10 @@ package io.javaoperatorsdk.operator.processing.event.source.informer; +import java.util.Collections; +import java.util.LinkedHashMap; import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import org.slf4j.Logger; @@ -34,15 +37,27 @@ public class TemporaryResourceCache<T extends HasMetadata> { private static final Logger log = LoggerFactory.getLogger(TemporaryResourceCache.class); + private static final int MAX_RESOURCE_VERSIONS = 256; private final Map<ResourceID, T> cache = new ConcurrentHashMap<>(); private final ManagedInformerEventSource<T, ?, ?> managedInformerEventSource; private final boolean parseResourceVersions; + private final Set<String> knownResourceVersions; public TemporaryResourceCache(ManagedInformerEventSource<T, ?, ?> managedInformerEventSource, boolean parseResourceVersions) { this.managedInformerEventSource = managedInformerEventSource; this.parseResourceVersions = parseResourceVersions; + if (parseResourceVersions) { + knownResourceVersions = Collections.newSetFromMap(new LinkedHashMap<String, Boolean>() { + @Override + protected boolean removeEldestEntry(java.util.Map.Entry<String, Boolean> eldest) { + return size() >= MAX_RESOURCE_VERSIONS; + } + }); + } else { + knownResourceVersions = null; + } } public synchronized void onEvent(T resource, boolean unknownState) { @@ -62,6 +77,9 @@ public synchronized void putAddedResource(T newResource) { * @param previousResourceVersion null indicates an add */ public synchronized void putResource(T newResource, String previousResourceVersion) { + if (knownResourceVersions != null) { + knownResourceVersions.add(newResource.getMetadata().getResourceVersion()); + } var resourceId = ResourceID.fromResource(newResource); var cachedResource = getResourceFromCache(resourceId) .orElse(managedInformerEventSource.get(resourceId).orElse(null)); @@ -79,6 +97,11 @@ public synchronized void putResource(T newResource, String previousResourceVersi } } + public boolean isKnownResourceVersion(T resource) { + return knownResourceVersions != null + && knownResourceVersions.contains(resource.getMetadata().getResourceVersion()); + } + /** * @return true if {@link InformerConfiguration#parseResourceVersions()} is enabled and the * resourceVersion of newResource is numerically greater than cachedResource, otherwise 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 c021e3d5b1..d641736739 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 @@ -91,12 +91,15 @@ void removesResourceFromCache() { void resourceVersionParsing() { this.temporaryResourceCache = new TemporaryResourceCache<>(informerEventSource, true); + assertThat(temporaryResourceCache.isKnownResourceVersion(testResource())).isFalse(); + ConfigMap testResource = propagateTestResourceToCache(); // an event with a newer version will not remove - temporaryResourceCache.onEvent(new ConfigMapBuilder(testResource()).editMetadata() + temporaryResourceCache.onEvent(new ConfigMapBuilder(testResource).editMetadata() .withResourceVersion("1").endMetadata().build(), false); + assertThat(temporaryResourceCache.isKnownResourceVersion(testResource)).isTrue(); assertThat(temporaryResourceCache.getResourceFromCache(ResourceID.fromResource(testResource))) .isPresent(); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/CreateUpdateInformerEventSourceEventFilterIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/CreateUpdateInformerEventSourceEventFilterIT.java index 0bd4767776..8a68c9a2b3 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/CreateUpdateInformerEventSourceEventFilterIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/CreateUpdateInformerEventSourceEventFilterIT.java @@ -31,7 +31,7 @@ void updateEventNotReceivedAfterCreateOrUpdate() { var createdResource = operator.create(resource); - assertData(operator, createdResource, 1); + assertData(operator, createdResource, 1, 1); CreateUpdateEventFilterTestCustomResource actualCreatedResource = operator.get(CreateUpdateEventFilterTestCustomResource.class, @@ -39,11 +39,11 @@ void updateEventNotReceivedAfterCreateOrUpdate() { actualCreatedResource.getSpec().setValue("2"); operator.replace(actualCreatedResource); - assertData(operator, actualCreatedResource, 2); + assertData(operator, actualCreatedResource, 2, 2); } static void assertData(LocallyRunOperatorExtension operator, - CreateUpdateEventFilterTestCustomResource resource, int executions) { + CreateUpdateEventFilterTestCustomResource resource, int minExecutions, int maxExecutions) { await() .atMost(Duration.ofSeconds(1)) .until(() -> { @@ -56,10 +56,10 @@ static void assertData(LocallyRunOperatorExtension operator, .equals(resource.getSpec().getValue()); }); - assertThat( - ((CreateUpdateEventFilterTestReconciler) operator.getFirstReconciler()) - .getNumberOfExecutions()) - .isEqualTo(executions); + int numberOfExecutions = ((CreateUpdateEventFilterTestReconciler) operator.getFirstReconciler()) + .getNumberOfExecutions(); + assertThat(numberOfExecutions).isGreaterThanOrEqualTo(minExecutions); + assertThat(numberOfExecutions).isLessThanOrEqualTo(maxExecutions); } static CreateUpdateEventFilterTestCustomResource prepareTestResource() { diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/PreviousAnnotationDisabledIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/PreviousAnnotationDisabledIT.java index 31b84ce1ad..c636737d0a 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/PreviousAnnotationDisabledIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/PreviousAnnotationDisabledIT.java @@ -24,7 +24,7 @@ void updateEventReceivedAfterCreateOrUpdate() { var createdResource = operator.create(resource); - CreateUpdateInformerEventSourceEventFilterIT.assertData(operator, createdResource, 2); + CreateUpdateInformerEventSourceEventFilterIT.assertData(operator, createdResource, 1, 2); CreateUpdateEventFilterTestCustomResource actualCreatedResource = operator.get(CreateUpdateEventFilterTestCustomResource.class, @@ -32,7 +32,7 @@ void updateEventReceivedAfterCreateOrUpdate() { actualCreatedResource.getSpec().setValue("2"); operator.replace(actualCreatedResource); - CreateUpdateInformerEventSourceEventFilterIT.assertData(operator, actualCreatedResource, 4); + CreateUpdateInformerEventSourceEventFilterIT.assertData(operator, actualCreatedResource, 2, 4); } }