Skip to content

Commit 0549899

Browse files
committed
fix: max reconciliation interval applies after retry exhaustion
1 parent e617061 commit 0549899

File tree

5 files changed

+54
-42
lines changed

5 files changed

+54
-42
lines changed

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ public Controller(Reconciler<P> reconciler,
7878
managedWorkflow =
7979
ManagedWorkflow.workflowFor(kubernetesClient, configuration.getDependentResources());
8080
eventSourceManager = new EventSourceManager<>(this);
81-
eventProcessor = new EventProcessor<>(eventSourceManager);
81+
eventProcessor = new EventProcessor<>(getConfiguration(), eventSourceManager);
8282
eventSourceManager.postProcessDefaultEventSourcesAfterProcessorInitializer();
8383
}
8484

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventProcessor.java

Lines changed: 32 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import io.javaoperatorsdk.operator.OperatorException;
1515
import io.javaoperatorsdk.operator.api.config.ConfigurationService;
1616
import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider;
17+
import io.javaoperatorsdk.operator.api.config.ControllerConfiguration;
1718
import io.javaoperatorsdk.operator.api.config.ExecutorServiceManager;
1819
import io.javaoperatorsdk.operator.api.monitoring.Metrics;
1920
import io.javaoperatorsdk.operator.api.reconciler.Constants;
@@ -37,6 +38,7 @@ public class EventProcessor<R extends HasMetadata> implements EventHandler, Life
3738
private static final long MINIMAL_RATE_LIMIT_RESCHEDULE_DURATION = 50;
3839

3940
private volatile boolean running;
41+
private final ControllerConfiguration<?> controllerConfiguration;
4042
private final ReconciliationDispatcher<R> reconciliationDispatcher;
4143
private final Retry retry;
4244
private final ExecutorService executor;
@@ -48,60 +50,55 @@ public class EventProcessor<R extends HasMetadata> implements EventHandler, Life
4850
private final ResourceStateManager resourceStateManager = new ResourceStateManager();
4951
private final Map<String, Object> metricsMetadata;
5052

51-
public EventProcessor(EventSourceManager<R> eventSourceManager) {
53+
54+
public EventProcessor(ControllerConfiguration<?> controllerConfiguration,
55+
EventSourceManager<R> eventSourceManager) {
5256
this(
57+
controllerConfiguration,
5358
eventSourceManager.getControllerResourceEventSource(),
5459
ExecutorServiceManager.instance().executorService(),
55-
eventSourceManager.getController().getConfiguration().getName(),
5660
new ReconciliationDispatcher<>(eventSourceManager.getController()),
57-
eventSourceManager.getController().getConfiguration().getRetry(),
5861
ConfigurationServiceProvider.instance().getMetrics(),
59-
eventSourceManager.getController().getConfiguration().getRateLimiter(),
6062
eventSourceManager);
6163
}
6264

6365
@SuppressWarnings("rawtypes")
6466
EventProcessor(
67+
ControllerConfiguration controllerConfiguration,
6568
ReconciliationDispatcher<R> reconciliationDispatcher,
6669
EventSourceManager<R> eventSourceManager,
67-
String relatedControllerName,
68-
Retry retry,
69-
RateLimiter rateLimiter,
7070
Metrics metrics) {
7171
this(
72+
controllerConfiguration,
7273
eventSourceManager.getControllerResourceEventSource(),
7374
null,
74-
relatedControllerName,
7575
reconciliationDispatcher,
76-
retry,
7776
metrics,
78-
rateLimiter,
7977
eventSourceManager);
8078
}
8179

8280
@SuppressWarnings({"rawtypes", "unchecked"})
8381
private EventProcessor(
82+
ControllerConfiguration controllerConfiguration,
8483
Cache<R> cache,
8584
ExecutorService executor,
86-
String relatedControllerName,
8785
ReconciliationDispatcher<R> reconciliationDispatcher,
88-
Retry retry,
8986
Metrics metrics,
90-
RateLimiter rateLimiter,
9187
EventSourceManager<R> eventSourceManager) {
88+
this.controllerConfiguration = controllerConfiguration;
9289
this.running = false;
9390
this.executor =
9491
executor == null
9592
? new ScheduledThreadPoolExecutor(
9693
ConfigurationService.DEFAULT_RECONCILIATION_THREADS_NUMBER)
9794
: executor;
98-
this.controllerName = relatedControllerName;
95+
this.controllerName = controllerConfiguration.getName();
9996
this.reconciliationDispatcher = reconciliationDispatcher;
100-
this.retry = retry;
97+
this.retry = controllerConfiguration.getRetry();
10198
this.cache = cache;
10299
this.metrics = metrics != null ? metrics : Metrics.NOOP;
103100
this.eventSourceManager = eventSourceManager;
104-
this.rateLimiter = rateLimiter;
101+
this.rateLimiter = controllerConfiguration.getRateLimiter();
105102

106103
metricsMetadata = Optional.ofNullable(eventSourceManager.getController())
107104
.map(Controller::getAssociatedGroupVersionKind)
@@ -272,18 +269,31 @@ synchronized void eventProcessingFinished(
272269
reScheduleExecutionIfInstructed(postExecutionControl, executionScope.getResource());
273270
}
274271
}
275-
276272
}
277273

