From 49d2eae7dbde5f871066ed5098498a162d5d11f1 Mon Sep 17 00:00:00 2001 From: Bryan Beaudreault Date: Tue, 30 Jun 2020 12:02:00 -0400 Subject: [PATCH] Make PropagationPolicy configurable when deletingExisting is configured --- CHANGELOG.md | 1 + .../client/dsl/CascadingDeletable.java | 2 +- ...ServerGetDeleteRecreateWaitApplicable.java | 6 +- ...rGetWatchDeleteRecreateWaitApplicable.java | 6 +- ...WatchDeleteRecreateWaitApplicableImpl.java | 4 +- ...hDeleteRecreateWaitApplicableListImpl.java | 4 +- .../io/fabric8/kubernetes/ResourceIT.java | 56 +++++++++++++++++++ .../kubernetes/client/mock/ResourceTest.java | 27 +++++++++ 8 files changed, 95 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e474e1132a..ae99b12ff42 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ * Fix #2656: Binding operations can be instantiated #### Improvements +* Fix: Allow specifying PropagationPolicy when using deleteExisting * Fix: CustomResourceDefinitionContext.fromCrd support for v1 CustomResourceDefinition * Fix #2642: Update kubernetes-examples to use apps/v1 Deployment rather than extensions/v1beta1 diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/CascadingDeletable.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/CascadingDeletable.java index e670a6559cb..661856e7ee4 100644 --- a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/CascadingDeletable.java +++ b/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 { +public interface CascadingDeletable extends Deletable, Cascading, Recreateable> { } diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/ListVisitFromServerGetDeleteRecreateWaitApplicable.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/ListVisitFromServerGetDeleteRecreateWaitApplicable.java index 6be63078d01..9f8a527aac5 100644 --- a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/ListVisitFromServerGetDeleteRecreateWaitApplicable.java +++ b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/ListVisitFromServerGetDeleteRecreateWaitApplicable.java @@ -26,8 +26,8 @@ public interface ListVisitFromServerGetDeleteRecreateWaitApplicable extends Visitable>, FromServerGettable>, RecreateApplicable, T>, - CascadingDeletable, + CascadingDeletable>, Waitable, T>, - GracePeriodConfigurable, - PropagationPolicyConfigurable { + GracePeriodConfigurable>>, + PropagationPolicyConfigurable>> { } diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/VisitFromServerGetWatchDeleteRecreateWaitApplicable.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/VisitFromServerGetWatchDeleteRecreateWaitApplicable.java index c2d656b7b6a..ced2a37f755 100644 --- a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/VisitFromServerGetWatchDeleteRecreateWaitApplicable.java +++ b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/VisitFromServerGetWatchDeleteRecreateWaitApplicable.java @@ -25,9 +25,9 @@ public interface VisitFromServerGetWatchDeleteRecreateWaitApplicable extends Visitable>, FromServerGettable, RecreateApplicable, - CascadingDeletable, + CascadingDeletable, Watchable>, Waitable, - GracePeriodConfigurable, - PropagationPolicyConfigurable { + GracePeriodConfigurable>, + PropagationPolicyConfigurable> { } diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableImpl.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableImpl.java index 03e23cb9ccd..536b4f7589f 100644 --- a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableImpl.java +++ b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableImpl.java @@ -200,12 +200,12 @@ public VisitFromServerGetWatchDeleteRecreateWaitApplicable accept(V } @Override - public CascadingDeletable withGracePeriod(long gracePeriodSeconds) { + public CascadingDeletable 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 withPropagationPolicy(DeletionPropagation propagationPolicy) { return new NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableImpl(client, config, fallbackNamespace, explicitNamespace, fromServer, true, visitors, item, gracePeriodSeconds, propagationPolicy, cascading, watchRetryInitialBackoffMillis, watchRetryBackoffMultiplier); } diff --git a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl.java b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl.java index 1f90a43487f..e86e74f4617 100644 --- a/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl.java +++ b/kubernetes-client/src/main/java/io/fabric8/kubernetes/client/dsl/internal/NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl.java @@ -382,12 +382,12 @@ public ListVisitFromServerGetDeleteRecreateWaitApplicable 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> 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> withPropagationPolicy(DeletionPropagation propagationPolicy) { return new NamespaceVisitFromServerGetWatchDeleteRecreateWaitApplicableListImpl(client, config, fallbackNamespace, explicitNamespace, fromServer, true, visitors, item, null, null, gracePeriodSeconds, propagationPolicy, cascading, watchRetryInitialBackoffMillis, watchRetryBackoffMultiplier); } diff --git a/kubernetes-itests/src/test/java/io/fabric8/kubernetes/ResourceIT.java b/kubernetes-itests/src/test/java/io/fabric8/kubernetes/ResourceIT.java index ba56829c552..0a87f27ccb0 100644 --- a/kubernetes-itests/src/test/java/io/fabric8/kubernetes/ResourceIT.java +++ b/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,60 @@ 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.BACKGROUND).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 diff --git a/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/ResourceTest.java b/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/ResourceTest.java index e96c71d4932..b33dd177eb7 100644 --- a/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/ResourceTest.java +++ b/kubernetes-tests/src/test/java/io/fabric8/kubernetes/client/mock/ResourceTest.java @@ -16,6 +16,8 @@ package io.fabric8.kubernetes.client.mock; +import io.fabric8.kubernetes.api.model.DeleteOptions; +import io.fabric8.kubernetes.api.model.DeletionPropagation; import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.api.model.Pod; import io.fabric8.kubernetes.api.model.PodBuilder; @@ -33,6 +35,7 @@ import io.fabric8.kubernetes.client.dsl.PodResource; import io.fabric8.kubernetes.client.internal.readiness.Readiness; import io.fabric8.kubernetes.client.server.mock.KubernetesServer; +import io.fabric8.kubernetes.client.utils.Serialization; import okhttp3.mockwebserver.RecordedRequest; import org.assertj.core.api.InstanceOfAssertFactories; import org.junit.Assert; @@ -118,6 +121,30 @@ void testCreateOrReplaceWithDeleteExisting() throws Exception { assertEquals("POST", request.getMethod()); } + @Test + void itPassesPropagationPolicyWithDeleteExisting() throws InterruptedException { + Pod pod1 = new PodBuilder().withNewMetadata().withName("pod1").withNamespace("test").and().build(); + + server.expect().delete().withPath("/api/v1/namespaces/ns1/pods/pod1").andReturn(HttpURLConnection.HTTP_OK, pod1).once(); + server.expect().post().withPath("/api/v1/namespaces/ns1/pods").andReturn(HttpURLConnection.HTTP_CREATED, pod1).once(); + + KubernetesClient client = server.getClient(); + HasMetadata response = client.resource(pod1).inNamespace("ns1").withPropagationPolicy(DeletionPropagation.FOREGROUND).deletingExisting().createOrReplace(); + assertEquals(pod1, response); + + assertEquals(2, server.getMockServer().getRequestCount()); + + RecordedRequest deleteRequest = server.getMockServer().takeRequest(); + assertEquals("/api/v1/namespaces/ns1/pods/pod1", deleteRequest.getPath()); + assertEquals("DELETE", deleteRequest.getMethod()); + DeleteOptions deleteOptions = Serialization.unmarshal(deleteRequest.getBody().readUtf8(), DeleteOptions.class); + assertEquals("Foreground", deleteOptions.getPropagationPolicy()); + + RecordedRequest postRequest = server.getLastRequest(); + assertEquals("/api/v1/namespaces/ns1/pods", postRequest.getPath()); + assertEquals("POST", postRequest.getMethod()); + } + @Test void testCreateOrReplaceWithDeleteExistingWithCreateFailed() { // Given