await for deletion if the just-deleted resource still exists when try…

…ing to create
fabric8io · Dec 17, 2020 · c4302bc · c4302bc
1 parent 49d2eae
commit c4302bc
Show file tree

Hide file tree

Showing 3 changed files with 105 additions and 6 deletions.
diff --git a/...rnetes-client/src/main/java/io/fabric8/kubernetes/client/utils/DeleteAndCreateHelper.java b/...rnetes-client/src/main/java/io/fabric8/kubernetes/client/utils/DeleteAndCreateHelper.java
@@ -23,15 +23,25 @@
 import io.fabric8.kubernetes.client.ResourceHandler;
 import okhttp3.OkHttpClient;
 
+import java.net.HttpURLConnection;
+import java.util.Objects;
+import java.util.concurrent.TimeUnit;
 import java.util.function.Function;
 import java.util.function.UnaryOperator;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 public class DeleteAndCreateHelper<T extends HasMetadata> {
+  private static final Logger LOG = LoggerFactory.getLogger(DeleteAndCreateHelper.class);
+
   private final UnaryOperator<T> createTask;
+  private final Function<T, Boolean> awaitDeleteTask;
   private final Function<T, Boolean> deleteTask;
 
-  public DeleteAndCreateHelper(UnaryOperator<T> createTask, Function<T, Boolean> deleteTask) {
+  public DeleteAndCreateHelper(UnaryOperator<T> createTask, Function<T, Boolean> deleteTask, Function<T, Boolean> awaitDeleteTask) {
     this.createTask = createTask;
+    this.awaitDeleteTask = awaitDeleteTask;
     this.deleteTask = deleteTask;
   }
 
@@ -40,15 +50,40 @@ public T deleteAndCreate(T item) {
     if (Boolean.FALSE.equals(deleted)) {
       throw new KubernetesClientException("Failed to delete existing item:" + item.getMetadata().getName());
     }
-    return createTask.apply(item);
+    try {
+      return createTask.apply(item);
+    } catch (KubernetesClientException e) {
+      // depending on the grace period, the object might not actually be deleted by the time we try to create
+      // if that's the case, give it some time.
+      if (e.getCode() == HttpURLConnection.HTTP_CONFLICT) {
+        if (Boolean.FALSE.equals(awaitDeleteTask.apply(item))) {
+          throw new KubernetesClientException("Timed out waiting for item to be deleted before recreating: " + item.getMetadata().getName(), e);
+        }
+        return createTask.apply(item);
+      }
+      throw e;
+    }
   }
 
   public static HasMetadata deleteAndCreateItem(OkHttpClient client, Config config, HasMetadata meta, ResourceHandler<HasMetadata, HasMetadataVisitiableBuilder> h, String namespaceToUse, DeletionPropagation propagationPolicy) {
     DeleteAndCreateHelper<HasMetadata> deleteAndCreateHelper = new DeleteAndCreateHelper<>(
       m -> h.create(client, config, namespaceToUse, m),
-      m -> h.delete(client, config, namespaceToUse, propagationPolicy, m)
+      m -> h.delete(client, config, namespaceToUse, propagationPolicy, m),
+      waitUntilDeletedOrInterrupted(client, config, h, namespaceToUse)
     );
 
     return deleteAndCreateHelper.deleteAndCreate(meta);
   }
+
+  private static <T extends HasMetadata> Function<T, Boolean> waitUntilDeletedOrInterrupted(OkHttpClient client, Config config, ResourceHandler<HasMetadata, HasMetadataVisitiableBuilder> h, String namespaceToUse) {
+    return m -> {
+      try {
+        return h.waitUntilCondition(client, config, namespaceToUse, m, Objects::isNull, 30 , TimeUnit.SECONDS) == null;
+      } catch (InterruptedException e) {
+        Thread.currentThread().interrupt();
+        LOG.warn("interrupted waiting for item to be deleted, assuming not deleted");
+        return false;
+      }
+    };
+  }
 }
diff --git a/...es-client/src/test/java/io/fabric8/kubernetes/client/utils/DeleteAndCreateHelperTest.java b/...es-client/src/test/java/io/fabric8/kubernetes/client/utils/DeleteAndCreateHelperTest.java
@@ -47,6 +47,7 @@ void testDeleteAndCreate() {
       .thenReturn(getPod());
     DeleteAndCreateHelper<Pod> podDeleteAndCreateHelper = new DeleteAndCreateHelper<>(
       createPodTask,
+      deletePodTask,
       p -> true
     );
 
@@ -55,7 +56,7 @@ void testDeleteAndCreate() {
 
     // Then
     assertNotNull(podCreated);
-    assertTrue(deletePodTask.apply(podCreated));
+    assertTrue(wasPodDeleted.get());
   }
 
   @Test
@@ -67,14 +68,72 @@ void testDeleteAndCreateWhenDeletionFailed() {
       HttpURLConnection.HTTP_CONFLICT, new StatusBuilder().withCode(HttpURLConnection.HTTP_CONFLICT).build()));
     DeleteAndCreateHelper<Pod> podDeleteAndCreateHelper = new DeleteAndCreateHelper<>(
       createPodTask,
-      p -> false
+      p -> false,
+      p -> {
+        throw new RuntimeException("should not be called because deletion failed");
+      }
     );
 
     // When
     Pod podToDeleteAndCreate = getPod();
     assertThrows(KubernetesClientException.class,() -> podDeleteAndCreateHelper.deleteAndCreate(podToDeleteAndCreate));
   }
 
