feat: improve workflow error reporting (#1613)

Author: metacosm

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/AggregatedOperatorException.java

@@ -1,31 +1,28 @@
 package io.javaoperatorsdk.operator;
 
-import java.util.Arrays;
 import java.util.Collections;
-import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
 
 public class AggregatedOperatorException extends OperatorException {
 
-  private final List<Exception> causes;
+  private final Map<String, Exception> causes;
 
-  public AggregatedOperatorException(String message, Exception... exceptions) {
-    super(message, exceptions != null && exceptions.length > 0 ? exceptions[0] : null);
-    this.causes = exceptions != null ? Arrays.asList(exceptions) : Collections.emptyList();
+  public AggregatedOperatorException(String message, Map<String, Exception> exceptions) {
+    super(message);
+    this.causes =
+        exceptions != null ? Collections.unmodifiableMap(exceptions) : Collections.emptyMap();
   }
 
-  public AggregatedOperatorException(String message, List<Exception> exceptions) {
-    super(message, exceptions != null && !exceptions.isEmpty() ? exceptions.get(0) : null);
-    this.causes = exceptions != null ? exceptions : Collections.emptyList();
-  }
-
-  public List<Exception> getAggregatedExceptions() {
+  @SuppressWarnings("unused")
+  public Map<String, Exception> getAggregatedExceptions() {
     return causes;
   }
 
   @Override
-  public String toString() {
-    return "AggregatedOperatorException{" +
-        "causes=" + causes +
-        '}';
+  public String getMessage() {
+    return super.getMessage() + " " + causes.entrySet().stream()
+        .map(entry -> entry.getKey() + " -> " + entry.getValue())
+        .collect(Collectors.joining("\n - ", "Details:\n", ""));
   }
 }

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

@@ -2,6 +2,7 @@
 
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
@@ -176,17 +177,14 @@ private boolean hasErroredDependent(DependentResourceNode dependentResourceNode)
   }
 
   private WorkflowCleanupResult createCleanupResult() {
-    var result = new WorkflowCleanupResult();
-    result.setErroredDependents(exceptionsDuringExecution
-        .entrySet().stream()
-        .collect(Collectors.toMap(e -> e.getKey().getDependentResource(), Map.Entry::getValue)));
-
-    result.setPostConditionNotMetDependents(
-        postDeleteConditionNotMet.stream().map(DependentResourceNode::getDependentResource)
-            .collect(Collectors.toList()));
-    result.setDeleteCalledOnDependents(
-        deleteCalled.stream().map(DependentResourceNode::getDependentResource)
-            .collect(Collectors.toList()));
-    return result;
+    final var erroredDependents = exceptionsDuringExecution.entrySet().stream()
+        .collect(Collectors.toMap(e -> e.getKey().getDependentResource(), Entry::getValue));
+    final var postConditionNotMet = postDeleteConditionNotMet.stream()
+        .map(DependentResourceNode::getDependentResource)
+        .collect(Collectors.toList());
+    final var deleteCalled =
+        this.deleteCalled.stream().map(DependentResourceNode::getDependentResource)
+            .collect(Collectors.toList());
+    return new WorkflowCleanupResult(erroredDependents, postConditionNotMet, deleteCalled);
   }
 }

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

@@ -1,64 +1,32 @@
 package io.javaoperatorsdk.operator.processing.dependent.workflow;
 
-import java.util.ArrayList;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
-import io.javaoperatorsdk.operator.AggregatedOperatorException;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
 
 @SuppressWarnings("rawtypes")
-public class WorkflowCleanupResult {
+public class WorkflowCleanupResult extends WorkflowResult {
 
-  private List<DependentResource> deleteCalledOnDependents = new ArrayList<>();
-  private List<DependentResource> postConditionNotMetDependents = new ArrayList<>();
-  private Map<DependentResource, Exception> erroredDependents = new HashMap<>();
+  private final List<DependentResource> deleteCalledOnDependents;
+  private final List<DependentResource> postConditionNotMetDependents;
 
-  public List<DependentResource> getDeleteCalledOnDependents() {
-    return deleteCalledOnDependents;
+  WorkflowCleanupResult(Map<DependentResource, Exception> erroredDependents,
+      List<DependentResource> postConditionNotMet, List<DependentResource> deleteCalled) {
+    super(erroredDependents);
+    this.deleteCalledOnDependents = deleteCalled;
+    this.postConditionNotMetDependents = postConditionNotMet;
   }
 
-  public WorkflowCleanupResult setDeleteCalledOnDependents(
-      List<DependentResource> deletedDependents) {
-    this.deleteCalledOnDependents = deletedDependents;
-    return this;
+  public List<DependentResource> getDeleteCalledOnDependents() {
+    return deleteCalledOnDependents;
   }
 
   public List<DependentResource> getPostConditionNotMetDependents() {
     return postConditionNotMetDependents;
   }
 
-  public WorkflowCleanupResult setPostConditionNotMetDependents(
-      List<DependentResource> postConditionNotMetDependents) {
-    this.postConditionNotMetDependents = postConditionNotMetDependents;
-    return this;
-  }
-
-  public Map<DependentResource, Exception> getErroredDependents() {
-    return erroredDependents;
-  }
-
-  public WorkflowCleanupResult setErroredDependents(
-      Map<DependentResource, Exception> erroredDependents) {
-    this.erroredDependents = erroredDependents;
-    return this;
-  }
-
   public boolean allPostConditionsMet() {
     return postConditionNotMetDependents.isEmpty();
   }
-
-  public boolean erroredDependentsExists() {
-    return !erroredDependents.isEmpty();
-  }
-
-  public void throwAggregateExceptionIfErrorsPresent() {
-    if (erroredDependentsExists()) {
-      throw new AggregatedOperatorException("Exception(s) during workflow execution.",
-          new ArrayList<>(erroredDependents.values()));
-    }
-  }
-
-
 }

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

@@ -1,35 +1,28 @@
 package io.javaoperatorsdk.operator.processing.dependent.workflow;
 
-import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 
-import io.javaoperatorsdk.operator.AggregatedOperatorException;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.ReconcileResult;
 
 @SuppressWarnings("rawtypes")
-public class WorkflowReconcileResult {
+public class WorkflowReconcileResult extends WorkflowResult {
 
   private final List<DependentResource> reconciledDependents;
   private final List<DependentResource> notReadyDependents;
-  private final Map<DependentResource, Exception> erroredDependents;
   private final Map<DependentResource, ReconcileResult> reconcileResults;
 
   public WorkflowReconcileResult(List<DependentResource> reconciledDependents,
       List<DependentResource> notReadyDependents,
       Map<DependentResource, Exception> erroredDependents,
       Map<DependentResource, ReconcileResult> reconcileResults) {
+    super(erroredDependents);
     this.reconciledDependents = reconciledDependents;
     this.notReadyDependents = notReadyDependents;
-    this.erroredDependents = erroredDependents;
     this.reconcileResults = reconcileResults;
   }
 
-  public Map<DependentResource, Exception> getErroredDependents() {
-    return erroredDependents;
-  }
-
   public List<DependentResource> getReconciledDependents() {
     return reconciledDependents;
   }
@@ -43,31 +36,7 @@ public Map<DependentResource, ReconcileResult> getReconcileResults() {
     return reconcileResults;
   }
 
-  public void throwAggregateExceptionIfErrorsPresent() {
-    if (!erroredDependents.isEmpty()) {
-      throw createFinalException();
-    }
-  }
-
-  private AggregatedOperatorException createFinalException() {
-    return new AggregatedOperatorException("Exception during workflow.",
-        new ArrayList<>(erroredDependents.values()));
-  }
-
   public boolean allDependentResourcesReady() {
     return notReadyDependents.isEmpty();
   }
-
-  /**
-   * @deprecated Use {@link #erroredDependentsExist()} instead
-   */
-  @Deprecated(forRemoval = true)
-  public boolean erroredDependentsExists() {
-    return !erroredDependents.isEmpty();
-  }
-
-  @SuppressWarnings("unused")
-  public boolean erroredDependentsExist() {
-    return !erroredDependents.isEmpty();
-  }
 }

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

@@ -0,0 +1,43 @@
+package io.javaoperatorsdk.operator.processing.dependent.workflow;
+
+import java.util.Map;
+import java.util.Map.Entry;
+import java.util.stream.Collectors;
+
+import io.javaoperatorsdk.operator.AggregatedOperatorException;
+import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
+
+@SuppressWarnings("rawtypes")
+class WorkflowResult {
+
+  private final Map<DependentResource, Exception> erroredDependents;
+
+  WorkflowResult(Map<DependentResource, Exception> erroredDependents) {
+    this.erroredDependents = erroredDependents;
+  }
+
+  public Map<DependentResource, Exception> getErroredDependents() {
+    return erroredDependents;
+  }
+
+  /**
+   * @deprecated Use {@link #erroredDependentsExist()} instead
+   */
+  @Deprecated(forRemoval = true)
+  public boolean erroredDependentsExists() {
+    return !erroredDependents.isEmpty();
+  }
+
+  @SuppressWarnings("unused")
+  public boolean erroredDependentsExist() {
+    return !erroredDependents.isEmpty();
+  }
+
+  public void throwAggregateExceptionIfErrorsPresent() {
+    if (erroredDependentsExist()) {
+      throw new AggregatedOperatorException("Exception(s) during workflow execution.",
+          erroredDependents.entrySet().stream()
+              .collect(Collectors.toMap(e -> e.getKey().getClass().getName(), Entry::getValue)));
+    }
+  }
+}