278274
private void reScheduleExecutionIfInstructed(
279275
PostExecutionControl<R> postExecutionControl, R customResource) {
276+
280277
postExecutionControl
281278
.getReScheduleDelay()
282-
.ifPresent(delay -> {
279+
.ifPresentOrElse(delay -> {
283280
var resourceID = ResourceID.fromResource(customResource);
284281
log.debug("ReScheduling event for resource: {} with delay: {}",
285282
resourceID, delay);
286283
retryEventSource().scheduleOnce(resourceID, delay);
284+
}, () -> scheduleExecutionForMaxReconciliationInterval(customResource));
285+
}
286+
287+
private void scheduleExecutionForMaxReconciliationInterval(R customResource) {
288+
this.controllerConfiguration
289+
.maxReconciliationInterval()
290+
.ifPresent(m -> {
291+
var resourceID = ResourceID.fromResource(customResource);
292+
var delay = m.toMillis();
293+
log.debug("ReScheduling event for resource because for max reconciliation interval: " +
294+
"{} with delay: {}",
295+
resourceID, delay);
296+
retryEventSource().scheduleOnce(resourceID, delay);
287297
});
288298
}
289299

@@ -319,7 +329,10 @@ private void handleRetryOnException(
319329
metrics.failedReconciliation(resourceID, exception, metricsMetadata);
320330
retryEventSource().scheduleOnce(resourceID, delay);
321331
},
322-
() -> log.error("Exhausted retries for {}", executionScope));
332+
() -> {
333+
log.error("Exhausted retries for {}", executionScope);
334+
scheduleExecutionForMaxReconciliationInterval(executionScope.getResource());
335+
});
323336
}
324337

