feat: silent exception handling in managed workflows (#2363)

Signed-off-by: Attila Mészáros <[email protected]>
Signed-off-by: Chris Laprun <[email protected]>
Co-authored-by: Chris Laprun <[email protected]>
Signed-off-by: Attila Mészáros <[email protected]>

Author: csviri

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/BaseConfigurationService.java

@@ -178,6 +178,12 @@ public List<DependentResourceSpec> getDependentResourceSpecs() {
         public boolean isExplicitInvocation() {
           return workflowAnnotation.explicitInvocation();
         }
+
+        @Override
+        public boolean handleExceptionsInReconciler() {
+          return workflowAnnotation.handleExceptionsInReconciler();
+        }
+
       };
       config.setWorkflowSpec(workflowSpec);
     }

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/workflow/WorkflowSpec.java

@@ -10,4 +10,6 @@ public interface WorkflowSpec {
   List<DependentResourceSpec> getDependentResourceSpecs();
 
   boolean isExplicitInvocation();
+
+  boolean handleExceptionsInReconciler();
 }

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Workflow.java

@@ -1,6 +1,10 @@
 package io.javaoperatorsdk.operator.api.reconciler;
 
-import java.lang.annotation.*;
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Inherited;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
 
 import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent;
@@ -13,10 +17,25 @@
   Dependent[] dependents();
 
   /**
-   * If true, managed workflow should be explicitly invoked within the reconciler implementation. If
-   * false workflow is invoked just before the {@link Reconciler#reconcile(HasMetadata, Context)}
-   * method.
+   * If {@code true}, the managed workflow should be explicitly invoked within the reconciler
+   * implementation. If {@code false}, the workflow is invoked just before the
+   * {@link Reconciler#reconcile(HasMetadata, Context)} method.
    */
   boolean explicitInvocation() default false;
 
+  /**
+   * If {@code true} and exceptions are thrown during the workflow's execution, the reconciler won't
+   * throw an {@link io.javaoperatorsdk.operator.AggregatedOperatorException} at the end of the
+   * execution as would normally be the case. Instead, it will proceed to its
+   * {@link Reconciler#reconcile(HasMetadata, Context)} method as if no error occurred. It is then
+   * up to the developer to decide how to proceed by retrieving the errored dependents (and their
+   * associated exception) via
+   * {@link io.javaoperatorsdk.operator.processing.dependent.workflow.WorkflowResult#erroredDependents},
+   * the workflow result itself being accessed from
+   * {@link Context#managedWorkflowAndDependentResourceContext()}. If {@code false}, an exception
+   * will be automatically thrown at the end of the workflow execution, presenting an aggregated
+   * view of what happened.
+   */
+  boolean handleExceptionsInReconciler() default false;
+
 }

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

@@ -70,6 +70,7 @@ public class Controller<P extends HasMetadata>
   private final boolean isCleaner;
   private final Metrics metrics;
   private final Workflow<P> managedWorkflow;
+  private final boolean explicitWorkflowInvocation;
 
   private final GroupVersionKind associatedGVK;
   private final EventProcessor<P> eventProcessor;
