improve: delete GC dependent if reconcile precondition not met (#1871)

operator-framework · Apr 26, 2023 · e63b720 · e63b720
1 parent b5d71c2
commit e63b720
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 4 deletions.
diff --git a/.../io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java b/.../io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java
@@ -149,7 +149,10 @@ protected void doRun(DependentResourceNode<R, P> dependentResourceNode,
         DependentResource<R, P> dependentResource) {
       var deletePostCondition = dependentResourceNode.getDeletePostcondition();
 
-      if (dependentResource.isDeletable()) {
+      // GarbageCollected status is irrelevant here, as this method is only called when a
+      // precondition does not hold,
+      // a deleter should be deleted even if it is otherwise garbage collected
+      if (dependentResource instanceof Deleter) {
         ((Deleter<P>) dependentResource).delete(primary, context);
       }
       boolean deletePostConditionMet = isConditionMet(deletePostCondition, dependentResource);

diff --git a/...o/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutorTest.java b/...o/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutorTest.java
@@ -117,15 +117,14 @@ void cleanupConditionDiamondWorkflow() {
 
   @Test
   void dontDeleteIfGarbageCollected() {
-    GarbageCollectedDeleter gcDel = new GarbageCollectedDeleter("GC_DELETER");
     var workflow = new WorkflowBuilder<TestCustomResource>()
-        .addDependentResource(gcDel)
+        .addDependentResource(gcDeleter)
         .build();
 
     var res = workflow.cleanup(new TestCustomResource(), null);
 
     assertThat(executionHistory)
-        .notReconciled(gcDel);
+        .notReconciled(gcDeleter);
 
     Assertions.assertThat(res.getDeleteCalledOnDependents()).isEmpty();
   }

diff --git a/...javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutorTest.java b/...javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutorTest.java
@@ -453,4 +453,29 @@ void diamondShareWithReadyCondition() {
     Assertions.assertThat(res.getNotReadyDependents()).containsExactlyInAnyOrder(dr2);
   }
 
+  @Test
+  void garbageCollectedResourceIsDeletedIfReconcilePreconditionDoesNotHold() {
+    var workflow = new WorkflowBuilder<TestCustomResource>()
+        .addDependentResource(gcDeleter).withReconcilePrecondition(not_met_reconcile_condition)
+        .build();
+
+    var res = workflow.reconcile(new TestCustomResource(), mockContext);
+
+    Assertions.assertThat(res.getErroredDependents()).isEmpty();
+    assertThat(executionHistory).deleted(gcDeleter);
+  }
+
+  @Test
+  void garbageCollectedDeepResourceIsDeletedIfReconcilePreconditionDoesNotHold() {
+    var workflow = new WorkflowBuilder<TestCustomResource>()
+        .addDependentResource(dr1).withReconcilePrecondition(not_met_reconcile_condition)
+        .addDependentResource(gcDeleter).dependsOn(dr1)
+        .build();
+
+    var res = workflow.reconcile(new TestCustomResource(), mockContext);
+
+    Assertions.assertThat(res.getErroredDependents()).isEmpty();
+    assertThat(executionHistory).deleted(gcDeleter);
+  }
+
 }