Allow specifying PropagationPolicy when using deleteExisting (#2676)

* Make PropagationPolicy configurable when deletingExisting is configured

await for deletion if the just-deleted resource still exists when trying to create

* trigger build

* fix resource test. fix DeleteAndCreateHelper so we don't fail on deleted=false, and update tests

* Make it a constant

CHANGELOG.md

@@ -12,6 +12,7 @@
 * Fix #2656: Binding operations can be instantiated
 
 #### Improvements
+* Fix: Allow specifying PropagationPolicy when using deleteExisting
 * Fix: Adds a convenience method for referring to Cache keys by namespace and name rather than item
 * Fix: CustomResourceDefinitionContext.fromCrd support for v1 CustomResourceDefinition
 * Fix #2642: Update kubernetes-examples to use apps/v1 Deployment rather than extensions/v1beta1

kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/CascadingDeletable.java

@@ -18,5 +18,5 @@
 /**
  * Created by iocanel on 9/15/16.
  */
-public interface CascadingDeletable extends Deletable, Cascading<Deletable> {
+public interface CascadingDeletable<T> extends Deletable, Cascading<Deletable>, Recreateable<Applicable<T>> {
 }

kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/ListVisitFromServerGetDeleteRecreateWaitApplicable.java

@@ -26,8 +26,8 @@
 public interface ListVisitFromServerGetDeleteRecreateWaitApplicable<T> extends Visitable<ListVisitFromServerGetDeleteRecreateWaitApplicable<T>>,
   FromServerGettable<List<T>>,
   RecreateApplicable<List<T>, T>,
-  CascadingDeletable,
+  CascadingDeletable<List<T>>,
   Waitable<List<T>, T>,
-  GracePeriodConfigurable<CascadingDeletable>,
-  PropagationPolicyConfigurable<CascadingDeletable> {
+  GracePeriodConfigurable<CascadingDeletable<List<T>>>,
+  PropagationPolicyConfigurable<CascadingDeletable<List<T>>> {
 }

kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/VisitFromServerGetWatchDeleteRecreateWaitApplicable.java

@@ -25,9 +25,9 @@
 
 public interface VisitFromServerGetWatchDeleteRecreateWaitApplicable<T> extends Visitable<VisitFromServerGetWatchDeleteRecreateWaitApplicable<T>>,
   FromServerGettable<T>, RecreateApplicable<T, T>,
-  CascadingDeletable,
+  CascadingDeletable<T>,
   Watchable<Watcher<T>>,
   Waitable<T, T>,
-  GracePeriodConfigurable<CascadingDeletable>,
-  PropagationPolicyConfigurable<CascadingDeletable> {
+  GracePeriodConfigurable<CascadingDeletable<T>>,
+  PropagationPolicyConfigurable<CascadingDeletable<T>> {
 }

kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableImpl.java

@@ -200,12 +200,12 @@ public VisitFromServerGetWatchDeleteRecreateWaitApplicable<HasMetadata> accept(V
   }
 
   @Override
-  public CascadingDeletable withGracePeriod(long gracePeriodSeconds) {
+  public CascadingDeletable<HasMetadata> withGracePeriod(long gracePeriodSeconds) {
     return new NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableImpl(client, config, fallbackNamespace, explicitNamespace, fromServer, true, visitors, item, gracePeriodSeconds, propagationPolicy, cascading, watchRetryInitialBackoffMillis, watchRetryBackoffMultiplier);
   }
 
   @Override
-  public CascadingDeletable withPropagationPolicy(DeletionPropagation propagationPolicy) {
+  public CascadingDeletable<HasMetadata> withPropagationPolicy(DeletionPropagation propagationPolicy) {
     return new NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableImpl(client, config, fallbackNamespace, explicitNamespace, fromServer, true, visitors, item, gracePeriodSeconds, propagationPolicy, cascading, watchRetryInitialBackoffMillis, watchRetryBackoffMultiplier);
   }
 

kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl.java

@@ -382,12 +382,12 @@ public ListVisitFromServerGetDeleteRecreateWaitApplicable<HasMetadata> accept(Vi
         return new NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl(client, config, fallbackNamespace, explicitNamespace, fromServer, true, newVisitors, item, null, null, gracePeriodSeconds, propagationPolicy, cascading, watchRetryInitialBackoffMillis, watchRetryBackoffMultiplier);
     }
 
-  @Override public CascadingDeletable withGracePeriod(long gracePeriodSeconds)
+  @Override public CascadingDeletable<List<HasMetadata>> withGracePeriod(long gracePeriodSeconds)
   {
     return new NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl(client, config, fallbackNamespace, explicitNamespace, fromServer, true, visitors, item, null, null, gracePeriodSeconds, propagationPolicy, cascading, watchRetryInitialBackoffMillis, watchRetryBackoffMultiplier);
   }
 
-  @Override public CascadingDeletable withPropagationPolicy(DeletionPropagation propagationPolicy)
+  @Override public CascadingDeletable<List<HasMetadata>> withPropagationPolicy(DeletionPropagation propagationPolicy)
   {
     return new NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl(client, config, fallbackNamespace, explicitNamespace, fromServer, true, visitors, item, null, null, gracePeriodSeconds, propagationPolicy, cascading, watchRetryInitialBackoffMillis, watchRetryBackoffMultiplier);
   }

kubernetes-client/src/main/java/io/fabric8/kubernetes/client/utils/DeleteAndCreateHelper.java

@@ -23,32 +23,76 @@
 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 static final int MAX_WAIT_SECONDS = 30;
+
   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;
   }
 
   public T deleteAndCreate(T item) {
     Boolean deleted = deleteTask.apply(item);
-    if (Boolean.FALSE.equals(deleted)) {
-      throw new KubernetesClientException("Failed to delete existing item:" + item.getMetadata().getName());
+    if (!deleted) {
+      LOG.debug("did not delete because item did not exist, continuing to create {}", item.getMetadata().getName());
+    }
+
+    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 (!deleted) {
+          LOG.error("there was no item to delete, but received HTTP_CONFLICT response upon creation of item {}", item.getMetadata().getName(), e);
+          throw e;
+        }
+
+        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;
     }
-    return createTask.apply(item);
   }
 
   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, MAX_WAIT_SECONDS , TimeUnit.SECONDS) == null;
+      } catch (InterruptedException e) {
+        Thread.currentThread().interrupt();
+        LOG.warn("interrupted waiting for item to be deleted, assuming not deleted");
+        return false;
+      }
+    };
+  }
 }