@@ -92,6 +93,9 @@ public Controller(Reconciler<P> reconciler,
 
     final var managed = configurationService.getWorkflowFactory().workflowFor(configuration);
     managedWorkflow = managed.resolve(kubernetesClient, configuration);
+    explicitWorkflowInvocation =
+        configuration.getWorkflowSpec().map(WorkflowSpec::isExplicitInvocation)
+            .orElse(false);
 
     eventSourceManager = new EventSourceManager<>(this);
     eventProcessor = new EventProcessor<>(eventSourceManager, configurationService);
@@ -143,7 +147,7 @@ public Map<String, Object> metadata() {
           public UpdateControl<P> execute() throws Exception {
             initContextIfNeeded(resource, context);
             configuration.getWorkflowSpec().ifPresent(ws -> {
-              if (!managedWorkflow.isEmpty() && !isWorkflowExplicitInvocation()) {
+              if (!managedWorkflow.isEmpty() && !explicitWorkflowInvocation) {
                   managedWorkflow.reconcile(resource, context);
               }
             });
@@ -188,7 +192,7 @@ public DeleteControl execute() {
               WorkflowCleanupResult workflowCleanupResult = null;
 
               // The cleanup is called also when explicit invocation is true, but the cleaner is not implemented
-              if (managedWorkflow.hasCleaner() || !isWorkflowExplicitInvocation()) {
+              if (managedWorkflow.hasCleaner() || !explicitWorkflowInvocation) {
                 workflowCleanupResult = managedWorkflow.cleanup(resource, context);
               }
 
@@ -449,7 +453,6 @@ public void reconcileManagedWorkflow(P primary, Context<P> context) {
       ((DefaultManagedWorkflowAndDependentResourceContext) context
           .managedWorkflowAndDependentResourceContext())
           .setWorkflowExecutionResult(res);
-      res.throwAggregateExceptionIfErrorsPresent();
     }
   }
 
@@ -459,15 +462,14 @@ public WorkflowCleanupResult cleanupManagedWorkflow(P resource, Context<P> conte
       ((DefaultManagedWorkflowAndDependentResourceContext) context
           .managedWorkflowAndDependentResourceContext())
           .setWorkflowCleanupResult(workflowCleanupResult);
-      workflowCleanupResult.throwAggregateExceptionIfErrorsPresent();
+
       return workflowCleanupResult;
     } else {
       return null;
     }
   }
 
   public boolean isWorkflowExplicitInvocation() {
-    return configuration.getWorkflowSpec().map(WorkflowSpec::isExplicitInvocation)
-        .orElse(false);
+    return explicitWorkflowInvocation;
   }
 }

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

@@ -16,7 +16,6 @@
 import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.KubernetesClientAware;
 
 import static io.javaoperatorsdk.operator.api.reconciler.Constants.NO_VALUE_SET;
-import static io.javaoperatorsdk.operator.processing.dependent.workflow.Workflow.THROW_EXCEPTION_AUTOMATICALLY_DEFAULT;
 
 @SuppressWarnings("rawtypes")
 public class DefaultManagedWorkflow<P extends HasMetadata> implements ManagedWorkflow<P> {
@@ -96,7 +95,8 @@ public Workflow<P> resolve(KubernetesClient client,
     final var top =
         topLevelResources.stream().map(alreadyResolved::get).collect(Collectors.toSet());
     return new DefaultWorkflow<>(alreadyResolved, bottom, top,
-        THROW_EXCEPTION_AUTOMATICALLY_DEFAULT, hasCleaner);
+        configuration.getWorkflowSpec().map(w -> !w.handleExceptionsInReconciler()).orElseThrow(),
+        hasCleaner);
   }
 
   @SuppressWarnings({"rawtypes", "unchecked"})

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

@@ -1,5 +1,6 @@
 package io.javaoperatorsdk.operator.processing.dependent.workflow;
 
+import java.util.Collections;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.stream.Collectors;
@@ -10,11 +11,10 @@
 @SuppressWarnings("rawtypes")
 class WorkflowResult {
 
-  private static final String NUMBER_DELIMITER = "_";
   private final Map<DependentResource, Exception> erroredDependents;
 
   WorkflowResult(Map<DependentResource, Exception> erroredDependents) {
-    this.erroredDependents = erroredDependents;
+    this.erroredDependents = erroredDependents != null ? erroredDependents : Collections.emptyMap();
   }
 
   public Map<DependentResource, Exception> getErroredDependents() {

operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowTest.java

@@ -73,6 +73,11 @@ public List<DependentResourceSpec> getDependentResourceSpecs() {
       public boolean isExplicitInvocation() {
         return false;
       }
+
+      @Override
+      public boolean handleExceptionsInReconciler() {
+        return false;
+      }
     };
     when(configuration.getWorkflowSpec()).thenReturn(Optional.of(ws));
 

operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowSilentExceptionHandlingIT.java

@@ -0,0 +1,47 @@
+package io.javaoperatorsdk.operator;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
+import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension;
+import io.javaoperatorsdk.operator.sample.workflowsilentexceptionhandling.HandleWorkflowExceptionsInReconcilerCustomResource;
+import io.javaoperatorsdk.operator.sample.workflowsilentexceptionhandling.HandleWorkflowExceptionsInReconcilerReconciler;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.awaitility.Awaitility.await;
+
+public class WorkflowSilentExceptionHandlingIT {
+
+  @RegisterExtension
+  LocallyRunOperatorExtension extension =
+      LocallyRunOperatorExtension.builder()
+          .withReconciler(HandleWorkflowExceptionsInReconcilerReconciler.class)
+          .build();
+
+  @Test
+  void handleExceptionsInReconciler() {
+    extension.create(testResource());
+    var reconciler =
+        extension.getReconcilerOfType(HandleWorkflowExceptionsInReconcilerReconciler.class);
+
+    await().untilAsserted(() -> {
+      assertThat(reconciler.isErrorsFoundInReconcilerResult()).isTrue();
+    });
+
+    extension.delete(testResource());
+
+    await().untilAsserted(() -> {
+      assertThat(reconciler.isErrorsFoundInCleanupResult()).isTrue();
+    });
+  }
+
+  HandleWorkflowExceptionsInReconcilerCustomResource testResource() {
+    var res = new HandleWorkflowExceptionsInReconcilerCustomResource();
+    res.setMetadata(new ObjectMetaBuilder()
+        .withName("test1")
+        .build());
+    return res;
+  }
+
+}

operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowsilentexceptionhandling/ConfigMapDependent.java

@@ -0,0 +1,28 @@
+package io.javaoperatorsdk.operator.sample.workflowsilentexceptionhandling;
+
+
+import io.fabric8.kubernetes.api.model.ConfigMap;
+import io.javaoperatorsdk.operator.api.reconciler.Context;
+import io.javaoperatorsdk.operator.api.reconciler.dependent.ReconcileResult;
+import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDNoGCKubernetesDependentResource;
+
+public class ConfigMapDependent extends
+    CRUDNoGCKubernetesDependentResource<ConfigMap, HandleWorkflowExceptionsInReconcilerCustomResource> {
+
+  public ConfigMapDependent() {
+    super(ConfigMap.class);
+  }
+
+  @Override
+  public ReconcileResult<ConfigMap> reconcile(
+      HandleWorkflowExceptionsInReconcilerCustomResource primary,
+      Context<HandleWorkflowExceptionsInReconcilerCustomResource> context) {
+    throw new RuntimeException("Exception thrown on purpose");
+  }
+
+  @Override
+  public void delete(HandleWorkflowExceptionsInReconcilerCustomResource primary,
+      Context<HandleWorkflowExceptionsInReconcilerCustomResource> context) {
+    throw new RuntimeException("Exception thrown on purpose");
+  }
+}

operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowsilentexceptionhandling/HandleWorkflowExceptionsInReconcilerCustomResource.java

@@ -0,0 +1,15 @@
+package io.javaoperatorsdk.operator.sample.workflowsilentexceptionhandling;
+
+import io.fabric8.kubernetes.api.model.Namespaced;
+import io.fabric8.kubernetes.client.CustomResource;
+import io.fabric8.kubernetes.model.annotation.Group;
+import io.fabric8.kubernetes.model.annotation.ShortNames;
+import io.fabric8.kubernetes.model.annotation.Version;
+
+@Group("sample.javaoperatorsdk")
+@Version("v1")
+@ShortNames("hweir")
+public class HandleWorkflowExceptionsInReconcilerCustomResource
+    extends CustomResource<Void, Void>
+    implements Namespaced {
+}