wip: record results for all conditions

Signed-off-by: Chris Laprun <claprun@redhat.com>

Author: metacosm

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

@@ -1,9 +1,7 @@
 package io.javaoperatorsdk.operator.processing.dependent.workflow;
 
 import java.util.Map;
-import java.util.Map.Entry;
 import java.util.Optional;
-import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Future;
@@ -20,25 +18,24 @@
 @SuppressWarnings("rawtypes")
 abstract class AbstractWorkflowExecutor<P extends HasMetadata> {
 
-  protected final Workflow<P> workflow;
+  protected final DefaultWorkflow<P> workflow;
   protected final P primary;
   protected final ResourceID primaryID;
   protected final Context<P> context;
+  protected final Map<DependentResourceNode<?, P>, WorkflowResult.DetailBuilder<?>> results;
   /**
    * Covers both deleted and reconciled
    */
-  private final Set<DependentResourceNode> alreadyVisited = ConcurrentHashMap.newKeySet();
   private final Map<DependentResourceNode, Future<?>> actualExecutions = new ConcurrentHashMap<>();
-  private final Map<DependentResourceNode, Exception> exceptionsDuringExecution =
-      new ConcurrentHashMap<>();
   private final ExecutorService executorService;
 
-  public AbstractWorkflowExecutor(Workflow<P> workflow, P primary, Context<P> context) {
+  protected AbstractWorkflowExecutor(DefaultWorkflow<P> workflow, P primary, Context<P> context) {
     this.workflow = workflow;
     this.primary = primary;
     this.context = context;
     this.primaryID = ResourceID.fromResource(primary);
     executorService = context.getWorkflowExecutorService();
+    results = new ConcurrentHashMap<>(workflow.getDependentResourcesByName().size());
   }
 
   protected abstract Logger logger();
@@ -75,11 +72,22 @@ protected boolean noMoreExecutionsScheduled() {
   }
 
   protected boolean alreadyVisited(DependentResourceNode<?, P> dependentResourceNode) {
-    return alreadyVisited.contains(dependentResourceNode);
+    return results.containsKey(dependentResourceNode);
   }
 
   protected void markAsVisited(DependentResourceNode<?, P> dependentResourceNode) {
-    alreadyVisited.add(dependentResourceNode);
+    createOrGetResultFor(dependentResourceNode);
+  }
+
+  protected boolean postDeleteConditionNotMet(DependentResourceNode<?, P> dependentResourceNode) {
+    final var builder = results.get(dependentResourceNode);
+    return builder != null && builder.hasPostDeleteConditionNotMet();
+  }
+
+  protected WorkflowResult.DetailBuilder createOrGetResultFor(
+      DependentResourceNode<?, P> dependentResourceNode) {
+    return results.computeIfAbsent(dependentResourceNode,
+        unused -> new WorkflowResult.DetailBuilder());
   }
 
   protected boolean isExecutingNow(DependentResourceNode<?, P> dependentResourceNode) {
@@ -94,17 +102,17 @@ protected void markAsExecuting(DependentResourceNode<?, P> dependentResourceNode
   protected synchronized void handleExceptionInExecutor(
       DependentResourceNode<?, P> dependentResourceNode,
       RuntimeException e) {
-    exceptionsDuringExecution.put(dependentResourceNode, e);
+    createOrGetResultFor(dependentResourceNode).withError(e);
   }
 
-  protected boolean isInError(DependentResourceNode<?, P> dependentResourceNode) {
-    return exceptionsDuringExecution.containsKey(dependentResourceNode);
+  protected boolean isNotReady(DependentResourceNode<?, P> dependentResourceNode) {
+    final var builder = results.get(dependentResourceNode);
+    return builder != null && builder.isNotReady();
   }
 
-  protected Map<DependentResource, Exception> getErroredDependents() {
-    return exceptionsDuringExecution.entrySet().stream()
-        .collect(
-            Collectors.toMap(e -> e.getKey().getDependentResource(), Entry::getValue));
+  protected boolean isInError(DependentResourceNode<?, P> dependentResourceNode) {
+    final var builder = results.get(dependentResourceNode);
+    return builder != null && builder.hasError();
   }
 
   protected synchronized void handleNodeExecutionFinish(
@@ -116,9 +124,17 @@ protected synchronized void handleNodeExecutionFinish(
     }
   }
 
-  protected <R> boolean isConditionMet(Optional<Condition<R, P>> condition,
-      DependentResource<R, P> dependentResource) {
-    return condition.map(c -> c.isMet(dependentResource, primary, context)).orElse(true);
+  @SuppressWarnings("unchecked")
+  protected <R> boolean isConditionMet(
+      Optional<DependentResourceNode.ConditionWithType<R, P, ?>> condition,
+      DependentResourceNode<R, P> dependentResource) {
+    final var dr = dependentResource.getDependentResource();
+    return condition.map(c -> {
+      final ResultCondition.Result<?> r = c.detailedIsMet(dr, primary, context);
+      results.computeIfAbsent(dependentResource, unused -> new WorkflowResult.DetailBuilder())
+          .withResultForCondition(c, r);
+      return r;
+    }).orElse(ResultCondition.Result.metWithoutResult).isSuccess();
   }
 
   protected <R> void submit(DependentResourceNode<R, P> dependentResourceNode,
@@ -145,4 +161,10 @@ protected <R> void registerOrDeregisterEventSourceBasedOnActivation(
       }
     }
   }
+
+  protected Map<DependentResource, WorkflowResult.Detail> asDetails() {
+    return results.entrySet().stream()
+        .collect(
+            Collectors.toMap(e -> e.getKey().getDependentResource(), e -> e.getValue().build()));
+  }
 }

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

@@ -6,6 +6,10 @@
 
 public interface Condition<R, P extends HasMetadata> {
 
+  enum Type {
+    ACTIVATION, DELETE, READY, RECONCILE
+  }
+
   /**
    * Checks whether a condition holds true for a given
    * {@link io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource} based on the

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

@@ -108,12 +108,10 @@ public WorkflowCleanupResult cleanup(P primary, Context<P> context) {
     return result;
   }
 
-  @Override
   public Set<DependentResourceNode> getTopLevelDependentResources() {
     return topLevelResources;
   }
 
-  @Override
   public Set<DependentResourceNode> getBottomLevelResource() {
     return bottomLevelResource;
   }

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

@@ -5,6 +5,7 @@
 import java.util.Optional;
 
 import io.fabric8.kubernetes.api.model.HasMetadata;
+import io.javaoperatorsdk.operator.api.reconciler.Context;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
 
 @SuppressWarnings("rawtypes")
@@ -13,23 +14,49 @@ class DependentResourceNode<R, P extends HasMetadata> {
   private final List<DependentResourceNode> dependsOn = new LinkedList<>();
   private final List<DependentResourceNode> parents = new LinkedList<>();
 
-  private Condition<R, P> reconcilePrecondition;
-  private Condition<R, P> deletePostcondition;
-  private Condition<R, P> readyPostcondition;
-  private Condition<R, P> activationCondition;
+  private ConditionWithType<R, P, ?> reconcilePrecondition;
+  private ConditionWithType<R, P, ?> deletePostcondition;
+  private ConditionWithType<R, P, ?> readyPostcondition;
+  private ConditionWithType<R, P, ?> activationCondition;
   private final DependentResource<R, P> dependentResource;
 
+  static class ConditionWithType<R, P extends HasMetadata, T> implements ResultCondition<R, P, T> {
+    private final Condition<R, P> condition;
+    private final Type type;
+
+    ConditionWithType(Condition<R, P> condition, Type type) {
+      this.condition = condition;
+      this.type = type;
+    }
+
+    public Type type() {
+      return type;
+    }
+
+    @SuppressWarnings("unchecked")
+    @Override
+    public Result<T> detailedIsMet(DependentResource<R, P> dependentResource, P primary,
+        Context<P> context) {
+      if (condition instanceof ResultCondition resultCondition) {
+        return resultCondition.detailedIsMet(dependentResource, primary, context);
+      } else {
+        return ResultCondition.Result
+            .withoutResult(condition.isMet(dependentResource, primary, context));
+      }
+    }
+  }
+
   DependentResourceNode(DependentResource<R, P> dependentResource) {
     this(null, null, null, null, dependentResource);
   }
 
   public DependentResourceNode(Condition<R, P> reconcilePrecondition,
       Condition<R, P> deletePostcondition, Condition<R, P> readyPostcondition,
       Condition<R, P> activationCondition, DependentResource<R, P> dependentResource) {
-    this.reconcilePrecondition = reconcilePrecondition;
-    this.deletePostcondition = deletePostcondition;
-    this.readyPostcondition = readyPostcondition;
-    this.activationCondition = activationCondition;
+    setReconcilePrecondition(reconcilePrecondition);
+    setDeletePostcondition(deletePostcondition);
+    setReadyPostcondition(readyPostcondition);
+    setActivationCondition(activationCondition);
     this.dependentResource = dependentResource;
   }
 
@@ -50,36 +77,40 @@ public List<DependentResourceNode> getParents() {
     return parents;
   }
 
-  public Optional<Condition<R, P>> getReconcilePrecondition() {
+  public Optional<ConditionWithType<R, P, ?>> getReconcilePrecondition() {
     return Optional.ofNullable(reconcilePrecondition);
   }
 
-  public Optional<Condition<R, P>> getDeletePostcondition() {
+  public Optional<ConditionWithType<R, P, ?>> getDeletePostcondition() {
     return Optional.ofNullable(deletePostcondition);
   }
 
-  public Optional<Condition<R, P>> getActivationCondition() {
+  public Optional<ConditionWithType<R, P, ?>> getActivationCondition() {
     return Optional.ofNullable(activationCondition);
   }
 
+  public Optional<ConditionWithType<R, P, ?>> getReadyPostcondition() {
+    return Optional.ofNullable(readyPostcondition);
+  }
+
   void setReconcilePrecondition(Condition<R, P> reconcilePrecondition) {
-    this.reconcilePrecondition = reconcilePrecondition;
+    this.reconcilePrecondition = reconcilePrecondition == null ? null
+        : new ConditionWithType<>(reconcilePrecondition, Condition.Type.RECONCILE);
   }
 
   void setDeletePostcondition(Condition<R, P> cleanupCondition) {
-    this.deletePostcondition = cleanupCondition;
+    this.deletePostcondition = deletePostcondition == null ? null
+        : new ConditionWithType<>(deletePostcondition, Condition.Type.DELETE);
   }
 
   void setActivationCondition(Condition<R, P> activationCondition) {
-    this.activationCondition = activationCondition;
-  }
-
-  public Optional<Condition<R, P>> getReadyPostcondition() {
-    return Optional.ofNullable(readyPostcondition);
+    this.activationCondition = activationCondition == null ? null
+        : new ConditionWithType<>(activationCondition, Condition.Type.ACTIVATION);
   }
 
   void setReadyPostcondition(Condition<R, P> readyPostcondition) {
-    this.readyPostcondition = readyPostcondition;
+    this.readyPostcondition = readyPostcondition == null ? null
+        : new ConditionWithType<>(readyPostcondition, Condition.Type.READY);
   }
 
   public DependentResource<R, P> getDependentResource() {

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

@@ -1,7 +1,6 @@
 package io.javaoperatorsdk.operator.processing.dependent.workflow;
 
 import io.fabric8.kubernetes.api.model.HasMetadata;
-import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
 
 abstract class NodeExecutor<R, P extends HasMetadata> implements Runnable {
 
@@ -17,9 +16,7 @@ protected NodeExecutor(DependentResourceNode<R, P> dependentResourceNode,
   @Override
   public void run() {
     try {
-      var dependentResource = dependentResourceNode.getDependentResource();
-
-      doRun(dependentResourceNode, dependentResource);
+      doRun(dependentResourceNode);
 
     } catch (RuntimeException e) {
       workflowExecutor.handleExceptionInExecutor(dependentResourceNode, e);
@@ -28,6 +25,5 @@ public void run() {
     }
   }
 
-  protected abstract void doRun(DependentResourceNode<R, P> dependentResourceNode,
-      DependentResource<R, P> dependentResource);
+  protected abstract void doRun(DependentResourceNode<R, P> dependentResourceNode);
 }

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

@@ -15,6 +15,34 @@ default boolean isMet(DependentResource<R, P> dependentResource, P primary, Cont
   }
 
   interface Result<T> {
+    Result metWithoutResult = new Result() {
+      @Override
+      public Object getResult() {
+        return NULL;
+      }
+
+      @Override
+      public boolean isSuccess() {
+        return true;
+      }
+    };
+
+    Result unmetWithoutResult = new Result() {
+      @Override
+      public Object getResult() {
+        return NULL;
+      }
+
+      @Override
+      public boolean isSuccess() {
+        return false;
+      }
+    };
+
+    static Result withoutResult(boolean success) {
+      return success ? metWithoutResult : unmetWithoutResult;
+    }
+
     T getResult();
 
     boolean isSuccess();

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

@@ -3,7 +3,6 @@
 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;
@@ -21,16 +20,6 @@ default WorkflowCleanupResult cleanup(P primary, Context<P> context) {
     throw new UnsupportedOperationException("Implement this");
   }
 
-  @SuppressWarnings("rawtypes")
-  default Set<DependentResourceNode> getTopLevelDependentResources() {
-    return Collections.emptySet();
-  }
-
-  @SuppressWarnings("rawtypes")
-  default Set<DependentResourceNode> getBottomLevelResource() {
-    return Collections.emptySet();
-  }
-
   default boolean hasCleaner() {
     return false;
   }

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

@@ -82,6 +82,10 @@ public WorkflowBuilder<P> withThrowExceptionFurther(boolean throwExceptionFurthe
   }
 
   public Workflow<P> build() {
+    return buildAsDefaultWorkflow();
+  }
+
+  DefaultWorkflow<P> buildAsDefaultWorkflow() {
     return new DefaultWorkflow(new HashSet<>(dependentResourceNodes.values()),
         throwExceptionAutomatically, isCleaner);
   }