Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fabric8 deletion behaviour fix #2241

Merged
merged 2 commits into from
Jan 17, 2020
Merged

Fabric8 deletion behaviour fix #2241

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
45340a4
Fabric8 deletion behaviour fix
sknot-rh Nov 25, 2019
565f44f
rebase
sknot-rh Jan 8, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions ...est/java/io/strimzi/operator/cluster/operator/assembly/KafkaAssemblyOperatorMockTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ public void testUpdateClusterWithoutKafkaSecrets(Params params, VertxTestContext
private void updateClusterWithoutSecrets(Params params, VertxTestContext context, String... secrets) throws InterruptedException, ExecutionException, TimeoutException {
KafkaAssemblyOperator kco = createCluster(params, context);
for (String secret: secrets) {
mockClient.secrets().inNamespace(NAMESPACE).withName(secret).delete();
mockClient.secrets().inNamespace(NAMESPACE).withName(secret).cascading(true).delete();
assertThat("Expected secret " + secret + " to be not exist",
mockClient.secrets().inNamespace(NAMESPACE).withName(secret).get(), is(nullValue()));
}
Expand All @@ -359,7 +359,7 @@ private void updateClusterWithoutServices(Params params, VertxTestContext contex
setFields(params);
KafkaAssemblyOperator kco = createCluster(params, context);
for (String service: services) {
mockClient.services().inNamespace(NAMESPACE).withName(service).delete();
mockClient.services().inNamespace(NAMESPACE).withName(service).cascading(true).delete();
assertThat("Expected service " + service + " to be not exist",
mockClient.services().inNamespace(NAMESPACE).withName(service).get(), is(nullValue()));
}
Expand Down Expand Up @@ -410,7 +410,7 @@ public void testUpdateClusterWithoutKafkaStatefulSet(Params params, VertxTestCon
private void updateClusterWithoutStatefulSet(Params params, VertxTestContext context, String statefulSet) throws InterruptedException, ExecutionException, TimeoutException {
KafkaAssemblyOperator kco = createCluster(params, context);

mockClient.apps().statefulSets().inNamespace(NAMESPACE).withName(statefulSet).delete();
mockClient.apps().statefulSets().inNamespace(NAMESPACE).withName(statefulSet).cascading(true).delete();
assertThat("Expected sts " + statefulSet + " to be not exist",
mockClient.apps().statefulSets().inNamespace(NAMESPACE).withName(statefulSet).get(), is(nullValue()));

Expand Down Expand Up @@ -608,7 +608,7 @@ public void testUpdateKafkaWithChangedDeleteClaim(Params params, VertxTestContex


LOGGER.info("Reconciling again -> delete");
kafkaAssembly(NAMESPACE, CLUSTER_NAME).delete();
kafkaAssembly(NAMESPACE, CLUSTER_NAME).cascading(true).delete();
Checkpoint deleteAsync = context.checkpoint();
kco.reconcile(new Reconciliation("test-trigger", Kafka.RESOURCE_KIND, NAMESPACE, CLUSTER_NAME)).setHandler(ar -> {
if (ar.failed()) ar.cause().printStackTrace();
Expand Down
2 changes: 1 addition & 1 deletion mockkube/src/main/java/io/strimzi/test/mockkube/DeploymentMockBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ protected void mockPatch(String resourceName, RollableScalableResource<Deploymen
// delete the first "old" Pod if there is one still remaining
if (podsForDeployments.get(deployment.getMetadata().getName()).size() > 0) {
String podToDelete = podsForDeployments.get(deployment.getMetadata().getName()).remove(0);
mockPods.inNamespace(deployment.getMetadata().getNamespace()).withName(podToDelete).delete();
mockPods.inNamespace(deployment.getMetadata().getNamespace()).withName(podToDelete).cascading(true).delete();
}

}
Expand Down
2 changes: 1 addition & 1 deletion mockkube/src/main/java/io/strimzi/test/mockkube/MockBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ protected void checkDoesExist(String resourceName) {
}

protected void mockDelete(String resourceName, R resource) {
when(resource.delete()).thenAnswer(i -> {
when(resource.cascading(true).delete()).thenAnswer(i -> {
return doDelete(resourceName);
});
}
Expand Down
6 changes: 3 additions & 3 deletions mockkube/src/main/java/io/strimzi/test/mockkube/StatefulSetMockBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,14 +193,14 @@ private void doRecreatePod(String namespace,

@Override
protected void mockDelete(String resourceName, RollableScalableResource<StatefulSet, DoneableStatefulSet> resource) {
when(resource.delete()).thenAnswer(i -> {
when(resource.cascading(true).delete()).thenAnswer(i -> {
LOGGER.debug("delete {} {}", resourceType, resourceName);
StatefulSet removed = db.remove(resourceName);
if (removed != null) {
fireWatchers(resourceName, removed, Watcher.Action.DELETED, "delete");
for (Map.Entry<String, Pod> pod : new HashMap<>(podDb).entrySet()) {
if (pod.getKey().matches(resourceName + "-[0-9]+")) {
mockPods.inNamespace(removed.getMetadata().getNamespace()).withName(pod.getKey()).delete();
mockPods.inNamespace(removed.getMetadata().getNamespace()).withName(pod.getKey()).cascading(true).delete();
}
}
}
Expand All @@ -226,7 +226,7 @@ private StatefulSet doPatch(String resourceName, StatefulSet argument) {
LOGGER.debug("scaling down {} {} from {} to {}", resourceType, resourceName, oldScale, newScale);
for (int i = oldScale - 1; i >= newScale; i--) {
String newPodName = argument.getMetadata().getName() + "-" + i;
mockPods.inNamespace(argument.getMetadata().getNamespace()).withName(newPodName).delete();
mockPods.inNamespace(argument.getMetadata().getNamespace()).withName(newPodName).cascading(true).delete();
}
} else {
db.put(resourceName, copyResource(argument));
Expand Down
4 changes: 2 additions & 2 deletions mockkube/src/test/java/io/strimzi/test/io/strimzi/test/mockkube/MockKubeRegressionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public void onClose(KubernetesClientException cause) {

}
});
client.pods().inNamespace("ns").withName(ns.get(0).getMetadata().getName()).delete();
client.pods().inNamespace("ns").withName(ns.get(0).getMetadata().getName()).cascading(true).delete();

assertThat(deleted.get(), is(true));
assertThat(recreated.get(), is(true));
Expand All @@ -81,7 +81,7 @@ public void onClose(KubernetesClientException cause) {
ns = client.pods().inNamespace("ns").list().getItems();
assertThat(ns.size(), is(3));

client.apps().statefulSets().inNamespace("ns").withName("foo").delete();
client.apps().statefulSets().inNamespace("ns").withName("foo").cascading(true).delete();

}
}
4 changes: 2 additions & 2 deletions mockkube/src/test/java/io/strimzi/test/io/strimzi/test/mockkube/MockKubeTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ public void podNameScopedCreateListGetDelete(Class<RT> cls,
// TODO assertNull(gotResource);

// Delete
assertThat(mixedOp.apply(client).withName(pod.getMetadata().getName()).delete(), is(true));
assertThat(mixedOp.apply(client).withName(pod.getMetadata().getName()).cascading(true).delete(), is(true));
assertThat(w.lastEvent().action, is(Watcher.Action.DELETED));
RT resource = (RT) w.lastEvent().resource;
resource.getMetadata().setResourceVersion(null);
Expand All @@ -256,7 +256,7 @@ public void podNameScopedCreateListGetDelete(Class<RT> cls,
gotResource = mixedOp.apply(client).withName(pod.getMetadata().getName()).get();
assertThat(gotResource, is(nullValue()));

assertThat(mixedOp.apply(client).withName(pod.getMetadata().getName()).delete(), is(false));
assertThat(mixedOp.apply(client).withName(pod.getMetadata().getName()).cascading(true).delete(), is(false));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the MockKube properly supports cascading deletion (i.e. I suspect it behaves the same for a cascading as a non-cascading delete), but if the tests pass I guess it doesn't matter.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the test pass.


// TODO Delete off a withLabels query, delete off a inNamespace
// TODO inAnyNamespace()
Expand Down
2 changes: 1 addition & 1 deletion .../src/test/java/io/strimzi/operator/common/operator/resource/KafkaBridgeCrdOperatorIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ protected KafkaBridge getResource() {
private Future<Void> deleteResource() {
// The resource has to be deleted this was and not using reconcile due to https://github.com/fabric8io/kubernetes-client/pull/1325
// Fix this override when project is using fabric8 version > 4.1.1
kafkaBridgeOperator.operation().inNamespace(namespace).withName(RESOURCE_NAME).delete();
kafkaBridgeOperator.operation().inNamespace(namespace).withName(RESOURCE_NAME).cascading(true).delete();

return kafkaBridgeOperator.waitFor(namespace, RESOURCE_NAME, 1_000, 60_000, (ignore1, ignore2) -> {
KafkaBridge deletion = kafkaBridgeOperator.get(namespace, RESOURCE_NAME);
Expand Down
2 changes: 1 addition & 1 deletion ...src/test/java/io/strimzi/operator/common/operator/resource/KafkaConnectCrdOperatorIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ protected KafkaConnect getResource() {
private Future<Void> deleteResource() {
// The resource has to be deleted this was and not using reconcile due to https://github.com/fabric8io/kubernetes-client/pull/1325
// Fix this override when project is using fabric8 version > 4.1.1
kafkaConnectOperator.operation().inNamespace(namespace).withName(RESOURCE_NAME).delete();
kafkaConnectOperator.operation().inNamespace(namespace).withName(RESOURCE_NAME).cascading(true).delete();

return kafkaConnectOperator.waitFor(namespace, RESOURCE_NAME, 1_000, 60_000, (ignore1, ignore2) -> {
KafkaConnect deletion = kafkaConnectOperator.get(namespace, RESOURCE_NAME);
Expand Down
2 changes: 1 addition & 1 deletion .../test/java/io/strimzi/operator/common/operator/resource/KafkaConnectS2IcrdOperatorIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ protected KafkaConnectS2I getResource() {
private Future<Void> deleteResource() {
// The resource has to be deleted this was and not using reconcile due to https://github.com/fabric8io/kubernetes-client/pull/1325
// Fix this override when project is using fabric8 version > 4.1.1
kafkaConnectS2Ioperator.operation().inNamespace(namespace).withName(RESOURCE_NAME).delete();
kafkaConnectS2Ioperator.operation().inNamespace(namespace).withName(RESOURCE_NAME).cascading(true).delete();

return kafkaConnectS2Ioperator.waitFor(namespace, RESOURCE_NAME, 1_000, 60_000, (ignore1, ignore2) -> {
KafkaConnectS2I deletion = kafkaConnectS2Ioperator.get(namespace, RESOURCE_NAME);
Expand Down
2 changes: 1 addition & 1 deletion ...common/src/test/java/io/strimzi/operator/common/operator/resource/KafkaCrdOperatorIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ protected Kafka getResource() {
private Future<Void> deleteResource() {
// The resource has to be deleted this was and not using reconcile due to https://github.com/fabric8io/kubernetes-client/pull/1325
// Fix this override when project is using fabric8 version > 4.1.1
kafkaOperator.operation().inNamespace(namespace).withName(RESOURCE_NAME).delete();
kafkaOperator.operation().inNamespace(namespace).withName(RESOURCE_NAME).cascading(true).delete();

return kafkaOperator.waitFor(namespace, RESOURCE_NAME, 1_000, 60_000, (ignore1, ignore2) -> {
Kafka deletion = kafkaOperator.get(namespace, RESOURCE_NAME);
Expand Down
2 changes: 1 addition & 1 deletion ...test/java/io/strimzi/operator/common/operator/resource/KafkaMirrorMakerCrdOperatorIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ protected KafkaMirrorMaker getResource() {
private Future<Void> deleteResource() {
// The resource has to be deleted this was and not using reconcile due to https://github.com/fabric8io/kubernetes-client/pull/1325
// Fix this override when project is using fabric8 version > 4.1.1
kafkaMirrorMakerOperator.operation().inNamespace(namespace).withName(RESOURCE_NAME).delete();
kafkaMirrorMakerOperator.operation().inNamespace(namespace).withName(RESOURCE_NAME).cascading(true).delete();

return kafkaMirrorMakerOperator.waitFor(namespace, RESOURCE_NAME, 1_000, 60_000, (ignore1, ignore2) -> {
KafkaMirrorMaker deletion = kafkaMirrorMakerOperator.get(namespace, RESOURCE_NAME);
Expand Down
2 changes: 1 addition & 1 deletion ...on/src/test/java/io/strimzi/operator/common/operator/resource/KafkaUserCrdOperatorIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ protected KafkaUser getResource() {
private Future<Void> deleteResource() {
// The resource has to be deleted this was and not using reconcile due to https://github.com/fabric8io/kubernetes-client/pull/1325
// Fix this override when project is using fabric8 version > 4.1.1
kafkaUserOperator.operation().inNamespace(namespace).withName(RESOURCE_NAME).delete();
kafkaUserOperator.operation().inNamespace(namespace).withName(RESOURCE_NAME).cascading(true).delete();

return kafkaUserOperator.waitFor(namespace, RESOURCE_NAME, 1_000, 60_000, (ignore1, ignore2) -> {
KafkaUser deletion = kafkaUserOperator.get(namespace, RESOURCE_NAME);
Expand Down
6 changes: 3 additions & 3 deletions pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,9 @@
<maven-jar-plugin.version>3.1.0</maven-jar-plugin.version>
<sundrio.version>0.19.0</sundrio.version>

<fabric8.kubernetes-client.version>4.6.2</fabric8.kubernetes-client.version>
<fabric8.openshift-client.version>4.6.2</fabric8.openshift-client.version>
<fabric8.kubernetes-model.version>4.6.2</fabric8.kubernetes-model.version>
<fabric8.kubernetes-client.version>4.6.4</fabric8.kubernetes-client.version>
<fabric8.openshift-client.version>4.6.4</fabric8.openshift-client.version>
<fabric8.kubernetes-model.version>4.6.4</fabric8.kubernetes-model.version>
<fabric8.zjsonpatch.version>0.3.0</fabric8.zjsonpatch.version>
<okhttp.version>3.12.0</okhttp.version>
<okio.version>1.15.0</okio.version>
Expand Down
2 changes: 1 addition & 1 deletion systemtest/src/test/java/io/strimzi/systemtest/TopicST.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ void testDeleteTopicEnableFalse() throws Exception {

String topicUid = KafkaTopicUtils.topicSnapshot(topicName);
LOGGER.info("Going to delete topic {}", topicName);
KafkaTopicResource.kafkaTopicClient().inNamespace(NAMESPACE).withName(topicName).delete();
KafkaTopicResource.kafkaTopicClient().inNamespace(NAMESPACE).withName(topicName).cascading(true).delete();
LOGGER.info("Topic {} deleted", topicName);

KafkaTopicUtils.waitTopicHasRolled(topicName, topicUid);
Expand Down
12 changes: 6 additions & 6 deletions test/src/main/java/io/strimzi/test/k8s/KubeClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public void createNamespace(String name) {
}

public void deleteNamespace(String name) {
client.namespaces().withName(name).delete();
client.namespaces().withName(name).cascading(true).delete();
}

/**
Expand All @@ -100,7 +100,7 @@ public boolean getNamespaceStatus(String namespaceName) {


public void deleteConfigMap(String configMapName) {
client.configMaps().inNamespace(getNamespace()).withName(configMapName).delete();
client.configMaps().inNamespace(getNamespace()).withName(configMapName).cascading(true).delete();
}

public ConfigMap getConfigMap(String configMapName) {
Expand Down Expand Up @@ -261,7 +261,7 @@ public String getStatefulSetUid(String statefulSetName) {
}

public void deleteStatefulSet(String statefulSetName) {
client.apps().statefulSets().inNamespace(getNamespace()).withName(statefulSetName).delete();
client.apps().statefulSets().inNamespace(getNamespace()).withName(statefulSetName).cascading(true).delete();
}

public Deployment createOrReplaceDeployment(Deployment deployment) {
Expand Down Expand Up @@ -308,7 +308,7 @@ public Map<String, String> getDeploymentConfigSelectors(String deploymentConfigN
* @param deploymentConfigName deployment config name
*/
public void deleteDeploymentConfig(String deploymentConfigName) {
client.adapt(OpenShiftClient.class).deploymentConfigs().inNamespace(getNamespace()).withName(deploymentConfigName).delete();
client.adapt(OpenShiftClient.class).deploymentConfigs().inNamespace(getNamespace()).withName(deploymentConfigName).cascading(true).delete();
}

/**
Expand Down Expand Up @@ -359,7 +359,7 @@ public Secret getSecret(String secretName) {
}

public boolean deleteSecret(String secretName) {
return client.secrets().inNamespace(getNamespace()).withName(secretName).delete();
return client.secrets().inNamespace(getNamespace()).withName(secretName).cascading(true).delete();
}

public Service createService(Service service) {
Expand Down Expand Up @@ -412,7 +412,7 @@ public List<Service> listServices() {
}

public void deleteService(String serviceName) {
client.services().inNamespace(getNamespace()).withName(serviceName).delete();
client.services().inNamespace(getNamespace()).withName(serviceName).cascading(true).delete();
}

public void deleteService(Service service) {
Expand Down
2 changes: 1 addition & 1 deletion topic-operator/src/test/java/io/strimzi/operator/topic/TopicOperatorBaseIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ public void teardown() throws InterruptedException, TimeoutException, ExecutionE

// Wait for the operator to delete all the existing topics in Kafka
for (KafkaTopic item : items) {
operation().inNamespace(NAMESPACE).withName(item.getMetadata().getName()).delete();
operation().inNamespace(NAMESPACE).withName(item.getMetadata().getName()).cascading(true).delete();
waitForTopicInKube(item.getMetadata().getName(), false);
}
}
Expand Down