IT, docs, impl

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

Author: csviri

Diff for: docs/documentation/workflows.md

@@ -338,6 +338,40 @@ and NOT `CRUDKubernetesDependentResource` since otherwise the Kubernetes Garbage
 In other words if a Kubernetes Dependent Resource depends on another dependent resource, it should not implement
 `GargageCollected` interface, otherwise the deletion order won't be guaranteed. 
 
+
+## Explicit Managed Workflow Invocation
+
+Managed workflow is execution before the `reconcile(Resource,Context)` is called. There are certain situations however
+when before the invocation there is additional validation or other logic needs to be executed or in some cases
+event the workflow execution skipped. For this reason managed workflow explicit invocation is possible.
+
+```java
+
+@Workflow(explicitInvocation = true,
+    dependents = @Dependent(type = ConfigMapDependent.class))
+@ControllerConfiguration
+public class WorkflowExplicitInvocationReconciler
+    implements Reconciler<WorkflowExplicitInvocationCustomResource> {
+    
+  @Override
+  public UpdateControl<WorkflowExplicitInvocationCustomResource> reconcile(
+      WorkflowExplicitInvocationCustomResource resource,
+      Context<WorkflowExplicitInvocationCustomResource> context) {
+    
+     // additional logic before the workflow is executed
+      
+     context.managedWorkflowAndDependentResourceContext().reconcileManagedWorkflow();
+     
+    return UpdateControl.noUpdate();
+  }
+
+```
+
+For `cleanup`, if the `Cleaner` interface is implemented, the `cleanupManageWorkflow()` needs to be called explicitly.
+However, if `Cleaner` interface is not implemented, it will be called implicitly. 
+
+Nothing prevents calling the workflow multiple times in a reconciler, however it is meant to be called at most once.
+
 ## Notes and Caveats
 
 - Delete is almost always called on every resource during the cleanup. However, it might be the case

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,7 @@ <R> Optional<R> getSecondaryResource(Class<R> expectedType,
 
   ControllerConfiguration<P> getControllerConfiguration();
 
-  ManagedDependentResourceContext managedDependentResourceContext();
+  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,15 +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<P> 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<>(controller, primaryResource, this);
+        new DefaultManagedWorkflowAndDependentResourceContext<>(controller, primaryResource, this);
   }
 
   @Override
@@ -80,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

@@ -12,7 +12,6 @@
 
   Dependent[] dependents();
 
