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.

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

feat: reconcile marked for deletion #2359

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ protected <P extends HasMetadata> ControllerConfiguration<P> configFor(Reconcile
Constants.NO_VALUE_SET),
null,
Utils.instantiate(annotation.itemStore(), ItemStore.class, context), dependentFieldManager,
this, informerListLimit);
this, informerListLimit, annotation.reconcileResourcesMarkedForDeletion());

ResourceEventFilter<P> answer = deprecatedEventFilter(annotation);
config.setEventFilter(answer != null ? answer : ResourceEventFilters.passthrough());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@

public interface ControllerConfiguration<P extends HasMetadata> extends ResourceConfiguration<P> {

boolean DEFAULT_RECONCILER_RESOURCES_MARKED_FOR_DELETION = false;


@SuppressWarnings("rawtypes")
RateLimiter DEFAULT_RATE_LIMITER = LinearRateLimiter.deactivatedRateLimiter();
/**
Expand Down Expand Up @@ -140,4 +143,8 @@ default String fieldManager() {
return getName();
}

default boolean reconcileResourcesMarkedForDeletion() {
return DEFAULT_RECONCILER_RESOURCES_MARKED_FOR_DELETION;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ public class ControllerConfigurationOverrider<R extends HasMetadata> {
private String name;
private String fieldManager;
private Long informerListLimit;
private Boolean reconcileResourcesMarkedForDeletion;

private ControllerConfigurationOverrider(ControllerConfiguration<R> original) {
this.finalizer = original.getFinalizerName();
Expand All @@ -59,6 +60,7 @@ private ControllerConfigurationOverrider(ControllerConfiguration<R> original) {
this.fieldManager = original.fieldManager();
this.informerListLimit = original.getInformerListLimit().orElse(null);
this.itemStore = original.getItemStore().orElse(null);
this.reconcileResourcesMarkedForDeletion = original.reconcileResourcesMarkedForDeletion();
}

public ControllerConfigurationOverrider<R> withFinalizer(String finalizer) {
Expand Down Expand Up @@ -179,6 +181,12 @@ public ControllerConfigurationOverrider<R> withFieldManager(
return this;
}

public ControllerConfigurationOverrider<R> withReconcileResourcesMarkedForDeletion(
boolean reconcileResourcesMarkedForDeletion) {
this.reconcileResourcesMarkedForDeletion = reconcileResourcesMarkedForDeletion;
return this;
}


/**
* Sets a max page size limit when starting the informer. This will result in pagination while
Expand Down Expand Up @@ -216,7 +224,7 @@ public ControllerConfiguration<R> build() {
reconciliationMaxInterval, onAddFilter, onUpdateFilter, genericFilter,
original.getDependentResources(),
namespaces, finalizer, labelSelector, configurations, itemStore, fieldManager,
original.getConfigurationService(), informerListLimit);
original.getConfigurationService(), informerListLimit, reconcileResourcesMarkedForDeletion);
overridden.setEventFilter(customResourcePredicate);
return overridden;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ public class ResolvedControllerConfiguration<P extends HasMetadata>
private final ItemStore<P> itemStore;
private final ConfigurationService configurationService;
private final String fieldManager;
private final boolean reconcileResourcesMarkedForDeletion;

private ResourceEventFilter<P> eventFilter;
private List<DependentResourceSpec> dependentResources;
Expand All @@ -47,7 +48,7 @@ public ResolvedControllerConfiguration(Class<P> resourceClass, ControllerConfigu
other.getFinalizerName(), other.getLabelSelector(), Collections.emptyMap(),
other.getItemStore().orElse(null), other.fieldManager(),
other.getConfigurationService(),
other.getInformerListLimit().orElse(null));
other.getInformerListLimit().orElse(null), other.reconcileResourcesMarkedForDeletion());
}

public static Duration getMaxReconciliationInterval(long interval, TimeUnit timeUnit) {
Expand Down Expand Up @@ -76,11 +77,12 @@ public ResolvedControllerConfiguration(Class<P> resourceClass, String name,
Set<String> namespaces, String finalizer, String labelSelector,
Map<DependentResourceSpec, Object> configurations, ItemStore<P> itemStore,
String fieldManager,
ConfigurationService configurationService, Long informerListLimit) {
ConfigurationService configurationService, Long informerListLimit,
Boolean reconcileResourcesMarkedForDeletion) {
this(resourceClass, name, generationAware, associatedReconcilerClassName, retry, rateLimiter,
maxReconciliationInterval, onAddFilter, onUpdateFilter, genericFilter,
namespaces, finalizer, labelSelector, configurations, itemStore, fieldManager,
configurationService, informerListLimit);
configurationService, informerListLimit, reconcileResourcesMarkedForDeletion);
setDependentResources(dependentResources);
}

Expand All @@ -92,7 +94,8 @@ protected ResolvedControllerConfiguration(Class<P> resourceClass, String name,
Set<String> namespaces, String finalizer, String labelSelector,
Map<DependentResourceSpec, Object> configurations, ItemStore<P> itemStore,
String fieldManager,
ConfigurationService configurationService, Long informerListLimit) {
ConfigurationService configurationService, Long informerListLimit,
boolean reconcileResourcesMarkedForDeletion) {
super(resourceClass, namespaces, labelSelector, onAddFilter, onUpdateFilter, genericFilter,
itemStore, informerListLimit);
this.configurationService = configurationService;
Expand All @@ -107,13 +110,14 @@ protected ResolvedControllerConfiguration(Class<P> resourceClass, String name,
this.finalizer =
ControllerConfiguration.ensureValidFinalizerName(finalizer, getResourceTypeName());
this.fieldManager = fieldManager;
this.reconcileResourcesMarkedForDeletion = reconcileResourcesMarkedForDeletion;
}

protected ResolvedControllerConfiguration(Class<P> resourceClass, String name,
Class<? extends Reconciler> reconcilerClas, ConfigurationService configurationService) {
this(resourceClass, name, false, getAssociatedReconcilerClassName(reconcilerClas), null, null,
null, null, null, null, null,
null, null, null, null, null, configurationService, null);
null, null, null, null, null, configurationService, null, false);
}

@Override
Expand Down Expand Up @@ -195,4 +199,9 @@ public Optional<ItemStore<P>> getItemStore() {
public String fieldManager() {
return fieldManager;
}

@Override
public boolean reconcileResourcesMarkedForDeletion() {
return reconcileResourcesMarkedForDeletion;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import io.javaoperatorsdk.operator.processing.retry.Retry;

import static io.javaoperatorsdk.operator.api.config.ControllerConfiguration.CONTROLLER_NAME_AS_FIELD_MANAGER;
import static io.javaoperatorsdk.operator.api.config.ControllerConfiguration.DEFAULT_RECONCILER_RESOURCES_MARKED_FOR_DELETION;
import static io.javaoperatorsdk.operator.api.reconciler.Constants.NO_LONG_VALUE_SET;

@Inherited
Expand Down Expand Up @@ -166,4 +167,11 @@ MaxReconciliationInterval maxReconciliationInterval() default @MaxReconciliation
* the informer cache.
*/
long informerListLimit() default NO_LONG_VALUE_SET;

/**
* if true and reconciler not implements {@link Cleaner} interface executes reconciliation even if
* controller is marked for deletion, thus when waiting for other finalizer removal. This allows
* to manage corner cases when not all resources will need a finalizer.
*/
boolean reconcileResourcesMarkedForDeletion() default DEFAULT_RECONCILER_RESOURCES_MARKED_FOR_DELETION;
}
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ private PostExecutionControl<P> handleDispatch(ExecutionScope<P> executionScope)
originalResource.getMetadata().getNamespace());

