fix: minor fixes (#767)

Author: csviri

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

@@ -6,16 +6,18 @@
 
 /**
  * If the custom resource's status implements this interface, the observed generation will be
- * automatically handled. The last observed generation will be updated on status when the status is
- * instructed to be updated (see below). In addition to that, controller configuration will be
- * checked if is set to generation aware. If generation aware config is turned off, this interface
- * is ignored.
- *
- * In order to work the status object returned by CustomResource.getStatus() should not be null. In
- * addition to that from the controller that the {@link UpdateControl#updateStatus(HasMetadata)} or
- * {@link UpdateControl#updateResourceAndStatus(HasMetadata)} should be returned. The observed
- * generation is not updated in other cases.
- *
+ * automatically handled. The last observed generation will be updated on status. In addition to
+ * that, controller configuration will be checked if is set to be generation aware. If generation
+ * aware config is turned off, this interface is ignored.
+ * <p>
+ * In order for this automatic handling to work the status object returned by
+ * {@link CustomResource#getStatus()} should not be null.
+ * <p>
+ * The observed generation is updated even when {@link UpdateControl#noUpdate()} or
+ * {@link UpdateControl#updateResource(HasMetadata)} is called. Although those results call normally
+ * does not result in a status update, there will be a subsequent status update Kubernetes API call
+ * in this case.
+ * 
  * @see ObservedGenerationAwareStatus
  */
 public interface ObservedGenerationAware {

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

@@ -8,9 +8,9 @@ public interface ErrorStatusHandler<T extends HasMetadata> {
 
   /**
    * <p>
-   * Reconcile can implement this interface in order to update the status sub-resource in the case
-   * when the last reconciliation retry attempt is failed on the Reconciler. In that case the
-   * updateErrorStatus is called automatically.
+   * Reconciler can implement this interface in order to update the status sub-resource in the case
+   * an exception in thrown. In that case
+   * {@link #updateErrorStatus(HasMetadata, RetryInfo, RuntimeException)} is called automatically.
    * <p>
    * The result of the method call is used to make a status update on the custom resource. This is
    * always a sub-resource update request, so no update on custom resource itself (like spec of

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

@@ -5,21 +5,30 @@
 public interface Reconciler<R extends HasMetadata> {
 
   /**
-   * Note that this method is used in combination of finalizers. If automatic finalizer handling is
-   * turned off for the controller, this method is not called.
+   * The implementation of this operation is required to be idempotent. Always use the UpdateControl
+   * object to make updates on custom resource if possible.
+   *
+   * @param resource the resource that has been created or updated
+   * @param context the context with which the operation is executed
+   * @return UpdateControl to manage updates on the custom resource (usually the status) after
+   *         reconciliation.
+   */
+  UpdateControl<R> reconcile(R resource, Context context);
+
+  /**
+   * Note that this method is used in combination with finalizers. If automatic finalizer handling
+   * is turned off for the controller, this method is not called.
    *
-   * The implementation should delete the associated component(s). Note that this is method is
-   * called when an object is marked for deletion. After it's executed the custom resource finalizer
-   * is automatically removed by the framework; unless the return value is
+   * The implementation should delete the associated component(s). This method is called when an
+   * object is marked for deletion. After it's executed the custom resource finalizer is
+   * automatically removed by the framework; unless the return value is
    * {@link DeleteControl#noFinalizerRemoval()}, which indicates that the controller has determined
    * that the resource should not be deleted yet. This is usually a corner case, when a cleanup is
    * tried again eventually.
    *
    * <p>
-   * It's important that this method be idempotent, as it could be called several times, depending
-   * on the conditions and the controller's configuration (for example, if the controller is
-   * configured to not use a finalizer but the resource does have finalizers, it might be be
-   * "offered" again for deletion several times until the finalizers are all removed.
+   * It's important for implementations of this method to be idempotent, since it can be called
+   * several times.
    *
    * @param resource the resource that is marked for deletion
    * @param context the context with which the operation is executed
@@ -32,20 +41,4 @@ default DeleteControl cleanup(R resource, Context context) {
     return DeleteControl.defaultDelete();
   }
 
-  /**
-   * The implementation of this operation is required to be idempotent. Always use the UpdateControl
-   * object to make updates on custom resource if possible. Also always use the custom resource
-   * parameter (not the custom resource that might be in the events)
-   *
-   * @param resource the resource that has been created or updated
-   * @param context the context with which the operation is executed
-   * @return The resource is updated in api server if the return value is not
-   *         {@link UpdateControl#noUpdate()}. This the common use cases. However in cases, for
-   *         example the operator is restarted, and we don't want to have an update call to k8s api
-   *         to be made unnecessarily, by returning {@link UpdateControl#noUpdate()} this update can
-   *         be skipped. <b>However we will always call an update if there is no finalizer on object
-   *         and it's not marked for deletion.</b>
-   */
-  UpdateControl<R> reconcile(R resource, Context context);
-
 }

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

@@ -31,10 +31,6 @@
 import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getName;
 import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getVersion;
 
-/**
- * Event handler that makes sure that events are processed in a "single threaded" way per resource
- * UID, while buffering events which are received during an execution.
- */
 class EventProcessor<R extends HasMetadata> implements EventHandler, LifecycleAware {
 
   private static final Logger log = LoggerFactory.getLogger(EventProcessor.class);

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

@@ -87,7 +87,8 @@ private PostExecutionControl<R> handleDispatch(ExecutionScope<R> executionScope)
   private boolean shouldNotDispatchToDelete(R resource) {
     // we don't dispatch to delete if the controller is configured to use a finalizer but that
     // finalizer is not present (which means it's already been removed)
-    return configuration().useFinalizer() && !resource.hasFinalizer(configuration().getFinalizer());
+    return !configuration().useFinalizer() || (configuration().useFinalizer()
+        && !resource.hasFinalizer(configuration().getFinalizer()));
   }
 
   private PostExecutionControl<R> handleReconcile(

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerResourceEventSource.java

@@ -26,9 +26,6 @@
 import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getUID;
 import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getVersion;
 
-/**
- * This is a special case since is not bound to a single custom resource
- */
 public class ControllerResourceEventSource<T extends HasMetadata> extends AbstractEventSource
     implements ResourceEventHandler<T> {
 

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/RetryExecution.java

@@ -7,10 +7,7 @@
 public interface RetryExecution extends RetryInfo {
 
   /**
-   * Calculates the delay for the next execution. This method should return 0, when called first
-   * time;
-   *
-   * @return the time to wait until the next execution in millisecondsz
+   * @return the time to wait until the next execution in milliseconds
    */
   Optional<Long> nextDelay();
 }

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

@@ -139,17 +139,14 @@ void callsDeleteIfObjectHasFinalizerAndMarkedForDelete() {
     verify(reconciler, times(1)).cleanup(eq(testCustomResource), any());
   }
 
-  /**
-   * Note that there could be more finalizers. Out of our control.
-   */
   @Test
-  void callDeleteOnControllerIfMarkedForDeletionWhenNoFinalizerIsConfigured() {
+  void doesNotCallDeleteOnControllerIfMarkedForDeletionWhenNoFinalizerIsConfigured() {
     configureToNotUseFinalizer();
     markForDeletion(testCustomResource);
 
     reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource));
 
-    verify(reconciler).cleanup(eq(testCustomResource), any());
+    verify(reconciler, times(0)).cleanup(eq(testCustomResource), any());
   }
 
   @Test