mostly removing the parse resource version term

and more of the previous annotation handling

Author: shawkins

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java

@@ -450,45 +450,15 @@ default Set<Class<? extends HasMetadata>> defaultNonSSAResource() {
   }
 
   /**
-   * For dependent resources, the framework can add an annotation to filter out events resulting
-   * directly from the framework's operation. There are, however, some resources that do not follow
-   * the Kubernetes API conventions that changes in metadata should not increase the generation of
-   * the resource (as recorded in the {@code generation} field of the resource's {@code metadata}).
-   * For these resources, this convention is not respected and results in a new event for the
-   * framework to process. If that particular case is not handled correctly in the resource matcher,
-   * the framework will consider that the resource doesn't match the desired state and therefore
-   * triggers an update, which in turn, will re-add the annotation, thus starting the loop again,
-   * infinitely.
-   *
-   * <p>As a workaround, we automatically skip adding previous annotation for those well-known
-   * resources. Note that if you are sure that the matcher works for your use case, and it should in
-   * most instances, you can remove the resource type from the blocklist.
-   *
-   * <p>The consequence of adding a resource type to the set is that the framework will not use
-   * event filtering to prevent events, initiated by changes made by the framework itself as a
-   * result of its processing of dependent resources, to trigger the associated reconciler again.
-   *
-   * <p>Note that this method only takes effect if annotating dependent resources to prevent
-   * dependent resources events from triggering the associated reconciler again is activated as
-   * controlled by {@link #previousAnnotationForDependentResourcesEventFiltering()}
-   *
-   * @return a Set of resource classes where the previous version annotation won't be used.
-   */
-  default Set<Class<? extends HasMetadata>> withPreviousAnnotationForDependentResourcesBlocklist() {
-    return Set.of(Deployment.class, StatefulSet.class);
-  }
-
-  /**
-   * If the event logic should parse the resourceVersion to determine the ordering of dependent
-   * resource events.
+   * If the event logic can compare resourceVersions.
    *
    * <p>Enabled by default as Kubernetes does support this interpretation of resourceVersions.
-   * Disable only if your api server provides non comparable resource versions..
+   * Disable only if your api server provides non comparable resource versions.
    *
-   * @return if resource version should be parsed (as integer)
-   * @since 4.5.0
+   * @return if resource versions are comparable
+   * @since 5.3.0
    */
