fix: isLastAttempt on error handler correct if max attempt 0 (#1595)

Author: csviri

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

@@ -38,11 +38,17 @@ class ReconciliationDispatcher<P extends HasMetadata> {
 
   private final Controller<P> controller;
   private final CustomResourceFacade<P> customResourceFacade;
+  // this is to handle corner case, when there is a retry, but it is actually limited to 0.
+  // Usually for testing purposes.
+  private final boolean retryConfigurationHasZeroAttempts;
 
   ReconciliationDispatcher(Controller<P> controller,
       CustomResourceFacade<P> customResourceFacade) {
     this.controller = controller;
     this.customResourceFacade = customResourceFacade;
+
+    var retry = controller.getConfiguration().getRetry();
+    retryConfigurationHasZeroAttempts = retry == null || retry.initExecution().isLastAttempt();
   }
 
   public ReconciliationDispatcher(Controller<P> controller) {
@@ -173,7 +179,9 @@ public int getAttemptCount() {
 
           @Override
           public boolean isLastAttempt() {
-            return controller.getConfiguration().getRetry() == null;
+            // check also if the retry is limited to 0
+            return retryConfigurationHasZeroAttempts ||
+                controller.getConfiguration().getRetry() == null;
           }
         });
         ((DefaultContext<P>) context).setRetryInfo(retryInfo);

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

@@ -11,6 +11,7 @@
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.mockito.ArgumentCaptor;
+import org.mockito.ArgumentMatcher;
 import org.mockito.ArgumentMatchers;
 import org.mockito.stubbing.Answer;
 
@@ -110,7 +111,10 @@ 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.getRetry()).thenReturn(new GenericRetry());
+    // needed so the retry can be predefined
+    if (configuration.getRetry() == null) {
+      when(configuration.getRetry()).thenReturn(new GenericRetry());
+    }
     when(configuration.maxReconciliationInterval())
         .thenReturn(Optional.of(Duration.ofHours(RECONCILIATION_MAX_INTERVAL)));
 
@@ -600,6 +604,34 @@ void errorStatusHandlerCanPatchResource() {
         any(), any());
   }
 
+  @Test
+  void ifRetryLimitedToZeroMaxAttemptsErrorHandlerGetsCorrectLastAttempt() {
+    var configuration =
+        MockControllerConfiguration
+            .forResource((Class<TestCustomResource>) testCustomResource.getClass());
+    when(configuration.getRetry()).thenReturn(new GenericRetry().setMaxAttempts(0));
+    reconciliationDispatcher =
+        init(testCustomResource, reconciler, configuration, customResourceFacade, false);
+
+    reconciler.reconcile = (r, c) -> {
+      throw new IllegalStateException("Error Status Test");
+    };
+    var mockErrorHandler = mock(ErrorStatusHandler.class);
+    when(mockErrorHandler.updateErrorStatus(any(), any(), any()))
+        .thenReturn(ErrorStatusUpdateControl.noStatusUpdate());
+    reconciler.errorHandler = mockErrorHandler;
+
+    reconciliationDispatcher.handleExecution(
+        new ExecutionScope(
+            testCustomResource, null));
+
+    verify(mockErrorHandler, times(1)).updateErrorStatus(any(),
+        ArgumentMatchers.argThat((ArgumentMatcher<Context<TestCustomResource>>) context -> {
+          var retryInfo = context.getRetryInfo().orElseThrow();
+          return retryInfo.isLastAttempt();
+        }), any());
+  }
+
   @Test
   void canSkipSchedulingMaxDelayIf() {
     testCustomResource.addFinalizer(DEFAULT_FINALIZER);