From 74cee293bb7ee81d064ab55117585996b3f0eae1 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Tue, 5 Jul 2022 12:45:56 +0200 Subject: [PATCH 01/16] refactor: do not create a new retry instance if not needed --- .../operator/api/config/ControllerConfiguration.java | 5 ++++- .../operator/processing/retry/GenericRetry.java | 2 ++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java index 339e06f894..ffa94eb675 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java @@ -34,7 +34,10 @@ default boolean isGenerationAware() { String getAssociatedReconcilerClassName(); default Retry getRetry() { - return GenericRetry.fromConfiguration(getRetryConfiguration()); // NOSONAR + final var configuration = getRetryConfiguration(); + return RetryConfiguration.DEFAULT != configuration + ? GenericRetry.fromConfiguration(configuration) + : GenericRetry.DEFAULT; // NOSONAR } /** diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/GenericRetry.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/GenericRetry.java index 7804586fa3..c98fc4d36f 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/GenericRetry.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/GenericRetry.java @@ -8,6 +8,8 @@ public class GenericRetry implements Retry { private double intervalMultiplier = RetryConfiguration.DEFAULT_MULTIPLIER; private long maxInterval = -1; + public static final Retry DEFAULT = fromConfiguration(RetryConfiguration.DEFAULT); + public static GenericRetry defaultLimitedExponentialRetry() { return new GenericRetry(); } From 88c17ee0717ae44c0e4367deac18ce4af7bafd26 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Tue, 5 Jul 2022 17:16:48 +0200 Subject: [PATCH 02/16] feat: initial reworking of configuration --- .../api/config/AnnotationConfigurable.java | 7 ++ .../AnnotationControllerConfiguration.java | 36 +++++++-- .../api/config/ControllerConfiguration.java | 16 +--- .../ControllerConfigurationOverrider.java | 7 -- .../api/config/DefaultRetryConfiguration.java | 4 - .../api/config/RetryConfiguration.java | 25 +++--- .../reconciler/ControllerConfiguration.java | 10 ++- .../event/rate/PeriodRateLimiter.java | 15 +++- .../processing/retry/GenericRetry.java | 24 +++--- .../operator/processing/retry/Retry.java | 1 + ...AnnotationControllerConfigurationTest.java | 81 +++++++++++++++++++ .../sample/ratelimit/RateLimitReconciler.java | 11 ++- .../operator/sample/WebPageReconciler.java | 25 ++++-- 13 files changed, 191 insertions(+), 71 deletions(-) create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationConfigurable.java delete mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultRetryConfiguration.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationConfigurable.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationConfigurable.java new file mode 100644 index 0000000000..53ee608568 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationConfigurable.java @@ -0,0 +1,7 @@ +package io.javaoperatorsdk.operator.api.config; + +import java.lang.annotation.Annotation; + +public interface AnnotationConfigurable { + void initFrom(C configuration); +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java index 560a79e5be..2bfb13f9e9 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java @@ -1,5 +1,6 @@ package io.javaoperatorsdk.operator.api.config; +import java.lang.annotation.Annotation; import java.lang.reflect.InvocationTargetException; import java.time.Duration; import java.util.Arrays; @@ -27,7 +28,6 @@ import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResourceConfig; import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition; -import io.javaoperatorsdk.operator.processing.event.rate.PeriodRateLimiter; import io.javaoperatorsdk.operator.processing.event.rate.RateLimiter; import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilter; import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilters; @@ -35,6 +35,7 @@ import io.javaoperatorsdk.operator.processing.event.source.filter.VoidOnAddFilter; import io.javaoperatorsdk.operator.processing.event.source.filter.VoidOnDeleteFilter; import io.javaoperatorsdk.operator.processing.event.source.filter.VoidOnUpdateFilter; +import io.javaoperatorsdk.operator.processing.retry.Retry; import static io.javaoperatorsdk.operator.api.reconciler.Constants.DEFAULT_NAMESPACES_SET; @@ -154,12 +155,33 @@ public Optional reconciliationMaxInterval() { @Override public RateLimiter getRateLimiter() { - if (annotation.rateLimit() != null) { - return new PeriodRateLimiter(Duration.of(annotation.rateLimit().refreshPeriod(), - annotation.rateLimit().refreshPeriodTimeUnit().toChronoUnit()), - annotation.rateLimit().limitForPeriod()); - } else { - return io.javaoperatorsdk.operator.api.config.ControllerConfiguration.super.getRateLimiter(); + final Class rateLimiterClass = annotation.rateLimiter(); + return instantiateAndConfigureIfNeeded(rateLimiterClass); + } + + @Override + public Retry getRetry() { + final Class retryClass = annotation.retry(); + return instantiateAndConfigureIfNeeded(retryClass); + } + + private T instantiateAndConfigureIfNeeded(Class targetClass) { + try { + final var instance = targetClass.getConstructor().newInstance(); + if (instance instanceof AnnotationConfigurable) { + AnnotationConfigurable configurable = (AnnotationConfigurable) instance; + final Class configurationClass = + (Class) Utils.getFirstTypeArgumentFromSuperClassOrInterface( + targetClass, AnnotationConfigurable.class); + final var configAnnotation = reconciler.getClass().getAnnotation(configurationClass); + if (configAnnotation != null) { + configurable.initFrom(configAnnotation); + } + } + return instance; + } catch (InstantiationException | IllegalAccessException | InvocationTargetException + | NoSuchMethodException e) { + throw new RuntimeException(e); } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java index ffa94eb675..ccea5da8e7 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java @@ -18,6 +18,7 @@ public interface ControllerConfiguration extends ResourceConfiguration { RateLimiter DEFAULT_RATE_LIMITER = new PeriodRateLimiter(); + Retry DEFAULT_RETRY = new GenericRetry(); default String getName() { return ReconcilerUtils.getDefaultReconcilerName(getAssociatedReconcilerClassName()); @@ -34,20 +35,7 @@ default boolean isGenerationAware() { String getAssociatedReconcilerClassName(); default Retry getRetry() { - final var configuration = getRetryConfiguration(); - return RetryConfiguration.DEFAULT != configuration - ? GenericRetry.fromConfiguration(configuration) - : GenericRetry.DEFAULT; // NOSONAR - } - - /** - * Use getRetry instead. - * - * @return configuration for retry. - */ - @Deprecated - default RetryConfiguration getRetryConfiguration() { - return RetryConfiguration.DEFAULT; + return DEFAULT_RETRY; } default RateLimiter getRateLimiter() { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java index 440a076428..3ce52429db 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java @@ -15,7 +15,6 @@ import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResourceConfig; import io.javaoperatorsdk.operator.processing.event.rate.RateLimiter; import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilter; -import io.javaoperatorsdk.operator.processing.retry.GenericRetry; import io.javaoperatorsdk.operator.processing.retry.Retry; import static io.javaoperatorsdk.operator.api.reconciler.Constants.DEFAULT_NAMESPACES_SET; @@ -111,12 +110,6 @@ public ControllerConfigurationOverrider withRetry(Retry retry) { return this; } - @Deprecated - public ControllerConfigurationOverrider withRetry(RetryConfiguration retry) { - this.retry = GenericRetry.fromConfiguration(retry); - return this; - } - public ControllerConfigurationOverrider withRateLimiter(RateLimiter rateLimiter) { this.rateLimiter = rateLimiter; return this; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultRetryConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultRetryConfiguration.java deleted file mode 100644 index 4e891b3fd3..0000000000 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultRetryConfiguration.java +++ /dev/null @@ -1,4 +0,0 @@ -package io.javaoperatorsdk.operator.api.config; - -public class DefaultRetryConfiguration implements RetryConfiguration { -} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/RetryConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/RetryConfiguration.java index dee54c3e8d..26ccbd28b7 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/RetryConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/RetryConfiguration.java @@ -1,26 +1,23 @@ package io.javaoperatorsdk.operator.api.config; -public interface RetryConfiguration { +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; - RetryConfiguration DEFAULT = new DefaultRetryConfiguration(); +@Retention(RetentionPolicy.RUNTIME) +@Target(ElementType.TYPE) +public @interface RetryConfiguration { int DEFAULT_MAX_ATTEMPTS = 5; long DEFAULT_INITIAL_INTERVAL = 2000L; double DEFAULT_MULTIPLIER = 1.5D; - default int getMaxAttempts() { - return DEFAULT_MAX_ATTEMPTS; - } + int maxAttempts() default DEFAULT_MAX_ATTEMPTS; - default long getInitialInterval() { - return DEFAULT_INITIAL_INTERVAL; - } + long initialInterval() default DEFAULT_INITIAL_INTERVAL; - default double getIntervalMultiplier() { - return DEFAULT_MULTIPLIER; - } + double intervalMultiplier() default DEFAULT_MULTIPLIER; - default long getMaxInterval() { - return (long) (DEFAULT_INITIAL_INTERVAL * Math.pow(DEFAULT_MULTIPLIER, DEFAULT_MAX_ATTEMPTS)); - } + long maxInterval() default -1; } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ControllerConfiguration.java index 3d86c74779..d5ec7be657 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ControllerConfiguration.java @@ -9,10 +9,14 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent; +import io.javaoperatorsdk.operator.processing.event.rate.PeriodRateLimiter; +import io.javaoperatorsdk.operator.processing.event.rate.RateLimiter; import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilter; import io.javaoperatorsdk.operator.processing.event.source.filter.VoidGenericFilter; import io.javaoperatorsdk.operator.processing.event.source.filter.VoidOnAddFilter; import io.javaoperatorsdk.operator.processing.event.source.filter.VoidOnUpdateFilter; +import io.javaoperatorsdk.operator.processing.retry.GenericRetry; +import io.javaoperatorsdk.operator.processing.retry.Retry; @Retention(RetentionPolicy.RUNTIME) @Target({ElementType.TYPE}) @@ -92,8 +96,6 @@ ReconciliationMaxInterval reconciliationMaxInterval() default @ReconciliationMax interval = 10); - RateLimit rateLimit() default @RateLimit; - /** * Optional list of {@link Dependent} configurations which associate a resource type to a * {@link io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource} implementation @@ -101,4 +103,8 @@ ReconciliationMaxInterval reconciliationMaxInterval() default @ReconciliationMax * @return the list of {@link Dependent} configurations */ Dependent[] dependents() default {}; + + Class retry() default GenericRetry.class; + + Class rateLimiter() default PeriodRateLimiter.class; } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/PeriodRateLimiter.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/PeriodRateLimiter.java index caa6075ee7..1c378d5841 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/PeriodRateLimiter.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/PeriodRateLimiter.java @@ -6,12 +6,14 @@ import java.util.Map; import java.util.Optional; +import io.javaoperatorsdk.operator.api.config.AnnotationConfigurable; +import io.javaoperatorsdk.operator.api.reconciler.RateLimit; import io.javaoperatorsdk.operator.processing.event.ResourceID; /** * A Simple rate limiter that limits the number of permission for a time interval. */ -public class PeriodRateLimiter implements RateLimiter { +public class PeriodRateLimiter implements RateLimiter, AnnotationConfigurable { public static final int DEFAULT_REFRESH_PERIOD_SECONDS = 10; public static final int DEFAULT_LIMIT_FOR_PERIOD = 3; @@ -22,9 +24,9 @@ public class PeriodRateLimiter implements RateLimiter { public static final int NO_LIMIT_PERIOD = -1; private Duration refreshPeriod; - private int limitForPeriod; + protected int limitForPeriod; - private Map limitData = new HashMap<>(); + private final Map limitData = new HashMap<>(); public PeriodRateLimiter() { this(DEFAULT_REFRESH_PERIOD, DEFAULT_LIMIT_FOR_PERIOD); @@ -58,4 +60,11 @@ public Optional acquirePermission(ResourceID resourceID) { public void clear(ResourceID resourceID) { limitData.remove(resourceID); } + + @Override + public void initFrom(RateLimit configuration) { + this.refreshPeriod = Duration.of(configuration.refreshPeriod(), + configuration.refreshPeriodTimeUnit().toChronoUnit()); + this.limitForPeriod = configuration.limitForPeriod(); + } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/GenericRetry.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/GenericRetry.java index c98fc4d36f..2caf092726 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/GenericRetry.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/GenericRetry.java @@ -1,15 +1,14 @@ package io.javaoperatorsdk.operator.processing.retry; +import io.javaoperatorsdk.operator.api.config.AnnotationConfigurable; import io.javaoperatorsdk.operator.api.config.RetryConfiguration; -public class GenericRetry implements Retry { +public class GenericRetry implements Retry, AnnotationConfigurable { private int maxAttempts = RetryConfiguration.DEFAULT_MAX_ATTEMPTS; private long initialInterval = RetryConfiguration.DEFAULT_INITIAL_INTERVAL; private double intervalMultiplier = RetryConfiguration.DEFAULT_MULTIPLIER; private long maxInterval = -1; - public static final Retry DEFAULT = fromConfiguration(RetryConfiguration.DEFAULT); - public static GenericRetry defaultLimitedExponentialRetry() { return new GenericRetry(); } @@ -22,15 +21,6 @@ public static GenericRetry every10second10TimesRetry() { return new GenericRetry().withLinearRetry().setMaxAttempts(10).setInitialInterval(10000); } - public static Retry fromConfiguration(RetryConfiguration configuration) { - return configuration == null ? defaultLimitedExponentialRetry() - : new GenericRetry() - .setInitialInterval(configuration.getInitialInterval()) - .setMaxAttempts(configuration.getMaxAttempts()) - .setIntervalMultiplier(configuration.getIntervalMultiplier()) - .setMaxInterval(configuration.getMaxInterval()); - } - @Override public GenericRetryExecution initExecution() { return new GenericRetryExecution(this); @@ -85,4 +75,14 @@ public GenericRetry withLinearRetry() { this.intervalMultiplier = 1; return this; } + + @Override + public void initFrom(RetryConfiguration configuration) { + this.initialInterval = configuration.initialInterval(); + this.maxAttempts = configuration.maxAttempts(); + this.intervalMultiplier = configuration.intervalMultiplier(); + this.maxInterval = configuration.maxInterval() < 0 + ? (long) (initialInterval * Math.pow(intervalMultiplier, maxAttempts)) + : configuration.maxInterval(); + } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/Retry.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/Retry.java index 3500a196f8..78ec6f189c 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/Retry.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/Retry.java @@ -1,5 +1,6 @@ package io.javaoperatorsdk.operator.processing.retry; +@FunctionalInterface public interface Retry { RetryExecution initExecution(); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/AnnotationControllerConfigurationTest.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/AnnotationControllerConfigurationTest.java index de54d2f3f4..e46521f7e9 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/AnnotationControllerConfigurationTest.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/AnnotationControllerConfigurationTest.java @@ -1,5 +1,9 @@ package io.javaoperatorsdk.operator.config.runtime; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; import java.util.List; import java.util.Optional; import java.util.Set; @@ -9,9 +13,12 @@ import io.fabric8.kubernetes.api.model.ConfigMap; import io.javaoperatorsdk.operator.OperatorException; +import io.javaoperatorsdk.operator.api.config.AnnotationConfigurable; +import io.javaoperatorsdk.operator.api.config.RetryConfiguration; import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.RateLimit; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent; @@ -19,10 +26,15 @@ import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependent; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResourceConfig; +import io.javaoperatorsdk.operator.processing.event.rate.PeriodRateLimiter; +import io.javaoperatorsdk.operator.processing.retry.GenericRetry; +import io.javaoperatorsdk.operator.processing.retry.Retry; +import io.javaoperatorsdk.operator.processing.retry.RetryExecution; import io.javaoperatorsdk.operator.sample.readonly.ReadOnlyDependent; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -39,6 +51,7 @@ void defaultValuesShouldBeConsistent() { assertNull(unannotated.labelSelector()); } + @SuppressWarnings("rawtypes") private KubernetesDependentResourceConfig extractDependentKubernetesResourceConfig( io.javaoperatorsdk.operator.api.config.ControllerConfiguration configuration, int index) { return (KubernetesDependentResourceConfig) configuration.getDependentResources().get(index) @@ -114,6 +127,30 @@ && findByNameOptional(dependents, DependentResource.defaultNameFor(ReadOnlyDepen .isPresent()); } + + @Test + void checkDefaultRateAndRetryConfigurations() { + var config = new AnnotationControllerConfiguration<>(new NoDepReconciler()); + final var retry = assertInstanceOf(GenericRetry.class, config.getRetry()); + assertEquals(RetryConfiguration.DEFAULT_MAX_ATTEMPTS, retry.getMaxAttempts()); + assertEquals(RetryConfiguration.DEFAULT_MULTIPLIER, retry.getIntervalMultiplier()); + assertEquals(RetryConfiguration.DEFAULT_INITIAL_INTERVAL, retry.getInitialInterval()); + + assertInstanceOf(PeriodRateLimiter.class, config.getRateLimiter()); + } + + @Test + void configuringRateAndRetryViaAnnotationsShouldWork() { + var config = + new AnnotationControllerConfiguration<>(new ConfigurableRateLimitAndRetryReconciler()); + final var retry = config.getRetry(); + final var testRetry = assertInstanceOf(TestRetry.class, retry); + assertEquals(12, testRetry.getValue()); + + final var rateLimiter = assertInstanceOf(TestRateLimiter.class, config.getRateLimiter()); + assertEquals(7, rateLimiter.getLimitForPeriod()); + } + @ControllerConfiguration(namespaces = OneDepReconciler.CONFIGURED_NS, dependents = @Dependent(type = ReadOnlyDependent.class)) private static class OneDepReconciler implements Reconciler { @@ -202,4 +239,48 @@ public UpdateControl reconcile(ConfigMap resource, Context return null; } } + + public static class TestRetry implements Retry, AnnotationConfigurable { + private int value; + + public TestRetry() {} + + @Override + public RetryExecution initExecution() { + return null; + } + + public int getValue() { + return value; + } + + @Override + public void initFrom(TestRetryConfiguration configuration) { + value = configuration.value(); + } + } + + @Target(ElementType.TYPE) + @Retention(RetentionPolicy.RUNTIME) + private @interface TestRetryConfiguration { + int value() default 42; + } + + public static class TestRateLimiter extends PeriodRateLimiter { + int getLimitForPeriod() { + return limitForPeriod; + } + } + + @TestRetryConfiguration(12) + @RateLimit(limitForPeriod = 7) + @ControllerConfiguration(retry = TestRetry.class, rateLimiter = TestRateLimiter.class) + private static class ConfigurableRateLimitAndRetryReconciler implements Reconciler { + + @Override + public UpdateControl reconcile(ConfigMap resource, Context context) + throws Exception { + return UpdateControl.noUpdate(); + } + } } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/ratelimit/RateLimitReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/ratelimit/RateLimitReconciler.java index b4a77a8fc0..52308618c5 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/ratelimit/RateLimitReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/ratelimit/RateLimitReconciler.java @@ -3,11 +3,16 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; -import io.javaoperatorsdk.operator.api.reconciler.*; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.RateLimit; +import io.javaoperatorsdk.operator.api.reconciler.Reconciler; +import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; -@ControllerConfiguration(rateLimit = @RateLimit(limitForPeriod = 1, +@RateLimit(limitForPeriod = 1, refreshPeriod = RateLimitReconciler.REFRESH_PERIOD, - refreshPeriodTimeUnit = TimeUnit.MILLISECONDS)) + refreshPeriodTimeUnit = TimeUnit.MILLISECONDS) +@ControllerConfiguration public class RateLimitReconciler implements Reconciler { diff --git a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconciler.java b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconciler.java index 2aa1674564..788a0861ff 100644 --- a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconciler.java +++ b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconciler.java @@ -3,7 +3,6 @@ import java.util.HashMap; import java.util.Map; import java.util.Objects; -import java.util.concurrent.TimeUnit; import org.apache.commons.lang3.StringUtils; import org.slf4j.Logger; @@ -19,16 +18,32 @@ import io.fabric8.kubernetes.client.KubernetesClient; import io.javaoperatorsdk.operator.ReconcilerUtils; import io.javaoperatorsdk.operator.api.config.informer.InformerConfiguration; -import io.javaoperatorsdk.operator.api.reconciler.*; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.ErrorStatusHandler; +import io.javaoperatorsdk.operator.api.reconciler.ErrorStatusUpdateControl; +import io.javaoperatorsdk.operator.api.reconciler.EventSourceContext; +import io.javaoperatorsdk.operator.api.reconciler.EventSourceInitializer; +import io.javaoperatorsdk.operator.api.reconciler.RateLimit; +import io.javaoperatorsdk.operator.api.reconciler.Reconciler; +import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; import io.javaoperatorsdk.operator.processing.event.source.EventSource; import io.javaoperatorsdk.operator.processing.event.source.informer.InformerEventSource; -import static io.javaoperatorsdk.operator.sample.Utils.*; +import static io.javaoperatorsdk.operator.sample.Utils.configMapName; +import static io.javaoperatorsdk.operator.sample.Utils.createStatus; +import static io.javaoperatorsdk.operator.sample.Utils.deploymentName; +import static io.javaoperatorsdk.operator.sample.Utils.handleError; +import static io.javaoperatorsdk.operator.sample.Utils.isValidHtml; +import static io.javaoperatorsdk.operator.sample.Utils.makeDesiredIngress; +import static io.javaoperatorsdk.operator.sample.Utils.serviceName; +import static io.javaoperatorsdk.operator.sample.Utils.setInvalidHtmlErrorMessage; +import static io.javaoperatorsdk.operator.sample.Utils.simulateErrorIfRequested; import static io.javaoperatorsdk.operator.sample.WebPageManagedDependentsReconciler.SELECTOR; /** Shows how to implement reconciler using the low level api directly. */ -@ControllerConfiguration(rateLimit = @RateLimit(limitForPeriod = 2, refreshPeriod = 3, - refreshPeriodTimeUnit = TimeUnit.SECONDS)) +@RateLimit(limitForPeriod = 2, refreshPeriod = 3) +@ControllerConfiguration public class WebPageReconciler implements Reconciler, ErrorStatusHandler, EventSourceInitializer { From 78b80363d104a29af9f654f3519dbb9567732159 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 6 Jul 2022 10:42:10 +0200 Subject: [PATCH 03/16] fix: make sure we always get a default deactivated rate limiter --- .../AnnotationControllerConfiguration.java | 5 +++- .../api/config/ControllerConfiguration.java | 4 ++-- .../DefaultControllerConfiguration.java | 4 +++- .../api/config/RetryConfiguration.java | 7 +++++- .../event/rate/PeriodRateLimiter.java | 24 +++++++++++++++---- .../processing/retry/GenericRetry.java | 6 ++--- .../processing/event/EventProcessorTest.java | 2 +- .../source/CustomResourceSelectorTest.java | 9 ++++--- ...AnnotationControllerConfigurationTest.java | 14 ++++------- 9 files changed, 50 insertions(+), 25 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java index 2bfb13f9e9..53263552c7 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java @@ -1,6 +1,7 @@ package io.javaoperatorsdk.operator.api.config; import java.lang.annotation.Annotation; +import java.lang.reflect.Constructor; import java.lang.reflect.InvocationTargetException; import java.time.Duration; import java.util.Arrays; @@ -167,7 +168,9 @@ public Retry getRetry() { private T instantiateAndConfigureIfNeeded(Class targetClass) { try { - final var instance = targetClass.getConstructor().newInstance(); + final Constructor constructor = targetClass.getDeclaredConstructor(); + constructor.setAccessible(true); + final var instance = constructor.newInstance(); if (instance instanceof AnnotationConfigurable) { AnnotationConfigurable configurable = (AnnotationConfigurable) instance; final Class configurationClass = diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java index ccea5da8e7..9bc0384e64 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java @@ -17,8 +17,8 @@ public interface ControllerConfiguration extends ResourceConfiguration { - RateLimiter DEFAULT_RATE_LIMITER = new PeriodRateLimiter(); - Retry DEFAULT_RETRY = new GenericRetry(); + RateLimiter DEFAULT_RATE_LIMITER = PeriodRateLimiter.deactivatedRateLimiter(); + Retry DEFAULT_RETRY = GenericRetry.defaultLimitedExponentialRetry(); default String getName() { return ReconcilerUtils.getDefaultReconcilerName(getAssociatedReconcilerClassName()); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultControllerConfiguration.java index 9cc6fd1608..29cdce05c9 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultControllerConfiguration.java @@ -10,6 +10,7 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; +import io.javaoperatorsdk.operator.processing.event.rate.PeriodRateLimiter; import io.javaoperatorsdk.operator.processing.event.rate.RateLimiter; import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilter; import io.javaoperatorsdk.operator.processing.retry.Retry; @@ -60,7 +61,8 @@ public DefaultControllerConfiguration( ? ControllerConfiguration.super.getRetry() : retry; this.resourceEventFilter = resourceEventFilter; - this.rateLimiter = rateLimiter; + this.rateLimiter = + rateLimiter != null ? rateLimiter : PeriodRateLimiter.deactivatedRateLimiter(); this.dependents = dependents != null ? dependents : Collections.emptyList(); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/RetryConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/RetryConfiguration.java index 26ccbd28b7..d3a299874c 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/RetryConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/RetryConfiguration.java @@ -13,11 +13,16 @@ long DEFAULT_INITIAL_INTERVAL = 2000L; double DEFAULT_MULTIPLIER = 1.5D; + long DEFAULT_MAX_INTERVAL = (long) (RetryConfiguration.DEFAULT_INITIAL_INTERVAL * Math.pow( + RetryConfiguration.DEFAULT_MULTIPLIER, RetryConfiguration.DEFAULT_MAX_ATTEMPTS)); + + long UNSET_VALUE = Long.MAX_VALUE; + int maxAttempts() default DEFAULT_MAX_ATTEMPTS; long initialInterval() default DEFAULT_INITIAL_INTERVAL; double intervalMultiplier() default DEFAULT_MULTIPLIER; - long maxInterval() default -1; + long maxInterval() default UNSET_VALUE; } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/PeriodRateLimiter.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/PeriodRateLimiter.java index 1c378d5841..50887bbcfa 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/PeriodRateLimiter.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/PeriodRateLimiter.java @@ -24,12 +24,16 @@ public class PeriodRateLimiter implements RateLimiter, AnnotationConfigurable limitData = new HashMap<>(); - public PeriodRateLimiter() { - this(DEFAULT_REFRESH_PERIOD, DEFAULT_LIMIT_FOR_PERIOD); + public static PeriodRateLimiter deactivatedRateLimiter() { + return new PeriodRateLimiter(); + } + + PeriodRateLimiter() { + this(DEFAULT_REFRESH_PERIOD, NO_LIMIT_PERIOD); } public PeriodRateLimiter(Duration refreshPeriod, int limitForPeriod) { @@ -39,7 +43,7 @@ public PeriodRateLimiter(Duration refreshPeriod, int limitForPeriod) { @Override public Optional acquirePermission(ResourceID resourceID) { - if (limitForPeriod <= 0) { + if (!isActivated()) { return Optional.empty(); } var actualState = limitData.computeIfAbsent(resourceID, r -> RateState.initialState()); @@ -67,4 +71,16 @@ public void initFrom(RateLimit configuration) { configuration.refreshPeriodTimeUnit().toChronoUnit()); this.limitForPeriod = configuration.limitForPeriod(); } + + public boolean isActivated() { + return limitForPeriod > 0; + } + + public int getLimitForPeriod() { + return limitForPeriod; + } + + public Duration getRefreshPeriod() { + return refreshPeriod; + } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/GenericRetry.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/GenericRetry.java index 2caf092726..4b4ba3e61e 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/GenericRetry.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/GenericRetry.java @@ -7,7 +7,7 @@ public class GenericRetry implements Retry, AnnotationConfigurable { @Override From 0d68396b80ffe64026f5ab4db870d7e3aefe1a2a Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 6 Jul 2022 11:03:18 +0200 Subject: [PATCH 04/16] fix: restore backwards compatibility --- .../api/config/ControllerConfiguration.java | 18 ++++++++- .../api/config/DefaultRetryConfiguration.java | 5 +++ .../api/config/RetryConfiguration.java | 37 +++++++++++-------- .../processing/retry/GenericRetry.java | 32 ++++++++++++---- .../processing/retry/RetryingGradually.java | 28 ++++++++++++++ ...AnnotationControllerConfigurationTest.java | 10 ++--- 6 files changed, 100 insertions(+), 30 deletions(-) create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultRetryConfiguration.java create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/RetryingGradually.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java index 9bc0384e64..563a041309 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java @@ -35,7 +35,23 @@ default boolean isGenerationAware() { String getAssociatedReconcilerClassName(); default Retry getRetry() { - return DEFAULT_RETRY; + final var configuration = getRetryConfiguration(); + return !RetryConfiguration.DEFAULT.equals(configuration) + ? GenericRetry.fromConfiguration(configuration) + : GenericRetry.DEFAULT; // NOSONAR + } + + /** + * Use {@link #getRetry()} instead. + * + * @return configuration for retry. + * @deprecated provide your own {@link Retry} implementation or use the + * {@link io.javaoperatorsdk.operator.processing.retry.RetryingGradually} annotation + * instead + */ + @Deprecated(forRemoval = true) + default RetryConfiguration getRetryConfiguration() { + return RetryConfiguration.DEFAULT; } default RateLimiter getRateLimiter() { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultRetryConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultRetryConfiguration.java new file mode 100644 index 0000000000..40fbb38aa7 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultRetryConfiguration.java @@ -0,0 +1,5 @@ +package io.javaoperatorsdk.operator.api.config; + +public class DefaultRetryConfiguration implements RetryConfiguration { + +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/RetryConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/RetryConfiguration.java index d3a299874c..bd9798e183 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/RetryConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/RetryConfiguration.java @@ -1,28 +1,33 @@ package io.javaoperatorsdk.operator.api.config; -import java.lang.annotation.ElementType; -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; -import java.lang.annotation.Target; +/** + * @deprecated specify your own {@link io.javaoperatorsdk.operator.processing.retry.Retry} + * implementation or use + * {@link io.javaoperatorsdk.operator.processing.retry.RetryingGradually} annotation + * instead + */ +@Deprecated(forRemoval = true) +public interface RetryConfiguration { -@Retention(RetentionPolicy.RUNTIME) -@Target(ElementType.TYPE) -public @interface RetryConfiguration { + RetryConfiguration DEFAULT = new DefaultRetryConfiguration(); int DEFAULT_MAX_ATTEMPTS = 5; long DEFAULT_INITIAL_INTERVAL = 2000L; double DEFAULT_MULTIPLIER = 1.5D; - long DEFAULT_MAX_INTERVAL = (long) (RetryConfiguration.DEFAULT_INITIAL_INTERVAL * Math.pow( - RetryConfiguration.DEFAULT_MULTIPLIER, RetryConfiguration.DEFAULT_MAX_ATTEMPTS)); + default int getMaxAttempts() { + return DEFAULT_MAX_ATTEMPTS; + } - long UNSET_VALUE = Long.MAX_VALUE; + default long getInitialInterval() { + return DEFAULT_INITIAL_INTERVAL; + } - int maxAttempts() default DEFAULT_MAX_ATTEMPTS; + default double getIntervalMultiplier() { + return DEFAULT_MULTIPLIER; + } - long initialInterval() default DEFAULT_INITIAL_INTERVAL; - - double intervalMultiplier() default DEFAULT_MULTIPLIER; - - long maxInterval() default UNSET_VALUE; + default long getMaxInterval() { + return (long) (DEFAULT_INITIAL_INTERVAL * Math.pow(DEFAULT_MULTIPLIER, DEFAULT_MAX_ATTEMPTS)); + } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/GenericRetry.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/GenericRetry.java index 4b4ba3e61e..a7e2c4cfd0 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/GenericRetry.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/GenericRetry.java @@ -3,11 +3,13 @@ import io.javaoperatorsdk.operator.api.config.AnnotationConfigurable; import io.javaoperatorsdk.operator.api.config.RetryConfiguration; -public class GenericRetry implements Retry, AnnotationConfigurable { - private int maxAttempts = RetryConfiguration.DEFAULT_MAX_ATTEMPTS; - private long initialInterval = RetryConfiguration.DEFAULT_INITIAL_INTERVAL; - private double intervalMultiplier = RetryConfiguration.DEFAULT_MULTIPLIER; - private long maxInterval = RetryConfiguration.DEFAULT_MAX_INTERVAL; +public class GenericRetry implements Retry, AnnotationConfigurable { + private int maxAttempts = RetryingGradually.DEFAULT_MAX_ATTEMPTS; + private long initialInterval = RetryingGradually.DEFAULT_INITIAL_INTERVAL; + private double intervalMultiplier = RetryingGradually.DEFAULT_MULTIPLIER; + private long maxInterval = RetryingGradually.DEFAULT_MAX_INTERVAL; + + public static final Retry DEFAULT = fromConfiguration(RetryConfiguration.DEFAULT); public static GenericRetry defaultLimitedExponentialRetry() { return new GenericRetry(); @@ -17,6 +19,20 @@ public static GenericRetry noRetry() { return new GenericRetry().setMaxAttempts(0); } + /** + * @deprecated Use the {@link RetryingGradually} annotation instead + */ + @Deprecated(forRemoval = true) + public static Retry fromConfiguration(RetryConfiguration configuration) { + return configuration == null ? defaultLimitedExponentialRetry() + : new GenericRetry() + .setInitialInterval(configuration.getInitialInterval()) + .setMaxAttempts(configuration.getMaxAttempts()) + .setIntervalMultiplier(configuration.getIntervalMultiplier()) + .setMaxInterval(configuration.getMaxInterval()); + } + + public static GenericRetry every10second10TimesRetry() { return new GenericRetry().withLinearRetry().setMaxAttempts(10).setInitialInterval(10000); } @@ -77,12 +93,12 @@ public GenericRetry withLinearRetry() { } @Override - public void initFrom(RetryConfiguration configuration) { + public void initFrom(RetryingGradually configuration) { this.initialInterval = configuration.initialInterval(); this.maxAttempts = configuration.maxAttempts(); this.intervalMultiplier = configuration.intervalMultiplier(); - this.maxInterval = configuration.maxInterval() == RetryConfiguration.UNSET_VALUE - ? RetryConfiguration.DEFAULT_MAX_INTERVAL + this.maxInterval = configuration.maxInterval() == RetryingGradually.UNSET_VALUE + ? RetryingGradually.DEFAULT_MAX_INTERVAL : configuration.maxInterval(); } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/RetryingGradually.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/RetryingGradually.java new file mode 100644 index 0000000000..0521df12c2 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/RetryingGradually.java @@ -0,0 +1,28 @@ +package io.javaoperatorsdk.operator.processing.retry; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +@Retention(RetentionPolicy.RUNTIME) +@Target(ElementType.TYPE) +public @interface RetryingGradually { + + int DEFAULT_MAX_ATTEMPTS = 5; + long DEFAULT_INITIAL_INTERVAL = 2000L; + double DEFAULT_MULTIPLIER = 1.5D; + + long DEFAULT_MAX_INTERVAL = (long) (RetryingGradually.DEFAULT_INITIAL_INTERVAL * Math.pow( + RetryingGradually.DEFAULT_MULTIPLIER, RetryingGradually.DEFAULT_MAX_ATTEMPTS)); + + long UNSET_VALUE = Long.MAX_VALUE; + + int maxAttempts() default DEFAULT_MAX_ATTEMPTS; + + long initialInterval() default DEFAULT_INITIAL_INTERVAL; + + double intervalMultiplier() default DEFAULT_MULTIPLIER; + + long maxInterval() default UNSET_VALUE; +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/AnnotationControllerConfigurationTest.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/AnnotationControllerConfigurationTest.java index 0db06a279c..91f82365e5 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/AnnotationControllerConfigurationTest.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/AnnotationControllerConfigurationTest.java @@ -14,7 +14,6 @@ import io.fabric8.kubernetes.api.model.ConfigMap; import io.javaoperatorsdk.operator.OperatorException; import io.javaoperatorsdk.operator.api.config.AnnotationConfigurable; -import io.javaoperatorsdk.operator.api.config.RetryConfiguration; import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; @@ -30,6 +29,7 @@ import io.javaoperatorsdk.operator.processing.retry.GenericRetry; import io.javaoperatorsdk.operator.processing.retry.Retry; import io.javaoperatorsdk.operator.processing.retry.RetryExecution; +import io.javaoperatorsdk.operator.processing.retry.RetryingGradually; import io.javaoperatorsdk.operator.sample.readonly.ReadOnlyDependent; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -132,10 +132,10 @@ && findByNameOptional(dependents, DependentResource.defaultNameFor(ReadOnlyDepen void checkDefaultRateAndRetryConfigurations() { var config = new AnnotationControllerConfiguration<>(new NoDepReconciler()); final var retry = assertInstanceOf(GenericRetry.class, config.getRetry()); - assertEquals(RetryConfiguration.DEFAULT_MAX_ATTEMPTS, retry.getMaxAttempts()); - assertEquals(RetryConfiguration.DEFAULT_MULTIPLIER, retry.getIntervalMultiplier()); - assertEquals(RetryConfiguration.DEFAULT_INITIAL_INTERVAL, retry.getInitialInterval()); - assertEquals(RetryConfiguration.DEFAULT_MAX_INTERVAL, retry.getMaxInterval()); + assertEquals(RetryingGradually.DEFAULT_MAX_ATTEMPTS, retry.getMaxAttempts()); + assertEquals(RetryingGradually.DEFAULT_MULTIPLIER, retry.getIntervalMultiplier()); + assertEquals(RetryingGradually.DEFAULT_INITIAL_INTERVAL, retry.getInitialInterval()); + assertEquals(RetryingGradually.DEFAULT_MAX_INTERVAL, retry.getMaxInterval()); final var limiter = assertInstanceOf(PeriodRateLimiter.class, config.getRateLimiter()); assertFalse(limiter.isActivated()); From 369f1e774ff5c78214296eef67ef35c2410611f9 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 6 Jul 2022 11:06:39 +0200 Subject: [PATCH 05/16] refactor: move RateLimit annotation to proper package --- .../operator/processing/event/rate/PeriodRateLimiter.java | 1 - .../{api/reconciler => processing/event/rate}/RateLimit.java | 4 +--- .../config/runtime/AnnotationControllerConfigurationTest.java | 2 +- .../operator/sample/ratelimit/RateLimitReconciler.java | 2 +- .../io/javaoperatorsdk/operator/sample/WebPageReconciler.java | 2 +- 5 files changed, 4 insertions(+), 7 deletions(-) rename operator-framework-core/src/main/java/io/javaoperatorsdk/operator/{api/reconciler => processing/event/rate}/RateLimit.java (81%) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/PeriodRateLimiter.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/PeriodRateLimiter.java index 50887bbcfa..b4759cf007 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/PeriodRateLimiter.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/PeriodRateLimiter.java @@ -7,7 +7,6 @@ import java.util.Optional; import io.javaoperatorsdk.operator.api.config.AnnotationConfigurable; -import io.javaoperatorsdk.operator.api.reconciler.RateLimit; import io.javaoperatorsdk.operator.processing.event.ResourceID; /** diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/RateLimit.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/RateLimit.java similarity index 81% rename from operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/RateLimit.java rename to operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/RateLimit.java index 6c4feb2b2a..78d0090d7e 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/RateLimit.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/RateLimit.java @@ -1,4 +1,4 @@ -package io.javaoperatorsdk.operator.api.reconciler; +package io.javaoperatorsdk.operator.processing.event.rate; import java.lang.annotation.ElementType; import java.lang.annotation.Retention; @@ -6,8 +6,6 @@ import java.lang.annotation.Target; import java.util.concurrent.TimeUnit; -import io.javaoperatorsdk.operator.processing.event.rate.PeriodRateLimiter; - @Retention(RetentionPolicy.RUNTIME) @Target({ElementType.TYPE}) public @interface RateLimit { diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/AnnotationControllerConfigurationTest.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/AnnotationControllerConfigurationTest.java index 91f82365e5..209f250227 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/AnnotationControllerConfigurationTest.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/AnnotationControllerConfigurationTest.java @@ -17,7 +17,6 @@ import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; -import io.javaoperatorsdk.operator.api.reconciler.RateLimit; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent; @@ -26,6 +25,7 @@ import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResourceConfig; import io.javaoperatorsdk.operator.processing.event.rate.PeriodRateLimiter; +import io.javaoperatorsdk.operator.processing.event.rate.RateLimit; import io.javaoperatorsdk.operator.processing.retry.GenericRetry; import io.javaoperatorsdk.operator.processing.retry.Retry; import io.javaoperatorsdk.operator.processing.retry.RetryExecution; diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/ratelimit/RateLimitReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/ratelimit/RateLimitReconciler.java index 52308618c5..3cb02519ea 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/ratelimit/RateLimitReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/ratelimit/RateLimitReconciler.java @@ -5,9 +5,9 @@ import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; -import io.javaoperatorsdk.operator.api.reconciler.RateLimit; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; +import io.javaoperatorsdk.operator.processing.event.rate.RateLimit; @RateLimit(limitForPeriod = 1, refreshPeriod = RateLimitReconciler.REFRESH_PERIOD, diff --git a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconciler.java b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconciler.java index 788a0861ff..128db67265 100644 --- a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconciler.java +++ b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconciler.java @@ -24,9 +24,9 @@ import io.javaoperatorsdk.operator.api.reconciler.ErrorStatusUpdateControl; import io.javaoperatorsdk.operator.api.reconciler.EventSourceContext; import io.javaoperatorsdk.operator.api.reconciler.EventSourceInitializer; -import io.javaoperatorsdk.operator.api.reconciler.RateLimit; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; +import io.javaoperatorsdk.operator.processing.event.rate.RateLimit; import io.javaoperatorsdk.operator.processing.event.source.EventSource; import io.javaoperatorsdk.operator.processing.event.source.informer.InformerEventSource; From a5ddb20d6b6380f7fc122a530d05cf59dd04a1e3 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 6 Jul 2022 14:51:24 +0200 Subject: [PATCH 06/16] refactor: rename RateLimit to be more explicit --- docs/documentation/features.md | 16 +++++++++---- ...Limit.java => LimitingRateOverPeriod.java} | 8 +++---- .../event/rate/PeriodRateLimiter.java | 24 +++++++------------ ...AnnotationControllerConfigurationTest.java | 7 ++++-- .../sample/ratelimit/RateLimitReconciler.java | 8 +++---- .../operator/sample/WebPageReconciler.java | 4 ++-- 6 files changed, 35 insertions(+), 32 deletions(-) rename operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/{RateLimit.java => LimitingRateOverPeriod.java} (62%) diff --git a/docs/documentation/features.md b/docs/documentation/features.md index 91778ee275..27a369e4c1 100644 --- a/docs/documentation/features.md +++ b/docs/documentation/features.md @@ -346,12 +346,18 @@ Users can override it by implementing their own [`RateLimiter`](https://github.com/java-operator-sdk/java-operator-sdk/blob/ce4d996ee073ebef5715737995fc3d33f4751275/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/RateLimiter.java) . -To configure the default rate limiter use `@ControllerConfiguration` annotation. The following -configuration limits -each resource to reconcile at most twice within a 3 second interval: +To configure the default rate limiter use the `@LimitingRateOverPeriod` annotation on your +`Reconciler` class. The following configuration limits each resource to reconcile at most twice +within a 3 second interval: -`@ControllerConfiguration(rateLimit = @RateLimit(limitForPeriod = 2,refreshPeriod = 3,refreshPeriodTimeUnit = TimeUnit.SECONDS))` -. +```java + +@LimitingRateOverPeriod(maxReconciliations = 2, within = 3, unit = TimeUnit.SECONDS) +@ControllerConfiguration +public class MyReconciler implements Reconciler { + +} +``` Thus, if a given resource was reconciled twice in one second, no further reconciliation for this resource will happen before two seconds have elapsed. Note that, since rate is limited on a diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/RateLimit.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/LimitingRateOverPeriod.java similarity index 62% rename from operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/RateLimit.java rename to operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/LimitingRateOverPeriod.java index 78d0090d7e..847e7d8214 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/RateLimit.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/LimitingRateOverPeriod.java @@ -8,14 +8,14 @@ @Retention(RetentionPolicy.RUNTIME) @Target({ElementType.TYPE}) -public @interface RateLimit { +public @interface LimitingRateOverPeriod { - int limitForPeriod() default PeriodRateLimiter.NO_LIMIT_PERIOD; + int maxReconciliations(); - int refreshPeriod() default PeriodRateLimiter.DEFAULT_REFRESH_PERIOD_SECONDS; + int within(); /** * @return time unit for max delay between reconciliations */ - TimeUnit refreshPeriodTimeUnit() default TimeUnit.SECONDS; + TimeUnit unit() default TimeUnit.SECONDS; } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/PeriodRateLimiter.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/PeriodRateLimiter.java index b4759cf007..641939dbb3 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/PeriodRateLimiter.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/PeriodRateLimiter.java @@ -12,18 +12,14 @@ /** * A Simple rate limiter that limits the number of permission for a time interval. */ -public class PeriodRateLimiter implements RateLimiter, AnnotationConfigurable { +public class PeriodRateLimiter + implements RateLimiter, AnnotationConfigurable { - public static final int DEFAULT_REFRESH_PERIOD_SECONDS = 10; - public static final int DEFAULT_LIMIT_FOR_PERIOD = 3; - public static final Duration DEFAULT_REFRESH_PERIOD = - Duration.ofSeconds(DEFAULT_REFRESH_PERIOD_SECONDS); - - /** To turn off rate limiting set limit fod period to a non-positive number */ + /** To turn off rate limiting set limit for period to a non-positive number */ public static final int NO_LIMIT_PERIOD = -1; private Duration refreshPeriod; - private int limitForPeriod; + private int limitForPeriod = NO_LIMIT_PERIOD; private final Map limitData = new HashMap<>(); @@ -31,9 +27,7 @@ public static PeriodRateLimiter deactivatedRateLimiter() { return new PeriodRateLimiter(); } - PeriodRateLimiter() { - this(DEFAULT_REFRESH_PERIOD, NO_LIMIT_PERIOD); - } + PeriodRateLimiter() {} public PeriodRateLimiter(Duration refreshPeriod, int limitForPeriod) { this.refreshPeriod = refreshPeriod; @@ -65,10 +59,10 @@ public void clear(ResourceID resourceID) { } @Override - public void initFrom(RateLimit configuration) { - this.refreshPeriod = Duration.of(configuration.refreshPeriod(), - configuration.refreshPeriodTimeUnit().toChronoUnit()); - this.limitForPeriod = configuration.limitForPeriod(); + public void initFrom(LimitingRateOverPeriod configuration) { + this.refreshPeriod = Duration.of(configuration.within(), + configuration.unit().toChronoUnit()); + this.limitForPeriod = configuration.maxReconciliations(); } public boolean isActivated() { diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/AnnotationControllerConfigurationTest.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/AnnotationControllerConfigurationTest.java index 209f250227..583b900494 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/AnnotationControllerConfigurationTest.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/AnnotationControllerConfigurationTest.java @@ -4,6 +4,7 @@ import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; +import java.time.Duration; import java.util.List; import java.util.Optional; import java.util.Set; @@ -24,8 +25,8 @@ import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependent; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResourceConfig; +import io.javaoperatorsdk.operator.processing.event.rate.LimitingRateOverPeriod; import io.javaoperatorsdk.operator.processing.event.rate.PeriodRateLimiter; -import io.javaoperatorsdk.operator.processing.event.rate.RateLimit; import io.javaoperatorsdk.operator.processing.retry.GenericRetry; import io.javaoperatorsdk.operator.processing.retry.Retry; import io.javaoperatorsdk.operator.processing.retry.RetryExecution; @@ -60,6 +61,7 @@ private KubernetesDependentResourceConfig extractDependentKubernetesResourceConf } @Test + @SuppressWarnings("rawtypes") void getDependentResources() { var configuration = new AnnotationControllerConfiguration<>(new NoDepReconciler()); var dependents = configuration.getDependentResources(); @@ -151,6 +153,7 @@ void configuringRateAndRetryViaAnnotationsShouldWork() { final var rateLimiter = assertInstanceOf(PeriodRateLimiter.class, config.getRateLimiter()); assertEquals(7, rateLimiter.getLimitForPeriod()); + assertEquals(Duration.ofSeconds(3), rateLimiter.getRefreshPeriod()); } @ControllerConfiguration(namespaces = OneDepReconciler.CONFIGURED_NS, @@ -269,7 +272,7 @@ public void initFrom(TestRetryConfiguration configuration) { } @TestRetryConfiguration(12) - @RateLimit(limitForPeriod = 7) + @LimitingRateOverPeriod(maxReconciliations = 7, within = 3) @ControllerConfiguration(retry = TestRetry.class) private static class ConfigurableRateLimitAndRetryReconciler implements Reconciler { diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/ratelimit/RateLimitReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/ratelimit/RateLimitReconciler.java index 3cb02519ea..8070249588 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/ratelimit/RateLimitReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/ratelimit/RateLimitReconciler.java @@ -7,11 +7,11 @@ import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; -import io.javaoperatorsdk.operator.processing.event.rate.RateLimit; +import io.javaoperatorsdk.operator.processing.event.rate.LimitingRateOverPeriod; -@RateLimit(limitForPeriod = 1, - refreshPeriod = RateLimitReconciler.REFRESH_PERIOD, - refreshPeriodTimeUnit = TimeUnit.MILLISECONDS) +@LimitingRateOverPeriod(maxReconciliations = 1, + within = RateLimitReconciler.REFRESH_PERIOD, + unit = TimeUnit.MILLISECONDS) @ControllerConfiguration public class RateLimitReconciler implements Reconciler { diff --git a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconciler.java b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconciler.java index 128db67265..1fde2e5bd4 100644 --- a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconciler.java +++ b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconciler.java @@ -26,7 +26,7 @@ import io.javaoperatorsdk.operator.api.reconciler.EventSourceInitializer; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; -import io.javaoperatorsdk.operator.processing.event.rate.RateLimit; +import io.javaoperatorsdk.operator.processing.event.rate.LimitingRateOverPeriod; import io.javaoperatorsdk.operator.processing.event.source.EventSource; import io.javaoperatorsdk.operator.processing.event.source.informer.InformerEventSource; @@ -42,7 +42,7 @@ import static io.javaoperatorsdk.operator.sample.WebPageManagedDependentsReconciler.SELECTOR; /** Shows how to implement reconciler using the low level api directly. */ -@RateLimit(limitForPeriod = 2, refreshPeriod = 3) +@LimitingRateOverPeriod(maxReconciliations = 2, within = 3) @ControllerConfiguration public class WebPageReconciler implements Reconciler, ErrorStatusHandler, EventSourceInitializer { From 5b240d398a514eab6c7debb2e384b48520e35831 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 6 Jul 2022 15:00:30 +0200 Subject: [PATCH 07/16] refactor: clean up --- .../operator/api/config/ControllerConfiguration.java | 1 - .../operator/processing/retry/GenericRetry.java | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java index 563a041309..464ace3a5e 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java @@ -18,7 +18,6 @@ public interface ControllerConfiguration extends ResourceConfiguration { RateLimiter DEFAULT_RATE_LIMITER = PeriodRateLimiter.deactivatedRateLimiter(); - Retry DEFAULT_RETRY = GenericRetry.defaultLimitedExponentialRetry(); default String getName() { return ReconcilerUtils.getDefaultReconcilerName(getAssociatedReconcilerClassName()); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/GenericRetry.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/GenericRetry.java index a7e2c4cfd0..71c7a3b0d4 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/GenericRetry.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/GenericRetry.java @@ -9,10 +9,10 @@ public class GenericRetry implements Retry, AnnotationConfigurable Date: Wed, 6 Jul 2022 17:47:29 +0200 Subject: [PATCH 08/16] chore(tests): add test to check `RetryingGradually` annotation --- ...AnnotationControllerConfigurationTest.java | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/AnnotationControllerConfigurationTest.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/AnnotationControllerConfigurationTest.java index 583b900494..efd9cfde06 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/AnnotationControllerConfigurationTest.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/AnnotationControllerConfigurationTest.java @@ -156,6 +156,19 @@ void configuringRateAndRetryViaAnnotationsShouldWork() { assertEquals(Duration.ofSeconds(3), rateLimiter.getRefreshPeriod()); } + @Test + void checkingRetryingGraduallyWorks() { + var config = new AnnotationControllerConfiguration<>(new CheckRetryingGraduallyConfiguration()); + final var retry = config.getRetry(); + final var genericRetry = assertInstanceOf(GenericRetry.class, retry); + assertEquals(CheckRetryingGraduallyConfiguration.INITIAL_INTERVAL, + genericRetry.getInitialInterval()); + assertEquals(CheckRetryingGraduallyConfiguration.MAX_ATTEMPTS, genericRetry.getMaxAttempts()); + assertEquals(CheckRetryingGraduallyConfiguration.INTERVAL_MULTIPLIER, + genericRetry.getIntervalMultiplier()); + assertEquals(CheckRetryingGraduallyConfiguration.MAX_INTERVAL, genericRetry.getMaxInterval()); + } + @ControllerConfiguration(namespaces = OneDepReconciler.CONFIGURED_NS, dependents = @Dependent(type = ReadOnlyDependent.class)) private static class OneDepReconciler implements Reconciler { @@ -282,4 +295,24 @@ public UpdateControl reconcile(ConfigMap resource, Context return UpdateControl.noUpdate(); } } + + @RetryingGradually( + maxAttempts = CheckRetryingGraduallyConfiguration.MAX_ATTEMPTS, + initialInterval = CheckRetryingGraduallyConfiguration.INITIAL_INTERVAL, + intervalMultiplier = CheckRetryingGraduallyConfiguration.INTERVAL_MULTIPLIER, + maxInterval = CheckRetryingGraduallyConfiguration.MAX_INTERVAL) + @ControllerConfiguration + private static class CheckRetryingGraduallyConfiguration implements Reconciler { + + public static final int MAX_ATTEMPTS = 7; + public static final int INITIAL_INTERVAL = 1000; + public static final int INTERVAL_MULTIPLIER = 2; + public static final int MAX_INTERVAL = 60000; + + @Override + public UpdateControl reconcile(ConfigMap resource, Context context) + throws Exception { + return UpdateControl.noUpdate(); + } + } } From aa7017b179460a28143b2e7c545347f522a82f23 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Thu, 7 Jul 2022 09:22:28 +0200 Subject: [PATCH 09/16] fix: better error message --- .../config/AnnotationControllerConfiguration.java | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java index 53263552c7..6164fd6a4d 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java @@ -157,16 +157,18 @@ public Optional reconciliationMaxInterval() { @Override public RateLimiter getRateLimiter() { final Class rateLimiterClass = annotation.rateLimiter(); - return instantiateAndConfigureIfNeeded(rateLimiterClass); + return instantiateAndConfigureIfNeeded(rateLimiterClass, RateLimiter.class); } @Override public Retry getRetry() { final Class retryClass = annotation.retry(); - return instantiateAndConfigureIfNeeded(retryClass); + return instantiateAndConfigureIfNeeded(retryClass, Retry.class); } - private T instantiateAndConfigureIfNeeded(Class targetClass) { + @SuppressWarnings("unchecked") + private T instantiateAndConfigureIfNeeded(Class targetClass, + Class expectedType) { try { final Constructor constructor = targetClass.getDeclaredConstructor(); constructor.setAccessible(true); @@ -184,7 +186,9 @@ private T instantiateAndConfigureIfNeeded(Class targetClass) { return instance; } catch (InstantiationException | IllegalAccessException | InvocationTargetException | NoSuchMethodException e) { - throw new RuntimeException(e); + throw new OperatorException("Couldn't instantiate " + expectedType.getSimpleName() + " '" + + targetClass.getName() + "' for '" + getName() + + "' reconciler. You need to provide an accessible no-arg constructor.", e); } } @@ -305,7 +309,7 @@ private String getName(Dependent dependent, Class d return name; } - @SuppressWarnings("rawtypes") + @SuppressWarnings({"rawtypes", "unchecked"}) private Object createKubernetesResourceConfig(Class dependentType) { Object config; From d11b0e29f23d8b93911347ac7137fc390a94a41c Mon Sep 17 00:00:00 2001 From: csviri Date: Mon, 11 Jul 2022 09:21:03 +0200 Subject: [PATCH 10/16] put back method for config override --- .../api/config/ControllerConfigurationOverrider.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java index 3ce52429db..b09e42e58a 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java @@ -15,6 +15,7 @@ import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResourceConfig; import io.javaoperatorsdk.operator.processing.event.rate.RateLimiter; import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilter; +import io.javaoperatorsdk.operator.processing.retry.GenericRetry; import io.javaoperatorsdk.operator.processing.retry.Retry; import static io.javaoperatorsdk.operator.api.reconciler.Constants.DEFAULT_NAMESPACES_SET; @@ -105,6 +106,12 @@ public ControllerConfigurationOverrider watchingAllNamespaces() { return this; } + @Deprecated(forRemoval = true) + public ControllerConfigurationOverrider withRetry(RetryConfiguration retry) { + this.retry = GenericRetry.fromConfiguration(retry); + return this; + } + public ControllerConfigurationOverrider withRetry(Retry retry) { this.retry = retry; return this; From b90272360c1175baacdf1a169ccc31c6ff97d693 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Mon, 11 Jul 2022 12:28:39 +0200 Subject: [PATCH 11/16] refactor: rename LimitingRateOverPeriod to RateLimited --- .../operator/processing/event/rate/PeriodRateLimiter.java | 4 ++-- .../rate/{LimitingRateOverPeriod.java => RateLimited.java} | 2 +- .../config/runtime/AnnotationControllerConfigurationTest.java | 4 ++-- .../operator/sample/ratelimit/RateLimitReconciler.java | 4 ++-- .../io/javaoperatorsdk/operator/sample/WebPageReconciler.java | 4 ++-- 5 files changed, 9 insertions(+), 9 deletions(-) rename operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/{LimitingRateOverPeriod.java => RateLimited.java} (91%) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/PeriodRateLimiter.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/PeriodRateLimiter.java index 641939dbb3..7eb3134217 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/PeriodRateLimiter.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/PeriodRateLimiter.java @@ -13,7 +13,7 @@ * A Simple rate limiter that limits the number of permission for a time interval. */ public class PeriodRateLimiter - implements RateLimiter, AnnotationConfigurable { + implements RateLimiter, AnnotationConfigurable { /** To turn off rate limiting set limit for period to a non-positive number */ public static final int NO_LIMIT_PERIOD = -1; @@ -59,7 +59,7 @@ public void clear(ResourceID resourceID) { } @Override - public void initFrom(LimitingRateOverPeriod configuration) { + public void initFrom(RateLimited configuration) { this.refreshPeriod = Duration.of(configuration.within(), configuration.unit().toChronoUnit()); this.limitForPeriod = configuration.maxReconciliations(); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/LimitingRateOverPeriod.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/RateLimited.java similarity index 91% rename from operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/LimitingRateOverPeriod.java rename to operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/RateLimited.java index 847e7d8214..7f425b73d9 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/LimitingRateOverPeriod.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/RateLimited.java @@ -8,7 +8,7 @@ @Retention(RetentionPolicy.RUNTIME) @Target({ElementType.TYPE}) -public @interface LimitingRateOverPeriod { +public @interface RateLimited { int maxReconciliations(); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/AnnotationControllerConfigurationTest.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/AnnotationControllerConfigurationTest.java index efd9cfde06..a8b16896cf 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/AnnotationControllerConfigurationTest.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/AnnotationControllerConfigurationTest.java @@ -25,8 +25,8 @@ import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependent; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResourceConfig; -import io.javaoperatorsdk.operator.processing.event.rate.LimitingRateOverPeriod; import io.javaoperatorsdk.operator.processing.event.rate.PeriodRateLimiter; +import io.javaoperatorsdk.operator.processing.event.rate.RateLimited; import io.javaoperatorsdk.operator.processing.retry.GenericRetry; import io.javaoperatorsdk.operator.processing.retry.Retry; import io.javaoperatorsdk.operator.processing.retry.RetryExecution; @@ -285,7 +285,7 @@ public void initFrom(TestRetryConfiguration configuration) { } @TestRetryConfiguration(12) - @LimitingRateOverPeriod(maxReconciliations = 7, within = 3) + @RateLimited(maxReconciliations = 7, within = 3) @ControllerConfiguration(retry = TestRetry.class) private static class ConfigurableRateLimitAndRetryReconciler implements Reconciler { diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/ratelimit/RateLimitReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/ratelimit/RateLimitReconciler.java index 8070249588..19a63952ef 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/ratelimit/RateLimitReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/ratelimit/RateLimitReconciler.java @@ -7,9 +7,9 @@ import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; -import io.javaoperatorsdk.operator.processing.event.rate.LimitingRateOverPeriod; +import io.javaoperatorsdk.operator.processing.event.rate.RateLimited; -@LimitingRateOverPeriod(maxReconciliations = 1, +@RateLimited(maxReconciliations = 1, within = RateLimitReconciler.REFRESH_PERIOD, unit = TimeUnit.MILLISECONDS) @ControllerConfiguration diff --git a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconciler.java b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconciler.java index 1fde2e5bd4..ca6710e049 100644 --- a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconciler.java +++ b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconciler.java @@ -26,7 +26,7 @@ import io.javaoperatorsdk.operator.api.reconciler.EventSourceInitializer; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; -import io.javaoperatorsdk.operator.processing.event.rate.LimitingRateOverPeriod; +import io.javaoperatorsdk.operator.processing.event.rate.RateLimited; import io.javaoperatorsdk.operator.processing.event.source.EventSource; import io.javaoperatorsdk.operator.processing.event.source.informer.InformerEventSource; @@ -42,7 +42,7 @@ import static io.javaoperatorsdk.operator.sample.WebPageManagedDependentsReconciler.SELECTOR; /** Shows how to implement reconciler using the low level api directly. */ -@LimitingRateOverPeriod(maxReconciliations = 2, within = 3) +@RateLimited(maxReconciliations = 2, within = 3) @ControllerConfiguration public class WebPageReconciler implements Reconciler, ErrorStatusHandler, EventSourceInitializer { From d2517350238490fcaac80ce549e61e3cb214d33f Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Mon, 11 Jul 2022 12:34:06 +0200 Subject: [PATCH 12/16] docs: add javadoc --- .../api/reconciler/ControllerConfiguration.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ControllerConfiguration.java index d5ec7be657..c5490609bb 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ControllerConfiguration.java @@ -104,7 +104,19 @@ ReconciliationMaxInterval reconciliationMaxInterval() default @ReconciliationMax */ Dependent[] dependents() default {}; + /** + * Optional {@link Retry} implementation for the associated controller to use. + * + * @return the class providing the {@link Retry} implementation to use, needs to provide an + * accessible no-arg constructor. + */ Class retry() default GenericRetry.class; + /** + * Optional {@link RateLimiter} implementation for the associated controller to use. + * + * @return the class providing the {@link RateLimiter} implementation to use, needs to provide an + * accessible no-arg constructor. + */ Class rateLimiter() default PeriodRateLimiter.class; } From 894574939c19febf4fc745689d676da253ee90c2 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Mon, 11 Jul 2022 12:34:41 +0200 Subject: [PATCH 13/16] fix: restore backwards compatibility --- .../api/config/ControllerConfigurationOverrider.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java index b09e42e58a..9b9f467386 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java @@ -117,6 +117,12 @@ public ControllerConfigurationOverrider withRetry(Retry retry) { return this; } + @Deprecated + public ControllerConfigurationOverrider withRetry(RetryConfiguration retry) { + this.retry = GenericRetry.fromConfiguration(retry); + return this; + } + public ControllerConfigurationOverrider withRateLimiter(RateLimiter rateLimiter) { this.rateLimiter = rateLimiter; return this; From 4c36bc14b37ff0362100681e6fcad74cb6bd46b2 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Mon, 11 Jul 2022 12:37:15 +0200 Subject: [PATCH 14/16] refactor: rename RetryingGradually to GradualRetry --- .../api/config/ControllerConfiguration.java | 6 +++--- .../api/config/RetryConfiguration.java | 6 +++--- .../processing/retry/GenericRetry.java | 18 +++++++++--------- ...etryingGradually.java => GradualRetry.java} | 6 +++--- .../AnnotationControllerConfigurationTest.java | 12 ++++++------ 5 files changed, 24 insertions(+), 24 deletions(-) rename operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/{RetryingGradually.java => GradualRetry.java} (74%) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java index 464ace3a5e..7488f48194 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java @@ -13,6 +13,7 @@ import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilter; import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilters; import io.javaoperatorsdk.operator.processing.retry.GenericRetry; +import io.javaoperatorsdk.operator.processing.retry.GradualRetry; import io.javaoperatorsdk.operator.processing.retry.Retry; public interface ControllerConfiguration extends ResourceConfiguration { @@ -44,9 +45,8 @@ default Retry getRetry() { * Use {@link #getRetry()} instead. * * @return configuration for retry. - * @deprecated provide your own {@link Retry} implementation or use the - * {@link io.javaoperatorsdk.operator.processing.retry.RetryingGradually} annotation - * instead + * @deprecated provide your own {@link Retry} implementation or use the {@link GradualRetry} + * annotation instead */ @Deprecated(forRemoval = true) default RetryConfiguration getRetryConfiguration() { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/RetryConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/RetryConfiguration.java index bd9798e183..b293c7e33f 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/RetryConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/RetryConfiguration.java @@ -1,10 +1,10 @@ package io.javaoperatorsdk.operator.api.config; +import io.javaoperatorsdk.operator.processing.retry.GradualRetry; + /** * @deprecated specify your own {@link io.javaoperatorsdk.operator.processing.retry.Retry} - * implementation or use - * {@link io.javaoperatorsdk.operator.processing.retry.RetryingGradually} annotation - * instead + * implementation or use {@link GradualRetry} annotation instead */ @Deprecated(forRemoval = true) public interface RetryConfiguration { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/GenericRetry.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/GenericRetry.java index 71c7a3b0d4..3c67b87966 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/GenericRetry.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/GenericRetry.java @@ -3,11 +3,11 @@ import io.javaoperatorsdk.operator.api.config.AnnotationConfigurable; import io.javaoperatorsdk.operator.api.config.RetryConfiguration; -public class GenericRetry implements Retry, AnnotationConfigurable { - private int maxAttempts = RetryingGradually.DEFAULT_MAX_ATTEMPTS; - private long initialInterval = RetryingGradually.DEFAULT_INITIAL_INTERVAL; - private double intervalMultiplier = RetryingGradually.DEFAULT_MULTIPLIER; - private long maxInterval = RetryingGradually.DEFAULT_MAX_INTERVAL; +public class GenericRetry implements Retry, AnnotationConfigurable { + private int maxAttempts = GradualRetry.DEFAULT_MAX_ATTEMPTS; + private long initialInterval = GradualRetry.DEFAULT_INITIAL_INTERVAL; + private double intervalMultiplier = GradualRetry.DEFAULT_MULTIPLIER; + private long maxInterval = GradualRetry.DEFAULT_MAX_INTERVAL; public static final Retry DEFAULT = new GenericRetry(); @@ -20,7 +20,7 @@ public static GenericRetry noRetry() { } /** - * @deprecated Use the {@link RetryingGradually} annotation instead + * @deprecated Use the {@link GradualRetry} annotation instead */ @Deprecated(forRemoval = true) public static Retry fromConfiguration(RetryConfiguration configuration) { @@ -93,12 +93,12 @@ public GenericRetry withLinearRetry() { } @Override - public void initFrom(RetryingGradually configuration) { + public void initFrom(GradualRetry configuration) { this.initialInterval = configuration.initialInterval(); this.maxAttempts = configuration.maxAttempts(); this.intervalMultiplier = configuration.intervalMultiplier(); - this.maxInterval = configuration.maxInterval() == RetryingGradually.UNSET_VALUE - ? RetryingGradually.DEFAULT_MAX_INTERVAL + this.maxInterval = configuration.maxInterval() == GradualRetry.UNSET_VALUE + ? GradualRetry.DEFAULT_MAX_INTERVAL : configuration.maxInterval(); } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/RetryingGradually.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/GradualRetry.java similarity index 74% rename from operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/RetryingGradually.java rename to operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/GradualRetry.java index 0521df12c2..66f6372b32 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/RetryingGradually.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/GradualRetry.java @@ -7,14 +7,14 @@ @Retention(RetentionPolicy.RUNTIME) @Target(ElementType.TYPE) -public @interface RetryingGradually { +public @interface GradualRetry { int DEFAULT_MAX_ATTEMPTS = 5; long DEFAULT_INITIAL_INTERVAL = 2000L; double DEFAULT_MULTIPLIER = 1.5D; - long DEFAULT_MAX_INTERVAL = (long) (RetryingGradually.DEFAULT_INITIAL_INTERVAL * Math.pow( - RetryingGradually.DEFAULT_MULTIPLIER, RetryingGradually.DEFAULT_MAX_ATTEMPTS)); + long DEFAULT_MAX_INTERVAL = (long) (GradualRetry.DEFAULT_INITIAL_INTERVAL * Math.pow( + GradualRetry.DEFAULT_MULTIPLIER, GradualRetry.DEFAULT_MAX_ATTEMPTS)); long UNSET_VALUE = Long.MAX_VALUE; diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/AnnotationControllerConfigurationTest.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/AnnotationControllerConfigurationTest.java index a8b16896cf..a8e9c8065b 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/AnnotationControllerConfigurationTest.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/AnnotationControllerConfigurationTest.java @@ -28,9 +28,9 @@ import io.javaoperatorsdk.operator.processing.event.rate.PeriodRateLimiter; import io.javaoperatorsdk.operator.processing.event.rate.RateLimited; import io.javaoperatorsdk.operator.processing.retry.GenericRetry; +import io.javaoperatorsdk.operator.processing.retry.GradualRetry; import io.javaoperatorsdk.operator.processing.retry.Retry; import io.javaoperatorsdk.operator.processing.retry.RetryExecution; -import io.javaoperatorsdk.operator.processing.retry.RetryingGradually; import io.javaoperatorsdk.operator.sample.readonly.ReadOnlyDependent; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -134,10 +134,10 @@ && findByNameOptional(dependents, DependentResource.defaultNameFor(ReadOnlyDepen void checkDefaultRateAndRetryConfigurations() { var config = new AnnotationControllerConfiguration<>(new NoDepReconciler()); final var retry = assertInstanceOf(GenericRetry.class, config.getRetry()); - assertEquals(RetryingGradually.DEFAULT_MAX_ATTEMPTS, retry.getMaxAttempts()); - assertEquals(RetryingGradually.DEFAULT_MULTIPLIER, retry.getIntervalMultiplier()); - assertEquals(RetryingGradually.DEFAULT_INITIAL_INTERVAL, retry.getInitialInterval()); - assertEquals(RetryingGradually.DEFAULT_MAX_INTERVAL, retry.getMaxInterval()); + assertEquals(GradualRetry.DEFAULT_MAX_ATTEMPTS, retry.getMaxAttempts()); + assertEquals(GradualRetry.DEFAULT_MULTIPLIER, retry.getIntervalMultiplier()); + assertEquals(GradualRetry.DEFAULT_INITIAL_INTERVAL, retry.getInitialInterval()); + assertEquals(GradualRetry.DEFAULT_MAX_INTERVAL, retry.getMaxInterval()); final var limiter = assertInstanceOf(PeriodRateLimiter.class, config.getRateLimiter()); assertFalse(limiter.isActivated()); @@ -296,7 +296,7 @@ public UpdateControl reconcile(ConfigMap resource, Context } } - @RetryingGradually( + @GradualRetry( maxAttempts = CheckRetryingGraduallyConfiguration.MAX_ATTEMPTS, initialInterval = CheckRetryingGraduallyConfiguration.INITIAL_INTERVAL, intervalMultiplier = CheckRetryingGraduallyConfiguration.INTERVAL_MULTIPLIER, From 4926cd6cba8d5e8dc1f45ed37347e64544086f51 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Mon, 11 Jul 2022 12:41:07 +0200 Subject: [PATCH 15/16] refactor: rename PeriodRateLimiter to LinearRateLimiter --- .../operator/api/config/ControllerConfiguration.java | 4 ++-- .../api/config/DefaultControllerConfiguration.java | 4 ++-- .../api/reconciler/ControllerConfiguration.java | 6 +++--- ...PeriodRateLimiter.java => LinearRateLimiter.java} | 12 ++++++------ .../processing/event/EventProcessorTest.java | 4 ++-- ...teLimiterTest.java => LinearRateLimiterTest.java} | 10 +++++----- .../AnnotationControllerConfigurationTest.java | 6 +++--- 7 files changed, 23 insertions(+), 23 deletions(-) rename operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/{PeriodRateLimiter.java => LinearRateLimiter.java} (87%) rename operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/rate/{PeriodRateLimiterTest.java => LinearRateLimiterTest.java} (85%) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java index 7488f48194..037703c355 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java @@ -8,7 +8,7 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.ReconcilerUtils; import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; -import io.javaoperatorsdk.operator.processing.event.rate.PeriodRateLimiter; +import io.javaoperatorsdk.operator.processing.event.rate.LinearRateLimiter; import io.javaoperatorsdk.operator.processing.event.rate.RateLimiter; import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilter; import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilters; @@ -18,7 +18,7 @@ public interface ControllerConfiguration extends ResourceConfiguration { - RateLimiter DEFAULT_RATE_LIMITER = PeriodRateLimiter.deactivatedRateLimiter(); + RateLimiter DEFAULT_RATE_LIMITER = LinearRateLimiter.deactivatedRateLimiter(); default String getName() { return ReconcilerUtils.getDefaultReconcilerName(getAssociatedReconcilerClassName()); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultControllerConfiguration.java index 29cdce05c9..98cc433f63 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultControllerConfiguration.java @@ -10,7 +10,7 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; -import io.javaoperatorsdk.operator.processing.event.rate.PeriodRateLimiter; +import io.javaoperatorsdk.operator.processing.event.rate.LinearRateLimiter; import io.javaoperatorsdk.operator.processing.event.rate.RateLimiter; import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilter; import io.javaoperatorsdk.operator.processing.retry.Retry; @@ -62,7 +62,7 @@ public DefaultControllerConfiguration( : retry; this.resourceEventFilter = resourceEventFilter; this.rateLimiter = - rateLimiter != null ? rateLimiter : PeriodRateLimiter.deactivatedRateLimiter(); + rateLimiter != null ? rateLimiter : LinearRateLimiter.deactivatedRateLimiter(); this.dependents = dependents != null ? dependents : Collections.emptyList(); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ControllerConfiguration.java index c5490609bb..a21dbd3214 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ControllerConfiguration.java @@ -9,7 +9,7 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent; -import io.javaoperatorsdk.operator.processing.event.rate.PeriodRateLimiter; +import io.javaoperatorsdk.operator.processing.event.rate.LinearRateLimiter; import io.javaoperatorsdk.operator.processing.event.rate.RateLimiter; import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilter; import io.javaoperatorsdk.operator.processing.event.source.filter.VoidGenericFilter; @@ -106,7 +106,7 @@ ReconciliationMaxInterval reconciliationMaxInterval() default @ReconciliationMax /** * Optional {@link Retry} implementation for the associated controller to use. - * + * * @return the class providing the {@link Retry} implementation to use, needs to provide an * accessible no-arg constructor. */ @@ -118,5 +118,5 @@ ReconciliationMaxInterval reconciliationMaxInterval() default @ReconciliationMax * @return the class providing the {@link RateLimiter} implementation to use, needs to provide an * accessible no-arg constructor. */ - Class rateLimiter() default PeriodRateLimiter.class; + Class rateLimiter() default LinearRateLimiter.class; } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/PeriodRateLimiter.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/LinearRateLimiter.java similarity index 87% rename from operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/PeriodRateLimiter.java rename to operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/LinearRateLimiter.java index 7eb3134217..4eb6758874 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/PeriodRateLimiter.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/rate/LinearRateLimiter.java @@ -10,9 +10,9 @@ import io.javaoperatorsdk.operator.processing.event.ResourceID; /** - * A Simple rate limiter that limits the number of permission for a time interval. + * A simple rate limiter that limits the number of permission for a time interval. */ -public class PeriodRateLimiter +public class LinearRateLimiter implements RateLimiter, AnnotationConfigurable { /** To turn off rate limiting set limit for period to a non-positive number */ @@ -23,13 +23,13 @@ public class PeriodRateLimiter private final Map limitData = new HashMap<>(); - public static PeriodRateLimiter deactivatedRateLimiter() { - return new PeriodRateLimiter(); + public static LinearRateLimiter deactivatedRateLimiter() { + return new LinearRateLimiter(); } - PeriodRateLimiter() {} + LinearRateLimiter() {} - public PeriodRateLimiter(Duration refreshPeriod, int limitForPeriod) { + public LinearRateLimiter(Duration refreshPeriod, int limitForPeriod) { this.refreshPeriod = refreshPeriod; this.limitForPeriod = limitForPeriod; } 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 4818e7cc1e..bcb7f3e739 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 @@ -17,7 +17,7 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.api.config.RetryConfiguration; import io.javaoperatorsdk.operator.api.monitoring.Metrics; -import io.javaoperatorsdk.operator.processing.event.rate.PeriodRateLimiter; +import io.javaoperatorsdk.operator.processing.event.rate.LinearRateLimiter; import io.javaoperatorsdk.operator.processing.event.rate.RateLimiter; import io.javaoperatorsdk.operator.processing.event.source.controller.ControllerResourceEventSource; import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceAction; @@ -258,7 +258,7 @@ void startProcessedMarkedEventReceivedBefore() { var crID = new ResourceID("test-cr", TEST_NAMESPACE); eventProcessor = spy(new EventProcessor(reconciliationDispatcherMock, eventSourceManagerMock, "Test", null, - PeriodRateLimiter.deactivatedRateLimiter(), + LinearRateLimiter.deactivatedRateLimiter(), metricsMock)); when(controllerResourceEventSourceMock.get(eq(crID))) .thenReturn(Optional.of(testCustomResource())); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/rate/PeriodRateLimiterTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/rate/LinearRateLimiterTest.java similarity index 85% rename from operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/rate/PeriodRateLimiterTest.java rename to operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/rate/LinearRateLimiterTest.java index 90ad8447cf..a9cd48bef4 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/rate/PeriodRateLimiterTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/rate/LinearRateLimiterTest.java @@ -9,14 +9,14 @@ import static org.assertj.core.api.Assertions.assertThat; -class PeriodRateLimiterTest { +class LinearRateLimiterTest { public static final Duration REFRESH_PERIOD = Duration.ofMillis(300); ResourceID resourceID = ResourceID.fromResource(TestUtils.testCustomResource()); @Test void acquirePermissionForNewResource() { - var rl = new PeriodRateLimiter(REFRESH_PERIOD, 2); + var rl = new LinearRateLimiter(REFRESH_PERIOD, 2); var res = rl.acquirePermission(resourceID); assertThat(res).isEmpty(); res = rl.acquirePermission(resourceID); @@ -28,7 +28,7 @@ void acquirePermissionForNewResource() { @Test void returnsMinimalDurationToAcquirePermission() { - var rl = new PeriodRateLimiter(REFRESH_PERIOD, 1); + var rl = new LinearRateLimiter(REFRESH_PERIOD, 1); var res = rl.acquirePermission(resourceID); assertThat(res).isEmpty(); @@ -40,7 +40,7 @@ void returnsMinimalDurationToAcquirePermission() { @Test void resetsPeriodAfterLimit() throws InterruptedException { - var rl = new PeriodRateLimiter(REFRESH_PERIOD, 1); + var rl = new LinearRateLimiter(REFRESH_PERIOD, 1); var res = rl.acquirePermission(resourceID); assertThat(res).isEmpty(); res = rl.acquirePermission(resourceID); @@ -55,7 +55,7 @@ void resetsPeriodAfterLimit() throws InterruptedException { @Test void rateLimitCanBeTurnedOff() { - var rl = new PeriodRateLimiter(REFRESH_PERIOD, PeriodRateLimiter.NO_LIMIT_PERIOD); + var rl = new LinearRateLimiter(REFRESH_PERIOD, LinearRateLimiter.NO_LIMIT_PERIOD); var res = rl.acquirePermission(resourceID); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/AnnotationControllerConfigurationTest.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/AnnotationControllerConfigurationTest.java index a8e9c8065b..731c7b7cd3 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/AnnotationControllerConfigurationTest.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/AnnotationControllerConfigurationTest.java @@ -25,7 +25,7 @@ import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependent; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResourceConfig; -import io.javaoperatorsdk.operator.processing.event.rate.PeriodRateLimiter; +import io.javaoperatorsdk.operator.processing.event.rate.LinearRateLimiter; import io.javaoperatorsdk.operator.processing.event.rate.RateLimited; import io.javaoperatorsdk.operator.processing.retry.GenericRetry; import io.javaoperatorsdk.operator.processing.retry.GradualRetry; @@ -139,7 +139,7 @@ void checkDefaultRateAndRetryConfigurations() { assertEquals(GradualRetry.DEFAULT_INITIAL_INTERVAL, retry.getInitialInterval()); assertEquals(GradualRetry.DEFAULT_MAX_INTERVAL, retry.getMaxInterval()); - final var limiter = assertInstanceOf(PeriodRateLimiter.class, config.getRateLimiter()); + final var limiter = assertInstanceOf(LinearRateLimiter.class, config.getRateLimiter()); assertFalse(limiter.isActivated()); } @@ -151,7 +151,7 @@ void configuringRateAndRetryViaAnnotationsShouldWork() { final var testRetry = assertInstanceOf(TestRetry.class, retry); assertEquals(12, testRetry.getValue()); - final var rateLimiter = assertInstanceOf(PeriodRateLimiter.class, config.getRateLimiter()); + final var rateLimiter = assertInstanceOf(LinearRateLimiter.class, config.getRateLimiter()); assertEquals(7, rateLimiter.getLimitForPeriod()); assertEquals(Duration.ofSeconds(3), rateLimiter.getRefreshPeriod()); } From 41de6408ed8ca19520d36f2eefa07011ed64e52c Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Mon, 11 Jul 2022 12:46:37 +0200 Subject: [PATCH 16/16] fix: remove duplicated method, add javadoc --- .../api/config/ControllerConfigurationOverrider.java | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java index 9b9f467386..82bd30a371 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java @@ -106,6 +106,9 @@ public ControllerConfigurationOverrider watchingAllNamespaces() { return this; } + /** + * @deprecated Use {@link #withRetry(Retry)} instead + */ @Deprecated(forRemoval = true) public ControllerConfigurationOverrider withRetry(RetryConfiguration retry) { this.retry = GenericRetry.fromConfiguration(retry); @@ -117,12 +120,6 @@ public ControllerConfigurationOverrider withRetry(Retry retry) { return this; } - @Deprecated - public ControllerConfigurationOverrider withRetry(RetryConfiguration retry) { - this.retry = GenericRetry.fromConfiguration(retry); - return this; - } - public ControllerConfigurationOverrider withRateLimiter(RateLimiter rateLimiter) { this.rateLimiter = rateLimiter; return this;