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,58 +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 targetKeys = bulkDependentResource.targetKeys(primary, context);
-      Map<String, R> actualResources =
-          bulkDependentResource.getSecondaryResources(primary, context);
-
-      deleteBulkResourcesIfRequired(targetKeys, actualResources, primary, context);
-      final List<ReconcileResult<R>> results = new ArrayList<>(targetKeys.size());
-
-      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 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 resource, String key,
-      Context<P> context) {
+  protected ReconcileResult<R> reconcile(P primary, R resource, Context<P> context) {
     if (creatable || updatable) {
       if (resource == null) {
         if (creatable) {
-          var desired = desiredIndexAware(primary, key, context);
+          var desired = desired(primary, context);
           throwIfNull(desired, primary, "Desired");
           logForOperation("Creating", primary, desired);
           var createdResource = handleCreate(desired, primary, context);
@@ -80,14 +55,10 @@ protected ReconcileResult<R> reconcileIndexAware(P primary, R resource, String k
       } else {
         if (updatable) {
           final Matcher.Result<R> match;
-          if (bulk) {
-            match = bulkDependentResource.match(resource, primary, key, context);
-          } else {
-            match = updater.match(resource, primary, context);
-          }
+          match = match(resource, primary, context);
           if (!match.matched()) {
             final var desired =
-                match.computedDesired().orElse(desiredIndexAware(primary, key, context));
+                match.computedDesired().orElse(desired(primary, context));
             throwIfNull(desired, primary, "Desired");
             logForOperation("Updating", primary, desired);
             var updatedResource = handleUpdate(resource, desired, primary, context);
@@ -105,9 +76,8 @@ protected ReconcileResult<R> reconcileIndexAware(P primary, R resource, String k
     return ReconcileResult.noOperation(resource);
   }
 
-  private R desiredIndexAware(P primary, String key, Context<P> context) {
-    return bulk ? bulkDependentResource.desired(primary, key, context)
-        : desired(primary, context);
+  public Result<R> match(R resource, P primary, Context<P> context) {
+    return updater.match(resource, primary, context);
   }
 
   @Override
@@ -174,12 +144,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) {
@@ -195,11 +160,9 @@ 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

@@ -18,9 +18,16 @@ public interface BulkDependentResource<R, P extends HasMetadata>
     extends Creator<R, P>, Deleter<P> {
 
   /**
-   * @return number of resources to create
+   * Retrieves the Set of keys identifying the set of secondary resources associated with the
+   * specified primary resource.
+   *
+   * @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 Set of identifiers allowing to associate a bulk secondary resource with the specified
+   *         primary
    */
-  Set<String> targetKeys(P primary, Context<P> context);
+  Set<String> desiredResourceKeys(P primary, Context<P> context);
 
   Map<String, R> getSecondaryResources(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

@@ -19,6 +19,7 @@
 import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.DependentResourceConfigurator;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.KubernetesClientAware;
 import io.javaoperatorsdk.operator.processing.dependent.AbstractEventSourceHolderDependentResource;
+import io.javaoperatorsdk.operator.processing.dependent.BulkDependentResource;
 import io.javaoperatorsdk.operator.processing.dependent.Matcher;
 import io.javaoperatorsdk.operator.processing.dependent.Matcher.Result;
 import io.javaoperatorsdk.operator.processing.event.ResourceID;
@@ -139,17 +140,18 @@ public Result<R> match(R actualResource, P primary, Context<P> context) {
     return matcher.match(actualResource, primary, context);
   }
 
+  @SuppressWarnings("unchecked")
   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);
+    final var desired = ((BulkDependentResource<R, P>) this).desired(primary, key, 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());
   }
 
-
+  @SuppressWarnings("unused")
   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,7 +32,7 @@ public ConfigMapDeleterBulkDependentResource() {
   }
 
   @Override
-  public Set<String> targetKeys(BulkDependentTestCustomResource primary,
+  public Set<String> desiredResourceKeys(BulkDependentTestCustomResource primary,
       Context<BulkDependentTestCustomResource> context) {
     var number = primary.getSpec().getNumberOfResources();
     Set<String> res = new HashSet<>();

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

@@ -60,7 +60,7 @@ private ResourceID toResourceID(ExternalResource externalResource) {
   }
 
   @Override
-  public Set<String> targetKeys(BulkDependentTestCustomResource primary,
+  public Set<String> desiredResourceKeys(BulkDependentTestCustomResource primary,
       Context<BulkDependentTestCustomResource> context) {
     var number = primary.getSpec().getNumberOfResources();
     Set<String> res = new HashSet<>();