Skip to content

Commit

Permalink
Fix #2855: .withPropagationPolicy and .withGracePeriod DSL methods ca…
Browse files Browse the repository at this point in the history
…n't be combined resource() deletion

+ Add gracePeriodSeconds parameter in ResourceHandler#delete
+ Modified CascadingDeletable interface so that it includes
GracePeriodConfigurable and PropagationPolicyConfiguratble
  • Loading branch information
rohanKanojia authored and manusa committed Mar 12, 2021
1 parent 170b762 commit 8d0957c
Show file tree
Hide file tree
Showing 19 changed files with 86 additions and 36 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* Fix #2828: Remove automatic instantiation of CustomResource spec and status as this feature was causing more issues than it was solving
* Fix #2857: Fix the log of an unexpected error from an Informer's EventHandler
* Fix #2853: Cannot change the type of the Service from ClusterIP to ExternalName with PATCH
* Fix #2855: `.withPropagationPolicy` and `.withGracePeriod` DSL methods can't be combined for Resource API deletion operations
* Fix #2783: OpenIDConnectionUtils#persistKubeConfigWithUpdatedToken persists access token instead of refresh token
* Fix #2871: Change longFileMode to LONGFILE\_POSIX for creating tar in PodUpload, improve exception handling in PodUpload.
* Fix #2746: SharedInformerFactory should use key formed from OperationContext
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ 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, boolean dryRun) {
return new ${model.name}OperationsImpl(client, config).withItem(item).inNamespace(namespace).withName(item.getMetadata().getName()).dryRun(dryRun).withPropagationPolicy(propagationPolicy).delete();
public Boolean delete(OkHttpClient client, Config config, String namespace, DeletionPropagation propagationPolicy, long gracePeriodSeconds, ${model.name} item, boolean dryRun) {
return new ${model.name}OperationsImpl(client, config).withItem(item).inNamespace(namespace).withName(item.getMetadata().getName()).dryRun(dryRun).withPropagationPolicy(propagationPolicy).withGracePeriod(gracePeriodSeconds).delete();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,8 @@ 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, boolean dryRun) {
return new ${model.name}OperationsImpl(client, config).withItem(item).inNamespace(namespace).withName(item.getMetadata().getName()).dryRun(dryRun).withPropagationPolicy(propagationPolicy).delete();
public Boolean delete(OkHttpClient client, Config config, String namespace, DeletionPropagation propagationPolicy, long gracePeriodSeconds, ${model.name} item, boolean dryRun) {
return new ${model.name}OperationsImpl(client, config).withItem(item).inNamespace(namespace).withName(item.getMetadata().getName()).dryRun(dryRun).withPropagationPolicy(propagationPolicy).withGracePeriod(gracePeriodSeconds).delete();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ 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, boolean dryRun) {
return new ${model.name}OperationsImpl(client, config).withItem(item).inNamespace(namespace).withName(item.getMetadata().getName()).dryRun(dryRun).withPropagationPolicy(propagationPolicy).delete();
public Boolean delete(OkHttpClient client, Config config, String namespace, DeletionPropagation propagationPolicy, long gracePeriodSeconds, ${model.name} item, boolean dryRun) {
return new ${model.name}OperationsImpl(client, config).withItem(item).inNamespace(namespace).withName(item.getMetadata().getName()).dryRun(dryRun).withPropagationPolicy(propagationPolicy).withGracePeriod(gracePeriodSeconds).delete();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ 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, boolean dryRun) {
return new ${model.name}OperationsImpl(client, config).withItem(item).inNamespace(namespace).withName(item.getMetadata().getName()).dryRun(dryRun).withPropagationPolicy(propagationPolicy).delete();
public Boolean delete(OkHttpClient client, Config config, String namespace, DeletionPropagation propagationPolicy, long gracePeriodSeconds, ${model.name} item, boolean dryRun) {
return new ${model.name}OperationsImpl(client, config).withItem(item).inNamespace(namespace).withName(item.getMetadata().getName()).dryRun(dryRun).withPropagationPolicy(propagationPolicy).withGracePeriod(gracePeriodSeconds).delete();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ 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, boolean dryRun) {
return new ${model.name}OperationsImpl(client, config).withItem(item).inNamespace(namespace).withName(item.getMetadata().getName()).dryRun(dryRun).withPropagationPolicy(propagationPolicy).delete();
public Boolean delete(OkHttpClient client, Config config, String namespace, DeletionPropagation propagationPolicy, long gracePeriodSeconds, ${model.name} item, boolean dryRun) {
return new ${model.name}OperationsImpl(client, config).withItem(item).inNamespace(namespace).withName(item.getMetadata().getName()).dryRun(dryRun).withPropagationPolicy(propagationPolicy).withGracePeriod(gracePeriodSeconds).delete();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,11 +122,12 @@ public boolean equals(Object obj) {
* @param config The client config.
* @param namespace The target namespace.
* @param propagationPolicy Whether and how garbage collection will be performed.
* @param gracePeriodSeconds The duration in seconds before the object should be deleted.
* @param item The resource to delete.
* @param dryRun enable dry run
* @return The true if the resource was successfully deleted.
*/
Boolean delete(OkHttpClient client, Config config, String namespace, DeletionPropagation propagationPolicy, T item, boolean dryRun);
Boolean delete(OkHttpClient client, Config config, String namespace, DeletionPropagation propagationPolicy, long gracePeriodSeconds, T item, boolean dryRun);


/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,15 @@
*/
package io.fabric8.kubernetes.client.dsl;

import io.fabric8.kubernetes.client.GracePeriodConfigurable;
import io.fabric8.kubernetes.client.PropagationPolicyConfigurable;

/**
* Created by iocanel on 9/15/16.
*/
public interface CascadingDeletable<T> extends Deletable, Cascading<Deletable>, Recreateable<Applicable<T>> {
public interface CascadingDeletable<T> extends Deletable,
Cascading<Deletable>,
GracePeriodConfigurable<CascadingDeletable<T>>,
PropagationPolicyConfigurable<CascadingDeletable<T>>,
Recreateable<Applicable<T>> {
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@

public class HasMetadataOperation<T extends HasMetadata, L extends KubernetesResourceList<T>, R extends Resource<T>> extends BaseOperation< T, L, R> {
public static final DeletionPropagation DEFAULT_PROPAGATION_POLICY = DeletionPropagation.BACKGROUND;
public static final long DEFAULT_GRACE_PERIOD_IN_SECONDS = -1L;

public HasMetadataOperation(OperationContext ctx) {
super(ctx);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@ public OperationSupport(OkHttpClient client, Config config) {
this(new OperationContext().withOkhttpClient(client).withConfig(config));
}

public OperationSupport(OkHttpClient client, Config config, String namespace, DeletionPropagation propagationPolicy) {
this(new OperationContext().withOkhttpClient(client).withConfig(config).withNamespace(namespace).withPropagationPolicy(propagationPolicy));
public OperationSupport(OkHttpClient client, Config config, String namespace, DeletionPropagation propagationPolicy, long gracePeriodInSeconds) {
this(new OperationContext().withOkhttpClient(client).withConfig(config).withNamespace(namespace).withPropagationPolicy(propagationPolicy).withGracePeriodSeconds(gracePeriodInSeconds));
}

public OperationSupport(OperationContext ctx) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@
import java.util.Arrays;
import java.util.List;

import static io.fabric8.kubernetes.client.dsl.base.HasMetadataOperation.DEFAULT_GRACE_PERIOD_IN_SECONDS;

public class KubernetesListOperationsImpl
extends OperationSupport
implements KubernetesListOperation,
Expand All @@ -55,11 +57,11 @@ public class KubernetesListOperationsImpl
private final Boolean deletingExisting;

public KubernetesListOperationsImpl(OkHttpClient client, Config config, String namespace) {
this(client, config, namespace, null, null, false, false, null, null, false);
this(client, config, namespace, null, null, DEFAULT_GRACE_PERIOD_IN_SECONDS, false, false, null, null, false);
}

public KubernetesListOperationsImpl(OkHttpClient client, Config config, String namespace, String name, DeletionPropagation propagationPolicy, Boolean fromServer, Boolean deletingExisting, KubernetesList item, String resourceVersion, boolean dryRun) {
super(client, config, namespace, propagationPolicy);
public KubernetesListOperationsImpl(OkHttpClient client, Config config, String namespace, String name, DeletionPropagation propagationPolicy, long gracePeriodSeconds, Boolean fromServer, Boolean deletingExisting, KubernetesList item, String resourceVersion, boolean dryRun) {
super(client, config, namespace, propagationPolicy, gracePeriodSeconds);
this.fromServer = fromServer;
this.deletingExisting = deletingExisting;
this.item = item;
Expand Down Expand Up @@ -116,7 +118,7 @@ public RecreateFromServerGettable<KubernetesList> load(String path) {

@Override
public RecreateFromServerGettable<KubernetesList> load(InputStream is) {
return new KubernetesListOperationsImpl(client, config, namespace, null, DeletionPropagation.BACKGROUND, fromServer, deletingExisting, unmarshal(is, KubernetesList.class), null, dryRun);
return new KubernetesListOperationsImpl(client, config, namespace, null, context.getPropagationPolicy(), context.getGracePeriodSeconds(), fromServer, deletingExisting, unmarshal(is, KubernetesList.class), null, dryRun);
}

@Override
Expand All @@ -142,7 +144,7 @@ public Boolean delete(List<KubernetesList> lists) {
for (KubernetesList list : lists) {
for (HasMetadata item : list.getItems()) {
ResourceHandler<HasMetadata, HasMetadataVisitiableBuilder> handler = Handlers.get(item.getKind(), item.getApiVersion());
if (!handler.delete(client, config, namespace, DeletionPropagation.BACKGROUND, item, dryRun)) {
if (!handler.delete(client, config, namespace, context.getPropagationPolicy(), context.getGracePeriodSeconds(), item, dryRun)) {
return false;
}
}
Expand All @@ -152,12 +154,12 @@ public Boolean delete(List<KubernetesList> lists) {

@Override
public Gettable<KubernetesList> fromServer() {
return new KubernetesListOperationsImpl(client, config, namespace, null, context.getPropagationPolicy(), true, deletingExisting, item, null, dryRun);
return new KubernetesListOperationsImpl(client, config, namespace, null, context.getPropagationPolicy(), context.getGracePeriodSeconds(), true, deletingExisting, item, null, dryRun);
}

@Override
public Createable<KubernetesList> deletingExisting() {
return new KubernetesListOperationsImpl(client, config, namespace, null, context.getPropagationPolicy(), fromServer, true, item, null, dryRun);
return new KubernetesListOperationsImpl(client, config, namespace, null, context.getPropagationPolicy(), context.getGracePeriodSeconds(), fromServer, true, item, null, dryRun);
}

private List<HasMetadata> createItemsInKubernetesList(KubernetesList list) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ public HasMetadata createOrReplace() {
ResourceHandler<HasMetadata, HasMetadataVisitiableBuilder> h = handlerOf(meta);
String namespaceToUse = meta.getMetadata().getNamespace();
if (Boolean.TRUE.equals(deletingExisting)) {
return deleteAndCreateItem(client, config, meta, h, namespaceToUse, propagationPolicy, dryRun);
return deleteAndCreateItem(client, config, meta, h, namespaceToUse, propagationPolicy, gracePeriodSeconds, dryRun);
}
return createOrReplaceItem(client, config, meta, h, namespaceToUse, dryRun);
}
Expand All @@ -154,7 +154,7 @@ public Boolean delete() {
//First pass check before deleting
HasMetadata meta = acceptVisitors(asHasMetadata(item), visitors);
ResourceHandler<HasMetadata, HasMetadataVisitiableBuilder> h = handlerOf(meta);
return h.delete(client, config, meta.getMetadata().getNamespace(), propagationPolicy, meta, dryRun);
return h.delete(client, config, meta.getMetadata().getNamespace(), propagationPolicy, gracePeriodSeconds, meta, dryRun);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ public Boolean delete() {
//Second pass do delete
for (HasMetadata meta : acceptVisitors(asHasMetadata(item, true), visitors)) {
ResourceHandler<HasMetadata, HasMetadataVisitiableBuilder> h = handlerOf(meta);
if (!h.delete(client, config, meta.getMetadata().getNamespace(), propagationPolicy, meta, dryRun)) {
if (!h.delete(client, config, meta.getMetadata().getNamespace(), propagationPolicy, gracePeriodSeconds, meta, dryRun)) {
return false;
}
}
Expand Down Expand Up @@ -449,7 +449,7 @@ private static <T> ResourceHandler handlerOf(T item) {

private HasMetadata createOrReplaceOrDeleteExisting(HasMetadata meta, ResourceHandler<HasMetadata, HasMetadataVisitiableBuilder> h, String namespaceToUse, boolean dryRun) {
if (Boolean.TRUE.equals(deletingExisting)) {
return deleteAndCreateItem(client, config, meta, h, namespaceToUse, propagationPolicy, dryRun);
return deleteAndCreateItem(client, config, meta, h, namespaceToUse, propagationPolicy, gracePeriodSeconds, dryRun);
}
return createOrReplaceItem(client, config, meta, h, namespaceToUse, dryRun);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@
import java.util.List;
import java.util.concurrent.TimeUnit;

import static io.fabric8.kubernetes.client.dsl.base.HasMetadataOperation.DEFAULT_GRACE_PERIOD_IN_SECONDS;
import static io.fabric8.kubernetes.client.dsl.base.HasMetadataOperation.DEFAULT_PROPAGATION_POLICY;

@Component
@org.apache.felix.scr.annotations.Service
public class KubernetesListHandler implements ResourceHandler<KubernetesList, KubernetesListBuilder> {
Expand All @@ -57,7 +60,7 @@ public String getApiVersion() {

@Override
public KubernetesList create(OkHttpClient client, Config config, String namespace, KubernetesList item, boolean dryRun) {
return new KubernetesListOperationsImpl(client, config, namespace, null, DeletionPropagation.BACKGROUND, false, false, item, null, dryRun).create();
return new KubernetesListOperationsImpl(client, config, namespace, null, DEFAULT_PROPAGATION_POLICY, DEFAULT_GRACE_PERIOD_IN_SECONDS, false, false, item, null, dryRun).create();
}

@Override
Expand Down Expand Up @@ -96,8 +99,8 @@ public KubernetesListBuilder edit(KubernetesList item) {
}

@Override
public Boolean delete(OkHttpClient client, Config config, String namespace, DeletionPropagation propagationPolicy, KubernetesList item, boolean dryRun) {
return new KubernetesListOperationsImpl(client, config, namespace, null, propagationPolicy, false, false, item, null, dryRun).delete(item);
public Boolean delete(OkHttpClient client, Config config, String namespace, DeletionPropagation propagationPolicy, long gracePeriodSeconds, KubernetesList item, boolean dryRun) {
return new KubernetesListOperationsImpl(client, config, namespace, null, propagationPolicy, gracePeriodSeconds, false, false, item, null, dryRun).delete(item);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,10 @@ public T deleteAndCreate(T item) {
}
}

public static HasMetadata deleteAndCreateItem(OkHttpClient client, Config config, HasMetadata meta, ResourceHandler<HasMetadata, HasMetadataVisitiableBuilder> h, String namespaceToUse, DeletionPropagation propagationPolicy, boolean dryRun) {
public static HasMetadata deleteAndCreateItem(OkHttpClient client, Config config, HasMetadata meta, ResourceHandler<HasMetadata, HasMetadataVisitiableBuilder> h, String namespaceToUse, DeletionPropagation propagationPolicy, long gracePeriodSeconds, boolean dryRun) {
DeleteAndCreateHelper<HasMetadata> deleteAndCreateHelper = new DeleteAndCreateHelper<>(
m -> h.create(client, config, namespaceToUse, m, dryRun),
m -> h.delete(client, config, namespaceToUse, propagationPolicy, m, dryRun),
m -> h.delete(client, config, namespaceToUse, propagationPolicy, gracePeriodSeconds, m, dryRun),
waitUntilDeletedOrInterrupted(client, config, h, namespaceToUse)
);

Expand Down
4 changes: 2 additions & 2 deletions kubernetes-client/src/main/resources/resource-handler.vm
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ 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, boolean dryRun) {
return new ${model.name}OperationsImpl(client, config, namespace).withItem(item).dryRun(dryRun).withPropagationPolicy(propagationPolicy).delete();
public Boolean delete(OkHttpClient client, Config config, String namespace, DeletionPropagation propagationPolicy, long gracePeriodSeconds, ${model.name} item, boolean dryRun) {
return new ${model.name}OperationsImpl(client, config, namespace).withItem(item).dryRun(dryRun).withPropagationPolicy(propagationPolicy).withGracePeriod(gracePeriodSeconds).delete();
}

@Override
Expand Down
Loading

0 comments on commit 8d0957c

Please sign in to comment.