feat: dependent resource in the condition instead of resource (#1690)

Author: csviri

docs/documentation/v4-3-migration.md

@@ -0,0 +1,36 @@
+---
+title: Migrating from v4.2 to v4.3
+description: Migrating from v4.2 to v4.3
+layout: docs
+permalink: /docs/v4-3-migration
+---
+
+# Migrating from v4.2 to v4.3
+
+## Condition API Change
+
+In Workflows the target of the condition was the managed resource itself, not a dependent resource. This changed, from
+not the API contains the dependent resource.
+
+New API:
+
+```java
+public interface Condition<R, P extends HasMetadata> {
+    
+  boolean isMet(DependentResource<R, P> dependentResource, P primary, Context<P> context);
+  
+}
+```
+
+Former API:
+
+```java
+public interface Condition<R, P extends HasMetadata> {
+
+  boolean isMet(P primary, R secondary, Context<P> context);
+  
+}
+```
+
+Migration is trivial. Since the secondary resource can be accessed from the dependent resource. So to access the secondary
+resource just use `dependentResource.getSecondaryResource(primary,context)`.

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

@@ -14,7 +14,6 @@
 import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.javaoperatorsdk.operator.api.reconciler.Context;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
-import io.javaoperatorsdk.operator.processing.dependent.BulkDependentResource;
 
 @SuppressWarnings("rawtypes")
 public abstract class AbstractWorkflowExecutor<P extends HasMetadata> {
@@ -103,15 +102,7 @@ protected synchronized void handleNodeExecutionFinish(
   @SuppressWarnings("unchecked")
   protected <R> boolean isConditionMet(Optional<Condition<R, P>> condition,
       DependentResource<R, P> dependentResource) {
-    if (condition.isEmpty()) {
-      return true;
-    }
-    var resources = dependentResource instanceof BulkDependentResource
-        ? ((BulkDependentResource) dependentResource).getSecondaryResources(primary, context)
-        : dependentResource.getSecondaryResource(primary, context).orElse(null);
-
-    return condition.map(c -> c.isMet(primary,
-        (R) resources, context))
+    return condition.map(c -> c.isMet(dependentResource, primary, context))
         .orElse(true);
   }
 }

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

@@ -2,6 +2,7 @@
 
 import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.javaoperatorsdk.operator.api.reconciler.Context;
+import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
 
 public interface Condition<R, P extends HasMetadata> {
 
@@ -10,12 +11,10 @@ public interface Condition<R, P extends HasMetadata> {
    * {@link io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource} based on the
    * observed cluster state.
    *
+   * @param dependentResource for which the condition applies to
    * @param primary the primary resource being considered
-   * @param secondary the secondary resource associated with the specified primary resource or
-   *        {@code null} if no such secondary resource exists (for example because it's been
-   *        deleted)
    * @param context the current reconciliation {@link Context}
    * @return {@code true} if the condition holds, {@code false} otherwise
    */
-  boolean isMet(P primary, R secondary, Context<P> context);
+  boolean isMet(DependentResource<R, P> dependentResource, P primary, Context<P> context);
 }

operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverriderTest.java

@@ -362,7 +362,9 @@ public UpdateControl<ConfigMap> reconcile(ConfigMap resource, Context<ConfigMap>
   private static class TestCondition implements Condition<ConfigMap, ConfigMap> {
 
     @Override
-    public boolean isMet(ConfigMap primary, ConfigMap secondary, Context<ConfigMap> context) {
+    public boolean isMet(DependentResource<ConfigMap, ConfigMap> dependentResource,
+        ConfigMap primary,
+        Context<ConfigMap> context) {
       return true;
     }
   }

operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/SampleBulkCondition.java

@@ -1,21 +1,25 @@
 package io.javaoperatorsdk.operator.sample.bulkdependent;
 
-import java.util.Map;
-
 import io.fabric8.kubernetes.api.model.ConfigMap;
 import io.javaoperatorsdk.operator.api.reconciler.Context;
+import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
 import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition;
 
 public class SampleBulkCondition
-    implements Condition<Map<String, ConfigMap>, BulkDependentTestCustomResource> {
+    implements Condition<ConfigMap, BulkDependentTestCustomResource> {
 
   // We use ConfigMaps here just to show how to check some properties of resources managed by a
   // BulkDependentResource. In real life example this would be rather based on some status of those
   // resources, like Pods.
 
   @Override
-  public boolean isMet(BulkDependentTestCustomResource primary, Map<String, ConfigMap> secondary,
+  public boolean isMet(
+      DependentResource<ConfigMap, BulkDependentTestCustomResource> dependentResource,
+      BulkDependentTestCustomResource primary,
       Context<BulkDependentTestCustomResource> context) {
-    return secondary.values().stream().noneMatch(cm -> cm.getData().isEmpty());
+
+    var resources = ((CRUDConfigMapBulkDependentResource) dependentResource)
+        .getSecondaryResources(primary, context);
+    return resources.values().stream().noneMatch(cm -> cm.getData().isEmpty());
   }
 }

operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/complexdependent/dependent/StatefulSetReadyCondition.java

@@ -2,17 +2,23 @@
 
 import io.fabric8.kubernetes.api.model.apps.StatefulSet;
 import io.javaoperatorsdk.operator.api.reconciler.Context;
+import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
 import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition;
 import io.javaoperatorsdk.operator.sample.complexdependent.ComplexDependentCustomResource;
 
 public class StatefulSetReadyCondition
     implements Condition<StatefulSet, ComplexDependentCustomResource> {
 
   @Override
-  public boolean isMet(ComplexDependentCustomResource primary, StatefulSet secondary,
+  public boolean isMet(
+      DependentResource<StatefulSet, ComplexDependentCustomResource> dependentResource,
+      ComplexDependentCustomResource primary,
       Context<ComplexDependentCustomResource> context) {
 
-    var readyReplicas = secondary.getStatus().getReadyReplicas();
-    return readyReplicas != null && readyReplicas > 0;
+    return dependentResource.getSecondaryResource(primary, context).map(secondary -> {
+      var readyReplicas = secondary.getStatus().getReadyReplicas();
+      return readyReplicas != null && readyReplicas > 0;
+    })
+        .orElse(false);
   }
 }

operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/ConfigMapDeletePostCondition.java

@@ -5,6 +5,7 @@
 
 import io.fabric8.kubernetes.api.model.ConfigMap;
 import io.javaoperatorsdk.operator.api.reconciler.Context;
+import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
 import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition;
 
 public class ConfigMapDeletePostCondition
@@ -14,9 +15,11 @@ public class ConfigMapDeletePostCondition
 
   @Override
   public boolean isMet(
-      WorkflowAllFeatureCustomResource primary, ConfigMap secondary,
+      DependentResource<ConfigMap, WorkflowAllFeatureCustomResource> dependentResource,
+      WorkflowAllFeatureCustomResource primary,
       Context<WorkflowAllFeatureCustomResource> context) {
-    var configMapDeleted = secondary == null;
+
+    var configMapDeleted = dependentResource.getSecondaryResource(primary, context).isEmpty();
     log.debug("Config Map Deleted: {}", configMapDeleted);
     return configMapDeleted;
   }

operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/ConfigMapReconcileCondition.java

@@ -2,14 +2,18 @@
 
 import io.fabric8.kubernetes.api.model.ConfigMap;
 import io.javaoperatorsdk.operator.api.reconciler.Context;
+import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
 import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition;
 
 public class ConfigMapReconcileCondition
     implements Condition<ConfigMap, WorkflowAllFeatureCustomResource> {
 
   @Override
-  public boolean isMet(WorkflowAllFeatureCustomResource primary, ConfigMap secondary,
+  public boolean isMet(
+      DependentResource<ConfigMap, WorkflowAllFeatureCustomResource> dependentResource,
+      WorkflowAllFeatureCustomResource primary,
       Context<WorkflowAllFeatureCustomResource> context) {
+
     return primary.getSpec().isCreateConfigMap();
   }
 }

operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/workflowallfeature/DeploymentReadyCondition.java

@@ -2,18 +2,21 @@
 
 import io.fabric8.kubernetes.api.model.apps.Deployment;
 import io.javaoperatorsdk.operator.api.reconciler.Context;
+import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
 import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition;
 
 public class DeploymentReadyCondition
     implements Condition<Deployment, WorkflowAllFeatureCustomResource> {
   @Override
-  public boolean isMet(WorkflowAllFeatureCustomResource primary, Deployment deployment,
+  public boolean isMet(
+      DependentResource<Deployment, WorkflowAllFeatureCustomResource> dependentResource,
+      WorkflowAllFeatureCustomResource primary,
       Context<WorkflowAllFeatureCustomResource> context) {
-    if (deployment == null) {
-      return false;
-    }
-    var readyReplicas = deployment.getStatus().getReadyReplicas();
-
-    return readyReplicas != null && deployment.getSpec().getReplicas().equals(readyReplicas);
+    return dependentResource.getSecondaryResource(primary, context)
+        .map(deployment -> {
+          var readyReplicas = deployment.getStatus().getReadyReplicas();
+          return readyReplicas != null && deployment.getSpec().getReplicas().equals(readyReplicas);
+        })
+        .orElse(false);
   }
 }

sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/ExposedIngressCondition.java

@@ -2,11 +2,14 @@
 
 import io.fabric8.kubernetes.api.model.networking.v1.Ingress;
 import io.javaoperatorsdk.operator.api.reconciler.Context;
+import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
 import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition;
 
 public class ExposedIngressCondition implements Condition<Ingress, WebPage> {
+
   @Override
-  public boolean isMet(WebPage primary, Ingress secondary, Context<WebPage> context) {
+  public boolean isMet(DependentResource<Ingress, WebPage> dependentResource,
+      WebPage primary, Context<WebPage> context) {
     return primary.getSpec().getExposed();
   }
 }