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

@@ -424,7 +424,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 {
 
 }

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

@@ -0,0 +1,54 @@
+package io.javaoperatorsdk.operator;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
+import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension;
+import io.javaoperatorsdk.operator.sample.manualobservedgeneration.ManualObservedGenerationCustomResource;
+import io.javaoperatorsdk.operator.sample.manualobservedgeneration.ManualObservedGenerationReconciler;
+import io.javaoperatorsdk.operator.sample.manualobservedgeneration.ManualObservedGenerationSpec;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.awaitility.Awaitility.await;
+
+public class ManualObservedGenerationIT {
+
+  public static final String RESOURCE_NAME = "test1";
+  @RegisterExtension
+  LocallyRunOperatorExtension extension =
+      LocallyRunOperatorExtension.builder().withReconciler(new ManualObservedGenerationReconciler())
+          .build();
+
+  @Test
+  void observedGenerationUpdated() {
+    extension.create(testResource());
+
+    await().untilAsserted(() -> {
+      var r = extension.get(ManualObservedGenerationCustomResource.class, RESOURCE_NAME);
+      assertThat(r).isNotNull();
+      assertThat(r.getStatus().getObservedGeneration()).isEqualTo(1);
+      assertThat(r.getStatus().getObservedGeneration()).isEqualTo(r.getMetadata().getGeneration());
+    });
+
+    var changed = testResource();
+    changed.getSpec().setValue("changed value");
+    extension.replace(changed);
+
+    await().untilAsserted(() -> {
+      var r = extension.get(ManualObservedGenerationCustomResource.class, RESOURCE_NAME);
+      assertThat(r.getStatus().getObservedGeneration()).isEqualTo(2);
+      assertThat(r.getStatus().getObservedGeneration()).isEqualTo(r.getMetadata().getGeneration());
+    });
+  }
+
+  ManualObservedGenerationCustomResource testResource() {
+    var res = new ManualObservedGenerationCustomResource();
+    res.setMetadata(new ObjectMetaBuilder()
+        .withName(RESOURCE_NAME)
+        .build());
+    res.setSpec(new ManualObservedGenerationSpec());
+    res.getSpec().setValue("Initial Value");
+    return res;
+  }
+}

operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/observedgeneration/ObservedGenStatus.java

@@ -11,12 +11,7 @@
 @ShortNames("cue")
 public class CreateUpdateEventFilterTestCustomResource
     extends
-    CustomResource<CreateUpdateEventFilterTestCustomResourceSpec, CreateUpdateEventFilterTestCustomResourceStatus>
+    CustomResource<CreateUpdateEventFilterTestCustomResourceSpec, Void>
     implements Namespaced {
 
-  @Override
-  protected CreateUpdateEventFilterTestCustomResourceStatus initStatus() {
-    return new CreateUpdateEventFilterTestCustomResourceStatus();
-  }
-
 }

operator-framework/src/test/java/io/javaoperatorsdk/operator/ManualObservedGenerationIT.java

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

operator-framework/src/test/java/io/javaoperatorsdk/operator/ObservedGenerationHandlingIT.java

@@ -1,7 +1,14 @@
 package io.javaoperatorsdk.operator.sample.gracefulstop;
 
-import io.javaoperatorsdk.operator.api.ObservedGenerationAwareStatus;
+public class GracefulStopTestCustomResourceStatus {
 
-public class GracefulStopTestCustomResourceStatus extends ObservedGenerationAwareStatus {
+  private long observedGeneration;
 
+  public long getObservedGeneration() {
+    return observedGeneration;
+  }
+
+  public void setObservedGeneration(long observedGeneration) {
+    this.observedGeneration = observedGeneration;
+  }
 }

operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/createupdateeventfilter/CreateUpdateEventFilterTestCustomResource.java

@@ -22,6 +22,7 @@ public UpdateControl<GracefulStopTestCustomResource> reconcile(
 
     numberOfExecutions.addAndGet(1);
     resource.setStatus(new GracefulStopTestCustomResourceStatus());
+    resource.getStatus().setObservedGeneration(resource.getMetadata().getGeneration());
     Thread.sleep(RECONCILER_SLEEP);
 
     return UpdateControl.patchStatus(resource);