Fix #2269: Setting a grace period when deleting resource using `withP…

…ropagationPolicy()`
fabric8io · Jul 2, 2020 · db78dc3 · db78dc3
1 parent 14c5aff
commit db78dc3
Show file tree

Hide file tree

Showing 12 changed files with 46 additions and 18 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,7 @@
 * Fix #2297: Resuscitate ProjectRequestHandler in openshift-client
 * Fix KubernetesAttributesExctractor to extract metadata from unregistered custom resources, such when using Raw CustomResource API 
 * Fix #2296: No adapter available for type:interface io.fabric8.kubernetes.client.dsl.V1APIGroupDSL
+* Fix #2269: Setting a grace period when deleting resource using `withPropagationPolicy()`
 
 #### Improvements
 * Fix #2233: client.service().getUrl(..) should be able to fetch URL for ClusterIP based services

diff --git a/doc/CHEATSHEET.md b/doc/CHEATSHEET.md
@@ -2050,6 +2050,10 @@ Boolean isDeleted = client.apps().deployments().inNamespace("default").withName(
 ```
 Boolean isDeleted = client.apps().deployments().inNamespace("default").withName("nginx-deploy").withPropagationPolicy("Foreground").delete();
 ```
+- Specifying grace period for deletion:
+```
+Boolean isDeleted = client.apps().deployments().inNamespace("ns1").withName("mydeployment").withPropagationPolicy(DeletionPropagation.FOREGROUND).withGracePeriod(10).delete();
+```
 
 ### Watch Options
 Kubernetes Client provides namely three different ways of using `Watch`:

diff --git a/extensions/knative/client/src/main/resources/resource-handler.vm b/extensions/knative/client/src/main/resources/resource-handler.vm
@@ -86,7 +86,7 @@ public class ${model.name}Handler implements ResourceHandler<${model.name}, ${mo
 
   @Override
   public Boolean delete(OkHttpClient client, Config config, String namespace, DeletionPropagation propagationPolicy, ${model.name} item) {
-    return new ${model.name}OperationsImpl(client, config).withItem(item).inNamespace(namespace).withPropagationPolicy(propagationPolicy).delete();
+    return new ${model.name}OperationsImpl(client, config).withItem(item).inNamespace(namespace).withName(item.getMetadata().getName()).withPropagationPolicy(propagationPolicy).delete();
   }
 
   @Override

diff --git a/extensions/service-catalog/client/src/main/resources/resource-handler.vm b/extensions/service-catalog/client/src/main/resources/resource-handler.vm
@@ -85,7 +85,7 @@ public class ${model.name}Handler implements ResourceHandler<${model.name}, ${mo
 
   @Override
   public Boolean delete(OkHttpClient client, Config config, String namespace, DeletionPropagation propagationPolicy, ${model.name} item) {
-    return new ${model.name}OperationsImpl(client, config).withItem(item).inNamespace(namespace).withPropagationPolicy(propagationPolicy).delete();
+    return new ${model.name}OperationsImpl(client, config).withItem(item).inNamespace(namespace).withName(item.getMetadata().getName()).withPropagationPolicy(propagationPolicy).delete();
   }
 
   @Override

diff --git a/extensions/tekton/client/src/main/resources/resource-handler.vm b/extensions/tekton/client/src/main/resources/resource-handler.vm
@@ -85,7 +85,7 @@ public class ${model.name}Handler implements ResourceHandler<${model.name}, ${mo
 
   @Override
   public Boolean delete(OkHttpClient client, Config config, String namespace, DeletionPropagation propagationPolicy, ${model.name} item) {
-    return new ${model.name}OperationsImpl(client, config).withItem(item).inNamespace(namespace).withPropagationPolicy(propagationPolicy).delete();
+    return new ${model.name}OperationsImpl(client, config).withItem(item).inNamespace(namespace).withName(item.getMetadata().getName()).withPropagationPolicy(propagationPolicy).delete();
   }
 
   @Override

diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/GracePeriodConfigurable.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/GracePeriodConfigurable.java
@@ -17,5 +17,14 @@
 
 public interface GracePeriodConfigurable<T>
 {
+  /**
+   * The duration in seconds before the object should be deleted. Value must be non-negative integer.
+   * The value zero indicates delete immediately. If this value is nil, the default grace period for
+   * the specified type will be used. Defaults to a per object value if not specified. zero means
+   * delete immediately.
+   *
+   * @param gracePeriodSeconds grace period integer value in seconds
+   * @return Deletable object
+   */
     T withGracePeriod(long gracePeriodSeconds);
 }
diff --git a/...nt/src/main/java/io/fabric8/kubernetes/client/dsl/CascadingEditReplacePatchDeletable.java b/...nt/src/main/java/io/fabric8/kubernetes/client/dsl/CascadingEditReplacePatchDeletable.java
@@ -16,8 +16,11 @@
 
 package io.fabric8.kubernetes.client.dsl;
 
+import io.fabric8.kubernetes.client.PropagationPolicyConfigurable;
+
 public interface CascadingEditReplacePatchDeletable<I,D,T,B> extends
   EditReplacePatchDeletable<I,D,T,B>,
   Cascading<EditReplacePatchDeletable<I,D,T,B>>,
+  PropagationPolicyConfigurable<EditReplacePatchDeletable<I,D,T,B>>,
   Lockable<Replaceable<I, I>> {
 }
diff --git a/...etes-client/src/main/java/io/fabric8/kubernetes/client/dsl/EditReplacePatchDeletable.java b/...etes-client/src/main/java/io/fabric8/kubernetes/client/dsl/EditReplacePatchDeletable.java
@@ -17,10 +17,9 @@
 package io.fabric8.kubernetes.client.dsl;
 
 import io.fabric8.kubernetes.client.GracePeriodConfigurable;
-import io.fabric8.kubernetes.client.PropagationPolicyConfigurable;
 
 public interface EditReplacePatchDeletable<I, T, D, B> extends EditReplacePatchable<I, T, D>, Deletable<B>,
-                                                          GracePeriodConfigurable<Deletable<B>>,
-                                                          PropagationPolicyConfigurable<Deletable<B>>
+                                                          GracePeriodConfigurable<Deletable<B>>
+
 {
 }
diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/WatchListDeletable.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/WatchListDeletable.java
@@ -16,11 +16,9 @@
 package io.fabric8.kubernetes.client.dsl;
 
 import io.fabric8.kubernetes.client.GracePeriodConfigurable;
-import io.fabric8.kubernetes.client.PropagationPolicyConfigurable;
 
 public interface WatchListDeletable<T, L, B, H, W> extends VersionWatchable<H, W>, Listable<L>, Deletable<B>,
                                                            GracePeriodConfigurable<Deletable<B>>,
-                                                           StatusUpdatable<T>,
-                                                           PropagationPolicyConfigurable<Deletable<B>>
+                                                           StatusUpdatable<T>
 {
 }
diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/base/BaseOperation.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/base/BaseOperation.java
@@ -998,7 +998,7 @@ public FilterWatchListDeletable<T, L, Boolean, Watch, Watcher<T>> withGracePerio
   }
 
   @Override
-  public FilterWatchListDeletable<T, L, Boolean, Watch, Watcher<T>> withPropagationPolicy(DeletionPropagation propagationPolicy)
+  public EditReplacePatchDeletable<T, T, D, Boolean> withPropagationPolicy(DeletionPropagation propagationPolicy)
   {
     return newInstance(context.withPropagationPolicy(propagationPolicy));
   }

diff --git a/kubernetes-client/src/test/java/io/fabric8/kubernetes/client/dsl/base/BaseOperationTest.java b/kubernetes-client/src/test/java/io/fabric8/kubernetes/client/dsl/base/BaseOperationTest.java
@@ -21,6 +21,7 @@
 import static org.hamcrest.CoreMatchers.notNullValue;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.Assert.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
 
 import java.net.MalformedURLException;
 import java.net.URL;
@@ -32,6 +33,7 @@
 import io.fabric8.kubernetes.api.model.ListOptionsBuilder;
 import io.fabric8.kubernetes.api.model.Pod;
 import io.fabric8.kubernetes.api.model.PodList;
+import io.fabric8.kubernetes.client.dsl.EditReplacePatchDeletable;
 import io.fabric8.kubernetes.client.dsl.Resource;
 import io.fabric8.kubernetes.client.utils.URLUtils;
 import org.junit.jupiter.api.Test;
@@ -90,14 +92,9 @@ public void testDefaultGracePeriod() {
   @Test
   public void testChainingGracePeriodAndPropagationPolicy() {
     final BaseOperation operation = new BaseOperation(new OperationContext());
-    FilterWatchListDeletable<?, ?, Boolean, Watch, Watcher<?>> operationWithGracePeriod = operation.withGracePeriod(10L);
-    FilterWatchListDeletable<?, ?, Boolean, Watch, Watcher<?>> operationWithPropagationPolicy = operation.withPropagationPolicy(DeletionPropagation.FOREGROUND);
-
-    Object chainedGracePeriod = operationWithPropagationPolicy.withGracePeriod(10);
-    Object chainedPropagationPolicy = operationWithGracePeriod.withPropagationPolicy(DeletionPropagation.FOREGROUND);
-
-    assertThat(chainedGracePeriod, is(notNullValue()));
-    assertThat(chainedPropagationPolicy, is(notNullValue()));
+    EditReplacePatchDeletable<?, ?, ?, Boolean> operationWithPropagationPolicy = operation.withPropagationPolicy(DeletionPropagation.FOREGROUND);
+    assertThat(operationWithPropagationPolicy, is(notNullValue()));
+    assertNotNull(operationWithPropagationPolicy.withGracePeriod(10));
   }
 
   @Test

diff --git a/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/PropagationPolicyTest.java b/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/PropagationPolicyTest.java
@@ -189,6 +189,23 @@ void testDeleteDeploymentWithExplicitPropagationPolicy() throws InterruptedExcep
     assertDeleteOptionsInLastRecordedRequest(DeletionPropagation.FOREGROUND.toString(), server.getLastRequest());
   }
 
+  @Test
+  @DisplayName("Should delete a Deployment with explicitly set PropagationPolicy And Grace Period")
+  void testDeleteDeploymentWithExplicitPropagationPolicyAndGracePeriod() throws InterruptedException {
+    // Given
+    server.expect().delete().withPath("/apis/apps/v1/namespaces/ns1/deployments/mydeployment")
+      .andReturn(HttpURLConnection.HTTP_OK, new DeploymentBuilder().build())
+      .once();
+    KubernetesClient client = server.getClient();
+
+    // When
+    Boolean isDeleted = client.apps().deployments().inNamespace("ns1").withName("mydeployment").withPropagationPolicy(DeletionPropagation.FOREGROUND).withGracePeriod(10).delete();
+
+    // Then
+    assertTrue(isDeleted);
+    assertEquals("{\"apiVersion\":\"v1\",\"kind\":\"DeleteOptions\",\"gracePeriodSeconds\":10,\"propagationPolicy\":\"" + DeletionPropagation.FOREGROUND + "\"}", server.getLastRequest().getBody().readUtf8());
+  }
+
   @Test
   @DisplayName("Should delete a StatefulSet with PropagationPolicy=Background")
   void testDeleteStatefulSet() throws InterruptedException {