Skip to content

feature: controller reconciliation max delay #871

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

Merged
merged 13 commits into from
Jan 31, 2022
28 changes: 25 additions & 3 deletions docs/documentation/features.md
Original file line number Diff line number Diff line change
Expand Up @@ -160,9 +160,7 @@ larger than the `.observedGeneration` field on status. In order to have this fea
.

If these conditions are fulfilled and generation awareness not turned off, the observed generation is automatically set
by the framework after the `reconcile` method is called. There is just one exception, when the reconciler returns
with `UpdateControl.updateResource`, in this case the status is not updated, just the custom resource - however, this
update will lead to a new reconciliation. Note that the observed generation is updated also
by the framework after the `reconcile` method is called. Note that the observed generation is updated also
when `UpdateControl.noUpdate()` is returned from the reconciler. See this feature working in
the [WebPage example](https://github.com/java-operator-sdk/java-operator-sdk/blob/b91221bb54af19761a617bf18eef381e8ceb3b4c/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageStatus.java#L5)
.
Expand Down Expand Up @@ -223,6 +221,30 @@ public class DeploymentReconciler
}
```

## Max Interval Between Reconciliations

In case informers are all in place and reconciler is implemented correctly, there is no need for additional triggers.
However, it's a [common practice](https://github.com/java-operator-sdk/java-operator-sdk/issues/848#issuecomment-1016419966)
to have a failsafe periodic trigger in place,
just to make sure the resources are reconciled after certain time. This functionality is in place by default, there
is quite high interval (currently 10 hours) while the reconciliation is triggered. See how to override this using
the standard annotation:

```java
@ControllerConfiguration(finalizerName = NO_FINALIZER,
reconciliationMaxInterval = @ReconciliationMaxInterval(
interval = 50,
timeUnit = TimeUnit.MILLISECONDS))
```

The event is not propagated in a fixed rate, rather it's scheduled after each reconciliation. So the
next reconciliation will after at most within the specified interval after last reconciliation.

This feature can be turned off by setting `reconciliationMaxInterval` to [`Constants.NO_RECONCILIATION_MAX_INTERVAL`](https://github.com/java-operator-sdk/java-operator-sdk/blob/442e7d8718e992a36880e42bd0a5c01affaec9df/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Constants.java#L8-L8)
or any non-positive number.

The automatic retries are not affected by this feature, in case of an error no schedule is set by this feature.

## Automatic Retries on Error

When an exception is thrown from a controller, the framework will schedule an automatic retry of the reconciliation. The
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package io.javaoperatorsdk.operator.api.config;

import java.lang.reflect.ParameterizedType;
import java.time.Duration;
import java.util.Collections;
import java.util.Optional;
import java.util.Set;

import io.fabric8.kubernetes.api.model.HasMetadata;
Expand Down Expand Up @@ -114,4 +116,8 @@ default boolean useFinalizer() {
default ResourceEventFilter<R> getEventFilter() {
return ResourceEventFilters.passthrough();
}

default Optional<Duration> reconciliationMaxInterval() {
return Optional.of(Duration.ofHours(10L));
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package io.javaoperatorsdk.operator.api.config;

import java.time.Duration;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
Expand All @@ -16,6 +17,7 @@ public class ControllerConfigurationOverrider<R extends HasMetadata> {
private String labelSelector;
private ResourceEventFilter<R> customResourcePredicate;
private final ControllerConfiguration<R> original;
private Duration reconciliationMaxInterval;

private ControllerConfigurationOverrider(ControllerConfiguration<R> original) {
finalizer = original.getFinalizer();
Expand All @@ -24,7 +26,9 @@ private ControllerConfigurationOverrider(ControllerConfiguration<R> original) {
retry = original.getRetryConfiguration();
labelSelector = original.getLabelSelector();
customResourcePredicate = original.getEventFilter();
reconciliationMaxInterval = original.reconciliationMaxInterval().orElse(null);
this.original = original;

}

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

public ControllerConfigurationOverrider<R> withReconciliationMaxInterval(
Duration reconciliationMaxInterval) {
this.reconciliationMaxInterval = reconciliationMaxInterval;
return this;
}

public ControllerConfiguration<R> build() {
return new DefaultControllerConfiguration<>(
original.getAssociatedReconcilerClassName(),
Expand All @@ -86,6 +96,7 @@ public ControllerConfiguration<R> build() {
labelSelector,
customResourcePredicate,
original.getResourceClass(),
reconciliationMaxInterval,
original.getConfigurationService());
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package io.javaoperatorsdk.operator.api.config;

import java.time.Duration;
import java.util.Collections;
import java.util.Optional;
import java.util.Set;

import io.fabric8.kubernetes.api.model.HasMetadata;
Expand All @@ -20,6 +22,7 @@ public class DefaultControllerConfiguration<R extends HasMetadata>
private final String labelSelector;
private final ResourceEventFilter<R> resourceEventFilter;
private final Class<R> resourceClass;
private final Duration reconciliationMaxInterval;
private ConfigurationService service;

public DefaultControllerConfiguration(
Expand All @@ -33,6 +36,7 @@ public DefaultControllerConfiguration(
String labelSelector,
ResourceEventFilter<R> resourceEventFilter,
Class<R> resourceClass,
Duration reconciliationMaxInterval,
ConfigurationService service) {
this.associatedControllerClassName = associatedControllerClassName;
this.name = name;
Expand All @@ -41,6 +45,7 @@ public DefaultControllerConfiguration(
this.generationAware = generationAware;
this.namespaces =
namespaces != null ? Collections.unmodifiableSet(namespaces) : Collections.emptySet();
this.reconciliationMaxInterval = reconciliationMaxInterval;
this.watchAllNamespaces = this.namespaces.isEmpty();
this.retryConfiguration =
retryConfiguration == null
Expand Down Expand Up @@ -122,4 +127,9 @@ public Class<R> getResourceClass() {
public ResourceEventFilter<R> getEventFilter() {
return resourceEventFilter;
}

@Override
public Optional<Duration> reconciliationMaxInterval() {
return Optional.ofNullable(reconciliationMaxInterval);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ public final class Constants {
public static final String EMPTY_STRING = "";
public static final String WATCH_CURRENT_NAMESPACE = "JOSDK_WATCH_CURRENT";
public static final String NO_FINALIZER = "JOSDK_NO_FINALIZER";
public static final long NO_RECONCILIATION_MAX_INTERVAL = -1L;

private Constants() {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
import java.util.concurrent.TimeUnit;

import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilter;

Expand Down Expand Up @@ -56,4 +57,8 @@
*/
@SuppressWarnings("rawtypes")
Class<? extends ResourceEventFilter>[] eventFilters() default {};

ReconciliationMaxInterval reconciliationMaxInterval() default @ReconciliationMaxInterval(
interval = 10, timeUnit = TimeUnit.HOURS);

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package io.javaoperatorsdk.operator.api.reconciler;

import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
import java.util.concurrent.TimeUnit;

@Retention(RetentionPolicy.RUNTIME)
@Target({ElementType.TYPE})
public @interface ReconciliationMaxInterval {

/**
* A max delay between two reconciliations. Having this value larger than zero, will ensure that a
* reconciliation is scheduled with a target interval after the last reconciliation. Note that
* this not applies for retries, in case of an exception reconciliation is not scheduled. This is
* not a fixed rate, in other words a new reconciliation is scheduled after each reconciliation.
* <p/>
* If an interval is specified by {@link UpdateControl} or {@link DeleteControl}, those take
* precedence.
* <p/>
* This is a fail-safe feature, in the sense that if informers are in place and the reconciler
* implementation is correct, this feature can be turned off.
* <p/>
* Use NO_RECONCILIATION_MAX_INTERVAL in {@link Constants} to turn off this feature.
*
* @return max delay between reconciliations
**/
long interval();

/**
* @return time unit for max delay between reconciliations
*/
TimeUnit timeUnit() default TimeUnit.HOURS;

}
Original file line number Diff line number Diff line change
Expand Up @@ -245,9 +245,12 @@ private PostExecutionControl<R> createPostExecutionControl(R updatedCustomResour
private void updatePostExecutionControlWithReschedule(
PostExecutionControl<R> postExecutionControl,
BaseControl<?> baseControl) {
baseControl.getScheduleDelay().ifPresent(postExecutionControl::withReSchedule);
baseControl.getScheduleDelay().ifPresentOrElse(postExecutionControl::withReSchedule,
() -> controller.getConfiguration().reconciliationMaxInterval()
.ifPresent(m -> postExecutionControl.withReSchedule(m.toMillis())));
}


private PostExecutionControl<R> handleCleanup(R resource, Context context) {
log.debug(
"Executing delete for resource: {} with version: {}",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ private static class TestControllerConfiguration<R extends HasMetadata>

public TestControllerConfiguration(Reconciler<R> controller, Class<R> crClass) {
super(null, getControllerName(controller),
CustomResource.getCRDName(crClass), null, false, null, null, null, null, crClass, null);
CustomResource.getCRDName(crClass), null, false, null, null, null, null, crClass,
null, null);
this.controller = controller;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package io.javaoperatorsdk.operator.processing.event;

import java.time.Duration;
import java.util.ArrayList;
import java.util.Optional;
import java.util.concurrent.TimeUnit;
Expand Down Expand Up @@ -47,13 +48,15 @@ class ReconciliationDispatcherTest {

private static final String DEFAULT_FINALIZER = "javaoperatorsdk.io/finalizer";
public static final String ERROR_MESSAGE = "ErrorMessage";
public static final long RECONCILIATION_MAX_INTERVAL = 10L;
private TestCustomResource testCustomResource;
private ReconciliationDispatcher<TestCustomResource> reconciliationDispatcher;
private final Reconciler<TestCustomResource> reconciler = mock(Reconciler.class,
withSettings().extraInterfaces(ErrorStatusHandler.class));
private final ConfigurationService configService = mock(ConfigurationService.class);
private final CustomResourceFacade<TestCustomResource> customResourceFacade =
mock(ReconciliationDispatcher.CustomResourceFacade.class);
private ControllerConfiguration configuration = mock(ControllerConfiguration.class);

@BeforeEach
void setup() {
Expand All @@ -63,17 +66,23 @@ void setup() {
}

private <R extends HasMetadata> ReconciliationDispatcher<R> init(R customResource,
Reconciler<R> reconciler, ControllerConfiguration<R> configuration,
Reconciler<R> reconciler, ControllerConfiguration configuration,
CustomResourceFacade<R> customResourceFacade, boolean useFinalizer) {

configuration = configuration == null ? mock(ControllerConfiguration.class) : configuration;
ReconciliationDispatcherTest.this.configuration = configuration;
final var finalizer = useFinalizer ? DEFAULT_FINALIZER : Constants.NO_FINALIZER;
when(configuration.getFinalizer()).thenReturn(finalizer);
when(configuration.useFinalizer()).thenCallRealMethod();
when(configuration.getName()).thenReturn("EventDispatcherTestController");
when(configuration.getResourceClass()).thenReturn((Class<R>) customResource.getClass());
when(configuration.getRetryConfiguration()).thenReturn(RetryConfiguration.DEFAULT);
when(configuration.reconciliationMaxInterval())
.thenReturn(Optional.of(Duration.ofHours(RECONCILIATION_MAX_INTERVAL)));

when(configuration.getConfigurationService()).thenReturn(configService);


/*
* We need this for mock reconcilers to properly generate the expected UpdateControl: without
* this, calls such as `when(reconciler.reconcile(eq(testCustomResource),
Expand Down Expand Up @@ -429,6 +438,35 @@ void callErrorStatusHandlerEvenOnFirstError() {
any(), any());
}

@Test
void schedulesReconciliationIfMaxDelayIsSet() {
testCustomResource.addFinalizer(DEFAULT_FINALIZER);

when(reconciler.reconcile(eq(testCustomResource), any()))
.thenReturn(UpdateControl.noUpdate());

PostExecutionControl control =
reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource));