-  default boolean parseResourceVersionsForEventFilteringAndCaching() {
+  default boolean comparableResourceVersions() {
     return DEFAULT_COMPARABLE_RESOURCE_VERSIONS;
   }
 

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java

@@ -51,11 +51,9 @@ public class ConfigurationServiceOverrider {
   private Duration reconciliationTerminationTimeout;
   private Boolean ssaBasedCreateUpdateMatchForDependentResources;
   private Set<Class<? extends HasMetadata>> defaultNonSSAResource;
-  private Boolean previousAnnotationForDependentResources;
-  private Boolean parseResourceVersions;
+  private Boolean comparableResourceVersions;
   private Boolean useSSAToPatchPrimaryResource;
   private Boolean cloneSecondaryResourcesWhenGettingFromCache;
-  private Set<Class<? extends HasMetadata>> previousAnnotationUsageBlocklist;
 
   @SuppressWarnings("rawtypes")
   private DependentResourceFactory dependentResourceFactory;
@@ -167,29 +165,24 @@ public ConfigurationServiceOverrider withDefaultNonSSAResource(
     this.defaultNonSSAResource = defaultNonSSAResource;
     return this;
   }
-
-  public ConfigurationServiceOverrider withPreviousAnnotationForDependentResources(boolean value) {
-    this.previousAnnotationForDependentResources = value;
-    return this;
-  }
-
+  
   /**
    * @param value true if internal algorithms can use metadata.resourceVersion as a numeric value.
    * @return this
    */
-  public ConfigurationServiceOverrider withParseResourceVersions(boolean value) {
-    this.parseResourceVersions = value;
+  public ConfigurationServiceOverrider withComparableResourceVersions(boolean value) {
+    this.comparableResourceVersions = value;
     return this;
   }
 
   /**
-   * @deprecated use withParseResourceVersions
+   * @deprecated use withComparableResourceVersions
    * @param value true if internal algorithms can use metadata.resourceVersion as a numeric value.
    * @return this
    */
   @Deprecated(forRemoval = true)
-  public ConfigurationServiceOverrider wihtParseResourceVersions(boolean value) {
-    this.parseResourceVersions = value;
+  public ConfigurationServiceOverrider withParseResourceVersions(boolean value) {
+    this.comparableResourceVersions = value;
     return this;
   }
 
@@ -204,12 +197,6 @@ public ConfigurationServiceOverrider withCloneSecondaryResourcesWhenGettingFromC
     return this;
   }
 
-  public ConfigurationServiceOverrider withPreviousAnnotationForDependentResourcesBlocklist(
-      Set<Class<? extends HasMetadata>> blocklist) {
-    this.previousAnnotationUsageBlocklist = blocklist;
-    return this;
-  }
-
   public ConfigurationService build() {
     return new BaseConfigurationService(original.getVersion(), cloner, client) {
       @Override
@@ -331,13 +318,6 @@ public Set<Class<? extends HasMetadata>> defaultNonSSAResources() {
             defaultNonSSAResource, ConfigurationService::defaultNonSSAResources);
       }
 
-      @Override
-      public boolean parseResourceVersionsForEventFilteringAndCaching() {
-        return overriddenValueOrDefault(
-            parseResourceVersions,
-            ConfigurationService::parseResourceVersionsForEventFilteringAndCaching);
-      }
-
       @Override
       public boolean useSSAToPatchPrimaryResource() {
         return overriddenValueOrDefault(
@@ -350,13 +330,12 @@ public boolean cloneSecondaryResourcesWhenGettingFromCache() {
             cloneSecondaryResourcesWhenGettingFromCache,
             ConfigurationService::cloneSecondaryResourcesWhenGettingFromCache);
       }
-
+      
       @Override
-      public Set<Class<? extends HasMetadata>>
-          withPreviousAnnotationForDependentResourcesBlocklist() {
+      public boolean comparableResourceVersions() {
         return overriddenValueOrDefault(
-            previousAnnotationUsageBlocklist,
-            ConfigurationService::withPreviousAnnotationForDependentResourcesBlocklist);
+            comparableResourceVersions,
+            ConfigurationService::comparableResourceVersions);
       }
     };
   }

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerEventSource.java

@@ -54,7 +54,7 @@ public ControllerEventSource(Controller<T> controller) {
         controller
             .getConfiguration()
             .getConfigurationService()
-            .parseResourceVersionsForEventFilteringAndCaching());
+            .comparableResourceVersions());
     this.controller = controller;
 
     final var config = controller.getConfiguration();

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java

@@ -97,15 +97,15 @@ public InformerEventSource(
   private InformerEventSource(
       InformerEventSourceConfiguration<R> configuration,
       KubernetesClient client,
-      boolean parseResourceVersions) {
+      boolean comparableResourceVersions) {
     super(
         configuration.name(),
         configuration
             .getGroupVersionKind()
             .map(gvk -> client.genericKubernetesResources(gvk.apiVersion(), gvk.getKind()))
             .orElseGet(() -> (MixedOperation) client.resources(configuration.getResourceClass())),
         configuration,
-        parseResourceVersions);
+        comparableResourceVersions);
     // If there is a primary to secondary mapper there is no need for primary to secondary index.
     primaryToSecondaryMapper = configuration.getPrimaryToSecondaryMapper();
     if (useSecondaryToPrimaryIndex()) {

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java

@@ -56,17 +56,17 @@ public abstract class ManagedInformerEventSource<
 
   private static final Logger log = LoggerFactory.getLogger(ManagedInformerEventSource.class);
   private InformerManager<R, C> cache;
-  private final boolean parseResourceVersions;
+  private final boolean comparableResourceVersions;
   private ControllerConfiguration<R> controllerConfiguration;
   private final C configuration;
   private final Map<String, Function<R, List<String>>> indexers = new HashMap<>();
   protected TemporaryResourceCache<R> temporaryResourceCache;
   protected MixedOperation client;
 
   protected ManagedInformerEventSource(
-      String name, MixedOperation client, C configuration, boolean parseResourceVersions) {
+      String name, MixedOperation client, C configuration, boolean comparableResourceVersions) {
     super(configuration.getResourceClass(), name);
-    this.parseResourceVersions = parseResourceVersions;
+    this.comparableResourceVersions = comparableResourceVersions;
     this.client = client;
     this.configuration = configuration;
   }
@@ -103,7 +103,7 @@ public synchronized void start() {
     if (isRunning()) {
       return;
     }
-    temporaryResourceCache = new TemporaryResourceCache<>(parseResourceVersions);
+    temporaryResourceCache = new TemporaryResourceCache<>(comparableResourceVersions);
     this.cache = new InformerManager<>(client, configuration, this);
     cache.setControllerConfiguration(controllerConfiguration);
     cache.addIndexers(indexers);
@@ -135,7 +135,7 @@ public void handleRecentResourceCreate(ResourceID resourceID, R resource) {
   public Optional<R> get(ResourceID resourceID) {
     var res = cache.get(resourceID);
     Optional<R> resource = temporaryResourceCache.getResourceFromCache(resourceID);
-    if (parseResourceVersions
+    if (comparableResourceVersions
         && resource.isPresent()
         && res.filter(
                 r ->

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryResourceCache.java

@@ -51,16 +51,16 @@ public class TemporaryResourceCache<T extends HasMetadata> {
   private static final Logger log = LoggerFactory.getLogger(TemporaryResourceCache.class);
 
   private final Map<ResourceID, T> cache = new ConcurrentHashMap<>();
-  private final boolean parseResourceVersions;
+  private final boolean comparableResourceVersions;
   private final Map<ResourceID, ReentrantLock> activelyModifying = new ConcurrentHashMap<>();
   private String latestResourceVersion;
 
-  public TemporaryResourceCache(boolean parseResourceVersions) {
-    this.parseResourceVersions = parseResourceVersions;
+  public TemporaryResourceCache(boolean comparableResourceVersions) {
+    this.comparableResourceVersions = comparableResourceVersions;
   }
 
   public void startModifying(ResourceID id) {
-    if (!parseResourceVersions) {
+    if (!comparableResourceVersions) {
       return;
     }
     activelyModifying
@@ -77,7 +77,7 @@ public void startModifying(ResourceID id) {
   }
 
   public void doneModifying(ResourceID id) {
-    if (!parseResourceVersions) {
+    if (!comparableResourceVersions) {
       return;
     }
     activelyModifying.computeIfPresent(
@@ -134,7 +134,7 @@ private boolean onEvent(T resource, boolean unknownState) {
 
   /** put the item into the cache if it's for a later state than what has already been observed. */
   public synchronized void putResource(T newResource) {
-    if (!parseResourceVersions) {
+    if (!comparableResourceVersions) {
       return;
     }
 

operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryPrimaryResourceCacheTest.java

@@ -108,7 +108,7 @@ void removesResourceFromCache() {
   }
 
   @Test
-  void resourceNoVersionParsing() {
+  void nonComparableResourceVersionsDisables() {
     this.temporaryResourceCache = new TemporaryResourceCache<>(false);
 
     this.temporaryResourceCache.putResource(testResource());

…filter/PreviousAnnotationDisabledIT.java …omparableResourceVersionsDisabledIT.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/createupdateeventfilter/PreviousAnnotationDisabledIT.java renamed to operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/createupdateeventfilter/ComparableResourceVersionsDisabledIT.java operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/createupdateeventfilter/PreviousAnnotationDisabledIT.java renamed to operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/createupdateeventfilter/ComparableResourceVersionsDisabledIT.java

@@ -15,19 +15,17 @@
  */
 package io.javaoperatorsdk.operator.baseapi.createupdateeventfilter;
 
+import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
 
-import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension;
-
-class PreviousAnnotationDisabledIT {
+class ComparableResourceVersionsDisabledIT {
 
   @RegisterExtension
   LocallyRunOperatorExtension operator =
       LocallyRunOperatorExtension.builder()
           .withReconciler(new CreateUpdateEventFilterTestReconciler())
-          .withConfigurationService(
-              overrider -> overrider.withPreviousAnnotationForDependentResources(false))
+          .withConfigurationService(overrider -> overrider.withComparableResourceVersions(false))
           .build();
 
   @Test