final var markedForDeletion = originalResource.isMarkedForDeletion();
if (markedForDeletion && shouldNotDispatchToCleanupWhenMarkedForDeletion(originalResource)) {
if (markedForDeletion && shouldNotDispatchWhenMarkedForDeletion(originalResource)) {
log.debug(
"Skipping cleanup of resource {} because finalizer(s) {} don't allow processing yet",
getName(originalResource),
Expand All @@ -85,21 +85,22 @@ private PostExecutionControl<P> handleDispatch(ExecutionScope<P> executionScope)

Context<P> context =
new DefaultContext<>(executionScope.getRetryInfo(), controller, originalResource);
if (markedForDeletion) {
if (markedForDeletion && controller.useFinalizer()) {
return handleCleanup(resourceForExecution, context);
} else {
return handleReconcile(executionScope, resourceForExecution, originalResource, context);
}
}

private boolean shouldNotDispatchToCleanupWhenMarkedForDeletion(P resource) {
private boolean shouldNotDispatchWhenMarkedForDeletion(P resource) {
var alreadyRemovedFinalizer = controller.useFinalizer()
&& !resource.hasFinalizer(configuration().getFinalizerName());
if (alreadyRemovedFinalizer) {
log.warn("This should not happen. Marked for deletion & already removed finalizer: {}",
ResourceID.fromResource(resource));
}
return !controller.useFinalizer() || alreadyRemovedFinalizer;
return !controller.getConfiguration().reconcileResourcesMarkedForDeletion() &&
(!controller.useFinalizer() || alreadyRemovedFinalizer);
}

private PostExecutionControl<P> handleReconcile(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,8 @@ public ControllerConfig(String finalizer, boolean generationAware,
null,
null,
null,
null, null, null, finalizer, null, null, null, new BaseConfigurationService(), null);
null, null, null, finalizer, null, null, null, new BaseConfigurationService(), null,
false);
setEventFilter(eventFilter);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ public TestConfiguration(boolean generationAware, OnAddFilter<TestCustomResource
null,
FINALIZER,
null, null, null, new BaseConfigurationService(),
null);
null, DEFAULT_RECONCILER_RESOURCES_MARKED_FOR_DELETION);
}
}
}
Loading