refactor: isolate bulk dependent resource handling more

Author: metacosm

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

@@ -11,6 +11,7 @@
 import io.javaoperatorsdk.operator.api.reconciler.ResourceDiscriminator;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.ReconcileResult;
+import io.javaoperatorsdk.operator.processing.dependent.Matcher.Result;
 import io.javaoperatorsdk.operator.processing.event.ResourceID;
 
 @Ignore
@@ -20,61 +21,32 @@ public abstract class AbstractDependentResource<R, P extends HasMetadata>
 
   private final boolean creatable = this instanceof Creator;
   private final boolean updatable = this instanceof Updater;
-  private final boolean bulk = this instanceof BulkDependentResource;
 
   protected Creator<R, P> creator;
   protected Updater<R, P> updater;
-  protected BulkDependentResource<R, P> bulkDependentResource;
   private ResourceDiscriminator<R, P> resourceDiscriminator;
+  private final DependentResourceReconciler<R, P> dependentResourceReconciler;
 
-  @SuppressWarnings({"unchecked", "rawtypes"})
+  @SuppressWarnings({"unchecked"})
   protected AbstractDependentResource() {
     creator = creatable ? (Creator<R, P>) this : null;
     updater = updatable ? (Updater<R, P>) this : null;
 
-    bulkDependentResource = bulk ? (BulkDependentResource) this : null;
+    dependentResourceReconciler = this instanceof BulkDependentResource
+        ? new BulkDependentResourceReconciler<>((BulkDependentResource<R, P>) this)
+        : new SingleDependentResourceReconciler<>(this);
   }
 
-
   @Override
   public ReconcileResult<R> reconcile(P primary, Context<P> context) {
-    if (bulk) {
-      final var targetResources = bulkDependentResource.desiredResources(primary, context);
-
-      Map<String, R> actualResources =
-          bulkDependentResource.getSecondaryResources(primary, context);
-
-      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));
-      });
-
-      return ReconcileResult.aggregatedResult(results);
-    } else {
-      var actualResource = getSecondaryResource(primary, context);
-      return reconcileIndexAware(primary, actualResource.orElse(null), null, null, context);
-    }
+    return dependentResourceReconciler.reconcile(primary, context);
   }
 
