wip

Signed-off-by: Attila Mészáros <a_meszaros@apple.com>

Author: csviri

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

@@ -28,8 +28,6 @@
 import io.fabric8.kubernetes.api.model.ConfigMap;
 import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.fabric8.kubernetes.api.model.Secret;
-import io.fabric8.kubernetes.api.model.apps.Deployment;
-import io.fabric8.kubernetes.api.model.apps.StatefulSet;
 import io.fabric8.kubernetes.client.Config;
 import io.fabric8.kubernetes.client.ConfigBuilder;
 import io.fabric8.kubernetes.client.CustomResource;

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

@@ -184,43 +184,43 @@ public Set<String> getKnownReconcilerNames() {
       }
 
       private <T> T overriddenValueOrDefault(
-              T value, Function<ConfigurationService, T> defaultValue) {
+          T value, Function<ConfigurationService, T> defaultValue) {
         return value != null ? value : defaultValue.apply(original);
       }
 
       @Override
       public boolean checkCRDAndValidateLocalModel() {
         return overriddenValueOrDefault(
-                checkCR, ConfigurationService::checkCRDAndValidateLocalModel);
+            checkCR, ConfigurationService::checkCRDAndValidateLocalModel);
       }
 
       @SuppressWarnings("rawtypes")
       @Override
       public DependentResourceFactory dependentResourceFactory() {
         return overriddenValueOrDefault(
-                dependentResourceFactory, ConfigurationService::dependentResourceFactory);
+            dependentResourceFactory, ConfigurationService::dependentResourceFactory);
       }
 
       @Override
       public int concurrentReconciliationThreads() {
         return Utils.ensureValid(
-                overriddenValueOrDefault(
-                        concurrentReconciliationThreads,
-                        ConfigurationService::concurrentReconciliationThreads),
-                "maximum reconciliation threads",
-                1,
-                original.concurrentReconciliationThreads());
+            overriddenValueOrDefault(
+                concurrentReconciliationThreads,
+                ConfigurationService::concurrentReconciliationThreads),
+            "maximum reconciliation threads",
+            1,
+            original.concurrentReconciliationThreads());
       }
 
       @Override
       public int concurrentWorkflowExecutorThreads() {
         return Utils.ensureValid(
-                overriddenValueOrDefault(
-                        concurrentWorkflowExecutorThreads,
-                        ConfigurationService::concurrentWorkflowExecutorThreads),
-                "maximum workflow execution threads",
-                1,
-                original.concurrentWorkflowExecutorThreads());
+            overriddenValueOrDefault(
+                concurrentWorkflowExecutorThreads,
+                ConfigurationService::concurrentWorkflowExecutorThreads),
+            "maximum workflow execution threads",
+            1,
+            original.concurrentWorkflowExecutorThreads());
       }
 
       @Override
@@ -254,22 +254,22 @@ public ExecutorService getWorkflowExecutorService() {
       @Override
       public Optional<LeaderElectionConfiguration> getLeaderElectionConfiguration() {
         return leaderElectionConfiguration != null
-                ? Optional.of(leaderElectionConfiguration)
-                : original.getLeaderElectionConfiguration();
+            ? Optional.of(leaderElectionConfiguration)
+            : original.getLeaderElectionConfiguration();
       }
 
       @Override
       public Optional<InformerStoppedHandler> getInformerStoppedHandler() {
         return informerStoppedHandler != null
-                ? Optional.of(informerStoppedHandler)
-                : original.getInformerStoppedHandler();
+            ? Optional.of(informerStoppedHandler)
+            : original.getInformerStoppedHandler();
       }
 
       @Override
       public boolean stopOnInformerErrorDuringStartup() {
         return overriddenValueOrDefault(
-                stopOnInformerErrorDuringStartup,
-                ConfigurationService::stopOnInformerErrorDuringStartup);
+            stopOnInformerErrorDuringStartup,
+            ConfigurationService::stopOnInformerErrorDuringStartup);
       }
 
       @Override
