error handler suppoer for conditional update

Author: csviri

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

@@ -9,23 +9,34 @@ public class ErrorStatusUpdateControl<P extends HasMetadata> {
   private final P resource;
   private final boolean patch;
   private boolean noRetry = false;
-
+  private final boolean onlyOnChange;
 
   public static <T extends HasMetadata> ErrorStatusUpdateControl<T> patchStatus(T resource) {
-    return new ErrorStatusUpdateControl<>(resource, true);
+    return new ErrorStatusUpdateControl<>(resource, true, false);
   }
 
   public static <T extends HasMetadata> ErrorStatusUpdateControl<T> updateStatus(T resource) {
-    return new ErrorStatusUpdateControl<>(resource, false);
+    return new ErrorStatusUpdateControl<>(resource, false, false);
+  }
+
+  public static <T extends HasMetadata> ErrorStatusUpdateControl<T> patchStatusIfChanged(
+      T resource) {
+    return new ErrorStatusUpdateControl<>(resource, true, true);
+  }
+
+  public static <T extends HasMetadata> ErrorStatusUpdateControl<T> updateStatusIfChanged(
+      T resource) {
+    return new ErrorStatusUpdateControl<>(resource, false, true);
   }
 
   public static <T extends HasMetadata> ErrorStatusUpdateControl<T> noStatusUpdate() {
-    return new ErrorStatusUpdateControl<>(null, true);
+    return new ErrorStatusUpdateControl<>(null, true, false);
   }
 
-  private ErrorStatusUpdateControl(P resource, boolean patch) {
+  private ErrorStatusUpdateControl(P resource, boolean patch, boolean onlyOnChange) {
     this.resource = resource;
     this.patch = patch;
+    this.onlyOnChange = onlyOnChange;
   }
 
   /**
@@ -49,4 +60,8 @@ public boolean isNoRetry() {
   public boolean isPatch() {
     return patch;
   }
+
+  public boolean isOnlyOnChange() {
+    return onlyOnChange;
+  }
 }

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

@@ -15,13 +15,7 @@
 import io.javaoperatorsdk.operator.api.ObservedGenerationAware;
 import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider;
 import io.javaoperatorsdk.operator.api.config.ControllerConfiguration;
-import io.javaoperatorsdk.operator.api.reconciler.BaseControl;
-import io.javaoperatorsdk.operator.api.reconciler.Context;
-import io.javaoperatorsdk.operator.api.reconciler.DefaultContext;
-import io.javaoperatorsdk.operator.api.reconciler.DeleteControl;
-import io.javaoperatorsdk.operator.api.reconciler.ErrorStatusHandler;
-import io.javaoperatorsdk.operator.api.reconciler.RetryInfo;
-import io.javaoperatorsdk.operator.api.reconciler.UpdateControl;
+import io.javaoperatorsdk.operator.api.reconciler.*;
 import io.javaoperatorsdk.operator.processing.Controller;
 
 import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getName;
@@ -185,21 +179,14 @@ public boolean isLastAttempt() {
         var errorStatusUpdateControl = ((ErrorStatusHandler<R>) controller.getReconciler())
             .updateErrorStatus(resource, context, e);
 
-        R updatedResource = null;
-        if (errorStatusUpdateControl.getResource().isPresent()) {
-          updatedResource = errorStatusUpdateControl.isPatch() ? customResourceFacade
-              .patchStatus(errorStatusUpdateControl.getResource().orElseThrow(), originalResource)
-              : customResourceFacade
-                  .updateStatus(errorStatusUpdateControl.getResource().orElseThrow());
-        }
+        Optional<R> updatedResource = errorStatusUpdateControl.getResource()
+            .flatMap(r -> updateForErrorHandling(originalResource, r,
+                errorStatusUpdateControl));
         if (errorStatusUpdateControl.isNoRetry()) {
-          if (updatedResource != null) {
-            return errorStatusUpdateControl.isPatch()
-                ? PostExecutionControl.customResourceStatusPatched(updatedResource)
-                : PostExecutionControl.customResourceUpdated(updatedResource);
-          } else {
-            return PostExecutionControl.defaultDispatch();
-          }
+          return updatedResource.map(r -> errorStatusUpdateControl.isPatch()
+              ? PostExecutionControl.customResourceStatusPatched(r)
+              : PostExecutionControl.customResourceUpdated(r))
+              .orElseGet(PostExecutionControl::defaultDispatch);
         }
       } catch (RuntimeException ex) {
         log.error("Error during error status handling.", ex);
@@ -208,6 +195,18 @@ public boolean isLastAttempt() {
     throw e;
   }
 
+  private Optional<R> updateForErrorHandling(R originalResource, R updatedResource,
+      ErrorStatusUpdateControl<R> errorStatusUpdateControl) {
+    if (errorStatusUpdateControl.isOnlyOnChange()
+        && statusEqual(originalResource, updatedResource)) {
+      return Optional.empty();
+    }
+    return Optional.of(errorStatusUpdateControl.isPatch() ? customResourceFacade
+        .patchStatus(errorStatusUpdateControl.getResource().orElseThrow(), originalResource)
+        : customResourceFacade
+            .updateStatus(errorStatusUpdateControl.getResource().orElseThrow()));
+  }
+
   private boolean isErrorStatusHandlerPresent() {
     return controller.getReconciler() instanceof ErrorStatusHandler;
   }

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

@@ -541,6 +541,7 @@ void callErrorStatusHandlerEvenOnFirstError() {
   @Test
   void errorHandlerCanInstructNoRetryWithUpdate() {
     testCustomResource.addFinalizer(DEFAULT_FINALIZER);
+    when(customResourceFacade.updateStatus(any())).thenReturn(testCustomResource);
     reconciler.reconcile = (r, c) -> {
       throw new IllegalStateException("Error Status Test");
     };
@@ -599,6 +600,34 @@ void errorStatusHandlerCanPatchResource() {
         any(), any());
   }
 
+  @Test
+  void errorStatusHandlerConditionallyUpdatesStatus() {
+    extracted((r, ri, e) -> ErrorStatusUpdateControl.updateStatusIfChanged(testCustomResource));
+  }
+
+  @Test
+  void errorStatusHandlerConditionallyPatchesStatus() {
+    extracted((r, ri, e) -> ErrorStatusUpdateControl.patchStatusIfChanged(testCustomResource));
+  }
+
+  private void extracted(ErrorStatusHandler<TestCustomResource> errorHandler) {
+    testCustomResource.addFinalizer(DEFAULT_FINALIZER);
+    reconciler.reconcile = (r, c) -> {
+      throw new IllegalStateException("Error Status Test");
+    };
+
+    reconciler.errorHandler = errorHandler;
+
+    reconciliationDispatcher.handleExecution(
+        new ExecutionScope(
+            testCustomResource, null));
+
+    verify(customResourceFacade, never()).updateStatus(eq(testCustomResource));
+    verify(customResourceFacade, never()).patchStatus(eq(testCustomResource), any());
+    verify(((ErrorStatusHandler) reconciler), times(1)).updateErrorStatus(eq(testCustomResource),
+        any(), any());
+  }
+
   @Test
   void schedulesReconciliationIfMaxDelayIsSet() {
     testCustomResource.addFinalizer(DEFAULT_FINALIZER);

sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/MySQLSchemaReconciler.java

@@ -57,7 +57,7 @@ public ErrorStatusUpdateControl<MySQLSchema> updateErrorStatus(MySQLSchema schem
     status.setSecretName(null);
     status.setStatus("ERROR: " + e.getMessage());
     schema.setStatus(status);
-    return ErrorStatusUpdateControl.patchStatus(schema);
+    return ErrorStatusUpdateControl.patchStatusIfChanged(schema);
   }
 
 

sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/Utils.java

@@ -31,7 +31,7 @@ static String serviceName(WebPage webPage) {
 
   static ErrorStatusUpdateControl<WebPage> handleError(WebPage resource, Exception e) {
     resource.getStatus().setErrorMessage("Error: " + e.getMessage());
-    return ErrorStatusUpdateControl.updateStatus(resource);
+    return ErrorStatusUpdateControl.updateStatusIfChanged(resource);
   }
 
   static void simulateErrorIfRequested(WebPage webPage) throws ErrorSimulationException {