kubernetes-client/src/test/java/io/fabric8/kubernetes/client/utils/DeleteAndCreateHelperTest.java

@@ -27,11 +27,12 @@
 import java.util.function.Function;
 import java.util.function.UnaryOperator;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.*;
 
 class DeleteAndCreateHelperTest {
   @Test
@@ -47,6 +48,7 @@ void testDeleteAndCreate() {
       .thenReturn(getPod());
     DeleteAndCreateHelper<Pod> podDeleteAndCreateHelper = new DeleteAndCreateHelper<>(
       createPodTask,
+      deletePodTask,
       p -> true
     );
 
@@ -55,26 +57,118 @@ void testDeleteAndCreate() {
 
     // Then
     assertNotNull(podCreated);
-    assertTrue(deletePodTask.apply(podCreated));
+    assertTrue(wasPodDeleted.get());
   }
 
   @Test
   void testDeleteAndCreateWhenDeletionFailed() {
+    // Given
+    AtomicBoolean wasPodDeleted = new AtomicBoolean(false);
+    Function<Pod, Boolean> deletePodTask = p -> {
+      wasPodDeleted.set(true);
+      return false;
+    };
+    UnaryOperator<Pod> createPodTask = Mockito.mock(UnaryOperator.class, Mockito.RETURNS_DEEP_STUBS);
+    when(createPodTask.apply(any())).thenAnswer(invocation -> invocation.getArgument(0));
+
+    DeleteAndCreateHelper<Pod> podDeleteAndCreateHelper = new DeleteAndCreateHelper<>(
+      createPodTask,
+      deletePodTask,
+      p -> {
+        throw new RuntimeException("should not be called because creation will succeed first");
+      }
+    );
+
+    // When
+    Pod podToDeleteAndCreate = getPod();
+    Pod result = podDeleteAndCreateHelper.deleteAndCreate(podToDeleteAndCreate);
+
+    // Then
+    assertEquals(podToDeleteAndCreate, result);
+    verify(createPodTask).apply(podToDeleteAndCreate);
+  }
+
+  @Test
+  void testThrowExceptionOnConflictAfterNoDelete() {
+    // Given
+    AtomicBoolean wasPodDeleted = new AtomicBoolean(false);
+    Function<Pod, Boolean> deletePodTask = p -> {
+      wasPodDeleted.set(true);
+      return false;
+    };
+
+    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,
+      deletePodTask,
+      p -> {
+        throw new RuntimeException("should not be called because creation will succeed first");
+      }
+    );
+
+    // 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 -> false
+      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()

kubernetes-itests/src/test/java/io/fabric8/kubernetes/ResourceIT.java

@@ -30,6 +30,7 @@
 import io.fabric8.kubernetes.api.model.ServiceBuilder;
 import io.fabric8.kubernetes.api.model.apps.Deployment;
 import io.fabric8.kubernetes.api.model.apps.DeploymentBuilder;
+import io.fabric8.kubernetes.api.model.apps.ReplicaSetList;
 import io.fabric8.kubernetes.client.KubernetesClient;
 import org.arquillian.cube.kubernetes.api.Session;
 import org.arquillian.cube.kubernetes.impl.requirement.RequiresKubernetes;
@@ -49,6 +50,7 @@
 import static junit.framework.TestCase.assertNotNull;
 import static org.awaitility.Awaitility.await;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertTrue;
 
 @RunWith(ArquillianConditionalRunner.class)
@@ -159,6 +161,65 @@ public void delete() {
     assertTrue(client.resource(pod1).inNamespace(session.getNamespace()).delete());
   }
 
+  @Test
+  public void testDeleteExistingWithOrphanDeletion() {
+    // Create Deployment
+    client.resource(deployment).inNamespace(session.getNamespace()).createOrReplace();
+    await().atMost(30, TimeUnit.SECONDS).until(resourceIsReady(deployment));
+
+    // get creationTimestamp of underlying replicaset. we expect this NOT to match later, meaning the orphan WAS deleted.
+    ReplicaSetList replicaSetList = client.apps().replicaSets().inNamespace(session.getNamespace()).withLabel("run", "deploy1").list();
+    assertEquals(1, replicaSetList.getItems().size());
+    String replicaSetCreationTimestamp = replicaSetList.getItems().get(0).getMetadata().getCreationTimestamp();
+
+    // Recreate deployment with deleteExisting
+    client.resource(deployment).inNamespace(session.getNamespace()).withPropagationPolicy(DeletionPropagation.FOREGROUND).deletingExisting().createOrReplace();
+    await().atMost(30, TimeUnit.SECONDS).until(resourceIsReady(deployment));
+
+    // check that creationTimestamp DOES NOT MATCH original, meaning the orphan WAS deleted
+    replicaSetList = client.apps().replicaSets().inNamespace(session.getNamespace()).withLabel("run", "deploy1").list();
+    assertEquals(1, replicaSetList.getItems().size());
+    assertNotEquals(replicaSetCreationTimestamp, replicaSetList.getItems().get(0).getMetadata().getCreationTimestamp());
+
+    // cleanup
+    assertEquals(true, client.resource(deployment).inNamespace(session.getNamespace()).delete());
+    // Check whether child resources are also deleted
+    await().atMost(30, TimeUnit.SECONDS)
+      .until(() -> client.apps().replicaSets().inNamespace(session.getNamespace()).withLabel("run", "deploy1").list().getItems().size() == 0);
+  }
+
+  @Test
+  public void testDeleteExistingWithoutOrphanDeletion() {
+    // Create Deployment
+    client.resource(deployment).inNamespace(session.getNamespace()).createOrReplace();
+    await().atMost(30, TimeUnit.SECONDS).until(resourceIsReady(deployment));
+
+    // get creationTimestamp of underlying replicaset. we expect this to match later, meaning the orphan was not deleted.
+    ReplicaSetList replicaSetList = client.apps().replicaSets().inNamespace(session.getNamespace()).withLabel("run", "deploy1").list();
+    assertEquals(1, replicaSetList.getItems().size());
+    String replicaSetCreationTimestamp = replicaSetList.getItems().get(0).getMetadata().getCreationTimestamp();
+
+    // Recreate deployment with deleteExisting
+    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
+    replicaSetList = client.apps().replicaSets().inNamespace(session.getNamespace()).withLabel("run", "deploy1").list();
+    assertEquals(1, replicaSetList.getItems().size());
+    assertEquals(replicaSetCreationTimestamp, replicaSetList.getItems().get(0).getMetadata().getCreationTimestamp());
+
+    // cleanup
+    assertEquals(true, client.resource(deployment).inNamespace(session.getNamespace()).delete());
+    // Check whether child resources are also deleted
+    await().atMost(30, TimeUnit.SECONDS)
+      .until(() -> client.apps().replicaSets().inNamespace(session.getNamespace()).withLabel("run", "deploy1").list().getItems().size() == 0);
+  }
+
   @Test
   public void testDeletionWithOrphanDeletion() {
     // Create Deployment