fix: deprecating UpdateControl method with confusing name

Author: csviri

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

@@ -9,32 +9,32 @@ public class UpdateControl<P extends HasMetadata> extends BaseControl<UpdateCont
   private final boolean updateStatus;
   private final boolean updateResource;
   private final boolean patchStatus;
-  private final boolean patchResource;
 
   private UpdateControl(
-      P resource, boolean updateStatus, boolean updateResource, boolean patchStatus,
-      boolean patchResource) {
+      P resource, boolean updateStatus, boolean updateResource, boolean patchStatus) {
     if ((updateResource || updateStatus) && resource == null) {
       throw new IllegalArgumentException("CustomResource cannot be null in case of update");
     }
     this.resource = resource;
     this.updateStatus = updateStatus;
     this.updateResource = updateResource;
     this.patchStatus = patchStatus;
-    this.patchResource = patchResource;
   }
 
   /**
    * Creates an update control instance that instructs the framework to do an update on resource
    * itself, not on the status. Note that usually as a results of a reconciliation should be a
    * status update not an update to the resource itself.
    *
+   * Using this update makes sure, that the resource in the next reconciliation is the updated one -
+   * this is not guaranteed by default if you do an update on a resource by the Kubernetes client.
+   *
    * @param <T> custom resource type
    * @param customResource customResource to use for update
    * @return initialized update control
    */
   public static <T extends HasMetadata> UpdateControl<T> updateResource(T customResource) {
-    return new UpdateControl<>(customResource, false, true, false, false);
+    return new UpdateControl<>(customResource, false, true, false);
   }
 
   /**
@@ -53,7 +53,7 @@ public static <T extends HasMetadata> UpdateControl<T> updateResource(T customRe
    * @return UpdateControl instance
    */
   public static <T extends HasMetadata> UpdateControl<T> patchStatus(T customResource) {
-    return new UpdateControl<>(customResource, true, false, true, false);
+    return new UpdateControl<>(customResource, true, false, true);
   }
 
   /**
@@ -69,7 +69,7 @@ public static <T extends HasMetadata> UpdateControl<T> patchStatus(T customResou
    * @return UpdateControl instance
    */
   public static <T extends HasMetadata> UpdateControl<T> updateStatus(T customResource) {
-    return new UpdateControl<>(customResource, true, false, false, false);
+    return new UpdateControl<>(customResource, true, false, false);
   }
 
   /**
@@ -82,22 +82,37 @@ public static <T extends HasMetadata> UpdateControl<T> updateStatus(T customReso
    */
   public static <T extends HasMetadata> UpdateControl<T> updateResourceAndStatus(
       T customResource) {
-    return new UpdateControl<>(customResource, true, true, false, false);
+    return new UpdateControl<>(customResource, true, true, false);
   }
 
+  /**
+   * Updates the resource - with optimistic locking - and patched the status without optimistic
+   * locking in place.
+   *
+   * @param customResource
+   * @return
+   * @param <T>
+   */
   public static <T extends HasMetadata> UpdateControl<T> updateResourceAndPatchStatus(
       T customResource) {
-    return new UpdateControl<>(customResource, true, true, true, false);
+    return new UpdateControl<>(customResource, true, true, true);
   }
 
