refactor: ReconciliationMaxInterval -> MaxReconciliationInterval (#1343)

metacosm · web-flow · commit eb2e5fac2ceb · 2022-07-15T14:03:04.000+02:00
diff --git a/docs/documentation/v3-1-migration.md b/docs/documentation/v3-1-migration.md
@@ -7,6 +7,11 @@ permalink: /docs/v3-1-migration
 
 # Migrating from v3 to v3.1
 
+## ReconciliationMaxInterval Annotation has been renamed to MaxReconciliationInterval
+
+Associated methods on both the `ControllerConfiguration` class and annotation have also been
+renamed accordingly.
+
 ## Workflows Impact on Managed Dependent Resources Behavior
 
 Version 3.1 comes with a workflow engine that replaces the previous behavior of managed dependent
diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java
@@ -30,7 +30,14 @@
 import io.javaoperatorsdk.operator.processing.event.rate.RateLimiter;
 import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilter;
 import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilters;
-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 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;
@@ -135,18 +142,12 @@ public ResourceEventFilter<P> getEventFilter() {
   }
 
   @Override
-  public Optional<Duration> reconciliationMaxInterval() {
-    if (annotation.reconciliationMaxInterval() != null) {
-      if (annotation.reconciliationMaxInterval().interval() <= 0) {
-        return Optional.empty();
-      }
-      return Optional.of(
-          Duration.of(
-              annotation.reconciliationMaxInterval().interval(),
-              annotation.reconciliationMaxInterval().timeUnit().toChronoUnit()));
-    } else {
-      return io.javaoperatorsdk.operator.api.config.ControllerConfiguration.super.reconciliationMaxInterval();
+  public Optional<Duration> maxReconciliationInterval() {
+    final var newConfig = annotation.maxReconciliationInterval();
+    if (newConfig != null && newConfig.interval() > 0) {
+      return Optional.of(Duration.of(newConfig.interval(), newConfig.timeUnit().toChronoUnit()));
     }
+    return Optional.empty();
   }
 
   @Override
diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java
@@ -8,6 +8,7 @@
 import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.javaoperatorsdk.operator.ReconcilerUtils;
 import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec;
+import io.javaoperatorsdk.operator.api.reconciler.MaxReconciliationInterval;
 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;
@@ -18,6 +19,7 @@
 
 public interface ControllerConfiguration<R extends HasMetadata> extends ResourceConfiguration<R> {
 
+  @SuppressWarnings("rawtypes")
   RateLimiter DEFAULT_RATE_LIMITER = LinearRateLimiter.deactivatedRateLimiter();
 
   default String getName() {
@@ -53,6 +55,7 @@ default RetryConfiguration getRetryConfiguration() {
     return RetryConfiguration.DEFAULT;
   }
 
+  @SuppressWarnings("rawtypes")
   default RateLimiter getRateLimiter() {
     return DEFAULT_RATE_LIMITER;
   }
@@ -77,8 +80,8 @@ default List<DependentResourceSpec> getDependentResources() {
     return Collections.emptyList();
   }
 
-  default Optional<Duration> reconciliationMaxInterval() {
-    return Optional.of(Duration.ofHours(10L));
+  default Optional<Duration> maxReconciliationInterval() {
+    return Optional.of(Duration.ofHours(MaxReconciliationInterval.DEFAULT_INTERVAL));
   }
 
   @SuppressWarnings("unused")
diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java
@@ -47,7 +47,7 @@ private ControllerConfigurationOverrider(ControllerConfiguration<R> original) {
     retry = original.getRetry();
     labelSelector = original.getLabelSelector();
     customResourcePredicate = original.getEventFilter();
-    reconciliationMaxInterval = original.reconciliationMaxInterval().orElse(null);
+    reconciliationMaxInterval = original.maxReconciliationInterval().orElse(null);
     // make the original specs modifiable
     final var dependentResources = original.getDependentResources();
     namedDependentResourceSpecs = new LinkedHashMap<>(dependentResources.size());
diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultControllerConfiguration.java
@@ -108,7 +108,7 @@ public List<DependentResourceSpec> getDependentResources() {
   }
 
   @Override
-  public Optional<Duration> reconciliationMaxInterval() {
+  public Optional<Duration> maxReconciliationInterval() {
     return Optional.ofNullable(reconciliationMaxInterval);
   }
 
diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Constants.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Constants.java
@@ -16,7 +16,8 @@ public final class Constants {
   public static final String NO_VALUE_SET = "";
   public static final String WATCH_CURRENT_NAMESPACE = "JOSDK_WATCH_CURRENT";
   public static final String WATCH_ALL_NAMESPACES = "JOSDK_ALL_NAMESPACES";
-  public static final long NO_RECONCILIATION_MAX_INTERVAL = -1L;
+
+  public static final long NO_MAX_RECONCILIATION_INTERVAL = -1L;
   public static final String SAME_AS_CONTROLLER = "JOSDK_SAME_AS_CONTROLLER";
 
   public static final String RESOURCE_GVK_KEY = "josdk.resource.gvk";
diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ControllerConfiguration.java
@@ -91,10 +91,10 @@
    * Optional configuration of the maximal interval the SDK will wait for a reconciliation request
    * to happen before one will be automatically triggered.
    *
-   * @return the maximal interval configuration
+   * @return the maximal reconciliation interval configuration
    */
-  ReconciliationMaxInterval reconciliationMaxInterval() default @ReconciliationMaxInterval(
-      interval = ReconciliationMaxInterval.DEFAULT_INTERVAL);
+  MaxReconciliationInterval maxReconciliationInterval() default @MaxReconciliationInterval(
+      interval = MaxReconciliationInterval.DEFAULT_INTERVAL);
 
 
   /**
diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/MaxReconciliationInterval.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/MaxReconciliationInterval.java
@@ -8,7 +8,8 @@
 
 @Retention(RetentionPolicy.RUNTIME)
 @Target({ElementType.TYPE})
-public @interface ReconciliationMaxInterval {
+public @interface MaxReconciliationInterval {
+
   long DEFAULT_INTERVAL = 10;
 
   /**
@@ -23,7 +24,7 @@
    * This is a fail-safe feature, in the sense that if informers are in place and the reconciler
    * implementation is correct, this feature can be turned off.
    * <p>
-   * Use NO_RECONCILIATION_MAX_INTERVAL in {@link Constants} to turn off this feature.
+   * Use {@link Constants#NO_MAX_RECONCILIATION_INTERVAL} to turn off this feature.
    *
    * @return max delay between reconciliations
    **/
diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java
@@ -270,7 +270,7 @@ private void updatePostExecutionControlWithReschedule(
       PostExecutionControl<R> postExecutionControl,
       BaseControl<?> baseControl) {
     baseControl.getScheduleDelay().ifPresentOrElse(postExecutionControl::withReSchedule,
-        () -> controller.getConfiguration().reconciliationMaxInterval()
+        () -> controller.getConfiguration().maxReconciliationInterval()
             .ifPresent(m -> postExecutionControl.withReSchedule(m.toMillis())));
   }
 
diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java
@@ -111,7 +111,7 @@ private <R extends HasMetadata> ReconciliationDispatcher<R> init(R customResourc
     when(configuration.getName()).thenReturn("EventDispatcherTestController");
     when(configuration.getResourceClass()).thenReturn(resourceClass);
     when(configuration.getRetry()).thenReturn(new GenericRetry());
-    when(configuration.reconciliationMaxInterval())
+    when(configuration.maxReconciliationInterval())
         .thenReturn(Optional.of(Duration.ofHours(RECONCILIATION_MAX_INTERVAL)));
 
     Controller<R> controller = new Controller<>(reconciler, configuration,
@@ -618,7 +618,7 @@ void canSkipSchedulingMaxDelayIf() {
     testCustomResource.addFinalizer(DEFAULT_FINALIZER);
 
     reconciler.reconcile = (r, c) -> UpdateControl.noUpdate();
-    when(reconciliationDispatcher.configuration().reconciliationMaxInterval())
+    when(reconciliationDispatcher.configuration().maxReconciliationInterval())
         .thenReturn(Optional.empty());
 
     PostExecutionControl control =
diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/AnnotationControllerConfigurationTest.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/AnnotationControllerConfigurationTest.java
@@ -8,6 +8,7 @@
 import java.util.List;
 import java.util.Optional;
 import java.util.Set;
+import java.util.concurrent.TimeUnit;
 
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
@@ -18,6 +19,7 @@
 import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec;
 import io.javaoperatorsdk.operator.api.reconciler.Context;
 import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration;
+import io.javaoperatorsdk.operator.api.reconciler.MaxReconciliationInterval;
 import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
 import io.javaoperatorsdk.operator.api.reconciler.UpdateControl;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent;
@@ -129,6 +131,11 @@ && findByNameOptional(dependents, DependentResource.defaultNameFor(ReadOnlyDepen
             .isPresent());
   }
 
+  @Test
+  void maxIntervalCanBeConfigured() {
+    var config = new AnnotationControllerConfiguration<>(new MaxIntervalReconciler());
+    assertEquals(50, config.maxReconciliationInterval().map(Duration::getSeconds).orElseThrow());
+  }
 
   @Test
   void checkDefaultRateAndRetryConfigurations() {
@@ -169,6 +176,18 @@ void checkingRetryingGraduallyWorks() {
     assertEquals(CheckRetryingGraduallyConfiguration.MAX_INTERVAL, genericRetry.getMaxInterval());
   }
 
+  @ControllerConfiguration(
+      maxReconciliationInterval = @MaxReconciliationInterval(interval = 50,
+          timeUnit = TimeUnit.SECONDS))
+  private static class MaxIntervalReconciler implements Reconciler<ConfigMap> {
+
+    @Override
+    public UpdateControl<ConfigMap> reconcile(ConfigMap resource, Context<ConfigMap> context)
+        throws Exception {
+      return null;
+    }
+  }
+
   @ControllerConfiguration(namespaces = OneDepReconciler.CONFIGURED_NS,
       dependents = @Dependent(type = ReadOnlyDependent.class))
   private static class OneDepReconciler implements Reconciler<ConfigMap> {
diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/maxinterval/MaxIntervalTestReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/maxinterval/MaxIntervalTestReconciler.java
@@ -3,10 +3,14 @@
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
 
-import io.javaoperatorsdk.operator.api.reconciler.*;
+import io.javaoperatorsdk.operator.api.reconciler.Context;
+import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration;
+import io.javaoperatorsdk.operator.api.reconciler.MaxReconciliationInterval;
+import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
+import io.javaoperatorsdk.operator.api.reconciler.UpdateControl;
 import io.javaoperatorsdk.operator.support.TestExecutionInfoProvider;
 
-@ControllerConfiguration(reconciliationMaxInterval = @ReconciliationMaxInterval(interval = 50,
+@ControllerConfiguration(maxReconciliationInterval = @MaxReconciliationInterval(interval = 50,
     timeUnit = TimeUnit.MILLISECONDS))
 public class MaxIntervalTestReconciler
     implements Reconciler<MaxIntervalTestCustomResource>, TestExecutionInfoProvider {

Original file line number	Diff line number	Diff line change
`@@ -108,7 +108,7 @@ public List<DependentResourceSpec> getDependentResources() {`
`108`	`108`	`}`
`109`	`109`
`110`	`110`	`@Override`
`111`		`- public Optional<Duration> reconciliationMaxInterval() {`
	`111`	`+ public Optional<Duration> maxReconciliationInterval() {`
`112`	`112`	`return Optional.ofNullable(reconciliationMaxInterval);`
`113`	`113`	`}`
`114`	`114`
Original file line number	Diff line number	Diff line change
`@@ -270,7 +270,7 @@ private void updatePostExecutionControlWithReschedule(`
`270`	`270`	`PostExecutionControl<R> postExecutionControl,`
`271`	`271`	`BaseControl<?> baseControl) {`
`272`	`272`	`baseControl.getScheduleDelay().ifPresentOrElse(postExecutionControl::withReSchedule,`
`273`		`- () -> controller.getConfiguration().reconciliationMaxInterval()`
	`273`	`+ () -> controller.getConfiguration().maxReconciliationInterval()`
`274`	`274`	`.ifPresent(m -> postExecutionControl.withReSchedule(m.toMillis())));`
`275`	`275`	`}`
`276`	`276`