inverted the logic to the informereventsource

moving where the annotation is handled for matching

Author: shawkins

Diff for: operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java

@@ -1,6 +1,7 @@
 package io.javaoperatorsdk.operator.processing.dependent.kubernetes;
 
-import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
 
@@ -37,8 +38,6 @@ public abstract class KubernetesDependentResource<R extends HasMetadata, P exten
     implements KubernetesClientAware,
     DependentResourceConfigurator<KubernetesDependentResourceConfig<R>> {
 
-  public static String PREVIOUS_ANNOTATION_KEY = "javaoperatorsdk.io/previous";
-
   private static final Logger log = LoggerFactory.getLogger(KubernetesDependentResource.class);
 
   protected KubernetesClient client;
@@ -147,17 +146,16 @@ public R update(R actual, R target, P primary, Context<P> context) {
     return updatedResource;
   }
 
-  void addPreviousAnnotation(String resourceVersion, HasMetadata target) {
-    String id =
-        ((InformerEventSource<HasMetadata, HasMetadata>) eventSource().orElseThrow()).getId();
-    target.getMetadata().getAnnotations().put(PREVIOUS_ANNOTATION_KEY,
-        id + Optional.ofNullable(resourceVersion).map(rv -> "," + rv).orElse(""));
+  private void addPreviousAnnotation(String resourceVersion, HasMetadata target) {
+    ((InformerEventSource<HasMetadata, HasMetadata>) eventSource().orElseThrow())
+        .addPreviousAnnotation(resourceVersion, target);
   }
 
   @Override
   public Result<R> match(R actualResource, P primary, Context<P> context) {
     final var desired = desired(primary, context);
     final boolean matches;
+    copySDKAnnotations(actualResource, desired);
     if (useSSA(context)) {
       addReferenceHandlingMetadata(desired, primary);
       matches = SSABasedGenericKubernetesResourceMatcher.getInstance()
@@ -171,6 +169,7 @@ public Result<R> match(R actualResource, P primary, Context<P> context) {
   @SuppressWarnings("unused")
   public Result<R> match(R actualResource, R desired, P primary, Context<P> context) {
     if (useSSA(context)) {
+      copySDKAnnotations(actualResource, desired);
       addReferenceHandlingMetadata(desired, primary);
       var matches = SSABasedGenericKubernetesResourceMatcher.getInstance()
           .matches(actualResource, desired, context);
@@ -182,6 +181,21 @@ public Result<R> match(R actualResource, R desired, P primary, Context<P> contex
     }
   }
 
+  private void copySDKAnnotations(R actualResource, final R desired) {
+    String actual = actualResource.getMetadata().getAnnotations()
+        .get(InformerEventSource.PREVIOUS_ANNOTATION_KEY);
+    Map<String, String> annotations = desired.getMetadata().getAnnotations();
+    if (actual != null) {
+      if (annotations == null) {
+        annotations = new LinkedHashMap<>();
+        desired.getMetadata().setAnnotations(annotations);
+      }
+      annotations.put(InformerEventSource.PREVIOUS_ANNOTATION_KEY, actual);
+    } else if (annotations != null) {
+      annotations.remove(InformerEventSource.PREVIOUS_ANNOTATION_KEY);
+    }
+  }
+
   private boolean useSSA(Context<P> context) {
     return context.getControllerConfiguration().getConfigurationService()
         .ssaBasedCreateUpdateMatchForDependentResources();
@@ -255,7 +269,7 @@ private boolean useDefaultAnnotationsToIdentifyPrimary() {
   private void addDefaultSecondaryToPrimaryMapperAnnotations(R desired, P primary) {
     var annotations = desired.getMetadata().getAnnotations();
     if (annotations == null) {
-      annotations = new HashMap<>();
+      annotations = new LinkedHashMap<>();
       desired.getMetadata().setAnnotations(annotations);
     }
     annotations.put(Mappers.DEFAULT_ANNOTATION_FOR_NAME, primary.getMetadata().getName());

Diff for: operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/SSABasedGenericKubernetesResourceMatcher.java

@@ -40,7 +40,6 @@
 // see also: https://kubernetes.slack.com/archives/C0123CNN8F3/p1686141087220719
 public class SSABasedGenericKubernetesResourceMatcher<R extends HasMetadata> {
 
-  private static final String ANNOTATIONS_KEY = "annotations";
   @SuppressWarnings("rawtypes")
   private static final SSABasedGenericKubernetesResourceMatcher INSTANCE =
       new SSABasedGenericKubernetesResourceMatcher<>();
@@ -91,8 +90,6 @@ public boolean matches(R actual, R desired, Context<?> context) {
     keepOnlyManagedFields(prunedActual, actualMap,
         managedFieldsEntry.getFieldsV1().getAdditionalProperties(), objectMapper);
 
-    removeSDKAnnotations(prunedActual);
-
     removeIrrelevantValues(desiredMap);
 
     if (LoggingUtils.isNotSensitiveResource(desired)) {
@@ -102,20 +99,6 @@ public boolean matches(R actual, R desired, Context<?> context) {
     return prunedActual.equals(desiredMap);
   }
 
-  @SuppressWarnings("unchecked")
-  private static void removeSDKAnnotations(HashMap<String, Object> prunedActual) {
-    Optional.ofNullable(((Map<String, Object>) prunedActual.get(METADATA_KEY)))
-        .ifPresent(m -> m.computeIfPresent(ANNOTATIONS_KEY,
-            (k, v) -> {
-              var annotations = (Map<String, Object>) v;
-              annotations.remove(KubernetesDependentResource.PREVIOUS_ANNOTATION_KEY);
-              if (annotations.isEmpty()) {
-                return null;
-              }
-              return annotations;
-            }));
-  }
-
   @SuppressWarnings("unchecked")
   private static void removeIrrelevantValues(Map<String, Object> desiredMap) {
     var metadata = (Map<String, Object>) desiredMap.get(METADATA_KEY);

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

@@ -19,7 +19,6 @@
 import io.javaoperatorsdk.operator.api.config.ConfigurationService;
 import io.javaoperatorsdk.operator.api.config.informer.InformerConfiguration;
 import io.javaoperatorsdk.operator.api.reconciler.EventSourceContext;
-import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource;
 import io.javaoperatorsdk.operator.processing.event.Event;
 import io.javaoperatorsdk.operator.processing.event.EventHandler;
 import io.javaoperatorsdk.operator.processing.event.ResourceID;
@@ -75,14 +74,16 @@ public class InformerEventSource<R extends HasMetadata, P extends HasMetadata>
     extends ManagedInformerEventSource<R, P, InformerConfiguration<R>>
     implements ResourceEventHandler<R> {
 
+  public static String PREVIOUS_ANNOTATION_KEY = "javaoperatorsdk.io/previous";
+
   private static final Logger log = LoggerFactory.getLogger(InformerEventSource.class);
 
   private final InformerConfiguration<R> configuration;
   // we need direct control for the indexer to propagate the just update resource also to the index
   private final PrimaryToSecondaryIndex<R> primaryToSecondaryIndex;
   private final PrimaryToSecondaryMapper<P> primaryToSecondaryMapper;
   private Map<String, Function<R, List<String>>> indexerBuffer = new HashMap<>();
-  private String id = UUID.randomUUID().toString();
+  private final String id = UUID.randomUUID().toString();
 
   public InformerEventSource(
       InformerConfiguration<R> configuration, EventSourceContext<P> context) {
@@ -110,10 +111,6 @@ public InformerEventSource(InformerConfiguration<R> configuration, KubernetesCli
     genericFilter = configuration.genericFilter().orElse(null);
   }
 
-  public String getId() {
-    return id;
-  }
-
   @Override
   public void onAdd(R newResource) {
     if (log.isDebugEnabled()) {
@@ -192,8 +189,7 @@ private boolean canSkipEvent(R newObject, R oldObject, ResourceID resourceID) {
   }
 
   private boolean isEventKnownFromAnnotation(R newObject, R oldObject) {
-    String previous = newObject.getMetadata().getAnnotations()
-        .get(KubernetesDependentResource.PREVIOUS_ANNOTATION_KEY);
+    String previous = newObject.getMetadata().getAnnotations().get(PREVIOUS_ANNOTATION_KEY);
     boolean known = false;
     if (previous != null) {
       String[] parts = previous.split(",");
@@ -320,4 +316,16 @@ public void addIndexers(Map<String, Function<R, List<String>>> indexers) {
     indexerBuffer.putAll(indexers);
   }
 
+  /**
+   * Add an annotation to the resource so that the subsequent will be omitted
+   *
+   * @param resourceVersion null if there is no prior version
+   * @param target mutable resource that will be returned
+   */
+  public R addPreviousAnnotation(String resourceVersion, R target) {
+    target.getMetadata().getAnnotations().put(PREVIOUS_ANNOTATION_KEY,
+        id + Optional.ofNullable(resourceVersion).map(rv -> "," + rv).orElse(""));
+    return target;
+  }
+
 }

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

@@ -16,7 +16,6 @@
 import io.javaoperatorsdk.operator.api.config.ConfigurationService;
 import io.javaoperatorsdk.operator.api.config.InformerStoppedHandler;
 import io.javaoperatorsdk.operator.api.config.informer.InformerConfiguration;
-import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource;
 import io.javaoperatorsdk.operator.processing.event.EventHandler;
 import io.javaoperatorsdk.operator.processing.event.ResourceID;
 import io.javaoperatorsdk.operator.processing.event.source.SecondaryToPrimaryMapper;
@@ -38,7 +37,6 @@ class InformerEventSourceTest {
 
   private static final String PREV_RESOURCE_VERSION = "0";
   private static final String DEFAULT_RESOURCE_VERSION = "1";
-  private static final String NEXT_RESOURCE_VERSION = "2";
 
   private InformerEventSource<Deployment, TestCustomResource> informerEventSource;
   private final KubernetesClient clientMock = MockKubernetesClient.client(Deployment.class);
@@ -81,21 +79,15 @@ void skipsEventPropagationIfResourceWithSameVersionInResourceCache() {
 
   @Test
   void skipsAddEventPropagationViaAnnotation() {
-    informerEventSource.onAdd(new DeploymentBuilder(testDeployment()).editMetadata()
-        .addToAnnotations(KubernetesDependentResource.PREVIOUS_ANNOTATION_KEY,
-            informerEventSource.getId())
-        .endMetadata().build());
+    informerEventSource.onAdd(informerEventSource.addPreviousAnnotation(null, testDeployment()));
 
     verify(eventHandlerMock, never()).handleEvent(any());
   }
 
   @Test
   void skipsUpdateEventPropagationViaAnnotation() {
     informerEventSource.onUpdate(testDeployment(),
-        new DeploymentBuilder(testDeployment()).editMetadata()
-            .addToAnnotations(KubernetesDependentResource.PREVIOUS_ANNOTATION_KEY,
-                informerEventSource.getId() + ",1")
-            .endMetadata().build());
+        informerEventSource.addPreviousAnnotation("1", testDeployment()));
 
     verify(eventHandlerMock, never()).handleEvent(any());
   }
@@ -110,7 +102,7 @@ void processEventPropagationWithoutAnnotation() {
   @Test
   void processEventPropagationWithIncorrectAnnotation() {
     informerEventSource.onAdd(new DeploymentBuilder(testDeployment()).editMetadata()
-        .addToAnnotations(KubernetesDependentResource.PREVIOUS_ANNOTATION_KEY, "invalid")
+        .addToAnnotations(InformerEventSource.PREVIOUS_ANNOTATION_KEY, "invalid")
         .endMetadata().build());
 
     verify(eventHandlerMock, times(1)).handleEvent(any());