-  @SuppressWarnings({"rawtypes"})
-  protected void deleteBulkResourcesIfRequired(Set targetKeys, Map<String, R> actualResources,
-      P primary, Context<P> context) {
-    actualResources.forEach((key, value) -> {
-      if (!targetKeys.contains(key)) {
-        bulkDependentResource.deleteBulkResource(primary, value, key, context);
-      }
-    });
-  }
-
-  protected ReconcileResult<R> reconcileIndexAware(P primary, R actualResource, R desiredResource,
-      String key,
-      Context<P> context) {
+  protected ReconcileResult<R> reconcile(P primary, R resource, Context<P> context) {
     if (creatable || updatable) {
       if (actualResource == null) {
         if (creatable) {
-          var desired = bulkAwareDesired(primary, desiredResource, context);
+          var desired = desired(primary, context);
           throwIfNull(desired, primary, "Desired");
           logForOperation("Creating", primary, desired);
           var createdResource = handleCreate(desired, primary, context);
@@ -83,15 +55,10 @@ protected ReconcileResult<R> reconcileIndexAware(P primary, R actualResource, R
       } else {
         if (updatable) {
           final Matcher.Result<R> match;
-          if (bulk) {
-            match =
-                bulkDependentResource.match(actualResource, desiredResource, primary, key, context);
-          } else {
-            match = updater.match(actualResource, primary, context);
-          }
+          match = match(resource, primary, context);
           if (!match.matched()) {
             final var desired =
-                match.computedDesired().orElse(bulkAwareDesired(primary, desiredResource, context));
+                match.computedDesired().orElse(desired(primary, context));
             throwIfNull(desired, primary, "Desired");
             logForOperation("Updating", primary, desired);
             var updatedResource = handleUpdate(actualResource, desired, primary, context);
@@ -110,9 +77,8 @@ protected ReconcileResult<R> reconcileIndexAware(P primary, R actualResource, R
     return ReconcileResult.noOperation(actualResource);
   }
 
-  private R bulkAwareDesired(P primary, R alreadyComputedDesire, Context<P> context) {
-    return bulk ? alreadyComputedDesire
-        : desired(primary, context);
+  public Result<R> match(R resource, P primary, Context<P> context) {
+    return updater.match(resource, primary, context);
   }
 
   @Override
@@ -179,12 +145,7 @@ protected R desired(P primary, Context<P> context) {
   }
 
   public void delete(P primary, Context<P> context) {
-    if (bulk) {
-      var actualResources = bulkDependentResource.getSecondaryResources(primary, context);
-      deleteBulkResourcesIfRequired(Collections.emptySet(), actualResources, primary, context);
-    } else {
-      handleDelete(primary, context);
-    }
+    dependentResourceReconciler.delete(primary, context);
   }
 
   protected void handleDelete(P primary, Context<P> context) {
@@ -200,11 +161,8 @@ protected boolean isCreatable() {
     return creatable;
   }
 
+  @SuppressWarnings("unused")
   protected boolean isUpdatable() {
     return updatable;
   }
-
-  protected boolean isBulk() {
-    return bulk;
-  }
 }

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

@@ -17,7 +17,14 @@ public interface BulkDependentResource<R, P extends HasMetadata>
     extends Creator<R, P>, Deleter<P> {
 
   /**
-   * @return number of resources to create
+   * Retrieves a map of desired secondary resources associated with the specified primary resource,
+   * identified by an arbitrary key.
+   *
+   * @param primary the primary resource with which we want to identify which secondary resources
+   *        are associated
+   * @param context the {@link Context} associated with the current reconciliation
+   * @return a Map associating bulk secondary resources with the specified primary via arbitrary
+   *         identifiers
    */
   Map<String, R> desiredResources(P primary, Context<P> context);
 

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

@@ -0,0 +1,97 @@
+package io.javaoperatorsdk.operator.processing.dependent;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import io.fabric8.kubernetes.api.model.HasMetadata;
+import io.javaoperatorsdk.operator.api.reconciler.Context;
+import io.javaoperatorsdk.operator.api.reconciler.dependent.ReconcileResult;
+import io.javaoperatorsdk.operator.processing.dependent.Matcher.Result;
+import io.javaoperatorsdk.operator.processing.event.ResourceID;
+
+class BulkDependentResourceReconciler<R, P extends HasMetadata>
+    implements DependentResourceReconciler<R, P> {
+
+  private final BulkDependentResource<R, P> bulkDependentResource;
+
+  BulkDependentResourceReconciler(BulkDependentResource<R, P> bulkDependentResource) {
+    this.bulkDependentResource = bulkDependentResource;
+  }
+
+  @Override
+  public ReconcileResult<R> reconcile(P primary, Context<P> context) {
+    final var targetKeys = bulkDependentResource.desiredResourceKeys(primary, context);
+    Map<String, R> actualResources =
+        bulkDependentResource.getSecondaryResources(primary, context);
+
+    deleteBulkResourcesIfRequired(targetKeys, actualResources, primary, context);
+
+    final List<ReconcileResult<R>> results = new ArrayList<>(targetKeys.size());
+    actualResources.forEach((key, value) -> {
+      final var instance = new BulkDependentResourceInstance<>(key, bulkDependentResource);
+      results.add(instance.reconcile(primary, value, context));
+    });
+
+    return ReconcileResult.aggregatedResult(results);
+  }
+
+  @Override
+  public void delete(P primary, Context<P> context) {
+    var actualResources = bulkDependentResource.getSecondaryResources(primary, context);
+    deleteBulkResourcesIfRequired(Collections.emptySet(), actualResources, primary, context);
+  }
+
+  protected void deleteBulkResourcesIfRequired(Set<String> expectedKeys,
+      Map<String, R> actualResources, P primary, Context<P> context) {
+    actualResources.forEach((key, value) -> {
+      if (!expectedKeys.contains(key)) {
+        bulkDependentResource.deleteBulkResource(primary, value, key, context);
+      }
+    });
+  }
+
+  private static class BulkDependentResourceInstance<R, P extends HasMetadata>
+      extends AbstractDependentResource<R, P> {
+    private final String key;
+    private final BulkDependentResource<R, P> bulkDependentResource;
+
+    private BulkDependentResourceInstance(String key,
+        BulkDependentResource<R, P> bulkDependentResource) {
+      this.key = key;
+      this.bulkDependentResource = bulkDependentResource;
+    }
+
+    @SuppressWarnings("unchecked")
+    private AbstractDependentResource<R, P> asAbstractDependentResource() {
+      return (AbstractDependentResource<R, P>) bulkDependentResource;
+    }
+
+    @Override
+    protected R desired(P primary, Context<P> context) {
+      return bulkDependentResource.desired(primary, key, context);
+    }
+
+    @Override
+    public Result<R> match(R resource, P primary, Context<P> context) {
+      return bulkDependentResource.match(resource, primary, key, context);
+    }
+
+    @Override
+    protected void onCreated(ResourceID primaryResourceId, R created) {
+      asAbstractDependentResource().onCreated(primaryResourceId, created);
+    }
+
+    @Override
+    protected void onUpdated(ResourceID primaryResourceId, R updated, R actual) {
+      asAbstractDependentResource().onUpdated(primaryResourceId, updated, actual);
+    }
+
+    @Override
+    public Class<R> resourceType() {
+      return asAbstractDependentResource().resourceType();
+    }
+  }
+}

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

@@ -0,0 +1,12 @@
+package io.javaoperatorsdk.operator.processing.dependent;
+
+import io.fabric8.kubernetes.api.model.HasMetadata;
+import io.javaoperatorsdk.operator.api.reconciler.Context;
+import io.javaoperatorsdk.operator.api.reconciler.dependent.ReconcileResult;
+
+interface DependentResourceReconciler<R, P extends HasMetadata> {
+
+  ReconcileResult<R> reconcile(P primary, Context<P> context);
+
+  void delete(P primary, Context<P> context);
+}

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

@@ -0,0 +1,26 @@
+package io.javaoperatorsdk.operator.processing.dependent;
+
+import io.fabric8.kubernetes.api.model.HasMetadata;
+import io.javaoperatorsdk.operator.api.reconciler.Context;
+import io.javaoperatorsdk.operator.api.reconciler.dependent.ReconcileResult;
+
+class SingleDependentResourceReconciler<R, P extends HasMetadata>
+    implements DependentResourceReconciler<R, P> {
+
+  private final AbstractDependentResource<R, P> instance;
+
+  SingleDependentResourceReconciler(AbstractDependentResource<R, P> dependentResource) {
+    this.instance = dependentResource;
+  }
+
+  @Override
+  public ReconcileResult<R> reconcile(P primary, Context<P> context) {
+    final var maybeActual = instance.getSecondaryResource(primary, context);
+    return instance.reconcile(primary, maybeActual.orElse(null), context);
+  }
+
+  @Override
+  public void delete(P primary, Context<P> context) {
+    instance.handleDelete(primary, context);
+  }
+}

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

@@ -148,6 +148,7 @@ protected void handleDelete(P primary, Context<P> context) {
     resource.ifPresent(r -> client.resource(r).delete());
   }
 
+  @SuppressWarnings("unused")
   public void deleteBulkResource(P primary, R resource, String key, Context<P> context) {
     client.resource(resource).delete();
   }