assertThat(control.getReScheduleDelay()).isPresent()
.hasValue(TimeUnit.HOURS.toMillis(RECONCILIATION_MAX_INTERVAL));
}

@Test
void canSkipSchedulingMaxDelayIf() {
testCustomResource.addFinalizer(DEFAULT_FINALIZER);

when(reconciler.reconcile(eq(testCustomResource), any()))
.thenReturn(UpdateControl.noUpdate());
when(configuration.reconciliationMaxInterval())
.thenReturn(Optional.empty());

PostExecutionControl control =
reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource));

assertThat(control.getReScheduleDelay()).isNotPresent();
}

private ObservedGenCustomResource createObservedGenCustomResource() {
ObservedGenCustomResource observedGenCustomResource = new ObservedGenCustomResource();
observedGenCustomResource.setMetadata(new ObjectMeta());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ public ControllerConfig(String finalizer, boolean generationAware,
null,
eventFilter,
customResourceClass,
null);
null, null);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ public TestConfiguration(boolean generationAware) {
null,
null,
TestCustomResource.class,
null,
null);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,12 @@ public List<Reconciler> getReconcilers() {
.collect(Collectors.toUnmodifiableList());
}

public Reconciler getFirstReconciler() {
return operator.getControllers().stream()
.map(Controller::getReconciler)
.findFirst().orElseThrow();
}