325338
private void cleanupOnSuccessfulExecution(ExecutionScope<R> executionScope) {

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -264,9 +264,7 @@ private PostExecutionControl<P> createPostExecutionControl(P updatedCustomResour
264264
private void updatePostExecutionControlWithReschedule(
265265
PostExecutionControl<P> postExecutionControl,
266266
BaseControl<?> baseControl) {
267-
baseControl.getScheduleDelay().ifPresentOrElse(postExecutionControl::withReSchedule,
268-
() -> controller.getConfiguration().maxReconciliationInterval()
269-
.ifPresent(m -> postExecutionControl.withReSchedule(m.toMillis())));
267+
baseControl.getScheduleDelay().ifPresent(postExecutionControl::withReSchedule);
270268
}
271269

272270

operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventProcessorTest.java

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import org.slf4j.LoggerFactory;
1616

1717
import io.fabric8.kubernetes.api.model.HasMetadata;
18+
import io.javaoperatorsdk.operator.api.config.ControllerConfiguration;
1819
import io.javaoperatorsdk.operator.api.config.RetryConfiguration;
1920
import io.javaoperatorsdk.operator.api.monitoring.Metrics;
2021
import io.javaoperatorsdk.operator.processing.event.rate.LinearRateLimiter;
@@ -25,6 +26,7 @@
2526
import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEvent;
2627
import io.javaoperatorsdk.operator.processing.event.source.timer.TimerEventSource;
2728
import io.javaoperatorsdk.operator.processing.retry.GenericRetry;
29+
import io.javaoperatorsdk.operator.processing.retry.Retry;
2830
import io.javaoperatorsdk.operator.sample.simple.TestCustomResource;
2931

3032
import static io.javaoperatorsdk.operator.TestUtils.markForDeletion;
@@ -70,12 +72,15 @@ void setup() {
7072
when(eventSourceManagerMock.getControllerResourceEventSource())
7173
.thenReturn(controllerResourceEventSourceMock);
7274
eventProcessor =
73-
spy(new EventProcessor(reconciliationDispatcherMock, eventSourceManagerMock, "Test", null,
74-
rateLimiterMock, null));
75+
spy(new EventProcessor(controllerConfiguration(null, rateLimiterMock),
76+
reconciliationDispatcherMock,
77+
eventSourceManagerMock, null));
7578
eventProcessor.start();
7679
eventProcessorWithRetry =
77-
spy(new EventProcessor(reconciliationDispatcherMock, eventSourceManagerMock, "Test",
78-
GenericRetry.defaultLimitedExponentialRetry(), rateLimiterMock, null));
80+
spy(new EventProcessor(
81+
controllerConfiguration(GenericRetry.defaultLimitedExponentialRetry(),
82+
rateLimiterMock),
83+
reconciliationDispatcherMock, eventSourceManagerMock, null));
7984
eventProcessorWithRetry.start();
8085
when(eventProcessor.retryEventSource()).thenReturn(retryTimerEventSourceMock);
8186
when(eventProcessorWithRetry.retryEventSource()).thenReturn(retryTimerEventSourceMock);
@@ -258,8 +263,9 @@ void cancelScheduleOnceEventsOnSuccessfulExecution() {
258263
void startProcessedMarkedEventReceivedBefore() {
259264
var crID = new ResourceID("test-cr", TEST_NAMESPACE);
260265
eventProcessor =
261-
spy(new EventProcessor(reconciliationDispatcherMock, eventSourceManagerMock, "Test", null,
262-
LinearRateLimiter.deactivatedRateLimiter(),
266+
spy(new EventProcessor(controllerConfiguration(null,
267+
LinearRateLimiter.deactivatedRateLimiter()), reconciliationDispatcherMock,
268+
eventSourceManagerMock,
263269
metricsMock));
264270
when(controllerResourceEventSourceMock.get(eq(crID)))
265271
.thenReturn(Optional.of(testCustomResource()));
@@ -407,4 +413,12 @@ private void overrideData(ResourceID id, HasMetadata applyTo) {
407413
applyTo.getMetadata().setNamespace(id.getNamespace().orElse(null));
408414
}
409415

416+
ControllerConfiguration controllerConfiguration(Retry retry, RateLimiter rateLimiter) {
417+
ControllerConfiguration res = mock(ControllerConfiguration.class);
418+
when(res.getName()).thenReturn("Test");
419+
when(res.getRetry()).thenReturn(retry);
420+
when(res.getRateLimiter()).thenReturn(rateLimiter);
421+
return res;
422+
}
423+
410424
}

operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -600,19 +600,6 @@ void errorStatusHandlerCanPatchResource() {
600600
any(), any());
601601
}
602602

603-
@Test
604-
void schedulesReconciliationIfMaxDelayIsSet() {
605-
testCustomResource.addFinalizer(DEFAULT_FINALIZER);
606-
607-
reconciler.reconcile = (r, c) -> UpdateControl.noUpdate();
608-
609-
PostExecutionControl control =
610-
reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource));
611-
612-
assertThat(control.getReScheduleDelay()).isPresent()
613-
.hasValue(TimeUnit.HOURS.toMillis(RECONCILIATION_MAX_INTERVAL));
614-
}
615-
616603
@Test
617604
void canSkipSchedulingMaxDelayIf() {
618605
testCustomResource.addFinalizer(DEFAULT_FINALIZER);

0 commit comments

Comments
 (0)