imrpove: replacing confusing UpdateControl method (#1762)

Author: csviri

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

@@ -8,24 +8,27 @@ public class UpdateControl<P extends HasMetadata> extends BaseControl<UpdateCont
   private final P resource;
   private final boolean updateStatus;
   private final boolean updateResource;
-  private final boolean patch;
+  private final boolean patchStatus;
 
   private UpdateControl(
-      P resource, boolean updateStatus, boolean updateResource, boolean patch) {
+      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.patch = patch;
+    this.patchStatus = patchStatus;
   }
 
   /**
    * 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
@@ -73,6 +76,9 @@ public static <T extends HasMetadata> UpdateControl<T> updateStatus(T customReso
    * As a results of this there will be two call to K8S API. First the custom resource will be
    * updates then the status sub-resource.
    *
+   * 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> resource type
    * @param customResource - custom resource to use in both API calls
    * @return UpdateControl instance
@@ -82,11 +88,36 @@ public static <T extends HasMetadata> UpdateControl<T> updateResourceAndStatus(
     return new UpdateControl<>(customResource, true, true, false);
   }
 
-  public static <T extends HasMetadata> UpdateControl<T> patchResourceAndStatus(
+  /**
+   * Updates the resource - with optimistic locking - and patches the status without optimistic
+   * locking in place.
+   *
+   * Note that using this method, it is not guaranteed that the most recent updated resource will be
+   * in case for next reconciliation.
+   *
+   * @param customResource to update
+   * @return UpdateControl instance
+   * @param <T> resource type
+   */
+  public static <T extends HasMetadata> UpdateControl<T> updateResourceAndPatchStatus(
       T customResource) {
     return new UpdateControl<>(customResource, true, true, true);
   }
 
+  /**
+   * Marked for removal, because of confusing name. It does not patch the resource but rather
+   * updates it.
+   *
+   * @deprecated use {@link UpdateControl#updateResourceAndPatchStatus(HasMetadata)}
+   *
+   * @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);
@@ -104,8 +135,8 @@ public boolean isUpdateResource() {
     return updateResource;
   }
 
-  public boolean isPatch() {
-    return patch;
+  public boolean isPatchStatus() {
+    return patchStatus;
   }
 
   public boolean isNoUpdate() {
@@ -115,4 +146,5 @@ public boolean isNoUpdate() {
   public boolean isUpdateResourceAndStatus() {
     return updateResource && updateStatus;
   }
+
 }

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

@@ -84,7 +84,7 @@ private PostExecutionControl<P> handleDispatch(ExecutionScope<P> executionScope)
     Context<P> context =
         new DefaultContext<>(executionScope.getRetryInfo(), controller, originalResource);
     if (markedForDeletion) {
-      return handleCleanup(originalResource, resourceForExecution, context);
+      return handleCleanup(resourceForExecution, context);
     } else {
       return handleReconcile(executionScope, resourceForExecution, originalResource, context);
     }
@@ -147,23 +147,24 @@ private PostExecutionControl<P> reconcileExecution(ExecutionScope<P> executionSc
           .setResourceVersion(updatedCustomResource.getMetadata().getResourceVersion());
       updatedCustomResource =
           updateStatusGenerationAware(updateControl.getResource(), originalResource,
-              updateControl.isPatch());
+              updateControl.isPatchStatus());
     } else if (updateControl.isUpdateStatus()) {
       updatedCustomResource =
           updateStatusGenerationAware(updateControl.getResource(), originalResource,
-              updateControl.isPatch());
+              updateControl.isPatchStatus());
     } else if (updateControl.isUpdateResource()) {
       updatedCustomResource =
           updateCustomResource(updateControl.getResource());
       if (shouldUpdateObservedGenerationAutomatically(updatedCustomResource)) {
         updatedCustomResource =
             updateStatusGenerationAware(updateControl.getResource(), originalResource,
-                updateControl.isPatch());
+                updateControl.isPatchStatus());
       }
     } else if (updateControl.isNoUpdate()
         && shouldUpdateObservedGenerationAutomatically(resourceForExecution)) {
       updatedCustomResource =
-          updateStatusGenerationAware(originalResource, originalResource, updateControl.isPatch());
+          updateStatusGenerationAware(originalResource, originalResource,
+              updateControl.isPatchStatus());
     }
     return createPostExecutionControl(updatedCustomResource, updateControl);
   }
@@ -259,7 +260,7 @@ private PostExecutionControl<P> createPostExecutionControl(P updatedCustomResour
       UpdateControl<P> updateControl) {
     PostExecutionControl<P> postExecutionControl;
     if (updatedCustomResource != null) {
-      if (updateControl.isUpdateStatus() && updateControl.isPatch()) {
+      if (updateControl.isUpdateStatus() && updateControl.isPatchStatus()) {
         postExecutionControl =
             PostExecutionControl.customResourceStatusPatched(updatedCustomResource);
       } else {
@@ -279,7 +280,7 @@ private void updatePostExecutionControlWithReschedule(
   }
 
 
-  private PostExecutionControl<P> handleCleanup(P originalResource, P resource,
+  private PostExecutionControl<P> handleCleanup(P resource,
       Context<P> context) {
     log.debug(
         "Executing delete for resource: {} with version: {}",
@@ -317,8 +318,10 @@ private P updateCustomResourceWithFinalizer(P resourceForExecution, P originalRe
   }
 
   private P updateCustomResource(P resource) {
-    log.debug("Updating resource: {} with version: {}", getUID(resource), getVersion(resource));
+    log.debug("Updating resource: {} with version: {}", getUID(resource),
+        getVersion(resource));
     log.trace("Resource before update: {}", resource);
+
     return customResourceFacade.updateResource(resource);
   }