improve: eventing on cleanup in case status is changed after marked for deletion (#2153)

Signed-off-by: Attila Mészáros <[email protected]>

Author: csviri

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

@@ -8,7 +8,11 @@ public class InternalEventFilters {
   private InternalEventFilters() {}
 
   static <T extends HasMetadata> OnUpdateFilter<T> onUpdateMarkedForDeletion() {
-    return (newResource, oldResource) -> newResource.isMarkedForDeletion();
+    // the old resource is checked since in corner cases users might still want to update the status
+    // for a resource that is marked for deletion
+
+    return (newResource, oldResource) -> !oldResource.isMarkedForDeletion()
+        && newResource.isMarkedForDeletion();
   }
 
   static <T extends HasMetadata> OnUpdateFilter<T> onUpdateGenerationAware(

operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/controller/InternalEventFiltersTest.java

@@ -17,8 +17,9 @@ class InternalEventFiltersTest {
 
   @Test
   void onUpdateMarkedForDeletion() {
+    var oldRes = TestUtils.testCustomResource();
     var res = markForDeletion(TestUtils.testCustomResource());
-    assertThat(InternalEventFilters.onUpdateMarkedForDeletion().accept(res, res)).isTrue();
+    assertThat(InternalEventFilters.onUpdateMarkedForDeletion().accept(res, oldRes)).isTrue();
   }
 
   @Test

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

@@ -0,0 +1,55 @@
+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.updatestatusincleanupandreschedule.UpdateStatusInCleanupAndRescheduleCustomResource;
+import io.javaoperatorsdk.operator.sample.updatestatusincleanupandreschedule.UpdateStatusInCleanupAndRescheduleReconciler;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.awaitility.Awaitility.await;
+
+public class UpdateStatusInCleanupAndRescheduleIT {
+
+  public static final String TEST_RESOURCE = "test1";
+  @RegisterExtension
+  LocallyRunOperatorExtension extension =
+      LocallyRunOperatorExtension.builder()
+          .withReconciler(UpdateStatusInCleanupAndRescheduleReconciler.class)
+          .build();
+
+
+  @Test
+  void testRescheduleAfterPatch() {
+    var res = extension.create(testResource());
+
+    await().untilAsserted(() -> {
+      var resource =
+          extension.get(UpdateStatusInCleanupAndRescheduleCustomResource.class, TEST_RESOURCE);
+      assertThat(resource.getMetadata().getFinalizers()).isNotEmpty();
+    });
+
+    extension.delete(res);
+
+    await().untilAsserted(() -> {
+      var resource =
+          extension.get(UpdateStatusInCleanupAndRescheduleCustomResource.class, TEST_RESOURCE);
+      assertThat(resource).isNull();
+    });
+
+    assertThat(extension.getReconcilerOfType(UpdateStatusInCleanupAndRescheduleReconciler.class)
+        .getRescheduleDelayWorked())
+        .isTrue();
+  }
+
+  UpdateStatusInCleanupAndRescheduleCustomResource testResource() {
+    var resource = new UpdateStatusInCleanupAndRescheduleCustomResource();
+    resource.setMetadata(new ObjectMetaBuilder()
+        .withName(TEST_RESOURCE)
+        .build());
+    return resource;
+  }
+
+}

operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/updatestatusincleanupandreschedule/UpdateStatusInCleanupAndRescheduleCustomResource.java

@@ -0,0 +1,17 @@
+package io.javaoperatorsdk.operator.sample.updatestatusincleanupandreschedule;
+
+import io.fabric8.kubernetes.api.model.Namespaced;
+import io.fabric8.kubernetes.client.CustomResource;
+import io.fabric8.kubernetes.model.annotation.Group;
+import io.fabric8.kubernetes.model.annotation.ShortNames;
+import io.fabric8.kubernetes.model.annotation.Version;
+
+@Group("sample.javaoperatorsdk")
+@Version("v1")
+@ShortNames("usc")
+public class UpdateStatusInCleanupAndRescheduleCustomResource
+    extends
+    CustomResource<Void, UpdateStatusInCleanupAndRescheduleCustomStatus>
+    implements Namespaced {
+
+}

operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/updatestatusincleanupandreschedule/UpdateStatusInCleanupAndRescheduleCustomStatus.java

@@ -0,0 +1,16 @@
+package io.javaoperatorsdk.operator.sample.updatestatusincleanupandreschedule;
+
+public class UpdateStatusInCleanupAndRescheduleCustomStatus {
+
+  private Integer cleanupAttempt;
+
+  public Integer getCleanupAttempt() {
+    return cleanupAttempt;
+  }
+
+  public UpdateStatusInCleanupAndRescheduleCustomStatus setCleanupAttempt(Integer cleanupAttempt) {
+    this.cleanupAttempt = cleanupAttempt;
+    return this;
+  }
+
+}

operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/updatestatusincleanupandreschedule/UpdateStatusInCleanupAndRescheduleReconciler.java

@@ -0,0 +1,60 @@
+package io.javaoperatorsdk.operator.sample.updatestatusincleanupandreschedule;
+
+import java.time.LocalTime;
+import java.time.temporal.ChronoUnit;
+
+import io.javaoperatorsdk.operator.api.reconciler.*;
+
+@ControllerConfiguration
+public class UpdateStatusInCleanupAndRescheduleReconciler
+    implements Reconciler<UpdateStatusInCleanupAndRescheduleCustomResource>,
+    Cleaner<UpdateStatusInCleanupAndRescheduleCustomResource> {
+
+  public static final Integer DELAY = 150;
+
+  private LocalTime lastCleanupExecution;
+
+  public Boolean rescheduleDelayWorked;
+
+  @Override
+  public UpdateControl<UpdateStatusInCleanupAndRescheduleCustomResource> reconcile(
+      UpdateStatusInCleanupAndRescheduleCustomResource resource,
+      Context<UpdateStatusInCleanupAndRescheduleCustomResource> context) {
+
+    return UpdateControl.noUpdate();
+  }
+
+  @Override
+  public DeleteControl cleanup(UpdateStatusInCleanupAndRescheduleCustomResource resource,
+      Context<UpdateStatusInCleanupAndRescheduleCustomResource> context) {
+
+    var status = resource.getStatus();
+    if (status == null) {
+      resource.setStatus(new UpdateStatusInCleanupAndRescheduleCustomStatus());
+      resource.getStatus().setCleanupAttempt(1);
+      lastCleanupExecution = LocalTime.now();
+    } else {
+      var currentAttempt = resource.getStatus().getCleanupAttempt();
+      resource.getStatus().setCleanupAttempt(currentAttempt + 1);
+      if (!Boolean.FALSE.equals(rescheduleDelayWorked)) {
+        var diff = ChronoUnit.MILLIS.between(lastCleanupExecution, LocalTime.now());
+        if (diff < DELAY) {
+          rescheduleDelayWorked = false;
+        } else {
+          rescheduleDelayWorked = true;
+        }
+      }
+    }
+    var res = context.getClient().resource(resource).updateStatus();
+
+    if (resource.getStatus().getCleanupAttempt() > 5) {
+      return DeleteControl.defaultDelete();
+    } else {
+      return DeleteControl.noFinalizerRemoval().rescheduleAfter(DELAY);
+    }
+  }
+
+  public Boolean getRescheduleDelayWorked() {
+    return rescheduleDelayWorked;
+  }
+}