fix: ready impl no tests

Author: csviri

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

@@ -23,7 +23,7 @@ public DependentResourceNode(DependentResource<R, P> dependentResource) {
   }
 
   public DependentResourceNode(DependentResource<R, P> dependentResource,
-      ReconcileCondition reconcileCondition) {
+      ReconcileCondition<P> reconcileCondition) {
     this(dependentResource, reconcileCondition, null);
   }
 
@@ -76,8 +76,8 @@ public DependentResourceNode<R, P> setCleanupCondition(CleanupCondition cleanupC
     return this;
   }
 
-  public ReadyCondition<R, P> getReadyCondition() {
-    return readyCondition;
+  public Optional<ReadyCondition<R, P>> getReadyCondition() {
+    return Optional.ofNullable(readyCondition);
   }
 
   public DependentResourceNode<R, P> setReadyCondition(ReadyCondition<R, P> readyCondition) {

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

@@ -18,27 +18,27 @@
  */
 public class Workflow<P extends HasMetadata> {
 
-  private final List<DependentResourceNode<?, ?>> dependentResourceNodes;
-  private final List<DependentResourceNode<?, ?>> topLevelResources = new ArrayList<>();
-  private Map<DependentResourceNode<?, ?>, List<DependentResourceNode<?, ?>>> dependents;
+  private final List<DependentResourceNode<?, P>> dependentResourceNodes;
+  private final List<DependentResourceNode<?, P>> topLevelResources = new ArrayList<>();
+  private Map<DependentResourceNode<?, P>, List<DependentResourceNode<?, P>>> dependents;
 
   // it's "global" executor service shared between multiple reconciliations running parallel
   private ExecutorService executorService;
 
-  public Workflow(List<DependentResourceNode<?, ?>> dependentResourceNodes) {
+  public Workflow(List<DependentResourceNode<?, P>> dependentResourceNodes) {
     this.executorService = ConfigurationServiceProvider.instance().getExecutorService();
     this.dependentResourceNodes = dependentResourceNodes;
     preprocessForReconcile();
   }
 
-  public Workflow(List<DependentResourceNode<?, ?>> dependentResourceNodes,
+  public Workflow(List<DependentResourceNode<?, P>> dependentResourceNodes,
       ExecutorService executorService) {
     this.executorService = executorService;
     this.dependentResourceNodes = dependentResourceNodes;
     preprocessForReconcile();
   }
 
-  public Workflow(List<DependentResourceNode<?, ?>> dependentResourceNodes, int globalParallelism) {
+  public Workflow(List<DependentResourceNode<?, P>> dependentResourceNodes, int globalParallelism) {
     this(dependentResourceNodes, Executors.newFixedThreadPool(globalParallelism));
   }
 
@@ -55,11 +55,11 @@ public void cleanup(P resource, Context<P> context) {
   // add cycle detection?
   private void preprocessForReconcile() {
     dependents = new ConcurrentHashMap<>(dependentResourceNodes.size());
-    for (DependentResourceNode<?, ?> node : dependentResourceNodes) {
+    for (DependentResourceNode<?, P> node : dependentResourceNodes) {
       if (node.getDependsOn().isEmpty()) {
         topLevelResources.add(node);
       } else {
-        for (DependentResourceNode<?, ?> dependsOn : node.getDependsOn()) {
+        for (DependentResourceNode<?, P> dependsOn : node.getDependsOn()) {
           dependents.computeIfAbsent(dependsOn, dr -> new ArrayList<>());
           dependents.get(dependsOn).add(node);
         }
@@ -71,11 +71,11 @@ public void setExecutorService(ExecutorService executorService) {
     this.executorService = executorService;
   }
 
-  List<DependentResourceNode<?, ?>> getTopLevelDependentResources() {
+  List<DependentResourceNode<?, P>> getTopLevelDependentResources() {
     return topLevelResources;
   }
 
-  Map<DependentResourceNode<?, ?>, List<DependentResourceNode<?, ?>>> getDependents() {
+  Map<DependentResourceNode<?, P>, List<DependentResourceNode<?, P>>> getDependents() {
     return dependents;
   }
 

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

@@ -10,6 +10,7 @@
 import io.javaoperatorsdk.operator.AggregatedOperatorException;
 import io.javaoperatorsdk.operator.api.reconciler.Context;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.Deleter;
+import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
 import io.javaoperatorsdk.operator.processing.dependent.workflow.condition.ReconcileCondition;
 
 public class WorkflowReconcileExecutor<P extends HasMetadata> {
@@ -20,6 +21,8 @@ public class WorkflowReconcileExecutor<P extends HasMetadata> {
 
   private final Set<DependentResourceNode<?, ?>> alreadyReconciled = ConcurrentHashMap.newKeySet();
   private final Set<DependentResourceNode<?, ?>> errored = ConcurrentHashMap.newKeySet();
+  private final Set<DependentResourceNode<?, ?>> reconciledButNotReady =
+      ConcurrentHashMap.newKeySet();
   private final Set<DependentResourceNode<?, ?>> reconcileConditionOrParentsConditionNotMet =
       ConcurrentHashMap.newKeySet();
 
@@ -39,9 +42,9 @@ public WorkflowReconcileExecutor(Workflow<P> workflow, P primary, Context<P> con
 
   // add reconcile results
   public synchronized void reconcile() {
-    for (DependentResourceNode<?, ?> dependentResourceNode : workflow
+    for (DependentResourceNode<?, P> dependentResourceNode : workflow
         .getTopLevelDependentResources()) {
-      handleReconcileOrDelete(dependentResourceNode, false);
+      handleReconcile(dependentResourceNode, false);
     }
     while (true) {
       try {
@@ -62,14 +65,14 @@ public synchronized void reconcile() {
     }
   }
 
-  private synchronized void handleReconcileOrDelete(
-      DependentResourceNode<?, ?> dependentResourceNode,
+  private synchronized void handleReconcile(
+      DependentResourceNode<?, P> dependentResourceNode,
       boolean onlyReconcileForPossibleDelete) {
     log.debug("Submitting for reconcile: {}", dependentResourceNode);
 
     if (alreadyReconciled(dependentResourceNode)
         || isReconcilingNow(dependentResourceNode)
-        || !allDependsReconciled(dependentResourceNode)
+        || !allDependsReconciledAndReady(dependentResourceNode)
         || hasErroredDependOn(dependentResourceNode)) {
       log.debug("Skipping submit of: {}, ", dependentResourceNode);
       return;
@@ -114,10 +117,10 @@ private boolean ownOrParentsReconcileConditionNotMet(
 
   private class NodeExecutor implements Runnable {
 
-    private final DependentResourceNode dependentResourceNode;
+    private final DependentResourceNode<?, P> dependentResourceNode;
     private final boolean onlyReconcileForPossibleDelete;
 
-    private NodeExecutor(DependentResourceNode<?, ?> dependentResourceNode,
+    private NodeExecutor(DependentResourceNode<?, P> dependentResourceNode,
         boolean onlyReconcileForDelete) {
       this.dependentResourceNode = dependentResourceNode;
       this.onlyReconcileForPossibleDelete = onlyReconcileForDelete;
@@ -127,16 +130,25 @@ private NodeExecutor(DependentResourceNode<?, ?> dependentResourceNode,
     @SuppressWarnings("unchecked")
     public void run() {
       try {
-        var dependentResource = dependentResourceNode.getDependentResource();
+        DependentResource dependentResource = dependentResourceNode.getDependentResource();
+        boolean handleDependents = true;
         if (onlyReconcileForPossibleDelete) {
           if (dependentResource instanceof Deleter) {
             ((Deleter<P>) dependentResource).delete(primary, context);
           }
         } else {
           dependentResource.reconcile(primary, context);
+          if (dependentResourceNode.getReadyCondition().isPresent()
+              && !dependentResourceNode.getReadyCondition().get()
+                  .isMet(dependentResource, primary, context)) {
+            handleDependents = false;
+            reconciledButNotReady.add(dependentResourceNode);
+          }
         }
         alreadyReconciled.add(dependentResourceNode);
-        handleDependentsReconcile(dependentResourceNode, onlyReconcileForPossibleDelete);
+        if (handleDependents) {
+          handleDependentsReconcile(dependentResourceNode, onlyReconcileForPossibleDelete);
+        }
       } catch (RuntimeException e) {
         handleExceptionInExecutor(dependentResourceNode, e);
       } finally {
@@ -150,10 +162,10 @@ private boolean isReconcilingNow(DependentResourceNode<?, ?> dependentResourceNo
   }
 
   private synchronized void handleDependentsReconcile(
-      DependentResourceNode<?, ?> dependentResourceNode, boolean onlyReconcileForPossibleDelete) {
+      DependentResourceNode<?, P> dependentResourceNode, boolean onlyReconcileForPossibleDelete) {
     var dependents = workflow.getDependents().get(dependentResourceNode);
     if (dependents != null) {
-      dependents.forEach(d -> handleReconcileOrDelete(d, onlyReconcileForPossibleDelete));
+      dependents.forEach(d -> handleReconcile(d, onlyReconcileForPossibleDelete));
     }
   }
 
@@ -180,11 +192,12 @@ private void handleReconcileCondition(DependentResourceNode<?, ?> dependentResou
     }
   }
 
-  private boolean allDependsReconciled(
+  private boolean allDependsReconciledAndReady(
       DependentResourceNode<?, ?> dependentResourceNode) {
     return dependentResourceNode.getDependsOn().isEmpty()
         || dependentResourceNode.getDependsOn().stream()
-            .allMatch(this::alreadyReconciled);
+            .allMatch(d -> alreadyReconciled(d)
+                && !reconciledButNotReady.contains(dependentResourceNode));
   }
 
   private boolean hasErroredDependOn(

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

@@ -3,6 +3,7 @@
 import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
 import io.javaoperatorsdk.operator.processing.dependent.workflow.DependentResourceNode;
+import io.javaoperatorsdk.operator.processing.dependent.workflow.condition.ReadyCondition;
 import io.javaoperatorsdk.operator.processing.dependent.workflow.condition.ReconcileCondition;
 
 public class DependentBuilder<P extends HasMetadata> {
@@ -28,6 +29,11 @@ public DependentBuilder<P> withReconcileCondition(ReconcileCondition reconcileCo
     return this;
   }
 
+  public DependentBuilder<P> withReadyCondition(ReadyCondition readyCondition) {
+    node.setReadyCondition(readyCondition);
+    return this;
+  }
+
   public WorkflowBuilder<P> build() {
     return workflowBuilder;
   }

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

@@ -12,10 +12,10 @@
 
 public class WorkflowBuilder<P extends HasMetadata> {
 
-  private List<DependentResourceNode<?, ?>> dependentResourceNodes = new ArrayList<>();
+  private List<DependentResourceNode<?, P>> dependentResourceNodes = new ArrayList<>();
 
-  public DependentBuilder<P> addDependent(DependentResource<?, ?> dependentResource) {
-    DependentResourceNode<?, ?> node = new DependentResourceNode<>(dependentResource);
+  public DependentBuilder<P> addDependent(DependentResource<?, P> dependentResource) {
+    DependentResourceNode<?, P> node = new DependentResourceNode<>(dependentResource);
     dependentResourceNodes.add(node);
     return new DependentBuilder<>(this, node);
   }

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

@@ -6,6 +6,6 @@
 
 public interface ReadyCondition<R, P extends HasMetadata> {
 
-  void isMet(DependentResource<R, P> dependentResource, P primary, Context<P> context);
+  boolean isMet(DependentResource<R, P> dependentResource, P primary, Context<P> context);
 
 }