diff --git a/bootstrapper-maven-plugin/pom.xml b/bootstrapper-maven-plugin/pom.xml index 93a253178f..9edf011170 100644 --- a/bootstrapper-maven-plugin/pom.xml +++ b/bootstrapper-maven-plugin/pom.xml @@ -22,7 +22,7 @@ io.javaoperatorsdk java-operator-sdk - 5.3.5-SNAPSHOT + 999-SNAPSHOT bootstrapper @@ -32,7 +32,7 @@ 3.15.2 - 3.9.16 + 3.9.15 3.1.0 3.15.2 diff --git a/caffeine-bounded-cache-support/pom.xml b/caffeine-bounded-cache-support/pom.xml index dc51e2b04f..be70ab9a2e 100644 --- a/caffeine-bounded-cache-support/pom.xml +++ b/caffeine-bounded-cache-support/pom.xml @@ -21,7 +21,7 @@ io.javaoperatorsdk java-operator-sdk - 5.3.5-SNAPSHOT + 999-SNAPSHOT caffeine-bounded-cache-support diff --git a/docs/content/en/docs/documentation/error-handling-retries.md b/docs/content/en/docs/documentation/error-handling-retries.md index eeecf54751..7bd4ad2e22 100644 --- a/docs/content/en/docs/documentation/error-handling-retries.md +++ b/docs/content/en/docs/documentation/error-handling-retries.md @@ -135,6 +135,9 @@ these features: 2. In case an exception is thrown, a retry is initiated. However, if an event is received meanwhile, it will be reconciled instantly, and this execution won't count as a retry attempt. + If that event-triggered reconciliation also fails inside the current retry window, the + existing retry deadline is preserved rather than reset — the failure does not advance the + retry counter unless the original deadline is imminent. 3. If the retry limit is reached (so no more automatic retry would happen), but a new event received, the reconciliation will still happen, but won't reset the retry, and will still be marked as the last attempt in the retry info. The point (1) still holds - thus successful reconciliation will reset the retry - but no retry will happen in case of an error. diff --git a/micrometer-support/pom.xml b/micrometer-support/pom.xml index 6f40d73afe..ae3c4d0be1 100644 --- a/micrometer-support/pom.xml +++ b/micrometer-support/pom.xml @@ -21,7 +21,7 @@ io.javaoperatorsdk java-operator-sdk - 5.3.5-SNAPSHOT + 999-SNAPSHOT micrometer-support diff --git a/migration/pom.xml b/migration/pom.xml index 182b81fc4c..5ceb39b6ca 100644 --- a/migration/pom.xml +++ b/migration/pom.xml @@ -21,7 +21,7 @@ io.javaoperatorsdk java-operator-sdk - 5.3.5-SNAPSHOT + 999-SNAPSHOT migration diff --git a/operator-framework-bom/pom.xml b/operator-framework-bom/pom.xml index b8ac4a4ccb..8bac943f16 100644 --- a/operator-framework-bom/pom.xml +++ b/operator-framework-bom/pom.xml @@ -21,7 +21,7 @@ io.javaoperatorsdk operator-framework-bom - 5.3.5-SNAPSHOT + 999-SNAPSHOT pom Operator SDK - Bill of Materials Java SDK for implementing Kubernetes operators @@ -54,7 +54,7 @@ 3.2.8 3.4.0 3.12.0 - 3.5.1 + 3.5.0 0.10.0 diff --git a/operator-framework-core/pom.xml b/operator-framework-core/pom.xml index 0f59f22eb8..2356433ca9 100644 --- a/operator-framework-core/pom.xml +++ b/operator-framework-core/pom.xml @@ -21,7 +21,7 @@ io.javaoperatorsdk java-operator-sdk - 5.3.5-SNAPSHOT + 999-SNAPSHOT ../pom.xml diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Context.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Context.java index 2df74d4298..75d12eb1ad 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Context.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Context.java @@ -114,6 +114,88 @@ default Stream getSecondaryResourcesAsStream(Class expectedType) { Optional getSecondaryResource(Class expectedType, String eventSourceName); + /** + * Retrieves a specific secondary resource by name and namespace from the event source identified + * by the given name. + * + *

This is a typed convenience over manually retrieving the {@link + * io.javaoperatorsdk.operator.processing.event.source.EventSource} and calling its cache. When + * the underlying event source implements {@link + * io.javaoperatorsdk.operator.processing.event.source.Cache}, the lookup is a direct cache lookup + * and read-cache-after-write consistent. + * + *

{@code eventSourceName} may be {@code null}. When {@code null} and {@code expectedType} is + * part of a managed workflow whose activation condition may not have registered the event source, + * an empty {@link Optional} is returned instead of throwing {@link + * io.javaoperatorsdk.operator.processing.event.NoEventSourceForClassException}. + * + * @param expectedType the class representing the type of secondary resource to retrieve + * @param eventSourceName the name of the event source to look in (may be {@code null}) + * @param name the name of the secondary resource + * @param namespace the namespace of the secondary resource (may be {@code null} for + * cluster-scoped resources) + * @param the type of secondary resource to retrieve + * @return an {@link Optional} containing the matching secondary resource, or {@link + * Optional#empty()} if none matches + * @throws io.javaoperatorsdk.operator.processing.event.NoEventSourceForClassException if no event + * source is registered for the given type and name (and no workflow activation condition + * accounts for it) + * @since 5.4.0 + */ + Optional getSecondaryResource( + Class expectedType, String eventSourceName, String name, String namespace); + + /** + * Convenience overload of {@link #getSecondaryResource(Class, String, String, String)} that uses + * the primary resource's namespace. + * + *

If the primary resource is cluster-scoped (no namespace), the lookup is performed against + * the cluster scope. To target a specific namespace from a cluster-scoped primary, use {@link + * #getSecondaryResource(Class, String, String, String)} directly. + * + *

{@code eventSourceName} may be {@code null} with the same semantics as in {@link + * #getSecondaryResource(Class, String, String, String)}. + * + * @param expectedType the class representing the type of secondary resource to retrieve + * @param eventSourceName the name of the event source to look in (may be {@code null}) + * @param name the name of the secondary resource (namespace inferred from the primary) + * @param the type of secondary resource to retrieve + * @return an {@link Optional} containing the matching secondary resource, or {@link + * Optional#empty()} if none matches + * @since 5.4.0 + */ + default Optional getSecondaryResource( + Class expectedType, String eventSourceName, String name) { + return getSecondaryResource( + expectedType, eventSourceName, name, getPrimaryResource().getMetadata().getNamespace()); + } + + /** + * Retrieves a {@link Stream} of the secondary resources of the specified type from the event + * source identified by the given name. Useful when several event sources are registered for the + * same type and you need to scope retrieval to one of them, or when you want to apply a custom + * filter at the call site. + * + *

When the underlying event source implements {@link ResourceCache}, the stream is + * read-cache-after-write consistent. + * + *

{@code eventSourceName} may be {@code null} with the same semantics as in {@link + * #getSecondaryResource(Class, String, String, String)}: when {@code null} and {@code + * expectedType} is part of a managed workflow whose activation condition may not have registered + * the event source, an empty {@link Stream} is returned instead of throwing {@link + * io.javaoperatorsdk.operator.processing.event.NoEventSourceForClassException}. + * + * @param expectedType the class representing the type of secondary resources to retrieve + * @param eventSourceName the name of the event source to look in (may be {@code null}) + * @param the type of secondary resources to retrieve + * @return a {@link Stream} of secondary resources of the specified type + * @throws io.javaoperatorsdk.operator.processing.event.NoEventSourceForClassException if no event + * source is registered for the given type and name (and no workflow activation condition + * accounts for it) + * @since 5.4.0 + */ + Stream getSecondaryResourcesAsStream(Class expectedType, String eventSourceName); + ControllerConfiguration

getControllerConfiguration(); /** diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java index ac5a7b41b9..2d9a22b6fa 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java @@ -36,6 +36,7 @@ import io.javaoperatorsdk.operator.processing.event.EventSourceRetriever; import io.javaoperatorsdk.operator.processing.event.NoEventSourceForClassException; import io.javaoperatorsdk.operator.processing.event.ResourceID; +import io.javaoperatorsdk.operator.processing.event.source.Cache; public class DefaultContext

implements Context

{ private RetryInfo retryInfo; @@ -95,6 +96,20 @@ public Stream getSecondaryResourcesAsStream(Class expectedType, boolea } } + /** + * Whether a missing event source for the given type is the expected case, in which case callers + * should return an empty result instead of propagating the {@link + * NoEventSourceForClassException}. + * + *

If a workflow has an activation condition there can be event sources which are only + * registered if the activation condition holds, but to provide a consistent API we return an + * empty result instead of throwing an exception. Note that not only the resource which has an + * activation condition might not be registered but dependents which depend on it. + */ + private boolean isMissingEventSourceExpected(String eventSourceName, Class expectedType) { + return eventSourceName == null && controller.workflowContainsDependentForType(expectedType); + } + private Map deduplicatedMap(Stream stream) { return stream.collect( Collectors.toUnmodifiableMap( @@ -120,19 +135,51 @@ public Optional getSecondaryResource(Class expectedType, String eventS .getEventSourceFor(expectedType, eventSourceName) .getSecondaryResource(primaryResource); } catch (NoEventSourceForClassException e) { - /* - * If a workflow has an activation condition there can be event sources which are only - * registered if the activation condition holds, but to provide a consistent API we return an - * Optional instead of throwing an exception. - * - * Note that not only the resource which has an activation condition might not be registered - * but dependents which depend on it. - */ - if (eventSourceName == null && controller.workflowContainsDependentForType(expectedType)) { + if (isMissingEventSourceExpected(eventSourceName, expectedType)) { return Optional.empty(); - } else { - throw e; } + throw e; + } + } + + @Override + public Optional getSecondaryResource( + Class expectedType, String eventSourceName, String name, String namespace) { + try { + final var eventSource = + controller.getEventSourceManager().getEventSourceFor(expectedType, eventSourceName); + final var resourceID = new ResourceID(name, namespace); + if (eventSource instanceof Cache cache) { + return cache.get(resourceID).map(expectedType::cast); + } + return eventSource.getSecondaryResources(primaryResource).stream() + .filter(r -> ResourceID.fromResource(r).equals(resourceID)) + .findFirst(); + } catch (NoEventSourceForClassException e) { + if (isMissingEventSourceExpected(eventSourceName, expectedType)) { + return Optional.empty(); + } + throw e; + } + } + + @Override + public Stream getSecondaryResourcesAsStream( + Class expectedType, String eventSourceName) { + try { + final var eventSource = + controller.getEventSourceManager().getEventSourceFor(expectedType, eventSourceName); + if (eventSource instanceof ResourceCache resourceCache) { + final var ns = primaryResource.getMetadata().getNamespace(); + final Stream stream = ns == null ? resourceCache.list() : resourceCache.list(ns); + return stream.map(expectedType::cast); + } + return eventSource.getSecondaryResources(primaryResource).stream(); + } catch (NoEventSourceForClassException e) { + if (isMissingEventSourceExpected(eventSourceName, expectedType)) { + return Stream.empty(); + } + throw e; } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventProcessor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventProcessor.java index 1aecac6c9a..83e42687bc 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventProcessor.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventProcessor.java @@ -49,6 +49,13 @@ public class EventProcessor

implements EventHandler, Life private static final Logger log = LoggerFactory.getLogger(EventProcessor.class); private static final long MINIMAL_RATE_LIMIT_RESCHEDULE_DURATION = 50; + /** + * Threshold below which an event-driven failed reconciliation that lands inside the current retry + * window is allowed to consume a retry attempt (i.e. advance the retry counter). Above this + * threshold the existing retry deadline is preserved instead. + */ + private static final long RETRY_DEADLINE_PRESERVE_THRESHOLD_MILLIS = 5_000; + private volatile boolean running; private final ControllerConfiguration controllerConfiguration; private final ReconciliationDispatcher

reconciliationDispatcher; @@ -369,6 +376,15 @@ private void handleRetryOnException(ExecutionScope

executionScope, Exception submitReconciliationExecution(state); return; } + Optional remaining = state.getRetry().remainingDurationUntilNextRetry(); + if (remaining.isPresent() + && remaining.get().toMillis() > RETRY_DEADLINE_PRESERVE_THRESHOLD_MILLIS) { + log.debug( + "Preserving existing retry deadline; remaining: {} ms. Not consuming a retry attempt.", + remaining.get().toMillis()); + retryEventSource().scheduleOnce(resourceID, remaining.get().toMillis()); + return; + } Optional nextDelay = state.getRetry().nextDelay(); nextDelay.ifPresentOrElse( delay -> { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/GenericRetryExecution.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/GenericRetryExecution.java index 4bdce57a77..fadc022de7 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/GenericRetryExecution.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/GenericRetryExecution.java @@ -15,6 +15,7 @@ */ package io.javaoperatorsdk.operator.processing.retry; +import java.time.Duration; import java.util.Optional; public class GenericRetryExecution implements RetryExecution { @@ -23,6 +24,7 @@ public class GenericRetryExecution implements RetryExecution { private int lastAttemptIndex = 0; private long currentInterval; + private Long lastNextDelayCallEpochMillis; public GenericRetryExecution(GenericRetry genericRetry) { this.genericRetry = genericRetry; @@ -40,6 +42,7 @@ public Optional nextDelay() { } } lastAttemptIndex++; + lastNextDelayCallEpochMillis = System.currentTimeMillis(); return Optional.of(currentInterval); } @@ -52,4 +55,16 @@ public boolean isLastAttempt() { public int getAttemptCount() { return lastAttemptIndex; } + + @Override + public Optional remainingDurationUntilNextRetry() { + if (lastNextDelayCallEpochMillis == null) { + return Optional.empty(); + } + long remaining = (lastNextDelayCallEpochMillis + currentInterval) - System.currentTimeMillis(); + if (remaining <= 0) { + return Optional.empty(); + } + return Optional.of(Duration.ofMillis(remaining)); + } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/RetryExecution.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/RetryExecution.java index caf71d7a33..a644a274ba 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/RetryExecution.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/RetryExecution.java @@ -15,6 +15,7 @@ */ package io.javaoperatorsdk.operator.processing.retry; +import java.time.Duration; import java.util.Optional; import io.javaoperatorsdk.operator.api.reconciler.RetryInfo; @@ -25,4 +26,15 @@ public interface RetryExecution extends RetryInfo { * @return the time to wait until the next execution in milliseconds */ Optional nextDelay(); + + /** + * Remaining time of the currently scheduled retry interval, i.e. the time until the previously + * computed retry delay would elapse. Returns an empty {@link Optional} if no retry has been + * scheduled yet (i.e. {@link #nextDelay()} has never been called) or if the deadline has already + * passed. + * + *

Used to decide whether an event-driven failed reconciliation that lands well inside the + * retry window should consume a retry attempt or simply be re-scheduled on the original deadline. + */ + Optional remainingDurationUntilNextRetry(); } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContextTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContextTest.java index 4df8df385b..7b9658f98d 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContextTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContextTest.java @@ -16,26 +16,34 @@ package io.javaoperatorsdk.operator.api.reconciler; import java.util.List; +import java.util.Optional; import java.util.Set; +import java.util.stream.Stream; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import io.fabric8.kubernetes.api.model.ConfigMap; +import io.fabric8.kubernetes.api.model.ConfigMapBuilder; import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; import io.fabric8.kubernetes.api.model.Pod; import io.fabric8.kubernetes.api.model.PodBuilder; import io.fabric8.kubernetes.api.model.Secret; +import io.fabric8.kubernetes.api.model.SecretBuilder; import io.javaoperatorsdk.operator.processing.Controller; import io.javaoperatorsdk.operator.processing.event.EventSourceManager; import io.javaoperatorsdk.operator.processing.event.NoEventSourceForClassException; import io.javaoperatorsdk.operator.processing.event.ResourceID; import io.javaoperatorsdk.operator.processing.event.source.EventSource; +import io.javaoperatorsdk.operator.processing.event.source.informer.ManagedInformerEventSource; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; class DefaultContextTest { @@ -63,6 +71,234 @@ void getSecondaryResourceReturnsEmptyOptionalOnNonActivatedDRType() { assertThat(res).isEmpty(); } + @Test + void getSecondaryResourceByNameAndNamespaceReturnsFromCacheFastPath() { + final var cm = + new ConfigMapBuilder() + .withNewMetadata() + .withName("cm-foo") + .withNamespace("ns") + .endMetadata() + .build(); + + final ManagedInformerEventSource cachingEventSource = mock(); + when(cachingEventSource.get(new ResourceID("cm-foo", "ns"))).thenReturn(Optional.of(cm)); + when(mockManager.getEventSourceFor(ConfigMap.class, "es-name")).thenReturn(cachingEventSource); + + final var res = context.getSecondaryResource(ConfigMap.class, "es-name", "cm-foo", "ns"); + + assertThat(res).contains(cm); + verify(cachingEventSource).get(new ResourceID("cm-foo", "ns")); + } + + @Test + void getSecondaryResourceByNameAndNamespaceReturnsEmptyOnCacheMiss() { + final ManagedInformerEventSource cachingEventSource = mock(); + when(cachingEventSource.get(new ResourceID("missing", "ns"))).thenReturn(Optional.empty()); + when(mockManager.getEventSourceFor(ConfigMap.class, "es-name")).thenReturn(cachingEventSource); + + assertThat(context.getSecondaryResource(ConfigMap.class, "es-name", "missing", "ns")).isEmpty(); + } + + @Test + void getSecondaryResourceByNameAndNamespaceFallsBackToGetSecondaryResources() { + final var match = + new ConfigMapBuilder() + .withNewMetadata() + .withName("cm-foo") + .withNamespace("ns") + .endMetadata() + .build(); + final var other = + new ConfigMapBuilder() + .withNewMetadata() + .withName("cm-bar") + .withNamespace("ns") + .endMetadata() + .build(); + + final EventSource nonCachingEventSource = mock(); + when(nonCachingEventSource.getSecondaryResources(any())).thenReturn(Set.of(match, other)); + when(mockManager.getEventSourceFor(ConfigMap.class, "es-name")) + .thenReturn(nonCachingEventSource); + + final var res = context.getSecondaryResource(ConfigMap.class, "es-name", "cm-foo", "ns"); + + assertThat(res).contains(match); + } + + @Test + void getSecondaryResourceByNameAndNamespaceFallbackReturnsEmptyWhenNoMatch() { + final var other = + new ConfigMapBuilder() + .withNewMetadata() + .withName("cm-other") + .withNamespace("ns") + .endMetadata() + .build(); + + final EventSource nonCachingEventSource = mock(); + when(nonCachingEventSource.getSecondaryResources(any())).thenReturn(Set.of(other)); + when(mockManager.getEventSourceFor(ConfigMap.class, "es-name")) + .thenReturn(nonCachingEventSource); + + assertThat(context.getSecondaryResource(ConfigMap.class, "es-name", "missing", "ns")).isEmpty(); + } + + @Test + void getSecondaryResourceByNameAndNamespaceRethrowsWhenNoEventSourceAndNotWorkflowManaged() { + when(mockManager.getEventSourceFor(ConfigMap.class, "es-name")) + .thenThrow(new NoEventSourceForClassException(ConfigMap.class)); + + assertThatThrownBy( + () -> context.getSecondaryResource(ConfigMap.class, "es-name", "cm-foo", "ns")) + .isInstanceOf(NoEventSourceForClassException.class); + } + + @Test + void getSecondaryResourceByNameAndNamespaceReturnsEmptyWhenNoEventSourceButWorkflowManaged() { + when(mockManager.getEventSourceFor(ConfigMap.class, null)) + .thenThrow(new NoEventSourceForClassException(ConfigMap.class)); + when(mockController.workflowContainsDependentForType(ConfigMap.class)).thenReturn(true); + + final var res = context.getSecondaryResource(ConfigMap.class, null, "cm-foo", "ns"); + + assertThat(res).isEmpty(); + } + + @Test + void getSecondaryResourceByNameUsesPrimaryNamespace() { + final var primaryNamespace = "primary-ns"; + final var namespacedPrimary = + new SecretBuilder() + .withNewMetadata() + .withName("primary") + .withNamespace(primaryNamespace) + .endMetadata() + .build(); + final DefaultContext namespacedContext = + new DefaultContext<>(null, mockController, namespacedPrimary, false, false); + + final var cm = + new ConfigMapBuilder() + .withNewMetadata() + .withName("cm-foo") + .withNamespace(primaryNamespace) + .endMetadata() + .build(); + + final ManagedInformerEventSource cachingEventSource = mock(); + when(cachingEventSource.get(new ResourceID("cm-foo", primaryNamespace))) + .thenReturn(Optional.of(cm)); + when(mockManager.getEventSourceFor(ConfigMap.class, "es-name")).thenReturn(cachingEventSource); + + final var res = namespacedContext.getSecondaryResource(ConfigMap.class, "es-name", "cm-foo"); + + assertThat(res).contains(cm); + } + + @Test + void getSecondaryResourcesAsStreamByEventSourceUsesResourceCacheFastPath() { + final var primaryNamespace = "primary-ns"; + final var namespacedPrimary = + new SecretBuilder() + .withNewMetadata() + .withName("primary") + .withNamespace(primaryNamespace) + .endMetadata() + .build(); + final DefaultContext namespacedContext = + new DefaultContext<>(null, mockController, namespacedPrimary, false, false); + + final var cm1 = + new ConfigMapBuilder() + .withNewMetadata() + .withName("cm-1") + .withNamespace(primaryNamespace) + .endMetadata() + .build(); + final var cm2 = + new ConfigMapBuilder() + .withNewMetadata() + .withName("cm-2") + .withNamespace(primaryNamespace) + .endMetadata() + .build(); + + final ManagedInformerEventSource resourceCacheEventSource = mock(); + when(resourceCacheEventSource.list(primaryNamespace)).thenReturn(Stream.of(cm1, cm2)); + when(mockManager.getEventSourceFor(ConfigMap.class, "es-name")) + .thenReturn(resourceCacheEventSource); + + final var res = + namespacedContext.getSecondaryResourcesAsStream(ConfigMap.class, "es-name").toList(); + + assertThat(res).containsExactlyInAnyOrder(cm1, cm2); + verify(resourceCacheEventSource).list(primaryNamespace); + } + + @Test + void getSecondaryResourcesAsStreamByEventSourceFastPathOnClusterScopedPrimary() { + // cluster-scoped primary: has metadata but no namespace set. + final var clusterScopedPrimary = + new SecretBuilder().withNewMetadata().withName("primary").endMetadata().build(); + final DefaultContext clusterScopedContext = + new DefaultContext<>(null, mockController, clusterScopedPrimary, false, false); + + final var cm1 = new ConfigMapBuilder().withNewMetadata().withName("cm-1").endMetadata().build(); + + final ManagedInformerEventSource resourceCacheEventSource = mock(); + when(resourceCacheEventSource.list()).thenReturn(Stream.of(cm1)); + when(mockManager.getEventSourceFor(ConfigMap.class, "es-name")) + .thenReturn(resourceCacheEventSource); + + final var res = + clusterScopedContext.getSecondaryResourcesAsStream(ConfigMap.class, "es-name").toList(); + + assertThat(res).containsExactly(cm1); + verify(resourceCacheEventSource).list(); + verify(resourceCacheEventSource, never()).list(any(String.class)); + } + + @Test + void getSecondaryResourcesAsStreamByEventSourceFallsBackToGetSecondaryResources() { + final var cm1 = + new ConfigMapBuilder() + .withNewMetadata() + .withName("cm-1") + .withNamespace("ns") + .endMetadata() + .build(); + + final EventSource nonCacheEventSource = mock(); + when(nonCacheEventSource.getSecondaryResources(any())).thenReturn(Set.of(cm1)); + when(mockManager.getEventSourceFor(ConfigMap.class, "es-name")).thenReturn(nonCacheEventSource); + + final var res = context.getSecondaryResourcesAsStream(ConfigMap.class, "es-name").toList(); + + assertThat(res).containsExactly(cm1); + } + + @Test + void getSecondaryResourcesAsStreamByEventSourceRethrowsWhenNotWorkflowManaged() { + when(mockManager.getEventSourceFor(ConfigMap.class, "es-name")) + .thenThrow(new NoEventSourceForClassException(ConfigMap.class)); + + assertThatThrownBy(() -> context.getSecondaryResourcesAsStream(ConfigMap.class, "es-name")) + .isInstanceOf(NoEventSourceForClassException.class); + } + + @Test + void getSecondaryResourcesAsStreamByEventSourceReturnsEmptyWhenWorkflowManaged() { + when(mockManager.getEventSourceFor(ConfigMap.class, null)) + .thenThrow(new NoEventSourceForClassException(ConfigMap.class)); + when(mockController.workflowContainsDependentForType(ConfigMap.class)).thenReturn(true); + + final var res = context.getSecondaryResourcesAsStream(ConfigMap.class, null).toList(); + + assertThat(res).isEmpty(); + } + @Test void setRetryInfo() { RetryInfo retryInfo = mock(); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventProcessorTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventProcessorTest.java index fb8f7c0805..f7864f2f16 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventProcessorTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventProcessorTest.java @@ -465,6 +465,98 @@ void schedulesRetryForMarReconciliationIntervalIfRetryExhausted() { verify(retryTimerEventSourceMock, times(1)).scheduleOnce((ResourceID) any(), anyLong()); } + @Test + void preservesRetryDeadlineWhenRemainingDurationAboveThreshold() { + RetryExecution mockRetryExecution = mock(RetryExecution.class); + when(mockRetryExecution.nextDelay()).thenReturn(Optional.of(60_000L)); + when(mockRetryExecution.remainingDurationUntilNextRetry()) + .thenReturn(Optional.of(Duration.ofMillis(50_000))); + Retry retry = mock(Retry.class); + when(retry.initExecution()).thenReturn(mockRetryExecution); + eventProcessorWithRetry = + spy( + new EventProcessor( + controllerConfiguration(retry, LinearRateLimiter.deactivatedRateLimiter()), + reconciliationDispatcherMock, + eventSourceManagerMock, + metricsMock)); + eventProcessorWithRetry.start(); + when(eventProcessorWithRetry.retryEventSource()).thenReturn(retryTimerEventSourceMock); + + TestCustomResource customResource = testCustomResource(); + ExecutionScope executionScope = + new ExecutionScope(null, null, false, false).setResource(customResource); + PostExecutionControl postExecutionControl = + PostExecutionControl.exceptionDuringExecution(new RuntimeException("test")); + + eventProcessorWithRetry.eventProcessingFinished(executionScope, postExecutionControl); + + verify(mockRetryExecution, never()).nextDelay(); + verify(retryTimerEventSourceMock, times(1)) + .scheduleOnce(eq(ResourceID.fromResource(customResource)), eq(50_000L)); + } + + @Test + void consumesRetryAttemptWhenRemainingDurationAtOrBelowThreshold() { + RetryExecution mockRetryExecution = mock(RetryExecution.class); + when(mockRetryExecution.nextDelay()).thenReturn(Optional.of(60_000L)); + when(mockRetryExecution.remainingDurationUntilNextRetry()) + .thenReturn(Optional.of(Duration.ofMillis(2_000))); + Retry retry = mock(Retry.class); + when(retry.initExecution()).thenReturn(mockRetryExecution); + eventProcessorWithRetry = + spy( + new EventProcessor( + controllerConfiguration(retry, LinearRateLimiter.deactivatedRateLimiter()), + reconciliationDispatcherMock, + eventSourceManagerMock, + metricsMock)); + eventProcessorWithRetry.start(); + when(eventProcessorWithRetry.retryEventSource()).thenReturn(retryTimerEventSourceMock); + + TestCustomResource customResource = testCustomResource(); + ExecutionScope executionScope = + new ExecutionScope(null, null, false, false).setResource(customResource); + PostExecutionControl postExecutionControl = + PostExecutionControl.exceptionDuringExecution(new RuntimeException("test")); + + eventProcessorWithRetry.eventProcessingFinished(executionScope, postExecutionControl); + + verify(mockRetryExecution, times(1)).nextDelay(); + verify(retryTimerEventSourceMock, times(1)) + .scheduleOnce(eq(ResourceID.fromResource(customResource)), eq(60_000L)); + } + + @Test + void firstFailureSchedulesUsingNextDelayWhenNoRemainingDuration() { + RetryExecution mockRetryExecution = mock(RetryExecution.class); + when(mockRetryExecution.nextDelay()).thenReturn(Optional.of(60_000L)); + when(mockRetryExecution.remainingDurationUntilNextRetry()).thenReturn(Optional.empty()); + Retry retry = mock(Retry.class); + when(retry.initExecution()).thenReturn(mockRetryExecution); + eventProcessorWithRetry = + spy( + new EventProcessor( + controllerConfiguration(retry, LinearRateLimiter.deactivatedRateLimiter()), + reconciliationDispatcherMock, + eventSourceManagerMock, + metricsMock)); + eventProcessorWithRetry.start(); + when(eventProcessorWithRetry.retryEventSource()).thenReturn(retryTimerEventSourceMock); + + TestCustomResource customResource = testCustomResource(); + ExecutionScope executionScope = + new ExecutionScope(null, null, false, false).setResource(customResource); + PostExecutionControl postExecutionControl = + PostExecutionControl.exceptionDuringExecution(new RuntimeException("test")); + + eventProcessorWithRetry.eventProcessingFinished(executionScope, postExecutionControl); + + verify(mockRetryExecution, times(1)).nextDelay(); + verify(retryTimerEventSourceMock, times(1)) + .scheduleOnce(eq(ResourceID.fromResource(customResource)), eq(60_000L)); + } + @Test void executionOfReconciliationShouldNotStartIfProcessorStopped() throws InterruptedException { when(reconciliationDispatcherMock.handleExecution(any())) diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/retry/GenericRetryExecutionTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/retry/GenericRetryExecutionTest.java index 8f5a446788..8d7ec55e37 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/retry/GenericRetryExecutionTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/retry/GenericRetryExecutionTest.java @@ -21,10 +21,10 @@ import static org.assertj.core.api.Assertions.assertThat; -public class GenericRetryExecutionTest { +class GenericRetryExecutionTest { @Test - public void noNextDelayIfMaxAttemptLimitReached() { + void noNextDelayIfMaxAttemptLimitReached() { RetryExecution retryExecution = GenericRetry.defaultLimitedExponentialRetry().setMaxAttempts(3).initExecution(); Optional res = callNextDelayNTimes(retryExecution, 2); @@ -35,7 +35,7 @@ public void noNextDelayIfMaxAttemptLimitReached() { } @Test - public void canLimitMaxIntervalLength() { + void canLimitMaxIntervalLength() { RetryExecution retryExecution = GenericRetry.defaultLimitedExponentialRetry() .setInitialInterval(2000) @@ -49,13 +49,13 @@ public void canLimitMaxIntervalLength() { } @Test - public void supportsNoRetry() { + void supportsNoRetry() { RetryExecution retryExecution = GenericRetry.noRetry().initExecution(); assertThat(retryExecution.nextDelay()).isEmpty(); } @Test - public void supportsIsLastExecution() { + void supportsIsLastExecution() { GenericRetryExecution execution = new GenericRetry().setMaxAttempts(2).initExecution(); assertThat(execution.isLastAttempt()).isFalse(); @@ -65,7 +65,7 @@ public void supportsIsLastExecution() { } @Test - public void returnAttemptIndex() { + void returnAttemptIndex() { RetryExecution retryExecution = GenericRetry.defaultLimitedExponentialRetry().initExecution(); assertThat(retryExecution.getAttemptCount()).isEqualTo(0); @@ -73,11 +73,59 @@ public void returnAttemptIndex() { assertThat(retryExecution.getAttemptCount()).isEqualTo(1); } - private RetryExecution getDefaultRetryExecution() { - return GenericRetry.defaultLimitedExponentialRetry().initExecution(); + @Test + void remainingDurationEmptyBeforeFirstNextDelay() { + RetryExecution retryExecution = GenericRetry.defaultLimitedExponentialRetry().initExecution(); + + assertThat(retryExecution.remainingDurationUntilNextRetry()).isEmpty(); + } + + @Test + void remainingDurationPresentAfterNextDelay() { + long interval = 60_000L; + RetryExecution retryExecution = new GenericRetry().setInitialInterval(interval).initExecution(); + + retryExecution.nextDelay(); + + Optional remaining = retryExecution.remainingDurationUntilNextRetry(); + assertThat(remaining).isPresent(); + assertThat(remaining.get().toMillis()).isPositive().isLessThanOrEqualTo(interval); + } + + @Test + void remainingDurationEmptyAfterIntervalElapsed() throws InterruptedException { + RetryExecution retryExecution = new GenericRetry().setInitialInterval(20).initExecution(); + + retryExecution.nextDelay(); + Thread.sleep(60); + + assertThat(retryExecution.remainingDurationUntilNextRetry()).isEmpty(); + } + + @Test + void remainingDurationReflectsUpdatedIntervalAfterSubsequentNextDelay() { + long initialInterval = 1000L; + double multiplier = 2.0; + RetryExecution retryExecution = + new GenericRetry() + .setInitialInterval(initialInterval) + .setIntervalMultiplier(multiplier) + .initExecution(); + + // first two calls keep the initial interval (multiplier only kicks in after attempt 1) + retryExecution.nextDelay(); + retryExecution.nextDelay(); + // third call doubles the interval to 2000ms + retryExecution.nextDelay(); + + Optional remaining = retryExecution.remainingDurationUntilNextRetry(); + assertThat(remaining).isPresent(); + assertThat(remaining.get().toMillis()) + .isPositive() + .isLessThanOrEqualTo((long) (initialInterval * multiplier)); } - public Optional callNextDelayNTimes(RetryExecution retryExecution, int n) { + Optional callNextDelayNTimes(RetryExecution retryExecution, int n) { for (int i = 0; i < n; i++) { retryExecution.nextDelay(); } diff --git a/operator-framework-junit/pom.xml b/operator-framework-junit/pom.xml index bff312ebfd..aa18d5c778 100644 --- a/operator-framework-junit/pom.xml +++ b/operator-framework-junit/pom.xml @@ -21,7 +21,7 @@ io.javaoperatorsdk java-operator-sdk - 5.3.5-SNAPSHOT + 999-SNAPSHOT operator-framework-junit diff --git a/operator-framework/pom.xml b/operator-framework/pom.xml index 8348d8dcf4..f94dfa757d 100644 --- a/operator-framework/pom.xml +++ b/operator-framework/pom.xml @@ -21,7 +21,7 @@ io.javaoperatorsdk java-operator-sdk - 5.3.5-SNAPSHOT + 999-SNAPSHOT operator-framework diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/retry/RetryIntervalHonoredOnFrequentEventsIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/retry/RetryIntervalHonoredOnFrequentEventsIT.java new file mode 100644 index 0000000000..df525e8056 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/retry/RetryIntervalHonoredOnFrequentEventsIT.java @@ -0,0 +1,107 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.baseapi.retry; + +import java.time.Duration; +import java.util.concurrent.TimeUnit; +import java.util.stream.IntStream; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import io.javaoperatorsdk.annotation.Sample; +import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; +import io.javaoperatorsdk.operator.processing.retry.GenericRetry; + +import static io.javaoperatorsdk.operator.baseapi.retry.RetryIT.createTestCustomResource; +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + +@Sample( + tldr = "Retry Interval Honored Despite Frequent Reconciliation Triggers", + description = + """ + Verifies that with a low max attempts (3) and a high retry interval (1 minute), \ + reconciliations triggered by external events (e.g. resource updates) during the retry \ + window do not consume retry attempts. The retry counter should only advance when the \ + scheduled retry deadline is approached, so the configured interval is honored. + """) +class RetryIntervalHonoredOnFrequentEventsIT { + + private static final Logger log = + LoggerFactory.getLogger(RetryIntervalHonoredOnFrequentEventsIT.class); + + public static final int MAX_RETRY_ATTEMPTS = 3; + public static final int RETRY_INTERVAL_MILLIS = 60_000; + public static final int ALL_EXECUTIONS_TO_FAIL = 99; + public static final int NUMBER_OF_UPDATES = 5; + + RetryTestCustomReconciler reconciler = new RetryTestCustomReconciler(ALL_EXECUTIONS_TO_FAIL); + + @RegisterExtension + LocallyRunOperatorExtension operator = + LocallyRunOperatorExtension.builder() + .withReconciler( + reconciler, + new GenericRetry() + .setInitialInterval(RETRY_INTERVAL_MILLIS) + .withLinearRetry() + .setMaxAttempts(MAX_RETRY_ATTEMPTS)) + .build(); + + @Test + void frequentEventsDuringRetryWindowDoNotExhaustRetryCounter() { + RetryTestCustomResource resource = createTestCustomResource("frequent-events"); + var created = operator.create(resource); + + // Wait until the initial reconciliation has been executed and failed; the retry timer is now + // armed for RETRY_INTERVAL_MILLIS in the future, retry counter is at 1. + await() + .pollInterval(Duration.ofMillis(50)) + .atMost(5, TimeUnit.SECONDS) + .untilAsserted( + () -> assertThat(reconciler.getNumberOfExecutions()).isGreaterThanOrEqualTo(1)); + + // Trigger several updates spaced apart so each results in its own reconciliation cycle. Each + // failed reconciliation lands well inside the 1 minute retry window, so the retry counter + // must NOT advance — only the original retry deadline matters. + IntStream.rangeClosed(1, NUMBER_OF_UPDATES) + .forEach( + i -> { + log.debug("replacing resource, iteration: {}", i); + var latest = + operator.get(RetryTestCustomResource.class, created.getMetadata().getName()); + latest.getSpec().setValue("update-" + i); + operator.replace(latest); + int expectedExecutions = i + 1; + await() + .pollInterval(Duration.ofMillis(50)) + .atMost(5, TimeUnit.SECONDS) + .untilAsserted( + () -> + assertThat(reconciler.getNumberOfExecutions()) + .isGreaterThanOrEqualTo(expectedExecutions)); + }); + + // Reconciliations did happen for every event (so events are not lost) but the retry counter + // observed inside the reconciler never went past 1: the configured 1 minute interval is + // honored even under a steady stream of external events. + assertThat(reconciler.getNumberOfExecutions()).isGreaterThanOrEqualTo(NUMBER_OF_UPDATES + 1); + assertThat(reconciler.getMaxObservedRetryAttempt()).isEqualTo(1); + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/retry/RetryTestCustomReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/retry/RetryTestCustomReconciler.java index 30a339fc4d..f981b9e1cb 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/retry/RetryTestCustomReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/retry/RetryTestCustomReconciler.java @@ -32,6 +32,7 @@ public class RetryTestCustomReconciler private static final Logger log = LoggerFactory.getLogger(RetryTestCustomReconciler.class); private final AtomicInteger numberOfExecutions = new AtomicInteger(0); + private final AtomicInteger maxObservedRetryAttempt = new AtomicInteger(0); private final AtomicInteger numberOfExecutionFails; @@ -43,6 +44,12 @@ public RetryTestCustomReconciler(int numberOfExecutionFails) { public UpdateControl reconcile( RetryTestCustomResource resource, Context context) { numberOfExecutions.addAndGet(1); + context + .getRetryInfo() + .ifPresent( + info -> + maxObservedRetryAttempt.updateAndGet( + prev -> Math.max(prev, info.getAttemptCount()))); log.info("Value: " + resource.getSpec().getValue()); @@ -70,4 +77,8 @@ private void ensureStatusExists(RetryTestCustomResource resource) { public int getNumberOfExecutions() { return numberOfExecutions.get(); } + + public int getMaxObservedRetryAttempt() { + return maxObservedRetryAttempt.get(); + } } diff --git a/pom.xml b/pom.xml index 86736cf727..b47541e8ec 100644 --- a/pom.xml +++ b/pom.xml @@ -21,7 +21,7 @@ io.javaoperatorsdk java-operator-sdk - 5.3.5-SNAPSHOT + 999-SNAPSHOT pom Operator SDK for Java Java SDK for implementing Kubernetes operators @@ -70,7 +70,7 @@ java-operator-sdk https://sonarcloud.io jdk - 6.1.0 + 6.0.3 7.7.0 2.0.18 2.26.0 @@ -85,7 +85,7 @@ 3.2.4 0.9.14 2.22.0 - 4.17 + 4.16 2.11 3.15.0 @@ -102,7 +102,7 @@ 3.1.4 10.0.0 3.5.1 - 3.5.1 + 3.5.0 diff --git a/sample-operators/controller-namespace-deletion/pom.xml b/sample-operators/controller-namespace-deletion/pom.xml index d43ac3cec4..af4be01972 100644 --- a/sample-operators/controller-namespace-deletion/pom.xml +++ b/sample-operators/controller-namespace-deletion/pom.xml @@ -22,7 +22,7 @@ io.javaoperatorsdk sample-operators - 5.3.5-SNAPSHOT + 999-SNAPSHOT sample-controller-namespace-deletion diff --git a/sample-operators/leader-election/pom.xml b/sample-operators/leader-election/pom.xml index 2e18435af3..4f896485d1 100644 --- a/sample-operators/leader-election/pom.xml +++ b/sample-operators/leader-election/pom.xml @@ -22,7 +22,7 @@ io.javaoperatorsdk sample-operators - 5.3.5-SNAPSHOT + 999-SNAPSHOT sample-leader-election diff --git a/sample-operators/mysql-schema/pom.xml b/sample-operators/mysql-schema/pom.xml index 45c85e4ef6..d2872c921a 100644 --- a/sample-operators/mysql-schema/pom.xml +++ b/sample-operators/mysql-schema/pom.xml @@ -22,7 +22,7 @@ io.javaoperatorsdk sample-operators - 5.3.5-SNAPSHOT + 999-SNAPSHOT sample-mysql-schema-operator diff --git a/sample-operators/operations/pom.xml b/sample-operators/operations/pom.xml index bbabcdbd8c..3dda5cac4c 100644 --- a/sample-operators/operations/pom.xml +++ b/sample-operators/operations/pom.xml @@ -22,7 +22,7 @@ io.javaoperatorsdk sample-operators - 5.3.5-SNAPSHOT + 999-SNAPSHOT sample-operations @@ -106,11 +106,6 @@ operations-operator - - - -Dlog4j.configurationFile=/config/log4j2.xml - - diff --git a/sample-operators/pom.xml b/sample-operators/pom.xml index c042f11aea..9313095584 100644 --- a/sample-operators/pom.xml +++ b/sample-operators/pom.xml @@ -22,7 +22,7 @@ io.javaoperatorsdk java-operator-sdk - 5.3.5-SNAPSHOT + 999-SNAPSHOT sample-operators diff --git a/sample-operators/tomcat-operator/pom.xml b/sample-operators/tomcat-operator/pom.xml index f6c7647098..ea964a2b07 100644 --- a/sample-operators/tomcat-operator/pom.xml +++ b/sample-operators/tomcat-operator/pom.xml @@ -22,7 +22,7 @@ io.javaoperatorsdk sample-operators - 5.3.5-SNAPSHOT + 999-SNAPSHOT sample-tomcat-operator diff --git a/sample-operators/webpage/pom.xml b/sample-operators/webpage/pom.xml index 0ae7c26fa4..d50e5ef03c 100644 --- a/sample-operators/webpage/pom.xml +++ b/sample-operators/webpage/pom.xml @@ -22,7 +22,7 @@ io.javaoperatorsdk sample-operators - 5.3.5-SNAPSHOT + 999-SNAPSHOT sample-webpage-operator diff --git a/test-index-processor/pom.xml b/test-index-processor/pom.xml index 4cc11dd26b..2ae7c5f454 100644 --- a/test-index-processor/pom.xml +++ b/test-index-processor/pom.xml @@ -22,7 +22,7 @@ io.javaoperatorsdk java-operator-sdk - 5.3.5-SNAPSHOT + 999-SNAPSHOT test-index-processor