-  public static <T extends HasMetadata> UpdateControl<T> patchResourceAndStatus(
-      T customResource) {
-    return new UpdateControl<>(customResource, true, true, true, false);
+  /**
+   * Market for removal, because of confusing name. It does not patch the resource just updates it.
+   * This method is same as updateResourceAndPatchStatus.
+   *
+   * @param customResource to update
+   * @return UpdateControl instance
+   * @param <T> resource type
+   */
+  @Deprecated(forRemoval = true)
+  public static <T extends HasMetadata> UpdateControl<T> patchResourceAndStatus(T customResource) {
+    return updateResourceAndStatus(customResource);
   }
 
-
   public static <T extends HasMetadata> UpdateControl<T> noUpdate() {
-    return new UpdateControl<>(null, false, false, false, false);
+    return new UpdateControl<>(null, false, false, false);
   }
 
   public P getResource() {
@@ -116,15 +131,12 @@ public boolean isPatchStatus() {
     return patchStatus;
   }
 
-  public boolean isPatchResource() {
-    return patchResource;
-  }
-
   public boolean isNoUpdate() {
     return !updateResource && !updateStatus;
   }
 
   public boolean isUpdateResourceAndStatus() {
     return updateResource && updateStatus;
   }
+
 }

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

@@ -9,49 +9,47 @@ final class PostExecutionControl<R extends HasMetadata> {
   private final boolean finalizerRemoved;
   private final R updatedCustomResource;
   private final boolean updateIsStatusPatch;
-  private final boolean updateIsResourcePatch;
   private final Exception runtimeException;
 
   private Long reScheduleDelay = null;
 
   private PostExecutionControl(
       boolean finalizerRemoved,
       R updatedCustomResource,
-      boolean updateIsStatusPatch, boolean updateIsResourcePatch, Exception runtimeException) {
+      boolean updateIsStatusPatch, Exception runtimeException) {
     this.finalizerRemoved = finalizerRemoved;
     this.updatedCustomResource = updatedCustomResource;
     this.updateIsStatusPatch = updateIsStatusPatch;
-    this.updateIsResourcePatch = updateIsResourcePatch;
     this.runtimeException = runtimeException;
   }
 
   public static <R extends HasMetadata> PostExecutionControl<R> onlyFinalizerAdded(
       R updatedCustomResource) {
-    return new PostExecutionControl<>(false, updatedCustomResource, false, false, null);
+    return new PostExecutionControl<>(false, updatedCustomResource, false, null);
   }
 
   public static <R extends HasMetadata> PostExecutionControl<R> defaultDispatch() {
-    return new PostExecutionControl<>(false, null, false, false, null);
+    return new PostExecutionControl<>(false, null, false, null);
   }
 
   public static <R extends HasMetadata> PostExecutionControl<R> customResourceStatusPatched(
       R updatedCustomResource) {
-    return new PostExecutionControl<>(false, updatedCustomResource, true, false, null);
+    return new PostExecutionControl<>(false, updatedCustomResource, true, null);
   }
 
   public static <R extends HasMetadata> PostExecutionControl<R> customResourceUpdated(
-      R updatedCustomResource, boolean patched) {
-    return new PostExecutionControl<>(false, updatedCustomResource, false, patched, null);
+      R updatedCustomResource) {
+    return new PostExecutionControl<>(false, updatedCustomResource, false, null);
   }
 
   public static <R extends HasMetadata> PostExecutionControl<R> customResourceFinalizerRemoved(
       R updatedCustomResource) {
-    return new PostExecutionControl<>(true, updatedCustomResource, false, false, null);
+    return new PostExecutionControl<>(true, updatedCustomResource, false, null);
   }
 
   public static <R extends HasMetadata> PostExecutionControl<R> exceptionDuringExecution(
       Exception exception) {
-    return new PostExecutionControl<>(false, null, false, false, exception);
+    return new PostExecutionControl<>(false, null, false, exception);
   }
 
   public Optional<R> getUpdatedCustomResource() {

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

@@ -140,7 +140,7 @@ private PostExecutionControl<P> reconcileExecution(ExecutionScope<P> executionSc
     P updatedCustomResource = null;
     if (updateControl.isUpdateResourceAndStatus()) {
       updatedCustomResource =
-          updateCustomResource(updateControl.getResource(), updateControl.isPatchResource());
+          updateCustomResource(updateControl.getResource());
       updateControl
           .getResource()
           .getMetadata()
@@ -154,7 +154,7 @@ private PostExecutionControl<P> reconcileExecution(ExecutionScope<P> executionSc
               updateControl.isPatchStatus());
     } else if (updateControl.isUpdateResource()) {
       updatedCustomResource =
-          updateCustomResource(updateControl.getResource(), updateControl.isPatchResource());
+          updateCustomResource(updateControl.getResource());
       if (shouldUpdateObservedGenerationAutomatically(updatedCustomResource)) {
         updatedCustomResource =
             updateStatusGenerationAware(updateControl.getResource(), originalResource,
@@ -203,8 +203,7 @@ public boolean isLastAttempt() {
           if (updatedResource != null) {
             return errorStatusUpdateControl.isPatch()
                 ? PostExecutionControl.customResourceStatusPatched(updatedResource)
-                // todo review if this is ok
-                : PostExecutionControl.customResourceUpdated(updatedResource, false);
+                : PostExecutionControl.customResourceUpdated(updatedResource);
           } else {
             return PostExecutionControl.defaultDispatch();
           }
@@ -265,8 +264,7 @@ private PostExecutionControl<P> createPostExecutionControl(P updatedCustomResour
         postExecutionControl =
             PostExecutionControl.customResourceStatusPatched(updatedCustomResource);
       } else {
-        postExecutionControl = PostExecutionControl.customResourceUpdated(updatedCustomResource,
-            updateControl.isPatchResource());
+        postExecutionControl = PostExecutionControl.customResourceUpdated(updatedCustomResource);
       }
     } else {
       postExecutionControl = PostExecutionControl.defaultDispatch();
@@ -319,13 +317,12 @@ private P updateCustomResourceWithFinalizer(P resourceForExecution, P originalRe
         r -> r.addFinalizer(configuration().getFinalizerName()));
   }
 
-  private P updateCustomResource(P resource, boolean isPatch) {
-    log.debug("Updating resource: {} with version: {} patch: {}", getUID(resource),
-        getVersion(resource), isPatch);
+  private P updateCustomResource(P resource) {
+    log.debug("Updating resource: {} with version: {}", getUID(resource),
+        getVersion(resource));
     log.trace("Resource before update: {}", resource);
 
-    return isPatch ? customResourceFacade.patchResource(resource)
-        : customResourceFacade.updateResource(resource);
+    return customResourceFacade.updateResource(resource);
   }
 
   ControllerConfiguration<P> configuration() {
@@ -381,11 +378,6 @@ public R getResource(String namespace, String name) {
       }
     }
 
-    public R patchResource(R resource) {
-      // todo
-      return null;
-    }
-
     public R updateResource(R resource) {
       log.debug(
           "Trying to replace resource {}, version: {}",