feat: retry and retry configuration decoupling (#1285)

csviri · csviri · commit 1815525ca51a · 2022-07-14T10:39:24.000+02:00
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
@@ -10,6 +10,8 @@
 import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec;
 import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilter;
 import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilters;
+import io.javaoperatorsdk.operator.processing.retry.GenericRetry;
+import io.javaoperatorsdk.operator.processing.retry.Retry;
 
 public interface ControllerConfiguration<R extends HasMetadata> extends ResourceConfiguration<R> {
 
@@ -27,6 +29,16 @@ default boolean isGenerationAware() {
 
   String getAssociatedReconcilerClassName();
 
+  default Retry getRetry() {
+    return GenericRetry.fromConfiguration(getRetryConfiguration()); // NOSONAR
+  }
+
+  /**
+   * Use getRetry instead.
+   *
+   * @return configuration for retry.
+   */
+  @Deprecated
   default RetryConfiguration getRetryConfiguration() {
     return RetryConfiguration.DEFAULT;
   }
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
@@ -9,6 +9,8 @@
 import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec;
 import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResourceConfig;
 import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilter;
+import io.javaoperatorsdk.operator.processing.retry.GenericRetry;
+import io.javaoperatorsdk.operator.processing.retry.Retry;
 
 import static io.javaoperatorsdk.operator.api.reconciler.Constants.DEFAULT_NAMESPACES_SET;
 import static io.javaoperatorsdk.operator.api.reconciler.Constants.WATCH_CURRENT_NAMESPACE_SET;
@@ -19,7 +21,7 @@ public class ControllerConfigurationOverrider<R extends HasMetadata> {
   private String finalizer;
   private boolean generationAware;
   private Set<String> namespaces;
-  private RetryConfiguration retry;
+  private Retry retry;
   private String labelSelector;
   private ResourceEventFilter<R> customResourcePredicate;
   private final ControllerConfiguration<R> original;
@@ -30,7 +32,7 @@ private ControllerConfigurationOverrider(ControllerConfiguration<R> original) {
     finalizer = original.getFinalizerName();
     generationAware = original.isGenerationAware();
     namespaces = new HashSet<>(original.getNamespaces());
-    retry = original.getRetryConfiguration();
+    retry = original.getRetry();
     labelSelector = original.getLabelSelector();
     customResourcePredicate = original.getEventFilter();
     reconciliationMaxInterval = original.reconciliationMaxInterval().orElse(null);
@@ -90,11 +92,17 @@ public ControllerConfigurationOverrider<R> watchingAllNamespaces() {
     return this;
   }
 
-  public ControllerConfigurationOverrider<R> withRetry(RetryConfiguration retry) {
+  public ControllerConfigurationOverrider<R> withRetry(Retry retry) {
     this.retry = retry;
     return this;
   }
 
+  @Deprecated
+  public ControllerConfigurationOverrider<R> withRetry(RetryConfiguration retry) {
+    this.retry = GenericRetry.fromConfiguration(retry);
+    return this;
+  }
+
   public ControllerConfigurationOverrider<R> withLabelSelector(String labelSelector) {
     this.labelSelector = labelSelector;
     return this;
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
@@ -9,6 +9,7 @@
 import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec;
 import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilter;
+import io.javaoperatorsdk.operator.processing.retry.Retry;
 
 @SuppressWarnings("rawtypes")
 public class DefaultControllerConfiguration<R extends HasMetadata>
@@ -20,7 +21,7 @@ public class DefaultControllerConfiguration<R extends HasMetadata>
   private final String crdName;
   private final String finalizer;
   private final boolean generationAware;
-  private final RetryConfiguration retryConfiguration;
+  private final Retry retry;
   private final ResourceEventFilter<R> resourceEventFilter;
   private final List<DependentResourceSpec> dependents;
   private final Duration reconciliationMaxInterval;
@@ -33,7 +34,7 @@ public DefaultControllerConfiguration(
       String finalizer,
       boolean generationAware,
       Set<String> namespaces,
-      RetryConfiguration retryConfiguration,
+      Retry retry,
       String labelSelector,
       ResourceEventFilter<R> resourceEventFilter,
       Class<R> resourceClass,
@@ -46,10 +47,10 @@ public DefaultControllerConfiguration(
     this.finalizer = finalizer;
     this.generationAware = generationAware;
     this.reconciliationMaxInterval = reconciliationMaxInterval;
-    this.retryConfiguration =
-        retryConfiguration == null
-            ? ControllerConfiguration.super.getRetryConfiguration()
-            : retryConfiguration;
+    this.retry =
+        retry == null
+            ? ControllerConfiguration.super.getRetry()
+            : retry;
     this.resourceEventFilter = resourceEventFilter;
 
     this.dependents = dependents != null ? dependents : Collections.emptyList();
@@ -81,8 +82,8 @@ public String getAssociatedReconcilerClassName() {
   }
 
   @Override
-  public RetryConfiguration getRetryConfiguration() {
-    return retryConfiguration;
+  public Retry getRetry() {
+    return retry;
   }
 
   @Override
diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventProcessor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventProcessor.java
@@ -25,7 +25,6 @@
 import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceAction;
 import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEvent;
 import io.javaoperatorsdk.operator.processing.event.source.timer.TimerEventSource;
-import io.javaoperatorsdk.operator.processing.retry.GenericRetry;
 import io.javaoperatorsdk.operator.processing.retry.Retry;
 import io.javaoperatorsdk.operator.processing.retry.RetryExecution;
 
@@ -54,8 +53,7 @@ class EventProcessor<R extends HasMetadata> implements EventHandler, LifecycleAw
         ExecutorServiceManager.instance().executorService(),
         eventSourceManager.getController().getConfiguration().getName(),
         new ReconciliationDispatcher<>(eventSourceManager.getController()),
-        GenericRetry.fromConfiguration(
-            eventSourceManager.getController().getConfiguration().getRetryConfiguration()),
+        eventSourceManager.getController().getConfiguration().getRetry(),
         ConfigurationServiceProvider.instance().getMetrics(),
         eventSourceManager);
   }
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
@@ -178,7 +178,7 @@ public int getAttemptCount() {
 
           @Override
           public boolean isLastAttempt() {
-            return controller.getConfiguration().getRetryConfiguration() == null;
+            return controller.getConfiguration().getRetry() == null;
           }
         });
         ((DefaultContext<R>) context).setRetryInfo(retryInfo);
diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/GenericRetry.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/GenericRetry.java
@@ -3,9 +3,9 @@
 import io.javaoperatorsdk.operator.api.config.RetryConfiguration;
 
 public class GenericRetry implements Retry {
-  private int maxAttempts = DEFAULT_MAX_ATTEMPTS;
-  private long initialInterval = DEFAULT_INITIAL_INTERVAL;
-  private double intervalMultiplier = DEFAULT_MULTIPLIER;
+  private int maxAttempts = RetryConfiguration.DEFAULT_MAX_ATTEMPTS;
+  private long initialInterval = RetryConfiguration.DEFAULT_INITIAL_INTERVAL;
+  private double intervalMultiplier = RetryConfiguration.DEFAULT_MULTIPLIER;
   private long maxInterval = -1;
 
   public static GenericRetry defaultLimitedExponentialRetry() {
diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/Retry.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/Retry.java
@@ -1,8 +1,7 @@
 package io.javaoperatorsdk.operator.processing.retry;
 
-import io.javaoperatorsdk.operator.api.config.RetryConfiguration;
-
-public interface Retry extends RetryConfiguration {
+public interface Retry {
 
   RetryExecution initExecution();
+
 }
diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventProcessorTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventProcessorTest.java
@@ -15,6 +15,7 @@
 import org.slf4j.LoggerFactory;
 
 import io.fabric8.kubernetes.api.model.HasMetadata;
+import io.javaoperatorsdk.operator.api.config.RetryConfiguration;
 import io.javaoperatorsdk.operator.api.monitoring.Metrics;
 import io.javaoperatorsdk.operator.processing.event.source.controller.ControllerResourceEventSource;
 import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceAction;
@@ -104,7 +105,7 @@ void schedulesAnEventRetryOnException() {
     eventProcessorWithRetry.eventProcessingFinished(executionScope, postExecutionControl);
 
     verify(retryTimerEventSourceMock, times(1))
-        .scheduleOnce(eq(customResource), eq(GenericRetry.DEFAULT_INITIAL_INTERVAL));
+        .scheduleOnce(eq(customResource), eq(RetryConfiguration.DEFAULT_INITIAL_INTERVAL));
   }
 
   @Test
@@ -135,7 +136,7 @@ void executesTheControllerInstantlyAfterErrorIfNewEventsReceived() {
     List<ExecutionScope> allValues = executionScopeArgumentCaptor.getAllValues();
     assertThat(allValues).hasSize(2);
     verify(retryTimerEventSourceMock, never())
-        .scheduleOnce(eq(customResource), eq(GenericRetry.DEFAULT_INITIAL_INTERVAL));
+        .scheduleOnce(eq(customResource), eq(RetryConfiguration.DEFAULT_INITIAL_INTERVAL));
   }
 
   @Test
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
@@ -25,7 +25,6 @@
 import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider;
 import io.javaoperatorsdk.operator.api.config.ControllerConfiguration;
 import io.javaoperatorsdk.operator.api.config.MockControllerConfiguration;
-import io.javaoperatorsdk.operator.api.config.RetryConfiguration;
 import io.javaoperatorsdk.operator.api.reconciler.Cleaner;
 import io.javaoperatorsdk.operator.api.reconciler.Context;
 import io.javaoperatorsdk.operator.api.reconciler.DeleteControl;
@@ -36,6 +35,7 @@
 import io.javaoperatorsdk.operator.api.reconciler.UpdateControl;
 import io.javaoperatorsdk.operator.processing.Controller;
 import io.javaoperatorsdk.operator.processing.event.ReconciliationDispatcher.CustomResourceFacade;
+import io.javaoperatorsdk.operator.processing.retry.GenericRetry;
 import io.javaoperatorsdk.operator.sample.observedgeneration.ObservedGenCustomResource;
 import io.javaoperatorsdk.operator.sample.simple.TestCustomResource;
 
@@ -110,7 +110,7 @@ private <R extends HasMetadata> ReconciliationDispatcher<R> init(R customResourc
     when(configuration.getFinalizerName()).thenReturn(DEFAULT_FINALIZER);
     when(configuration.getName()).thenReturn("EventDispatcherTestController");
     when(configuration.getResourceClass()).thenReturn(resourceClass);
-    when(configuration.getRetryConfiguration()).thenReturn(RetryConfiguration.DEFAULT);
+    when(configuration.getRetry()).thenReturn(new GenericRetry());
     when(configuration.reconciliationMaxInterval())
         .thenReturn(Optional.of(Duration.ofHours(RECONCILIATION_MAX_INTERVAL)));
 
diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/retry/GenericRetryExecutionTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/retry/GenericRetryExecutionTest.java
@@ -4,33 +4,36 @@
 
 import org.junit.jupiter.api.Test;
 
-import static io.javaoperatorsdk.operator.processing.retry.GenericRetry.DEFAULT_INITIAL_INTERVAL;
+import io.javaoperatorsdk.operator.api.config.RetryConfiguration;
+
 import static org.assertj.core.api.Assertions.assertThat;
 
 public class GenericRetryExecutionTest {
 
   @Test
   public void forFirstBackOffAlwaysReturnsInitialInterval() {
-    assertThat(getDefaultRetryExecution().nextDelay().get()).isEqualTo(DEFAULT_INITIAL_INTERVAL);
+    assertThat(getDefaultRetryExecution().nextDelay().get())
+        .isEqualTo(RetryConfiguration.DEFAULT_INITIAL_INTERVAL);
   }
 
   @Test
   public void delayIsMultipliedEveryNextDelayCall() {
     RetryExecution retryExecution = getDefaultRetryExecution();
 
     Optional<Long> res = callNextDelayNTimes(retryExecution, 1);
-    assertThat(res.get()).isEqualTo(DEFAULT_INITIAL_INTERVAL);
+    assertThat(res.get()).isEqualTo(RetryConfiguration.DEFAULT_INITIAL_INTERVAL);
 
     res = retryExecution.nextDelay();
     assertThat(res.get())
-        .isEqualTo((long) (DEFAULT_INITIAL_INTERVAL * GenericRetry.DEFAULT_MULTIPLIER));
+        .isEqualTo((long) (RetryConfiguration.DEFAULT_INITIAL_INTERVAL
+            * RetryConfiguration.DEFAULT_MULTIPLIER));
 
     res = retryExecution.nextDelay();
     assertThat(res.get())
         .isEqualTo(
-            (long) (DEFAULT_INITIAL_INTERVAL
-                * GenericRetry.DEFAULT_MULTIPLIER
-                * GenericRetry.DEFAULT_MULTIPLIER));
+            (long) (RetryConfiguration.DEFAULT_INITIAL_INTERVAL
+                * RetryConfiguration.DEFAULT_MULTIPLIER
+                * RetryConfiguration.DEFAULT_MULTIPLIER));
   }
 
   @Test

Original file line number	Diff line number	Diff line change
`@@ -178,7 +178,7 @@ public int getAttemptCount() {`
`178`	`178`
`179`	`179`	`@Override`
`180`	`180`	`public boolean isLastAttempt() {`
`181`		`- return controller.getConfiguration().getRetryConfiguration() == null;`
	`181`	`+ return controller.getConfiguration().getRetry() == null;`
`182`	`182`	`}`
`183`	`183`	`});`
`184`	`184`	`((DefaultContext<R>) context).setRetryInfo(retryInfo);`
Original file line number	Diff line number	Diff line change
`@@ -1,8 +1,7 @@`
`1`	`1`	`package io.javaoperatorsdk.operator.processing.retry;`
`2`	`2`
`3`		`-import io.javaoperatorsdk.operator.api.config.RetryConfiguration;`
`4`		`-`
`5`		`-public interface Retry extends RetryConfiguration {`
	`3`	`+public interface Retry {`
`6`	`4`
`7`	`5`	`RetryExecution initExecution();`
	`6`	`+`
`8`	`7`	`}`