feat: explicit workflow invocation (#2289)

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

Author: csviri

Diff for: docs/documentation/v5-0-migration.md

@@ -17,3 +17,5 @@ permalink: /docs/v5-0-migration
    [`EventSourceUtils`](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/EventSourceUtils.java#L11-L11)
    now contains all the utility methods used for event sources naming that were previously defined in
    the `EventSourceInitializer` interface.
+3. `ManagedDependentResourceContext` has been renamed to `ManagedWorkflowAndDependentResourceContext` and is accessed
+   via the accordingly renamed `managedWorkflowAndDependentResourceContext` method.

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

@@ -169,7 +169,7 @@ protected <P extends HasMetadata> ControllerConfiguration<P> configFor(Reconcile
         io.javaoperatorsdk.operator.api.reconciler.Workflow.class);
     if (workflowAnnotation != null) {
       List<DependentResourceSpec> specs = dependentResources(workflowAnnotation, config);
-      WorkflowSpec workflowSpec = new WorkflowSpec(specs);
+      WorkflowSpec workflowSpec = new WorkflowSpec(specs, workflowAnnotation.explicitInvocation());
       config.setWorkflowSpec(workflowSpec);
     }
 

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

@@ -8,12 +8,19 @@ public class WorkflowSpec {
 
   @SuppressWarnings("rawtypes")
   private final List<DependentResourceSpec> dependentResourceSpecs;
+  private final boolean explicitInvocation;
 
-  public WorkflowSpec(List<DependentResourceSpec> dependentResourceSpecs) {
+  public WorkflowSpec(List<DependentResourceSpec> dependentResourceSpecs,
+      boolean explicitInvocation) {
     this.dependentResourceSpecs = dependentResourceSpecs;
+    this.explicitInvocation = explicitInvocation;
   }
 
   public List<DependentResourceSpec> getDependentResourceSpecs() {
     return dependentResourceSpecs;
   }
+
+  public boolean isExplicitInvocation() {
+    return explicitInvocation;
+  }
 }

Diff for: operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Context.java

@@ -8,7 +8,7 @@
 import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.fabric8.kubernetes.client.KubernetesClient;
 import io.javaoperatorsdk.operator.api.config.ControllerConfiguration;
-import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.ManagedDependentResourceContext;
+import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.ManagedWorkflowAndDependentResourceContext;
 import io.javaoperatorsdk.operator.processing.event.EventSourceRetriever;
 import io.javaoperatorsdk.operator.processing.event.source.IndexerResourceCache;
 
@@ -34,7 +34,14 @@ <R> Optional<R> getSecondaryResource(Class<R> expectedType,
 
   ControllerConfiguration<P> getControllerConfiguration();
 
-  ManagedDependentResourceContext managedDependentResourceContext();
+  /**
+   * Retrieve the {@link ManagedWorkflowAndDependentResourceContext} used to interact with
+   * {@link io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource}s and associated
+   * {@link io.javaoperatorsdk.operator.processing.dependent.workflow.Workflow}
+   *
+   * @return the {@link ManagedWorkflowAndDependentResourceContext}
+   */
+  ManagedWorkflowAndDependentResourceContext managedWorkflowAndDependentResourceContext();
 
   EventSourceRetriever<P> eventSourceRetriever();
 

Diff for: operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java

@@ -9,8 +9,8 @@
 import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.fabric8.kubernetes.client.KubernetesClient;
 import io.javaoperatorsdk.operator.api.config.ControllerConfiguration;
-import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.DefaultManagedDependentResourceContext;
-import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.ManagedDependentResourceContext;
+import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.DefaultManagedWorkflowAndDependentResourceContext;
+import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.ManagedWorkflowAndDependentResourceContext;
 import io.javaoperatorsdk.operator.processing.Controller;
 import io.javaoperatorsdk.operator.processing.event.EventSourceRetriever;
 import io.javaoperatorsdk.operator.processing.event.ResourceID;
@@ -21,14 +21,15 @@ public class DefaultContext<P extends HasMetadata> implements Context<P> {
   private final Controller<P> controller;
   private final P primaryResource;
   private final ControllerConfiguration<P> controllerConfiguration;
-  private final DefaultManagedDependentResourceContext defaultManagedDependentResourceContext;
+  private final DefaultManagedWorkflowAndDependentResourceContext<P> defaultManagedDependentResourceContext;
 
   public DefaultContext(RetryInfo retryInfo, Controller<P> controller, P primaryResource) {
     this.retryInfo = retryInfo;
     this.controller = controller;
     this.primaryResource = primaryResource;
     this.controllerConfiguration = controller.getConfiguration();
-    this.defaultManagedDependentResourceContext = new DefaultManagedDependentResourceContext();
+    this.defaultManagedDependentResourceContext =
+        new DefaultManagedWorkflowAndDependentResourceContext<>(controller, primaryResource, this);
   }
 
   @Override
@@ -79,7 +80,7 @@ public ControllerConfiguration<P> getControllerConfiguration() {
   }
 
   @Override
-  public ManagedDependentResourceContext managedDependentResourceContext() {
+  public ManagedWorkflowAndDependentResourceContext managedWorkflowAndDependentResourceContext() {
     return defaultManagedDependentResourceContext;
   }
 

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

@@ -2,6 +2,7 @@
 
 import java.lang.annotation.*;
 
+import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent;
 
 @Inherited
@@ -11,4 +12,11 @@
 
   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.
+   */
+  boolean explicitInvocation() default false;
+
 }

Diff for: operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/managed/DefaultManagedDependentResourceContext.java renamed to operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/managed/DefaultManagedWorkflowAndDependentResourceContext.java

@@ -3,14 +3,30 @@
 import java.util.Optional;
 import java.util.concurrent.ConcurrentHashMap;
 
+import io.fabric8.kubernetes.api.model.HasMetadata;
+import io.javaoperatorsdk.operator.api.reconciler.Context;
+import io.javaoperatorsdk.operator.processing.Controller;
 import io.javaoperatorsdk.operator.processing.dependent.workflow.WorkflowCleanupResult;
 import io.javaoperatorsdk.operator.processing.dependent.workflow.WorkflowReconcileResult;
 
 @SuppressWarnings("rawtypes")
-public class DefaultManagedDependentResourceContext implements ManagedDependentResourceContext {
-  public static final Object RECONCILE_RESULT_KEY = new Object();
-  public static final Object CLEANUP_RESULT_KEY = new Object();
+public class DefaultManagedWorkflowAndDependentResourceContext<P extends HasMetadata>
+    implements ManagedWorkflowAndDependentResourceContext {
+
+    public static final Object RECONCILE_RESULT_KEY = new Object();
+    public static final Object CLEANUP_RESULT_KEY = new Object();
   private final ConcurrentHashMap attributes = new ConcurrentHashMap();
+  private final Controller<P> controller;
+  private final P primaryResource;
+  private final Context<P> context;
+
+  public DefaultManagedWorkflowAndDependentResourceContext(Controller<P> controller,
+      P primaryResource,
+      Context<P> context) {
+    this.controller = controller;
+    this.primaryResource = primaryResource;
+    this.context = context;
+  }
 
   @Override
   public <T> Optional<T> get(Object key, Class<T> expectedType) {
@@ -45,4 +61,21 @@ public Optional<WorkflowReconcileResult> getWorkflowReconcileResult() {
   public Optional<WorkflowCleanupResult> getWorkflowCleanupResult() {
     return get(CLEANUP_RESULT_KEY, WorkflowCleanupResult.class);
   }
+
+  @Override
+  public void reconcileManagedWorkflow() {
+    if (!controller.isWorkflowExplicitInvocation()) {
+      throw new IllegalStateException("Workflow explicit invocation is not set.");
+    }
+    controller.reconcileManagedWorkflow(primaryResource, context);
+  }
+
+  @Override
+  public void cleanupManageWorkflow() {
+    if (!controller.isWorkflowExplicitInvocation()) {
+      throw new IllegalStateException("Workflow explicit invocation is not set.");
+    }
+    controller.cleanupManagedWorkflow(primaryResource, context);
+  }
+
 }

Diff for: operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/managed/ManagedDependentResourceContext.java renamed to operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/managed/ManagedWorkflowAndDependentResourceContext.java

@@ -10,7 +10,7 @@
  * Contextual information related to {@link DependentResource} either to retrieve the actual
  * implementations to interact with them or to pass information between them and/or the reconciler
  */
-public interface ManagedDependentResourceContext {
+public interface ManagedWorkflowAndDependentResourceContext {
 
   /**
    * Retrieve a contextual object, if it exists and is of the specified expected type, associated
@@ -37,7 +37,6 @@ public interface ManagedDependentResourceContext {
    * @return an Optional containing the previous value associated with the key or
    *         {@link Optional#empty()} if none existed
    */
-  @SuppressWarnings("unchecked")
   <T> T put(Object key, T value);
 
   /**
@@ -54,5 +53,25 @@ public interface ManagedDependentResourceContext {
 
   WorkflowReconcileResult getWorkflowReconcileResult();
 
+  @SuppressWarnings("unused")
   WorkflowCleanupResult getWorkflowCleanupResult();
+
+  /**
+   * Explicitly reconcile the declared workflow for the associated
+   * {@link io.javaoperatorsdk.operator.api.reconciler.Reconciler}
+   *
+   * @throws IllegalStateException if called when explicit invocation is not requested
+   */
+  void reconcileManagedWorkflow();
+
+  /**
+   * Explicitly clean-up dependent resources in the declared workflow for the associated
+   * {@link io.javaoperatorsdk.operator.api.reconciler.Reconciler}. Note that calling this method is
+   * only needed if the associated reconciler implements the
+   * {@link io.javaoperatorsdk.operator.api.reconciler.Cleaner} interface.
+   *
+   * @throws IllegalStateException if called when explicit invocation is not requested
+   */
+  void cleanupManageWorkflow();
+
 }

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

@@ -1,6 +1,11 @@
 package io.javaoperatorsdk.operator.processing;
 
-import java.util.*;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -18,9 +23,18 @@
 import io.javaoperatorsdk.operator.RegisteredController;
 import io.javaoperatorsdk.operator.api.config.ControllerConfiguration;
 import io.javaoperatorsdk.operator.api.config.ExecutorServiceManager;
+import io.javaoperatorsdk.operator.api.config.workflow.WorkflowSpec;
 import io.javaoperatorsdk.operator.api.monitoring.Metrics;
 import io.javaoperatorsdk.operator.api.monitoring.Metrics.ControllerExecution;
-import io.javaoperatorsdk.operator.api.reconciler.*;
+import io.javaoperatorsdk.operator.api.reconciler.Cleaner;
+import io.javaoperatorsdk.operator.api.reconciler.Constants;
+import io.javaoperatorsdk.operator.api.reconciler.Context;
+import io.javaoperatorsdk.operator.api.reconciler.ContextInitializer;
+import io.javaoperatorsdk.operator.api.reconciler.DeleteControl;
+import io.javaoperatorsdk.operator.api.reconciler.EventSourceContext;
+import io.javaoperatorsdk.operator.api.reconciler.Ignore;
+import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
+import io.javaoperatorsdk.operator.api.reconciler.UpdateControl;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.EventSourceNotFoundException;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.EventSourceProvider;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.EventSourceReferencer;
@@ -129,9 +143,11 @@ public Map<String, Object> metadata() {
           @Override
           public UpdateControl<P> execute() throws Exception {
             initContextIfNeeded(resource, context);
-            if (!managedWorkflow.isEmpty()) {
-              managedWorkflow.reconcile(resource, context);
-            }
+            configuration.getWorkflowSpec().ifPresent(ws -> {
+              if (!managedWorkflow.isEmpty() && !isWorkflowExplicitInvocation()) {
+                  managedWorkflow.reconcile(resource, context);
+              }
+            });
             return reconciler.reconcile(resource, context);
           }
         });
@@ -171,9 +187,12 @@ public Map<String, Object> metadata() {
             public DeleteControl execute() {
               initContextIfNeeded(resource, context);
               WorkflowCleanupResult workflowCleanupResult = null;
-              if (managedWorkflow.hasCleaner()) {
+
+              // The cleanup is called also when explicit invocation is true, but the cleaner is not implemented
+              if (managedWorkflow.hasCleaner() || !isWorkflowExplicitInvocation()) {
                 workflowCleanupResult = managedWorkflow.cleanup(resource, context);
               }
+
               if (isCleaner) {
                 var cleanupResult = ((Cleaner<P>) reconciler).cleanup(resource, context);
                 if (!cleanupResult.isRemoveFinalizer()) {
@@ -430,4 +449,32 @@ public ExecutorServiceManager getExecutorServiceManager() {
   public EventSourceContext<P> eventSourceContext() {
     return eventSourceContext;
   }
+
+  public void reconcileManagedWorkflow(P primary, Context<P> context) {
+    if (!managedWorkflow.isEmpty()) {
+      var res = managedWorkflow.reconcile(primary, context);
+      ((DefaultManagedWorkflowAndDependentResourceContext) context
+          .managedWorkflowAndDependentResourceContext())
+          .setWorkflowExecutionResult(res);
+      res.throwAggregateExceptionIfErrorsPresent();
+    }
+  }
+
+  public WorkflowCleanupResult cleanupManagedWorkflow(P resource, Context<P> context) {
+    if (managedWorkflow.hasCleaner()) {
+      var workflowCleanupResult = managedWorkflow.cleanup(resource, context);
+      ((DefaultManagedWorkflowAndDependentResourceContext) context
+          .managedWorkflowAndDependentResourceContext())
+          .setWorkflowCleanupResult(workflowCleanupResult);
+      workflowCleanupResult.throwAggregateExceptionIfErrorsPresent();
+      return workflowCleanupResult;
+    } else {
+      return null;
+    }
+  }
+
+  public boolean isWorkflowExplicitInvocation() {
+    return configuration.getWorkflowSpec().map(WorkflowSpec::isExplicitInvocation)
+        .orElse(false);
+  }
 }

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

@@ -64,7 +64,7 @@ ManagedWorkflow managedWorkflow(DependentResourceSpec... specs) {
     final var configuration = mock(ControllerConfiguration.class);
     final var specList = List.of(specs);
 
-    var ws = new WorkflowSpec(specList);
+    var ws = new WorkflowSpec(specList, false);
     when(configuration.getWorkflowSpec()).thenReturn(Optional.of(ws));
 
     return new BaseConfigurationService().getWorkflowFactory()

Diff for: operator-framework/src/test/java/io/javaoperatorsdk/operator/WorkflowExplicitCleanupIT.java

@@ -0,0 +1,50 @@
+package io.javaoperatorsdk.operator;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.fabric8.kubernetes.api.model.ConfigMap;
+import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
+import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension;
+import io.javaoperatorsdk.operator.sample.workflowexplicitcleanup.WorkflowExplicitCleanupCustomResource;
+import io.javaoperatorsdk.operator.sample.workflowexplicitcleanup.WorkflowExplicitCleanupReconciler;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.awaitility.Awaitility.await;
+
+public class WorkflowExplicitCleanupIT {
+
+  public static final String RESOURCE_NAME = "test1";
+
+  @RegisterExtension
+  LocallyRunOperatorExtension extension =
+      LocallyRunOperatorExtension.builder()
+          .withReconciler(WorkflowExplicitCleanupReconciler.class)
+          .build();
+
+  @Test
+  void workflowInvokedExplicitly() {
+    var res = extension.create(testResource());
+
+    await().untilAsserted(() -> {
+      assertThat(extension.get(ConfigMap.class, RESOURCE_NAME)).isNotNull();
+    });
+
+    extension.delete(res);
+
+    // The ConfigMap is not garbage collected, this tests that even if the cleaner is not
+    // implemented the workflow cleanup still called even if there is explicit invocation
+    await().untilAsserted(() -> {
+      assertThat(extension.get(ConfigMap.class, RESOURCE_NAME)).isNull();
+    });
+  }
+
+  WorkflowExplicitCleanupCustomResource testResource() {
+    var res = new WorkflowExplicitCleanupCustomResource();
+    res.setMetadata(new ObjectMetaBuilder()
+        .withName(RESOURCE_NAME)
+        .build());
+    return res;
+  }
+
+}