refactor: clean-up WorkflowReconcileResult and Condition (#1388)

Should make it easier to handle Conditions and Filters.

Author: metacosm

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

@@ -22,7 +22,6 @@
 import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource;
-import io.javaoperatorsdk.operator.api.reconciler.dependent.VoidCondition;
 import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependent;
 import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource;
 import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResourceConfig;
@@ -34,10 +33,6 @@
 import io.javaoperatorsdk.operator.processing.event.source.filter.OnAddFilter;
 import io.javaoperatorsdk.operator.processing.event.source.filter.OnDeleteFilter;
 import io.javaoperatorsdk.operator.processing.event.source.filter.OnUpdateFilter;
-import io.javaoperatorsdk.operator.processing.event.source.filter.VoidGenericFilter;
-import io.javaoperatorsdk.operator.processing.event.source.filter.VoidOnAddFilter;
-import io.javaoperatorsdk.operator.processing.event.source.filter.VoidOnDeleteFilter;
-import io.javaoperatorsdk.operator.processing.event.source.filter.VoidOnUpdateFilter;
 import io.javaoperatorsdk.operator.processing.retry.Retry;
 
 import static io.javaoperatorsdk.operator.api.reconciler.Constants.DEFAULT_NAMESPACES_SET;
@@ -46,6 +41,10 @@
 public class AnnotationControllerConfiguration<P extends HasMetadata>
     implements io.javaoperatorsdk.operator.api.config.ControllerConfiguration<P> {
 
+  private static final String CONTROLLER_CONFIG_ANNOTATION =
+      ControllerConfiguration.class.getSimpleName();
+  private static final String KUBE_DEPENDENT_NAME = KubernetesDependent.class.getSimpleName();
+
   protected final Reconciler<P> reconciler;
   private final ControllerConfiguration annotation;
   private List<DependentResourceSpec> specs;
@@ -153,18 +152,19 @@ public Optional<Duration> maxReconciliationInterval() {
   @Override
   public RateLimiter getRateLimiter() {
     final Class<? extends RateLimiter> rateLimiterClass = annotation.rateLimiter();
-    return instantiateAndConfigureIfNeeded(rateLimiterClass, RateLimiter.class);
+    return instantiateAndConfigureIfNeeded(rateLimiterClass, RateLimiter.class,
+        CONTROLLER_CONFIG_ANNOTATION);
   }
 
   @Override
   public Retry getRetry() {
     final Class<? extends Retry> retryClass = annotation.retry();
-    return instantiateAndConfigureIfNeeded(retryClass, Retry.class);
+    return instantiateAndConfigureIfNeeded(retryClass, Retry.class, CONTROLLER_CONFIG_ANNOTATION);
   }
 
   @SuppressWarnings("unchecked")
-  private <T> T instantiateAndConfigureIfNeeded(Class<? extends T> targetClass,
-      Class<T> expectedType) {
+  protected <T> T instantiateAndConfigureIfNeeded(Class<? extends T> targetClass,
+      Class<T> expectedType, String context) {
     try {
       final Constructor<? extends T> constructor = targetClass.getDeclaredConstructor();
       constructor.setAccessible(true);
@@ -184,57 +184,39 @@ private <T> T instantiateAndConfigureIfNeeded(Class<? extends T> targetClass,
         | NoSuchMethodException e) {
       throw new OperatorException("Couldn't instantiate " + expectedType.getSimpleName() + " '"
           + targetClass.getName() + "' for '" + getName()
-          + "' reconciler. You need to provide an accessible no-arg constructor.", e);
+          + "' reconciler in " + context
+          + ". You need to provide an accessible no-arg constructor.", e);
     }
   }
 
   @Override
   @SuppressWarnings("unchecked")
   public Optional<OnAddFilter<P>> onAddFilter() {
-    return (Optional<OnAddFilter<P>>) createFilter(annotation.onAddFilter(), FilterType.onAdd,
-        annotation.getClass().getSimpleName());
-  }
-
-  private enum FilterType {
-    onAdd(VoidOnAddFilter.class), onUpdate(VoidOnUpdateFilter.class), onDelete(
-        VoidOnDeleteFilter.class), generic(VoidGenericFilter.class);
-
-    final Class<?> defaultValue;
-
-    FilterType(Class<?> defaultValue) {
-      this.defaultValue = defaultValue;
-    }
+    return (Optional<OnAddFilter<P>>) createFilter(annotation.onAddFilter(), OnAddFilter.class,
+        CONTROLLER_CONFIG_ANNOTATION);
   }
 
-  private <T> Optional<T> createFilter(Class<T> filter, FilterType filterType, String origin) {
-    if (filterType.defaultValue.equals(filter)) {
+  protected <T> Optional<? extends T> createFilter(Class<? extends T> filter, Class<T> defaultValue,
+      String origin) {
+    if (defaultValue.equals(filter)) {
       return Optional.empty();
     } else {
-      try {
-        var instance = (T) filter.getDeclaredConstructor().newInstance();
-        return Optional.of(instance);
-      } catch (InstantiationException | IllegalAccessException | InvocationTargetException
-          | NoSuchMethodException e) {
-        throw new OperatorException(
-            "Couldn't create " + filterType + " filter from " + filter.getName() + " class in "
-                + origin + " for reconciler " + getName(),
-            e);
-      }
+      return Optional.of(instantiateAndConfigureIfNeeded(filter, defaultValue, origin));
     }
   }
 
   @SuppressWarnings("unchecked")
   @Override
   public Optional<OnUpdateFilter<P>> onUpdateFilter() {
     return (Optional<OnUpdateFilter<P>>) createFilter(annotation.onUpdateFilter(),
-        FilterType.onUpdate, annotation.getClass().getSimpleName());
+        OnUpdateFilter.class, CONTROLLER_CONFIG_ANNOTATION);
   }
 
   @SuppressWarnings("unchecked")
   @Override
   public Optional<GenericFilter<P>> genericFilter() {
     return (Optional<GenericFilter<P>>) createFilter(annotation.genericFilter(),
-        FilterType.generic, annotation.getClass().getSimpleName());
+        GenericFilter.class, CONTROLLER_CONFIG_ANNOTATION);
   }
 
   @SuppressWarnings({"rawtypes", "unchecked"})
@@ -260,13 +242,14 @@ public List<DependentResourceSpec> getDependentResources() {
         var spec = specsMap.get(name);
         if (spec != null) {
           throw new IllegalArgumentException(
-              "A DependentResource named: " + name + " already exists: " + spec);
+              "A DependentResource named '" + name + "' already exists: " + spec);
         }
+        final var context = "DependentResource of type '" + dependentType.getName() + "'";
         spec = new DependentResourceSpec(dependentType, config, name,
             Set.of(dependent.dependsOn()),
-            instantiateConditionIfNotVoid(dependent.readyPostcondition()),
-            instantiateConditionIfNotVoid(dependent.reconcilePrecondition()),
-            instantiateConditionIfNotVoid(dependent.deletePostcondition()));
+            instantiateConditionIfNotDefault(dependent.readyPostcondition(), context),
+            instantiateConditionIfNotDefault(dependent.reconcilePrecondition(), context),
+            instantiateConditionIfNotDefault(dependent.deletePostcondition(), context));
         specsMap.put(name, spec);
       }
 
@@ -275,9 +258,10 @@ public List<DependentResourceSpec> getDependentResources() {
     return specs;
   }
 
-  private Condition<?, ?> instantiateConditionIfNotVoid(Class<? extends Condition> condition) {
-    if (condition != VoidCondition.class) {
-      return instantiateAndConfigureIfNeeded(condition, Condition.class);
+  protected Condition<?, ?> instantiateConditionIfNotDefault(Class<? extends Condition> condition,
+      String context) {
+    if (condition != Condition.class) {
+      return instantiateAndConfigureIfNeeded(condition, Condition.class, context);
     }
     return null;
   }
@@ -313,17 +297,19 @@ private Object createKubernetesResourceConfig(Class<? extends DependentResource>
       final var fromAnnotation = kubeDependent.labelSelector();
       labelSelector = Constants.NO_VALUE_SET.equals(fromAnnotation) ? null : fromAnnotation;
 
-      final var kubeDependentName = KubernetesDependent.class.getSimpleName();
-      onAddFilter = createFilter(kubeDependent.onAddFilter(), FilterType.onAdd, kubeDependentName)
+
+      final var context =
+          KUBE_DEPENDENT_NAME + " annotation on " + dependentType.getName() + " DependentResource";
+      onAddFilter = createFilter(kubeDependent.onAddFilter(), OnAddFilter.class, context)
           .orElse(null);
       onUpdateFilter =
-          createFilter(kubeDependent.onUpdateFilter(), FilterType.onUpdate, kubeDependentName)
+          createFilter(kubeDependent.onUpdateFilter(), OnUpdateFilter.class, context)
               .orElse(null);
       onDeleteFilter =
-          createFilter(kubeDependent.onDeleteFilter(), FilterType.onDelete, kubeDependentName)
+          createFilter(kubeDependent.onDeleteFilter(), OnDeleteFilter.class, context)
               .orElse(null);
       genericFilter =
-          createFilter(kubeDependent.genericFilter(), FilterType.generic, kubeDependentName)
+          createFilter(kubeDependent.genericFilter(), GenericFilter.class, context)
               .orElse(null);
     }
 

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

@@ -6,17 +6,13 @@
 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;
 import io.javaoperatorsdk.operator.processing.event.rate.LinearRateLimiter;
 import io.javaoperatorsdk.operator.processing.event.rate.RateLimiter;
 import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilter;
 import io.javaoperatorsdk.operator.processing.event.source.filter.GenericFilter;
 import io.javaoperatorsdk.operator.processing.event.source.filter.OnAddFilter;
 import io.javaoperatorsdk.operator.processing.event.source.filter.OnUpdateFilter;
-import io.javaoperatorsdk.operator.processing.event.source.filter.VoidGenericFilter;
-import io.javaoperatorsdk.operator.processing.event.source.filter.VoidOnAddFilter;
-import io.javaoperatorsdk.operator.processing.event.source.filter.VoidOnUpdateFilter;
 import io.javaoperatorsdk.operator.processing.retry.GenericRetry;
 import io.javaoperatorsdk.operator.processing.retry.Retry;
 
@@ -79,15 +75,15 @@
   /**
    * Filter of onAdd events of resources.
    **/
-  Class<? extends OnAddFilter<? extends HasMetadata>> onAddFilter() default VoidOnAddFilter.class;
+  Class<? extends OnAddFilter> onAddFilter() default OnAddFilter.class;
 
   /** Filter of onUpdate events of resources. */
-  Class<? extends OnUpdateFilter<? extends HasMetadata>> onUpdateFilter() default VoidOnUpdateFilter.class;
+  Class<? extends OnUpdateFilter> onUpdateFilter() default OnUpdateFilter.class;
 
   /**
    * Filter applied to all operations (add, update, delete). Used to ignore some resources.
    **/
-  Class<? extends GenericFilter<? extends HasMetadata>> genericFilter() default VoidGenericFilter.class;
+  Class<? extends GenericFilter> genericFilter() default GenericFilter.class;
 
   /**
    * Optional configuration of the maximal interval the SDK will wait for a reconciliation request

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

@@ -4,21 +4,57 @@
 
 import static io.javaoperatorsdk.operator.api.reconciler.Constants.NO_VALUE_SET;
 
+/**
+ * The annotation used to create managed {@link DependentResource} associated with a given
+ * {@link io.javaoperatorsdk.operator.api.reconciler.Reconciler}
+ */
 public @interface Dependent {
 
   @SuppressWarnings("rawtypes")
   Class<? extends DependentResource> type();
 
+  /**
+   * The name of this dependent. This is needed to be able to refer to it when creating dependencies
+   * between dependent resources.
+   *
+   * @return the name if it has been set,
+   *         {@link io.javaoperatorsdk.operator.api.reconciler.Constants#NO_VALUE_SET} otherwise
+   */
   String name() default NO_VALUE_SET;
 
-  @SuppressWarnings("rawtypes")
-  Class<? extends Condition> readyPostcondition() default VoidCondition.class;
-
-  @SuppressWarnings("rawtypes")
-  Class<? extends Condition> reconcilePrecondition() default VoidCondition.class;
-
-  @SuppressWarnings("rawtypes")
-  Class<? extends Condition> deletePostcondition() default VoidCondition.class;
-
+  /**
+   * The condition (if it exists) that needs to become true before the workflow can further proceed.
+   *
+   * @return a {@link Condition} implementation, defaulting to the interface itself if no value is
+   *         set
+   */
+  Class<? extends Condition> readyPostcondition() default Condition.class;
+
+  /**
+   * The condition (if it exists) that needs to become true before the associated
+   * {@link DependentResource} is reconciled. Note that if this condition is set and the condition
+   * doesn't hold true, the associated secondary will be deleted.
+   *
+   * @return a {@link Condition} implementation, defaulting to the interface itself if no value is
+   *         set
+   */
+  Class<? extends Condition> reconcilePrecondition() default Condition.class;
+
+  /**
+   * The condition (if it exists) that needs to become true before further reconciliation of
+   * dependents can proceed after the secondary resource associated with this dependent is supposed
+   * to have been deleted.
+   *
+   * @return a {@link Condition} implementation, defaulting to the interface itself if no value is
+   *         set
+   */
+  Class<? extends Condition> deletePostcondition() default Condition.class;
+
+  /**
+   * The list of named dependents that need to be reconciled before this one can be.
+   *
+   * @return the list (possibly empty) of named dependents that need to be reconciled before this
+   *         one can be
+   */
   String[] dependsOn() default {};
 }

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

@@ -5,9 +5,11 @@
 import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
 
-import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.javaoperatorsdk.operator.api.reconciler.Constants;
-import io.javaoperatorsdk.operator.processing.event.source.filter.*;
+import io.javaoperatorsdk.operator.processing.event.source.filter.GenericFilter;
+import io.javaoperatorsdk.operator.processing.event.source.filter.OnAddFilter;
+import io.javaoperatorsdk.operator.processing.event.source.filter.OnDeleteFilter;
+import io.javaoperatorsdk.operator.processing.event.source.filter.OnUpdateFilter;
 
 import static io.javaoperatorsdk.operator.api.reconciler.Constants.NO_VALUE_SET;
 
@@ -35,11 +37,35 @@
    */
   String labelSelector() default NO_VALUE_SET;
 
-  Class<? extends OnAddFilter<? extends HasMetadata>> onAddFilter() default VoidOnAddFilter.class;
+  /**
+   * Optional {@link OnAddFilter} to filter events sent to this KubernetesDependent
+   *
+   * @return the {@link OnAddFilter} filter implementation to use, defaulting to the interface
+   *         itself if no value is set
+   */
+  Class<? extends OnAddFilter> onAddFilter() default OnAddFilter.class;
 
-  Class<? extends OnUpdateFilter<? extends HasMetadata>> onUpdateFilter() default VoidOnUpdateFilter.class;
+  /**
+   * Optional {@link OnUpdateFilter} to filter events sent to this KubernetesDependent
+   *
+   * @return the {@link OnUpdateFilter} filter implementation to use, defaulting to the interface
+   *         itself if no value is set
+   */
+  Class<? extends OnUpdateFilter> onUpdateFilter() default OnUpdateFilter.class;
 
-  Class<? extends OnDeleteFilter<? extends HasMetadata>> onDeleteFilter() default VoidOnDeleteFilter.class;
+  /**
+   * Optional {@link OnDeleteFilter} to filter events sent to this KubernetesDependent
+   *
+   * @return the {@link OnDeleteFilter} filter implementation to use, defaulting to the interface
+   *         itself if no value is set
+   */
+  Class<? extends OnDeleteFilter> onDeleteFilter() default OnDeleteFilter.class;
 
-  Class<? extends GenericFilter<? extends HasMetadata>> genericFilter() default VoidGenericFilter.class;
+  /**
+   * Optional {@link GenericFilter} to filter events sent to this KubernetesDependent
+   *
+   * @return the {@link GenericFilter} filter implementation to use, defaulting to the interface
+   *         itself if no value is set
+   */
+  Class<? extends GenericFilter> genericFilter() default GenericFilter.class;
 }

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependent.java

@@ -2,9 +2,20 @@
 
 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> {
 
-  boolean isMet(DependentResource<R, P> dependentResource, P primary, Context<P> context);
+  /**
+   * Checks whether a condition holds true for a given
+   * {@link io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource} based on the
+   * observed cluster state.
+   *
+   * @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);
 }