refactor: improve bulk dependent resource api (#1525)

* feat: decouple event source from cache + list discriminator (#1378)

* feat: bulk dependent resources (#1448)

* feat: optional eventsource on dependent resources (#1479)

* refactor: simplify handling of reused event sources (#1518)



Co-authored-by: Chris Laprun <metacosm@gmail.com>

* refactor: isolate index handling to BulkDependentResource interface (#1517)

* feat: key based bulk resource creation (#1521)

* improvement: bulk dependent resource api

* merge

Co-authored-by: Chris Laprun <metacosm@gmail.com>
Co-authored-by: Chris Laprun <metacosm@users.noreply.github.com>

Author: csviri

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/AbstractDependentResource.java

@@ -39,20 +39,22 @@ protected AbstractDependentResource() {
   @Override
   public ReconcileResult<R> reconcile(P primary, Context<P> context) {
     if (bulk) {
-      final var targetKeys = bulkDependentResource.targetKeys(primary, context);
+      final var targetResources = bulkDependentResource.desiredResources(primary, context);
+
       Map<String, R> actualResources =
           bulkDependentResource.getSecondaryResources(primary, context);
 
-      deleteBulkResourcesIfRequired(targetKeys, actualResources, primary, context);
-      final List<ReconcileResult<R>> results = new ArrayList<>(targetKeys.size());
+      deleteBulkResourcesIfRequired(targetResources.keySet(), actualResources, primary, context);
+      final List<ReconcileResult<R>> results = new ArrayList<>(targetResources.size());
+
+      targetResources.forEach((key, resource) -> {
+        results.add(reconcileIndexAware(primary, actualResources.get(key), resource, key, context));
+      });
 
-      for (String key : targetKeys) {
-        results.add(reconcileIndexAware(primary, actualResources.get(key), key, context));
-      }
       return ReconcileResult.aggregatedResult(results);
     } else {
       var actualResource = getSecondaryResource(primary, context);
-      return reconcileIndexAware(primary, actualResource.orElse(null), null, context);
+      return reconcileIndexAware(primary, actualResource.orElse(null), null, null, context);
     }
   }
 
@@ -66,12 +68,13 @@ protected void deleteBulkResourcesIfRequired(Set targetKeys, Map<String, R> actu
     });
   }
 
-  protected ReconcileResult<R> reconcileIndexAware(P primary, R resource, String key,
+  protected ReconcileResult<R> reconcileIndexAware(P primary, R actualResource, R desiredResource,
+      String key,
       Context<P> context) {
     if (creatable || updatable) {
-      if (resource == null) {
+      if (actualResource == null) {
         if (creatable) {
-          var desired = desiredIndexAware(primary, key, context);
+          var desired = bulkAwareDesired(primary, desiredResource, context);
           throwIfNull(desired, primary, "Desired");
           logForOperation("Creating", primary, desired);
           var createdResource = handleCreate(desired, primary, context);
@@ -81,32 +84,34 @@ protected ReconcileResult<R> reconcileIndexAware(P primary, R resource, String k
         if (updatable) {
           final Matcher.Result<R> match;
           if (bulk) {
-            match = bulkDependentResource.match(resource, primary, key, context);
+            match =
+                bulkDependentResource.match(actualResource, desiredResource, primary, key, context);
           } else {
-            match = updater.match(resource, primary, context);
+            match = updater.match(actualResource, primary, context);
           }
           if (!match.matched()) {
             final var desired =
-                match.computedDesired().orElse(desiredIndexAware(primary, key, context));
+                match.computedDesired().orElse(bulkAwareDesired(primary, desiredResource, context));
             throwIfNull(desired, primary, "Desired");
             logForOperation("Updating", primary, desired);
-            var updatedResource = handleUpdate(resource, desired, primary, context);
+            var updatedResource = handleUpdate(actualResource, desired, primary, context);
             return ReconcileResult.resourceUpdated(updatedResource);
           }
         } else {
-          log.debug("Update skipped for dependent {} as it matched the existing one", resource);
+          log.debug("Update skipped for dependent {} as it matched the existing one",
+              actualResource);
         }
       }
     } else {
       log.debug(
           "Dependent {} is read-only, implement Creator and/or Updater interfaces to modify it",
           getClass().getSimpleName());
     }
-    return ReconcileResult.noOperation(resource);
+    return ReconcileResult.noOperation(actualResource);
   }
 
-  private R desiredIndexAware(P primary, String key, Context<P> context) {
-    return bulk ? bulkDependentResource.desired(primary, key, context)
+  private R bulkAwareDesired(P primary, R alreadyComputedDesire, Context<P> context) {
+    return bulk ? alreadyComputedDesire
         : desired(primary, context);
   }
 

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/BulkDependentResource.java

@@ -1,7 +1,6 @@
 package io.javaoperatorsdk.operator.processing.dependent;
 
 import java.util.Map;
-import java.util.Set;
 
 import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.javaoperatorsdk.operator.api.reconciler.Context;
@@ -20,12 +19,10 @@ public interface BulkDependentResource<R, P extends HasMetadata>
   /**
    * @return number of resources to create
    */
-  Set<String> targetKeys(P primary, Context<P> context);
+  Map<String, R> desiredResources(P primary, Context<P> context);
 
   Map<String, R> getSecondaryResources(P primary, Context<P> context);
 
-  R desired(P primary, String key, Context<P> context);
-
   /**
    * Used to delete resource if the desired count is lower than the actual count of a resource.
    *
@@ -50,6 +47,6 @@ public interface BulkDependentResource<R, P extends HasMetadata>
    *         convenience methods ({@link Result#nonComputed(boolean)} and
    *         {@link Result#computed(boolean, Object)})
    */
-  Result<R> match(R actualResource, P primary, String key, Context<P> context);
+  Result<R> match(R actualResource, R desired, P primary, String key, Context<P> context);
 
 }

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

@@ -139,17 +139,15 @@ public Result<R> match(R actualResource, P primary, Context<P> context) {
     return matcher.match(actualResource, primary, context);
   }
 
-  public Result<R> match(R actualResource, P primary, String key, Context<P> context) {
-    final var desired = bulkDependentResource.desired(primary, key, context);
-    return GenericKubernetesResourceMatcher.match((R) desired, actualResource, false);
+  public Result<R> match(R actualResource, R desired, P primary, String key, Context<P> context) {
+    return GenericKubernetesResourceMatcher.match(desired, actualResource, false);
   }
 
   protected void handleDelete(P primary, Context<P> context) {
     var resource = getSecondaryResource(primary, context);
     resource.ifPresent(r -> client.resource(r).delete());
   }
 
-
   public void deleteBulkResource(P primary, R resource, String key, Context<P> context) {
     client.resource(resource).delete();
   }

operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/ConfigMapDeleterBulkDependentResource.java

@@ -32,17 +32,17 @@ public ConfigMapDeleterBulkDependentResource() {
   }
 
   @Override
-  public Set<String> targetKeys(BulkDependentTestCustomResource primary,
+  public Map<String, ConfigMap> desiredResources(BulkDependentTestCustomResource primary,
       Context<BulkDependentTestCustomResource> context) {
     var number = primary.getSpec().getNumberOfResources();
-    Set<String> res = new HashSet<>();
+    Map<String, ConfigMap> res = new HashMap<>();
     for (int i = 0; i < number; i++) {
-      res.add(Integer.toString(i));
+      var key = Integer.toString(i);
+      res.put(key, desired(primary, key, context));
     }
     return res;
   }
 
-  @Override
   public ConfigMap desired(BulkDependentTestCustomResource primary, String key,
       Context<BulkDependentTestCustomResource> context) {
     ConfigMap configMap = new ConfigMap();

operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/external/ExternalBulkDependentResource.java

@@ -60,12 +60,14 @@ private ResourceID toResourceID(ExternalResource externalResource) {
   }
 
   @Override
-  public Set<String> targetKeys(BulkDependentTestCustomResource primary,
+  public Map<String, ExternalResource> desiredResources(BulkDependentTestCustomResource primary,
       Context<BulkDependentTestCustomResource> context) {
     var number = primary.getSpec().getNumberOfResources();
-    Set<String> res = new HashSet<>();
+    Map<String, ExternalResource> res = new HashMap<>();
     for (int i = 0; i < number; i++) {
-      res.add(Integer.toString(i));
+      var key = Integer.toString(i);
+      res.put(key, new ExternalResource(toExternalResourceId(primary, key),
+          primary.getSpec().getAdditionalData()));
     }
     return res;
   }
@@ -84,13 +86,6 @@ public Map<String, ExternalResource> getSecondaryResources(
             r -> r));
   }
 
-  @Override
-  public ExternalResource desired(BulkDependentTestCustomResource primary, String key,
-      Context<BulkDependentTestCustomResource> context) {
-    return new ExternalResource(toExternalResourceId(primary, key),
-        primary.getSpec().getAdditionalData());
-  }
-
   @Override
   public void deleteBulkResource(BulkDependentTestCustomResource primary, ExternalResource resource,
       String key,
@@ -100,9 +95,9 @@ public void deleteBulkResource(BulkDependentTestCustomResource primary, External
 
   @Override
   public Matcher.Result<ExternalResource> match(ExternalResource actualResource,
+      ExternalResource desired,
       BulkDependentTestCustomResource primary, String index,
       Context<BulkDependentTestCustomResource> context) {
-    var desired = desired(primary, index, context);
     return Matcher.Result.computed(desired.equals(actualResource), desired);
   }
 }