fix: race condition on get when add received after informer cache read - alt 1

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

@@ -191,20 +191,36 @@ public void handleRecentResourceCreate(ResourceID resourceID, R resource) {
 
   @Override
   public Optional<R> get(ResourceID resourceID) {
+    // The order of reading from these caches matters
     Optional<R> resource = temporaryResourceCache.getResourceFromCache(resourceID);
     var res = cache.get(resourceID);
-    if (comparableResourceVersions
-        && resource.isPresent()
-        && ReconcilerUtilsInternal.compareResourceVersions(
-                resource.get().getMetadata().getResourceVersion(),
-                manager().lastSyncResourceVersion(resource.get().getMetadata().getNamespace()))
-            > 0) {
-      log.debug("Latest resource found in temporary cache for Resource ID: {}", resourceID);
-      return resource;
+    if (comparableResourceVersions && resource.isPresent()) {
+      var comp =
+          ReconcilerUtilsInternal.compareResourceVersions(
+              resource.get().getMetadata().getResourceVersion(),
+              manager().lastSyncResourceVersion(resource.get().getMetadata().getNamespace()));
+      if (comp > 0) {
+        log.debug("Latest resource found in temporary cache for Resource ID: {}", resourceID);
+        return resource;
+      } else {
+        log.debug(
+            "Latest resource not found in temporary but was obsolete: {}. Re-reading the informer"
+                + " cache: {}",
+            resourceID,
+            res.isEmpty());
+        if (res.isEmpty()) {
+          // re-reading resource from informer cache since an add even might have received
+          // after the first read.
+          return cache.get(resourceID);
+        } else {
+          return res;
+        }
+      }
     }
     log.debug(
-        "Resource not found, or older, in temporary cache. Found in informer cache {}, for"
-            + " Resource ID: {}",
+        "Resource found in temp cache: {}, or older, in temporary cache. Found in informer cache"
+            + " {}, for Resource ID: {}",
+        resource.isPresent(),
         res.isPresent(),
         resourceID);
     return res;

operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/cachingfilteringupdate/CachingFilteringUpdateReconciler.java

@@ -43,27 +43,39 @@ public UpdateControl<CachingFilteringUpdateCustomResource> reconcile(
       CachingFilteringUpdateCustomResource resource,
       Context<CachingFilteringUpdateCustomResource> context) {
 
-    context.resourceOperations().serverSideApply(prepareCM(resource));
+    context.resourceOperations().serverSideApply(prepareCM(resource, 1));
     var cachedCM = context.getSecondaryResource(ConfigMap.class);
     if (cachedCM.isEmpty()) {
       issueFound.set(true);
       throw new IllegalStateException("Error for resource: " + ResourceID.fromResource(resource));
     }
 
+    var updated = context.resourceOperations().serverSideApply(prepareCM(resource, 2));
+    cachedCM = context.getSecondaryResource(ConfigMap.class);
+    if (!cachedCM
+        .orElseThrow()
+        .getMetadata()
+        .getResourceVersion()
+        .equals(updated.getMetadata().getResourceVersion())) {
+      issueFound.set(true);
+      throw new IllegalStateException(
+          "Update error for resource: " + ResourceID.fromResource(resource));
+    }
+
     ensureStatusExists(resource);
     resource.getStatus().setUpdated(true);
     return UpdateControl.patchStatus(resource);
   }
 
-  private static ConfigMap prepareCM(CachingFilteringUpdateCustomResource p) {
+  private static ConfigMap prepareCM(CachingFilteringUpdateCustomResource p, int num) {
     var cm =
         new ConfigMapBuilder()
             .withMetadata(
                 new ObjectMetaBuilder()
                     .withName(p.getMetadata().getName())
                     .withNamespace(p.getMetadata().getNamespace())
                     .build())
-            .withData(Map.of("name", p.getMetadata().getName()))
+            .withData(Map.of("name", p.getMetadata().getName(), "num", "" + num))
             .build();
     cm.addOwnerReference(p);
     return cm;