@@ -280,34 +280,35 @@ public Duration cacheSyncTimeout() {
       @Override
       public Duration reconciliationTerminationTimeout() {
         return overriddenValueOrDefault(
-                reconciliationTerminationTimeout,
-                ConfigurationService::reconciliationTerminationTimeout);
+            reconciliationTerminationTimeout,
+            ConfigurationService::reconciliationTerminationTimeout);
       }
 
       @Override
       public boolean ssaBasedCreateUpdateMatchForDependentResources() {
         return overriddenValueOrDefault(
-                ssaBasedCreateUpdateMatchForDependentResources,
-                ConfigurationService::ssaBasedCreateUpdateMatchForDependentResources);
+            ssaBasedCreateUpdateMatchForDependentResources,
+            ConfigurationService::ssaBasedCreateUpdateMatchForDependentResources);
       }
 
       @Override
       public Set<Class<? extends HasMetadata>> defaultNonSSAResources() {
         return overriddenValueOrDefault(
-                defaultNonSSAResource, ConfigurationService::defaultNonSSAResources);
+            defaultNonSSAResource, ConfigurationService::defaultNonSSAResources);
       }
 
       @Override
       public boolean useSSAToPatchPrimaryResource() {
         return overriddenValueOrDefault(
-                useSSAToPatchPrimaryResource, ConfigurationService::useSSAToPatchPrimaryResource);
+            useSSAToPatchPrimaryResource, ConfigurationService::useSSAToPatchPrimaryResource);
       }
 
       @Override
       public boolean cloneSecondaryResourcesWhenGettingFromCache() {
         return overriddenValueOrDefault(
-                cloneSecondaryResourcesWhenGettingFromCache,
-                ConfigurationService::cloneSecondaryResourcesWhenGettingFromCache);
+            cloneSecondaryResourcesWhenGettingFromCache,
+            ConfigurationService::cloneSecondaryResourcesWhenGettingFromCache);
       }
     };
-}}
+  }
+}

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

