feature: controller reconciliation max delay (#871)

Author: csviri

docs/documentation/features.md

@@ -160,9 +160,7 @@ larger than the `.observedGeneration` field on status. In order to have this fea
   .
 
 If these conditions are fulfilled and generation awareness not turned off, the observed generation is automatically set
-by the framework after the `reconcile` method is called. There is just one exception, when the reconciler returns
-with `UpdateControl.updateResource`, in this case the status is not updated, just the custom resource - however, this
-update will lead to a new reconciliation. Note that the observed generation is updated also
+by the framework after the `reconcile` method is called. Note that the observed generation is updated also
 when `UpdateControl.noUpdate()` is returned from the reconciler. See this feature working in
 the [WebPage example](https://github.com/java-operator-sdk/java-operator-sdk/blob/b91221bb54af19761a617bf18eef381e8ceb3b4c/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageStatus.java#L5)
 .
@@ -224,6 +222,30 @@ public class DeploymentReconciler
   }
 ```
 
+## Max Interval Between Reconciliations
+
+In case informers are all in place and reconciler is implemented correctly, there is no need for additional triggers. 
+However, it's a [common practice](https://github.com/java-operator-sdk/java-operator-sdk/issues/848#issuecomment-1016419966) 
+to have a failsafe periodic trigger in place,
+just to make sure the resources are reconciled after certain time. This functionality is in place by default, there
+is quite high interval (currently 10 hours) while the reconciliation is triggered. See how to override this using 
+the standard annotation:
+
+```java
+@ControllerConfiguration(finalizerName = NO_FINALIZER,
+        reconciliationMaxInterval = @ReconciliationMaxInterval(
+                interval = 50,
+                timeUnit = TimeUnit.MILLISECONDS))
+```
+
+The event is not propagated in a fixed rate, rather it's scheduled after each reconciliation. So the 
+next reconciliation will after at most within the specified interval after last reconciliation.
+
+This feature can be turned off by setting `reconciliationMaxInterval` to [`Constants.NO_RECONCILIATION_MAX_INTERVAL`](https://github.com/java-operator-sdk/java-operator-sdk/blob/442e7d8718e992a36880e42bd0a5c01affaec9df/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Constants.java#L8-L8)
+or any non-positive number.
+
+The automatic retries are not affected by this feature, in case of an error no schedule is set by this feature. 
+
 ## Automatic Retries on Error
 
 When an exception is thrown from a controller, the framework will schedule an automatic retry of the reconciliation. The

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

@@ -1,7 +1,9 @@
 package io.javaoperatorsdk.operator.api.config;
 
 import java.lang.reflect.ParameterizedType;
+import java.time.Duration;
 import java.util.Collections;
+import java.util.Optional;
 import java.util.Set;
 
 import io.fabric8.kubernetes.api.model.HasMetadata;
@@ -114,4 +116,8 @@ default boolean useFinalizer() {
   default ResourceEventFilter<R> getEventFilter() {
     return ResourceEventFilters.passthrough();
   }
+
+  default Optional<Duration> reconciliationMaxInterval() {
+    return Optional.of(Duration.ofHours(10L));
+  }
 }

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

@@ -1,5 +1,6 @@
 package io.javaoperatorsdk.operator.api.config;
 
+import java.time.Duration;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
@@ -16,6 +17,7 @@ public class ControllerConfigurationOverrider<R extends HasMetadata> {
   private String labelSelector;
   private ResourceEventFilter<R> customResourcePredicate;
   private final ControllerConfiguration<R> original;
+  private Duration reconciliationMaxInterval;
 
   private ControllerConfigurationOverrider(ControllerConfiguration<R> original) {
     finalizer = original.getFinalizer();
@@ -24,7 +26,9 @@ private ControllerConfigurationOverrider(ControllerConfiguration<R> original) {
     retry = original.getRetryConfiguration();
     labelSelector = original.getLabelSelector();
     customResourcePredicate = original.getEventFilter();
+    reconciliationMaxInterval = original.reconciliationMaxInterval().orElse(null);
     this.original = original;
+
   }
 
   public ControllerConfigurationOverrider<R> withFinalizer(String finalizer) {
@@ -74,6 +78,12 @@ public ControllerConfigurationOverrider<R> withCustomResourcePredicate(
     return this;
   }
 
+  public ControllerConfigurationOverrider<R> withReconciliationMaxInterval(
+      Duration reconciliationMaxInterval) {
+    this.reconciliationMaxInterval = reconciliationMaxInterval;
+    return this;
+  }
+
   public ControllerConfiguration<R> build() {
     return new DefaultControllerConfiguration<>(
         original.getAssociatedReconcilerClassName(),
@@ -86,6 +96,7 @@ public ControllerConfiguration<R> build() {
         labelSelector,
         customResourcePredicate,
         original.getResourceClass(),
+        reconciliationMaxInterval,
         original.getConfigurationService());
   }
 

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

@@ -1,6 +1,8 @@
 package io.javaoperatorsdk.operator.api.config;
 
+import java.time.Duration;
 import java.util.Collections;
+import java.util.Optional;
 import java.util.Set;
 
 import io.fabric8.kubernetes.api.model.HasMetadata;
@@ -20,6 +22,7 @@ public class DefaultControllerConfiguration<R extends HasMetadata>
   private final String labelSelector;
   private final ResourceEventFilter<R> resourceEventFilter;
   private final Class<R> resourceClass;
+  private final Duration reconciliationMaxInterval;
   private ConfigurationService service;
 
   public DefaultControllerConfiguration(
@@ -33,6 +36,7 @@ public DefaultControllerConfiguration(
       String labelSelector,
       ResourceEventFilter<R> resourceEventFilter,
       Class<R> resourceClass,
+      Duration reconciliationMaxInterval,
       ConfigurationService service) {
     this.associatedControllerClassName = associatedControllerClassName;
     this.name = name;
@@ -41,6 +45,7 @@ public DefaultControllerConfiguration(
     this.generationAware = generationAware;
     this.namespaces =
         namespaces != null ? Collections.unmodifiableSet(namespaces) : Collections.emptySet();
+    this.reconciliationMaxInterval = reconciliationMaxInterval;
     this.watchAllNamespaces = this.namespaces.isEmpty();
     this.retryConfiguration =
         retryConfiguration == null
@@ -122,4 +127,9 @@ public Class<R> getResourceClass() {
   public ResourceEventFilter<R> getEventFilter() {
     return resourceEventFilter;
   }
+
+  @Override
+  public Optional<Duration> reconciliationMaxInterval() {
+    return Optional.ofNullable(reconciliationMaxInterval);
+  }
 }

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

@@ -5,6 +5,7 @@ public final class Constants {
   public static final String EMPTY_STRING = "";
   public static final String WATCH_CURRENT_NAMESPACE = "JOSDK_WATCH_CURRENT";
   public static final String NO_FINALIZER = "JOSDK_NO_FINALIZER";
+  public static final long NO_RECONCILIATION_MAX_INTERVAL = -1L;
 
   private Constants() {}
 }

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

@@ -4,6 +4,7 @@
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
+import java.util.concurrent.TimeUnit;
 
 import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilter;
 
@@ -56,4 +57,8 @@
    */
   @SuppressWarnings("rawtypes")
   Class<? extends ResourceEventFilter>[] eventFilters() default {};
+
+  ReconciliationMaxInterval reconciliationMaxInterval() default @ReconciliationMaxInterval(
+      interval = 10, timeUnit = TimeUnit.HOURS);
+
 }

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

@@ -0,0 +1,36 @@
+package io.javaoperatorsdk.operator.api.reconciler;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+import java.util.concurrent.TimeUnit;
+
+@Retention(RetentionPolicy.RUNTIME)
+@Target({ElementType.TYPE})
+public @interface ReconciliationMaxInterval {
+
+  /**
+   * A max delay between two reconciliations. Having this value larger than zero, will ensure that a
+   * reconciliation is scheduled with a target interval after the last reconciliation. Note that
+   * this not applies for retries, in case of an exception reconciliation is not scheduled. This is
+   * not a fixed rate, in other words a new reconciliation is scheduled after each reconciliation.
+   * <p/>
+   * If an interval is specified by {@link UpdateControl} or {@link DeleteControl}, those take
+   * precedence.
+   * <p/>
+   * 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.
+   *
+   * @return max delay between reconciliations
+   **/
+  long interval();
+
+  /**
+   * @return time unit for max delay between reconciliations
+   */
+  TimeUnit timeUnit() default TimeUnit.HOURS;
+
+}

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java

@@ -245,9 +245,12 @@ private PostExecutionControl<R> createPostExecutionControl(R updatedCustomResour
   private void updatePostExecutionControlWithReschedule(
       PostExecutionControl<R> postExecutionControl,
       BaseControl<?> baseControl) {
-    baseControl.getScheduleDelay().ifPresent(postExecutionControl::withReSchedule);
+    baseControl.getScheduleDelay().ifPresentOrElse(postExecutionControl::withReSchedule,
+        () -> controller.getConfiguration().reconciliationMaxInterval()
+            .ifPresent(m -> postExecutionControl.withReSchedule(m.toMillis())));
   }
 
+
   private PostExecutionControl<R> handleCleanup(R resource, Context context) {
     log.debug(
         "Executing delete for resource: {} with version: {}",

operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ControllerManagerTest.java

@@ -60,7 +60,8 @@ private static class TestControllerConfiguration<R extends HasMetadata>
 
     public TestControllerConfiguration(Reconciler<R> controller, Class<R> crClass) {
       super(null, getControllerName(controller),
-          CustomResource.getCRDName(crClass), null, false, null, null, null, null, crClass, null);
+          CustomResource.getCRDName(crClass), null, false, null, null, null, null, crClass,
+          null, null);
       this.controller = controller;
     }
 

operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java

@@ -1,5 +1,6 @@
 package io.javaoperatorsdk.operator.processing.event;
 
+import java.time.Duration;
 import java.util.ArrayList;
 import java.util.Optional;
 import java.util.concurrent.TimeUnit;
@@ -47,13 +48,15 @@ class ReconciliationDispatcherTest {
 
   private static final String DEFAULT_FINALIZER = "javaoperatorsdk.io/finalizer";
   public static final String ERROR_MESSAGE = "ErrorMessage";
+  public static final long RECONCILIATION_MAX_INTERVAL = 10L;
   private TestCustomResource testCustomResource;
   private ReconciliationDispatcher<TestCustomResource> reconciliationDispatcher;
   private final Reconciler<TestCustomResource> reconciler = mock(Reconciler.class,
       withSettings().extraInterfaces(ErrorStatusHandler.class));
   private final ConfigurationService configService = mock(ConfigurationService.class);
   private final CustomResourceFacade<TestCustomResource> customResourceFacade =
       mock(ReconciliationDispatcher.CustomResourceFacade.class);
+  private ControllerConfiguration configuration = mock(ControllerConfiguration.class);
 
   @BeforeEach
   void setup() {
@@ -63,17 +66,23 @@ void setup() {
   }
 
   private <R extends HasMetadata> ReconciliationDispatcher<R> init(R customResource,
-      Reconciler<R> reconciler, ControllerConfiguration<R> configuration,
+      Reconciler<R> reconciler, ControllerConfiguration configuration,
       CustomResourceFacade<R> customResourceFacade, boolean useFinalizer) {
+
     configuration = configuration == null ? mock(ControllerConfiguration.class) : configuration;
+    ReconciliationDispatcherTest.this.configuration = configuration;
     final var finalizer = useFinalizer ? DEFAULT_FINALIZER : Constants.NO_FINALIZER;
     when(configuration.getFinalizer()).thenReturn(finalizer);
     when(configuration.useFinalizer()).thenCallRealMethod();
     when(configuration.getName()).thenReturn("EventDispatcherTestController");
     when(configuration.getResourceClass()).thenReturn((Class<R>) customResource.getClass());
     when(configuration.getRetryConfiguration()).thenReturn(RetryConfiguration.DEFAULT);
+    when(configuration.reconciliationMaxInterval())
+        .thenReturn(Optional.of(Duration.ofHours(RECONCILIATION_MAX_INTERVAL)));
+
     when(configuration.getConfigurationService()).thenReturn(configService);
 
+
     /*
      * We need this for mock reconcilers to properly generate the expected UpdateControl: without
      * this, calls such as `when(reconciler.reconcile(eq(testCustomResource),
@@ -429,6 +438,35 @@ void callErrorStatusHandlerEvenOnFirstError() {
         any(), any());
   }
 
+  @Test
+  void schedulesReconciliationIfMaxDelayIsSet() {
+    testCustomResource.addFinalizer(DEFAULT_FINALIZER);
+
+    when(reconciler.reconcile(eq(testCustomResource), any()))
+        .thenReturn(UpdateControl.noUpdate());
+
+    PostExecutionControl control =
+        reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource));
+
+    assertThat(control.getReScheduleDelay()).isPresent()
+        .hasValue(TimeUnit.HOURS.toMillis(RECONCILIATION_MAX_INTERVAL));
+  }
+
+  @Test
+  void canSkipSchedulingMaxDelayIf() {
+    testCustomResource.addFinalizer(DEFAULT_FINALIZER);
+
+    when(reconciler.reconcile(eq(testCustomResource), any()))
+        .thenReturn(UpdateControl.noUpdate());
+    when(configuration.reconciliationMaxInterval())
+        .thenReturn(Optional.empty());
+
+    PostExecutionControl control =
+        reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource));
+
+    assertThat(control.getReScheduleDelay()).isNotPresent();
+  }
+
   private ObservedGenCustomResource createObservedGenCustomResource() {
     ObservedGenCustomResource observedGenCustomResource = new ObservedGenCustomResource();
     observedGenCustomResource.setMetadata(new ObjectMeta());