-  // todo maybe better naming? "explicitReconciliation" ?
   /**
    * 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)}

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

@@ -10,8 +10,8 @@
 import io.javaoperatorsdk.operator.processing.dependent.workflow.WorkflowReconcileResult;
 
 @SuppressWarnings("rawtypes")
-public class DefaultManagedDependentResourceContext<P extends HasMetadata>
-    implements ManagedDependentResourceContext {
+public class DefaultManagedWorkflowAndDependentResourceContext<P extends HasMetadata>
+    implements ManagedWorkflowAndDependentResourceContext {
 
   private final ConcurrentHashMap attributes = new ConcurrentHashMap();
   private final Controller<P> controller;
@@ -20,7 +20,7 @@ public class DefaultManagedDependentResourceContext<P extends HasMetadata>
   private WorkflowReconcileResult workflowReconcileResult;
   private WorkflowCleanupResult workflowCleanupResult;
 
-  public DefaultManagedDependentResourceContext(Controller<P> controller,
+  public DefaultManagedWorkflowAndDependentResourceContext(Controller<P> controller,
       P primaryResource,
       Context<P> context) {
     this.controller = controller;
@@ -52,13 +52,13 @@ public <T> T getMandatory(Object key, Class<T> expectedType) {
             + ") is missing or not of the expected type"));
   }
 
-  public DefaultManagedDependentResourceContext setWorkflowExecutionResult(
+  public DefaultManagedWorkflowAndDependentResourceContext setWorkflowExecutionResult(
       WorkflowReconcileResult workflowReconcileResult) {
     this.workflowReconcileResult = workflowReconcileResult;
     return this;
   }
 
-  public DefaultManagedDependentResourceContext setWorkflowCleanupResult(
+  public DefaultManagedWorkflowAndDependentResourceContext setWorkflowCleanupResult(
       WorkflowCleanupResult workflowCleanupResult) {
     this.workflowCleanupResult = workflowCleanupResult;
     return this;
@@ -75,8 +75,19 @@ public WorkflowCleanupResult getWorkflowCleanupResult() {
   }
 
   @Override
-  public void invokeWorkflow() {
-    controller.invokeManagedWorkflow(primaryResource, context);
+  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
@@ -56,6 +56,8 @@ public interface ManagedDependentResourceContext {
 
   WorkflowCleanupResult getWorkflowCleanupResult();
 
-  void invokeWorkflow();
+  void reconcileManagedWorkflow();
+
+  void cleanupManageWorkflow();
 
 }

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

@@ -18,13 +18,14 @@
 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.dependent.EventSourceNotFoundException;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.EventSourceProvider;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.EventSourceReferencer;
-import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.DefaultManagedDependentResourceContext;
+import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.DefaultManagedWorkflowAndDependentResourceContext;
 import io.javaoperatorsdk.operator.health.ControllerHealthInfo;
 import io.javaoperatorsdk.operator.processing.dependent.workflow.Workflow;
 import io.javaoperatorsdk.operator.processing.dependent.workflow.WorkflowCleanupResult;
@@ -131,8 +132,8 @@ public Map<String, Object> metadata() {
           public UpdateControl<P> execute() throws Exception {
             initContextIfNeeded(resource, context);
             configuration.getWorkflowSpec().ifPresent(ws -> {
-              if (!ws.isExplicitInvocation()) {
-                invokeManagedWorkflow(resource, context);
+              if (!isWorkflowExplicitInvocation()) {
+                reconcileManagedWorkflow(resource, context);
               }
             });
             return reconciler.reconcile(resource, context);
@@ -174,12 +175,14 @@ public Map<String, Object> metadata() {
             public DeleteControl execute() {
               initContextIfNeeded(resource, context);
               WorkflowCleanupResult workflowCleanupResult = null;
-              if (managedWorkflow.hasCleaner()) {
-                workflowCleanupResult = managedWorkflow.cleanup(resource, context);
-                ((DefaultManagedDependentResourceContext) context.managedDependentResourceContext())
-                    .setWorkflowCleanupResult(workflowCleanupResult);
-                workflowCleanupResult.throwAggregateExceptionIfErrorsPresent();
+
+              // The cleanup is called also when explicit invocation is true, but the cleaner is not
+              // implemented
+              if (!isCleaner
+                  || !isWorkflowExplicitInvocation()) {
+                workflowCleanupResult = cleanupManagedWorkflow(resource, context);
               }
+
               if (isCleaner) {
                 var cleanupResult = ((Cleaner<P>) reconciler).cleanup(resource, context);
                 if (!cleanupResult.isRemoveFinalizer()) {
@@ -429,12 +432,31 @@ public EventSourceContext<P> eventSourceContext() {
     return eventSourceContext;
   }
 
-  public void invokeManagedWorkflow(P primary, Context<P> context) {
+  public void reconcileManagedWorkflow(P primary, Context<P> context) {
     if (!managedWorkflow.isEmpty()) {
       var res = managedWorkflow.reconcile(primary, context);
-      ((DefaultManagedDependentResourceContext) context.managedDependentResourceContext())
+      ((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/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;
+  }
+
+}

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

@@ -0,0 +1,66 @@
+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.workflowexplicitinvocation.WorkflowExplicitInvocationCustomResource;
+import io.javaoperatorsdk.operator.sample.workflowexplicitinvocation.WorkflowExplicitInvocationReconciler;
+import io.javaoperatorsdk.operator.sample.workflowexplicitinvocation.WorkflowExplicitInvocationSpec;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.awaitility.Awaitility.await;
+
+public class WorkflowExplicitInvocationIT {
+
+  public static final String RESOURCE_NAME = "test1";
+
+  @RegisterExtension
+  LocallyRunOperatorExtension extension =
+      LocallyRunOperatorExtension.builder()
+          .withReconciler(WorkflowExplicitInvocationReconciler.class)
+          .build();
+
+  @Test
+  void workflowInvokedExplicitly() {
+    var res = extension.create(testResource());
+    var reconciler = extension.getReconcilerOfType(WorkflowExplicitInvocationReconciler.class);
+
+    await().untilAsserted(() -> {
+      assertThat(reconciler.getNumberOfExecutions()).isEqualTo(1);
+      assertThat(extension.get(ConfigMap.class, RESOURCE_NAME)).isNull();
+    });
+
+    reconciler.setInvokeWorkflow(true);
+
+    // trigger reconciliation
+    res.getSpec().setValue("changed value");
+    res = extension.replace(res);
+
+    await().untilAsserted(() -> {
+      assertThat(reconciler.getNumberOfExecutions()).isEqualTo(2);
+      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();
+    });
+  }
+
+  WorkflowExplicitInvocationCustomResource testResource() {
+    var res = new WorkflowExplicitInvocationCustomResource();
+    res.setMetadata(new ObjectMetaBuilder()
+        .withName(RESOURCE_NAME)
+        .build());
+    res.setSpec(new WorkflowExplicitInvocationSpec());
+    res.getSpec().setValue("initial value");
+    return res;
+  }
+
+}

Diff for: operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/ManagedBulkDependentWithReadyConditionReconciler.java

@@ -19,7 +19,7 @@ public UpdateControl<BulkDependentTestCustomResource> reconcile(
       Context<BulkDependentTestCustomResource> context) throws Exception {
     numberOfExecutions.incrementAndGet();
 
-    var ready = context.managedDependentResourceContext().getWorkflowReconcileResult()
+    var ready = context.managedWorkflowAndDependentResourceContext().getWorkflowReconcileResult()
         .allDependentResourcesReady();
 
 

Diff for: operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/complexdependent/ComplexDependentReconciler.java

@@ -40,7 +40,7 @@ public class ComplexDependentReconciler implements Reconciler<ComplexDependentCu
   public UpdateControl<ComplexDependentCustomResource> reconcile(
       ComplexDependentCustomResource resource,
       Context<ComplexDependentCustomResource> context) throws Exception {
-    var ready = context.managedDependentResourceContext().getWorkflowReconcileResult()
+    var ready = context.managedWorkflowAndDependentResourceContext().getWorkflowReconcileResult()
         .allDependentResourcesReady();
 
     var status = Objects.requireNonNullElseGet(resource.getStatus(), ComplexDependentStatus::new);