@SuppressWarnings({"rawtypes"})
public <T extends Reconciler> T getControllerOfType(Class<T> type) {
return operator.getControllers().stream()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package io.javaoperatorsdk.operator.config.runtime;

import java.time.Duration;
import java.util.Optional;
import java.util.Set;
import java.util.function.Function;

Expand Down Expand Up @@ -108,6 +110,21 @@ public ResourceEventFilter<R> getEventFilter() {
: ResourceEventFilters.passthrough();
}



@Override
public Optional<Duration> reconciliationMaxInterval() {
if (annotation.reconciliationMaxInterval() != null) {
if (annotation.reconciliationMaxInterval().interval() <= 0) {
return Optional.empty();
}
return Optional.of(Duration.of(annotation.reconciliationMaxInterval().interval(),
annotation.reconciliationMaxInterval().timeUnit().toChronoUnit()));
} else {
return io.javaoperatorsdk.operator.api.config.ControllerConfiguration.super.reconciliationMaxInterval();
}
}

public static <T> T valueOrDefault(ControllerConfiguration controllerConfiguration,
Function<ControllerConfiguration, T> mapper,
T defaultValue) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
import static org.assertj.core.api.Assertions.assertThat;
import static org.awaitility.Awaitility.await;

public class ConcurrencyIT {
class ConcurrencyIT {
public static final int NUMBER_OF_RESOURCES_CREATED = 50;
public static final int NUMBER_OF_RESOURCES_DELETED = 30;
public static final int NUMBER_OF_RESOURCES_UPDATED = 20;
Expand All @@ -34,7 +34,7 @@ public class ConcurrencyIT {
.build();

@Test
public void manyResourcesGetCreatedUpdatedAndDeleted() throws InterruptedException {
void manyResourcesGetCreatedUpdatedAndDeleted() throws InterruptedException {
log.info("Creating {} new resources", NUMBER_OF_RESOURCES_CREATED);
for (int i = 0; i < NUMBER_OF_RESOURCES_CREATED; i++) {
TestCustomResource tcr = TestUtils.testCustomResourceWithPrefix(String.valueOf(i));
Expand Down
Loading