@@ -47,7 +47,7 @@ public class ControllerEventSource<T extends HasMetadata>
 
   @SuppressWarnings({"unchecked", "rawtypes"})
   public ControllerEventSource(Controller<T> controller) {
-    super(NAME, controller.getCRClient(), controller.getConfiguration(), false);
+    super(NAME, controller.getCRClient(), controller.getConfiguration());
     this.controller = controller;
 
     final var config = controller.getConfiguration();

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

@@ -82,15 +82,12 @@ public class InformerEventSource<R extends HasMetadata, P extends HasMetadata>
 
   public InformerEventSource(
       InformerEventSourceConfiguration<R> configuration, EventSourceContext<P> context) {
-    this(
-        configuration,
-        configuration.getKubernetesClient().orElse(context.getClient()));
+    this(configuration, configuration.getKubernetesClient().orElse(context.getClient()));
   }
 
+  // visible for testing
   @SuppressWarnings({"unchecked", "rawtypes"})
-  private InformerEventSource(
-      InformerEventSourceConfiguration<R> configuration,
-      KubernetesClient client) {
+  InformerEventSource(InformerEventSourceConfiguration<R> configuration, KubernetesClient client) {
     super(
         configuration.name(),
         configuration
@@ -199,7 +196,7 @@ private synchronized void onAddOrUpdate(
   private boolean canSkipEvent(R newObject, R oldObject, ResourceID resourceID) {
     var res = temporaryResourceCache.getResourceFromCache(resourceID);
     if (res.isEmpty()) {
-      return isEventKnownFromAnnotation(newObject, oldObject);
+      return false;
     }
     boolean resVersionsEqual =
         newObject
@@ -214,24 +211,6 @@ private boolean canSkipEvent(R newObject, R oldObject, ResourceID resourceID) {
         || temporaryResourceCache.isLaterResourceVersion(resourceID, res.get(), newObject);
   }
 
-  private boolean isEventKnownFromAnnotation(R newObject, R oldObject) {
-    String previous = newObject.getMetadata().getAnnotations().get(PREVIOUS_ANNOTATION_KEY);
-    boolean known = false;
-    if (previous != null) {
-      String[] parts = previous.split(",");
-      if (id.equals(parts[0])) {
-        if (oldObject == null && parts.length == 1) {
-          known = true;
-        } else if (oldObject != null
-            && parts.length == 2
-            && oldObject.getMetadata().getResourceVersion().equals(parts[1])) {
-          known = true;
-        }
-      }
-    }
-    return known;
-  }
-
   private void propagateEvent(R object) {
     var primaryResourceIdSet =
         configuration().getSecondaryToPrimaryMapper().toPrimaryResourceIDs(object);

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

@@ -61,8 +61,7 @@ public abstract class ManagedInformerEventSource<
   protected TemporaryResourceCache<R> temporaryResourceCache;
   protected MixedOperation client;
 
-  protected ManagedInformerEventSource(
-      String name, MixedOperation client, C configuration) {
+  protected ManagedInformerEventSource(String name, MixedOperation client, C configuration) {
     super(configuration.getResourceClass(), name);
     this.client = client;
     this.configuration = configuration;

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

@@ -24,7 +24,6 @@
 import org.slf4j.LoggerFactory;
 
 import io.fabric8.kubernetes.api.model.HasMetadata;
-import io.javaoperatorsdk.operator.api.config.ConfigurationService;
 import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource;
 import io.javaoperatorsdk.operator.processing.event.ResourceID;
 
@@ -95,8 +94,7 @@ void clean() {
   private final ExpirationCache<String> tombstones = new ExpirationCache<>(1000000, 1200000);
   private final ManagedInformerEventSource<T, ?, ?> managedInformerEventSource;
 
-  public TemporaryResourceCache(
-      ManagedInformerEventSource<T, ?, ?> managedInformerEventSource) {
+  public TemporaryResourceCache(ManagedInformerEventSource<T, ?, ?> managedInformerEventSource) {
     this.managedInformerEventSource = managedInformerEventSource;
   }
 
@@ -161,10 +159,10 @@ public synchronized void putResource(T newResource, String previousResourceVersi
     }
   }
 
-  private boolean isLaterResourceVersion(ResourceID resourceId, T newResource, T cachedResource) {
+  public boolean isLaterResourceVersion(ResourceID resourceId, T newResource, T cachedResource) {
     try {
-       return Long.parseLong(newResource.getMetadata().getResourceVersion())
-              > Long.parseLong(cachedResource.getMetadata().getResourceVersion());
+      return Long.parseLong(newResource.getMetadata().getResourceVersion())
+          > Long.parseLong(cachedResource.getMetadata().getResourceVersion());
     } catch (NumberFormatException e) {
       log.warn(
           "Could not compare resourceVersions {} and {} for {}",

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

@@ -104,21 +104,6 @@ void skipsEventPropagationIfResourceWithSameVersionInResourceCache() {
     verify(eventHandlerMock, never()).handleEvent(any());
   }
 
-  @Test
-  void skipsAddEventPropagationViaAnnotation() {
-    informerEventSource.onAdd(informerEventSource.addPreviousAnnotation(null, testDeployment()));
-
-    verify(eventHandlerMock, never()).handleEvent(any());
-  }
-
-  @Test
-  void skipsUpdateEventPropagationViaAnnotation() {
-    informerEventSource.onUpdate(
-        testDeployment(), informerEventSource.addPreviousAnnotation("1", testDeployment()));
-
-    verify(eventHandlerMock, never()).handleEvent(any());
-  }
-
   @Test
   void processEventPropagationWithoutAnnotation() {
     informerEventSource.onUpdate(testDeployment(), testDeployment());

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

@@ -46,7 +46,7 @@ class TemporaryPrimaryResourceCacheTest {
   @BeforeEach
   void setup() {
     informerEventSource = mock(InformerEventSource.class);
-    temporaryResourceCache = new TemporaryResourceCache<>(informerEventSource, false);
+    temporaryResourceCache = new TemporaryResourceCache<>(informerEventSource);
   }
 
   @Test
@@ -109,7 +109,7 @@ void removesResourceFromCache() {
 
   @Test
   void resourceVersionParsing() {
-    this.temporaryResourceCache = new TemporaryResourceCache<>(informerEventSource, true);
+    this.temporaryResourceCache = new TemporaryResourceCache<>(informerEventSource);
 
     ConfigMap testResource = propagateTestResourceToCache();