+  @Test
+  void testDeleteAndCreateWhenDeletionSucceedsButNotFinishedInTime() {
+    // Given
+    UnaryOperator<Pod> createPodTask = Mockito.mock(UnaryOperator.class, Mockito.RETURNS_DEEP_STUBS);
+    when(createPodTask.apply(any())).thenThrow(new KubernetesClientException("The POST operation could not be completed at " +
+      "this time, please try again",
+      HttpURLConnection.HTTP_CONFLICT, new StatusBuilder().withCode(HttpURLConnection.HTTP_CONFLICT).build()));
+    DeleteAndCreateHelper<Pod> podDeleteAndCreateHelper = new DeleteAndCreateHelper<>(
+      createPodTask,
+      p -> true, // deletion succeeds
+      p -> false // but doesn't finish in time
+    );
+
+    // When
+    Pod podToDeleteAndCreate = getPod();
+    assertThrows(KubernetesClientException.class,() -> podDeleteAndCreateHelper.deleteAndCreate(podToDeleteAndCreate));
+  }
+
+  @Test
+  void testDeleteAndCreateAfterWaitingForItemToBeDeleted() {
+    // Given
+    AtomicBoolean wasPodDeleted = new AtomicBoolean(false);
+    Function<Pod, Boolean> deletePodTask = p -> {
+      wasPodDeleted.set(true);
+      return true;
+    };
+
+    AtomicBoolean awaitedDeletion = new AtomicBoolean(false);
+    Function<Pod, Boolean> awaitDeletionTask = p -> {
+      awaitedDeletion.set(true);
+      return true;
+    };
+
+    UnaryOperator<Pod> createPodTask = Mockito.mock(UnaryOperator.class, Mockito.RETURNS_DEEP_STUBS);
+    when(createPodTask.apply(any()))
+      .thenThrow(new KubernetesClientException("The POST operation could not be completed at " +
+        "this time, please try again",
+        HttpURLConnection.HTTP_CONFLICT, new StatusBuilder().withCode(HttpURLConnection.HTTP_CONFLICT).build()))
+      .thenReturn(getPod());
+
+    DeleteAndCreateHelper<Pod> podDeleteAndCreateHelper = new DeleteAndCreateHelper<>(
+      createPodTask,
+      deletePodTask,
+      awaitDeletionTask
+    );
+
+    // When
+    Pod podCreated = podDeleteAndCreateHelper.deleteAndCreate(getPod());
+
+    // Then
+    assertNotNull(podCreated);
+    assertTrue(wasPodDeleted.get());
+    assertTrue(awaitedDeletion.get());
+  }
+
   private Pod getPod() {
     return new PodBuilder()
       .withNewMetadata().withName("p1").endMetadata()

diff --git a/kubernetes-itests/src/test/java/io/fabric8/kubernetes/ResourceIT.java b/kubernetes-itests/src/test/java/io/fabric8/kubernetes/ResourceIT.java
@@ -200,7 +200,12 @@ public void testDeleteExistingWithoutOrphanDeletion() {
     String replicaSetCreationTimestamp = replicaSetList.getItems().get(0).getMetadata().getCreationTimestamp();
 
     // Recreate deployment with deleteExisting
-    client.resource(deployment).inNamespace(session.getNamespace()).withPropagationPolicy(DeletionPropagation.ORPHAN).deletingExisting().createOrReplace();
+    client.resource(deployment)
+      .inNamespace(session.getNamespace())
+      .withPropagationPolicy(DeletionPropagation.ORPHAN)
+      .deletingExisting()
+      .createOrReplace();
+
     await().atMost(30, TimeUnit.SECONDS).until(resourceIsReady(deployment));
 
     // check that creationTimestamp matches original, meaning the orphan was not deleted