feat: remove automatic observed generation handling (#2382)

Signed-off-by: Attila Mészáros <csviri@gmail.com>
Signed-off-by: Chris Laprun <claprun@redhat.com>
Co-authored-by: Chris Laprun <claprun@redhat.com>
Signed-off-by: Attila Mészáros <csviri@gmail.com>

Author: csviri

README.md

@@ -40,7 +40,6 @@ It makes it easy to implement best practices and patterns for an Operator. Featu
 * Handling dependent resources, related events, and caching.
 * Automatic Retries
 * Smart event scheduling
-* Handling Observed Generations automatically
 * Easy to use Error Handling
 * ... and everything that a batteries included framework needs 
 

bootstrapper-maven-plugin/src/main/resources/templates/Status.java

@@ -1,7 +1,5 @@
 package {{groupId}};
 
-import io.javaoperatorsdk.operator.api.ObservedGenerationAwareStatus;
-
-public class {{artifactClassId}}Status extends ObservedGenerationAwareStatus {
+public class {{artifactClassId}}Status {
 
 }

caffeine-bounded-cache-support/src/test/java/io/javaoperatorsdk/operator/processing/event/source/cache/sample/namespacescope/BoundedCacheTestStatus.java

@@ -1,6 +1,4 @@
 package io.javaoperatorsdk.operator.processing.event.source.cache.sample.namespacescope;
 
-import io.javaoperatorsdk.operator.api.ObservedGenerationAwareStatus;
-
-public class BoundedCacheTestStatus extends ObservedGenerationAwareStatus {
+public class BoundedCacheTestStatus {
 }

docs/documentation/v5-0-migration.md

@@ -44,11 +44,6 @@ permalink: /docs/v5-0-migration
    where it is demonstrated. Also, the related part of
    a [workaround](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework/src/test/java/io/javaoperatorsdk/operator/StatusPatchSSAMigrationIT.java#L110-L116).
 
-   Related automatic observed generation handling changes:
-   Automated Observed Generation (see features in docs), is automatically handled for non-SSA, even if
-   the status sub-resource is not instructed to be updated. This is not true for SSA, observed generation is updated
-   only when patch status is instructed by `UpdateControl`.
-
 6. `ManagedDependentResourceContext` has been renamed to `ManagedWorkflowAndDependentResourceContext` and is accessed
    via the accordingly renamed `managedWorkflowAndDependentResourceContext` method.
 7. `ResourceDiscriminator` was removed. In most of the cases you can just delete the discriminator, everything should
@@ -57,3 +52,9 @@ permalink: /docs/v5-0-migration
 8. `ConfigurationService.getTerminationTimeoutSeconds` and associated overriding mechanism have been removed,
    use `Operator.stop(Duration)` instead.
 9. `Operator.installShutdownHook()` has been removed, use `Operator.installShutdownHook(Duration)` instead
+10. Automated observed generation handling feature was removed (`ObservedGenerationAware` interface
+    and `ObservedGenerationAwareStatus` class were deleted). Manually handling observed generation is fairly easy to do
+    in your reconciler, however, it cannot be done automatically when using SSA. We therefore removed the feature since
+    it would have been confusing to have a different behavior for SSA and non-SSA cases. For an example of how to do
+    observed generation handling manually in your reconciler, see
+    [this sample](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/manualobservedgeneration/ManualObservedGenerationReconciler.java).

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

@@ -395,7 +395,7 @@ default boolean parseResourceVersionsForEventFilteringAndCaching() {
   /**
    * {@link io.javaoperatorsdk.operator.api.reconciler.UpdateControl} patch resource or status can
    * either use simple patches or SSA. Setting this to {@code true}, controllers will use SSA for
-   * adding finalizers, managing observed generation, patching resources and status.
+   * adding finalizers, patching resources and status.
    *
    * @return {@code true} if Server-Side Apply (SSA) should be used when patching the primary
    *         resources, {@code false} otherwise

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

@@ -10,14 +10,12 @@
 import io.fabric8.kubernetes.api.model.KubernetesResourceList;
 import io.fabric8.kubernetes.api.model.Namespaced;
 import io.fabric8.kubernetes.api.model.ObjectMeta;
-import io.fabric8.kubernetes.client.CustomResource;
 import io.fabric8.kubernetes.client.KubernetesClientException;
 import io.fabric8.kubernetes.client.dsl.MixedOperation;
 import io.fabric8.kubernetes.client.dsl.Resource;
 import io.fabric8.kubernetes.client.dsl.base.PatchContext;
 import io.fabric8.kubernetes.client.dsl.base.PatchType;
 import io.javaoperatorsdk.operator.OperatorException;
-import io.javaoperatorsdk.operator.api.ObservedGenerationAware;
 import io.javaoperatorsdk.operator.api.config.Cloner;
 import io.javaoperatorsdk.operator.api.config.ControllerConfiguration;
 import io.javaoperatorsdk.operator.api.reconciler.BaseControl;
@@ -166,13 +164,8 @@ private PostExecutionControl<P> reconcileExecution(ExecutionScope<P> executionSc
       }
     }
 
-    // check if status also needs to be updated
-    final var updateObservedGeneration = updateControl.isNoUpdate()
-        ? shouldUpdateObservedGenerationAutomatically(resourceForExecution)
-        : shouldUpdateObservedGenerationAutomatically(updatedCustomResource);
-    // if using SSA the observed generation is updated only if user instructs patching the status
-    if (updateControl.isPatchStatus() || (updateObservedGeneration && !useSSA)) {
-      updatedCustomResource = patchStatusGenerationAware(toUpdate, originalResource);
+    if (updateControl.isPatchStatus()) {
+      customResourceFacade.patchStatus(toUpdate, originalResource);
     }
     return createPostExecutionControl(updatedCustomResource, updateControl);
   }
@@ -202,9 +195,8 @@ public boolean isLastAttempt() {
 
         P updatedResource = null;
         if (errorStatusUpdateControl.getResource().isPresent()) {
-          updatedResource =
-              patchStatusGenerationAware(errorStatusUpdateControl.getResource().orElseThrow(),
-                  originalResource);
+          updatedResource = customResourceFacade
+              .patchStatus(errorStatusUpdateControl.getResource().orElseThrow(), originalResource);
         }
         if (errorStatusUpdateControl.isNoRetry()) {
           PostExecutionControl<P> postExecutionControl;
@@ -230,38 +222,9 @@ private boolean isErrorStatusHandlerPresent() {
   }
 
   private P patchStatusGenerationAware(P resource, P originalResource) {
-    updateStatusObservedGenerationIfRequired(resource);
     return customResourceFacade.patchStatus(resource, originalResource);
   }
 
-  @SuppressWarnings("rawtypes")
-  private boolean shouldUpdateObservedGenerationAutomatically(P resource) {
-    if (configuration().isGenerationAware() && resource instanceof CustomResource<?, ?>) {
-      var customResource = (CustomResource) resource;
-      var status = customResource.getStatus();
-      // Note that if status is null we won't update the observed generation.
-      if (status instanceof ObservedGenerationAware) {
-        var observedGen = ((ObservedGenerationAware) status).getObservedGeneration();
-        var currentGen = resource.getMetadata().getGeneration();
-        return !currentGen.equals(observedGen);
-      }
-    }
-    return false;
-  }
-
-  @SuppressWarnings("rawtypes")
-  private void updateStatusObservedGenerationIfRequired(P resource) {
-    if (configuration().isGenerationAware() && resource instanceof CustomResource<?, ?>) {
-      var customResource = (CustomResource) resource;
-      var status = customResource.getStatus();
-      // Note that if status is null we won't update the observed generation.
-      if (status instanceof ObservedGenerationAware) {
-        ((ObservedGenerationAware) status)
-            .setObservedGeneration(resource.getMetadata().getGeneration());
-      }
-    }
-  }
-
   private PostExecutionControl<P> createPostExecutionControl(P updatedCustomResource,
       UpdateControl<P> updateControl) {
     PostExecutionControl<P> postExecutionControl;

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

@@ -435,29 +435,6 @@ void reScheduleOnDeleteWithoutFinalizerRemoval() {
         .isEqualTo(1000L);
   }
 
-  @Test
-  void setObservedGenerationForStatusIfNeeded() throws Exception {
-    var observedGenResource = createObservedGenCustomResource();
-
-    Reconciler<ObservedGenCustomResource> reconciler = mock(Reconciler.class);
-    ControllerConfiguration<ObservedGenCustomResource> config =
-        MockControllerConfiguration.forResource(ObservedGenCustomResource.class);
-    CustomResourceFacade<ObservedGenCustomResource> facade = mock(CustomResourceFacade.class);
-    var dispatcher = init(observedGenResource, reconciler, config, facade, true);
-
-    when(config.isGenerationAware()).thenReturn(true);
-
-    when(reconciler.reconcile(any(), any()))
-        .thenReturn(UpdateControl.patchStatus(observedGenResource));
-    when(facade.patchStatus(eq(observedGenResource), any())).thenReturn(observedGenResource);
-
-    PostExecutionControl<ObservedGenCustomResource> control = dispatcher.handleExecution(
-        executionScopeWithCREvent(observedGenResource));
-    assertThat(control.getUpdatedCustomResource().orElseGet(() -> fail("Missing optional"))
-        .getStatus().getObservedGeneration())
-        .isEqualTo(1L);
-  }
-
   @Test
   void doesNotUpdatesObservedGenerationIfStatusIsNotPatchedWhenUsingSSA() throws Exception {
     var observedGenResource = createObservedGenCustomResource();
@@ -476,28 +453,6 @@ void doesNotUpdatesObservedGenerationIfStatusIsNotPatchedWhenUsingSSA() throws E
     assertThat(control.getUpdatedCustomResource()).isEmpty();
   }
 
-  @Test
-  void patchObservedGenerationOnCustomResourcePatchIfNoSSA() throws Exception {
-    var observedGenResource = createObservedGenCustomResource();
-
-    Reconciler<ObservedGenCustomResource> reconciler = mock(Reconciler.class);
-    final var config = MockControllerConfiguration.forResource(ObservedGenCustomResource.class);
-    CustomResourceFacade<ObservedGenCustomResource> facade = mock(CustomResourceFacade.class);
-    when(config.isGenerationAware()).thenReturn(true);
-    when(reconciler.reconcile(any(), any()))
-        .thenReturn(UpdateControl.patchResource(observedGenResource));
-    when(facade.patchResource(any(), any())).thenReturn(observedGenResource);
-    when(facade.patchStatus(eq(observedGenResource), any())).thenReturn(observedGenResource);
-    initConfigService(false);
-    var dispatcher = init(observedGenResource, reconciler, config, facade, true);
-
-    PostExecutionControl<ObservedGenCustomResource> control = dispatcher.handleExecution(
-        executionScopeWithCREvent(observedGenResource));
-    assertThat(control.getUpdatedCustomResource().orElseGet(() -> fail("Missing optional"))
-        .getStatus().getObservedGeneration())
-        .isEqualTo(1L);
-  }
-
   @Test
   void doesNotPatchObservedGenerationOnCustomResourcePatch() throws Exception {
     var observedGenResource = createObservedGenCustomResource();

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

@@ -1,7 +1,5 @@
 package io.javaoperatorsdk.operator.sample.observedgeneration;
 
-import io.javaoperatorsdk.operator.api.ObservedGenerationAwareStatus;
-
-public class ObservedGenStatus extends ObservedGenerationAwareStatus {
+public class ObservedGenStatus {
 
 }