From aa9d3c228cd91bcf1b9aabff59d79161f5c6281e Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 9 Mar 2022 13:41:53 +0100 Subject: [PATCH 01/19] feat: retrieve ConfigurationService from ConfigurationServiceProvider --- .../io/javaoperatorsdk/operator/Operator.java | 8 ++++ .../config/ConfigurationServiceOverrider.java | 26 +++++++----- .../config/ConfigurationServiceProvider.java | 41 +++++++++++++++++++ .../junit/AbstractOperatorExtension.java | 9 ++-- .../runtime/DefaultConfigurationService.java | 8 +++- .../operator/ConcurrencyIT.java | 6 +-- .../operator/ControllerExecutionIT.java | 6 +-- .../CreateUpdateDependentEventFilterIT.java | 2 - .../operator/CustomResourceFilterIT.java | 6 +-- .../operator/ErrorStatusHandlerIT.java | 2 - .../operator/EventSourceIT.java | 6 +-- .../operator/InformerEventSourceIT.java | 2 - .../KubernetesResourceStatusUpdateIT.java | 6 +-- .../operator/MaxIntervalIT.java | 6 +-- .../operator/MultiVersionCRDIT.java | 2 - .../ObservedGenerationHandlingIT.java | 6 +-- .../io/javaoperatorsdk/operator/RetryIT.java | 2 - .../operator/RetryMaxAttemptIT.java | 2 - .../StandaloneDependentResourceIT.java | 6 +-- .../operator/SubResourceUpdateIT.java | 6 +-- .../operator/UpdatingResAndSubResIT.java | 6 +-- .../DefaultConfigurationServiceTest.java | 19 +++++---- .../operator/sample/MySQLSchemaOperator.java | 3 +- .../sample/MySQLSchemaOperatorE2E.java | 6 +-- .../operator/sample/TomcatOperator.java | 3 +- .../operator/sample/TomcatOperatorE2E.java | 3 -- .../operator/sample/WebPageOperator.java | 3 +- .../WebPageOperatorDependentResourcesE2E.java | 3 -- .../operator/sample/WebPageOperatorE2E.java | 3 -- .../sample/PureJavaApplicationRunner.java | 3 +- .../operator/sample/Config.java | 5 ++- 31 files changed, 106 insertions(+), 109 deletions(-) create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProvider.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java index adc3410405..7f51c4d8f7 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java @@ -14,6 +14,7 @@ import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.Version; import io.javaoperatorsdk.operator.api.config.ConfigurationService; +import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.config.ControllerConfigurationOverrider; import io.javaoperatorsdk.operator.api.config.ExecutorServiceManager; @@ -28,6 +29,13 @@ public class Operator implements LifecycleAware { private final ConfigurationService configurationService; private final ControllerManager controllers = new ControllerManager(); + public Operator() { + this(new DefaultKubernetesClient(), ConfigurationServiceProvider.instance()); + } + + public Operator(KubernetesClient kubernetesClient) { + this(kubernetesClient, ConfigurationServiceProvider.instance()); + } public Operator(ConfigurationService configurationService) { this(new DefaultKubernetesClient(), configurationService); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java index e2251eea64..b5e26d7cad 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java @@ -19,8 +19,7 @@ public class ConfigurationServiceOverrider { private boolean closeClientOnStop; private ExecutorService executorService = null; - public ConfigurationServiceOverrider( - ConfigurationService original) { + ConfigurationServiceOverrider(ConfigurationService original) { this.original = original; this.clientConfig = original.getClientConfiguration(); this.checkCR = original.checkCRDAndValidateLocalModel(); @@ -73,12 +72,11 @@ public ConfigurationServiceOverrider withExecutorService(ExecutorService executo } public ConfigurationService build() { - return new ConfigurationService() { + final var overridden = new BaseConfigurationService(original.getVersion()) { @Override public ControllerConfiguration getConfigurationFor( Reconciler reconciler) { - ControllerConfiguration controllerConfiguration = - original.getConfigurationFor(reconciler); + final var controllerConfiguration = original.getConfigurationFor(reconciler); controllerConfiguration.setConfigurationService(this); return controllerConfiguration; } @@ -88,11 +86,6 @@ public Set getKnownReconcilerNames() { return original.getKnownReconcilerNames(); } - @Override - public Version getVersion() { - return original.getVersion(); - } - @Override public Config getClientConfiguration() { return clientConfig; @@ -133,12 +126,23 @@ public ExecutorService getExecutorService() { if (executorService != null) { return executorService; } else { - return ConfigurationService.super.getExecutorService(); + return super.getExecutorService(); } } }; + // make sure we set the overridden version on the provider so that it is made available + ConfigurationServiceProvider.set(overridden, true); + return overridden; + } + + public static ConfigurationServiceOverrider overrideCurrent() { + return new ConfigurationServiceOverrider(ConfigurationServiceProvider.instance()); } + /** + * @deprecated Use {@link #overrideCurrent()} instead + */ + @Deprecated(since = "2.2.0") public static ConfigurationServiceOverrider override(ConfigurationService original) { return new ConfigurationServiceOverrider(original); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProvider.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProvider.java new file mode 100644 index 0000000000..9542e491ee --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProvider.java @@ -0,0 +1,41 @@ +package io.javaoperatorsdk.operator.api.config; + +public class ConfigurationServiceProvider { + static ConfigurationService instance; + private static ConfigurationService defaultConfigurationService; + private static boolean alreadyConfigured = false; + + public static ConfigurationService instance() { + if (instance == null) { + if (defaultConfigurationService == null) { + throw new IllegalStateException( + "A ConfigurationService needs to be configured using the `set` method first"); + } + set(defaultConfigurationService); + } + return instance; + } + + public static void set(ConfigurationService instance) { + set(instance, false); + } + + static void set(ConfigurationService instance, boolean overriding) { + if ((overriding && alreadyConfigured) || + (ConfigurationServiceProvider.instance != null + && !ConfigurationServiceProvider.instance.equals(instance))) { + throw new IllegalStateException( + "A ConfigurationService has already been set and cannot be set again. Current: " + + ConfigurationServiceProvider.instance.getClass().getCanonicalName()); + } + + if (overriding) { + alreadyConfigured = true; + } + ConfigurationServiceProvider.instance = instance; + } + + public static void setDefault(ConfigurationService defaultConfigurationService) { + ConfigurationServiceProvider.defaultConfigurationService = defaultConfigurationService; + } +} diff --git a/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/AbstractOperatorExtension.java b/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/AbstractOperatorExtension.java index 839b550231..7b1c550ac5 100644 --- a/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/AbstractOperatorExtension.java +++ b/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/AbstractOperatorExtension.java @@ -22,9 +22,8 @@ import io.fabric8.kubernetes.client.dsl.Resource; import io.fabric8.kubernetes.client.utils.KubernetesResourceUtil; import io.fabric8.kubernetes.client.utils.Utils; -import io.javaoperatorsdk.operator.api.config.BaseConfigurationService; import io.javaoperatorsdk.operator.api.config.ConfigurationService; -import io.javaoperatorsdk.operator.api.config.Version; +import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; public abstract class AbstractOperatorExtension implements HasKubernetesClient, BeforeAllCallback, @@ -200,7 +199,7 @@ public static abstract class AbstractBuilder> { protected boolean oneNamespacePerClass; protected AbstractBuilder() { - this.configurationService = new BaseConfigurationService(Version.UNKNOWN); + this.configurationService = ConfigurationServiceProvider.instance(); this.infrastructure = new ArrayList<>(); this.infrastructureTimeout = Duration.ofMinutes(1); @@ -233,6 +232,10 @@ public T oneNamespacePerClass(boolean value) { return (T) this; } + /** + * @deprecated There is no need to set the ConfigurationService manually anymore + */ + @Deprecated public T withConfigurationService(ConfigurationService value) { configurationService = value; return (T) this; diff --git a/operator-framework/src/main/java/io/javaoperatorsdk/operator/config/runtime/DefaultConfigurationService.java b/operator-framework/src/main/java/io/javaoperatorsdk/operator/config/runtime/DefaultConfigurationService.java index d34d6fee55..34e0b3f1d7 100644 --- a/operator-framework/src/main/java/io/javaoperatorsdk/operator/config/runtime/DefaultConfigurationService.java +++ b/operator-framework/src/main/java/io/javaoperatorsdk/operator/config/runtime/DefaultConfigurationService.java @@ -2,6 +2,7 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.api.config.BaseConfigurationService; +import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; @@ -10,11 +11,16 @@ public class DefaultConfigurationService extends BaseConfigurationService { private static final DefaultConfigurationService instance = new DefaultConfigurationService(); private boolean createIfNeeded = super.createIfNeeded(); + static { + // register this as the default configuration service + ConfigurationServiceProvider.setDefault(instance); + } + private DefaultConfigurationService() { super(); } - public static DefaultConfigurationService instance() { + static DefaultConfigurationService instance() { return instance; } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/ConcurrencyIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/ConcurrencyIT.java index 12cf3ca4d8..a9fe44191e 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/ConcurrencyIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/ConcurrencyIT.java @@ -10,7 +10,6 @@ import org.slf4j.LoggerFactory; import io.fabric8.kubernetes.api.model.ConfigMap; -import io.javaoperatorsdk.operator.config.runtime.DefaultConfigurationService; import io.javaoperatorsdk.operator.junit.OperatorExtension; import io.javaoperatorsdk.operator.sample.simple.TestCustomResource; import io.javaoperatorsdk.operator.sample.simple.TestReconciler; @@ -28,10 +27,7 @@ class ConcurrencyIT { @RegisterExtension OperatorExtension operator = - OperatorExtension.builder() - .withConfigurationService(DefaultConfigurationService.instance()) - .withReconciler(new TestReconciler(true)) - .build(); + OperatorExtension.builder().withReconciler(new TestReconciler(true)).build(); @Test void manyResourcesGetCreatedUpdatedAndDeleted() throws InterruptedException { diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/ControllerExecutionIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/ControllerExecutionIT.java index dbd015587e..0a3ed114a1 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/ControllerExecutionIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/ControllerExecutionIT.java @@ -6,7 +6,6 @@ import org.junit.jupiter.api.extension.RegisterExtension; import io.fabric8.kubernetes.api.model.ConfigMap; -import io.javaoperatorsdk.operator.config.runtime.DefaultConfigurationService; import io.javaoperatorsdk.operator.junit.OperatorExtension; import io.javaoperatorsdk.operator.sample.simple.TestCustomResource; import io.javaoperatorsdk.operator.sample.simple.TestReconciler; @@ -18,10 +17,7 @@ class ControllerExecutionIT { @RegisterExtension OperatorExtension operator = - OperatorExtension.builder() - .withConfigurationService(DefaultConfigurationService.instance()) - .withReconciler(new TestReconciler(true)) - .build(); + OperatorExtension.builder().withReconciler(new TestReconciler(true)).build(); @Test void configMapGetsCreatedForTestCustomResource() { diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/CreateUpdateDependentEventFilterIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/CreateUpdateDependentEventFilterIT.java index 57fbc78838..58dc0a541d 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/CreateUpdateDependentEventFilterIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/CreateUpdateDependentEventFilterIT.java @@ -7,7 +7,6 @@ import io.fabric8.kubernetes.api.model.ConfigMap; import io.fabric8.kubernetes.api.model.ObjectMeta; -import io.javaoperatorsdk.operator.config.runtime.DefaultConfigurationService; import io.javaoperatorsdk.operator.junit.OperatorExtension; import io.javaoperatorsdk.operator.sample.createupdateeventfilter.CreateUpdateEventFilterTestCustomResource; import io.javaoperatorsdk.operator.sample.createupdateeventfilter.CreateUpdateEventFilterTestCustomResourceSpec; @@ -22,7 +21,6 @@ public class CreateUpdateDependentEventFilterIT { @RegisterExtension OperatorExtension operator = OperatorExtension.builder() - .withConfigurationService(DefaultConfigurationService.instance()) .withReconciler(new CreateUpdateEventFilterTestReconciler()) .build(); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/CustomResourceFilterIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/CustomResourceFilterIT.java index 70a3f04777..45387d2b97 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/CustomResourceFilterIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/CustomResourceFilterIT.java @@ -4,7 +4,6 @@ import org.junit.jupiter.api.extension.RegisterExtension; import io.fabric8.kubernetes.api.model.ObjectMeta; -import io.javaoperatorsdk.operator.config.runtime.DefaultConfigurationService; import io.javaoperatorsdk.operator.junit.OperatorExtension; import io.javaoperatorsdk.operator.sample.customfilter.CustomFilteringTestReconciler; import io.javaoperatorsdk.operator.sample.customfilter.CustomFilteringTestResource; @@ -16,10 +15,7 @@ class CustomResourceFilterIT { @RegisterExtension OperatorExtension operator = - OperatorExtension.builder() - .withConfigurationService(DefaultConfigurationService.instance()) - .withReconciler(new CustomFilteringTestReconciler()) - .build(); + OperatorExtension.builder().withReconciler(new CustomFilteringTestReconciler()).build(); @Test void doesCustomFiltering() throws InterruptedException { diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/ErrorStatusHandlerIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/ErrorStatusHandlerIT.java index fbf54a1ad1..8b0e7bab11 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/ErrorStatusHandlerIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/ErrorStatusHandlerIT.java @@ -6,7 +6,6 @@ import org.junit.jupiter.api.extension.RegisterExtension; import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; -import io.javaoperatorsdk.operator.config.runtime.DefaultConfigurationService; import io.javaoperatorsdk.operator.junit.OperatorExtension; import io.javaoperatorsdk.operator.processing.retry.GenericRetry; import io.javaoperatorsdk.operator.sample.errorstatushandler.ErrorStatusHandlerTestCustomResource; @@ -23,7 +22,6 @@ class ErrorStatusHandlerIT { @RegisterExtension OperatorExtension operator = OperatorExtension.builder() - .withConfigurationService(DefaultConfigurationService.instance()) .withReconciler(reconciler, new GenericRetry().setMaxAttempts(MAX_RETRY_ATTEMPTS).withLinearRetry()) .build(); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/EventSourceIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/EventSourceIT.java index c91ebaad43..eea996cb3b 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/EventSourceIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/EventSourceIT.java @@ -6,7 +6,6 @@ import org.junit.jupiter.api.extension.RegisterExtension; import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; -import io.javaoperatorsdk.operator.config.runtime.DefaultConfigurationService; import io.javaoperatorsdk.operator.junit.OperatorExtension; import io.javaoperatorsdk.operator.sample.event.EventSourceTestCustomReconciler; import io.javaoperatorsdk.operator.sample.event.EventSourceTestCustomResource; @@ -19,10 +18,7 @@ class EventSourceIT { @RegisterExtension OperatorExtension operator = - OperatorExtension.builder() - .withConfigurationService(DefaultConfigurationService.instance()) - .withReconciler(EventSourceTestCustomReconciler.class) - .build(); + OperatorExtension.builder().withReconciler(EventSourceTestCustomReconciler.class).build(); @Test void receivingPeriodicEvents() { diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/InformerEventSourceIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/InformerEventSourceIT.java index 7ff36bd375..ddd4113c22 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/InformerEventSourceIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/InformerEventSourceIT.java @@ -8,7 +8,6 @@ import io.fabric8.kubernetes.api.model.ConfigMap; import io.fabric8.kubernetes.api.model.ObjectMeta; -import io.javaoperatorsdk.operator.config.runtime.DefaultConfigurationService; import io.javaoperatorsdk.operator.junit.OperatorExtension; import io.javaoperatorsdk.operator.sample.informereventsource.InformerEventSourceTestCustomReconciler; import io.javaoperatorsdk.operator.sample.informereventsource.InformerEventSourceTestCustomResource; @@ -27,7 +26,6 @@ class InformerEventSourceIT { @RegisterExtension OperatorExtension operator = OperatorExtension.builder() - .withConfigurationService(DefaultConfigurationService.instance()) .withReconciler(new InformerEventSourceTestCustomReconciler()) .build(); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/KubernetesResourceStatusUpdateIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/KubernetesResourceStatusUpdateIT.java index a2c7378ff5..603dc2ab1a 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/KubernetesResourceStatusUpdateIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/KubernetesResourceStatusUpdateIT.java @@ -11,7 +11,6 @@ import io.fabric8.kubernetes.api.model.*; import io.fabric8.kubernetes.api.model.apps.Deployment; import io.fabric8.kubernetes.api.model.apps.DeploymentSpec; -import io.javaoperatorsdk.operator.config.runtime.DefaultConfigurationService; import io.javaoperatorsdk.operator.junit.OperatorExtension; import io.javaoperatorsdk.operator.sample.deployment.DeploymentReconciler; @@ -23,10 +22,7 @@ class KubernetesResourceStatusUpdateIT { @RegisterExtension OperatorExtension operator = - OperatorExtension.builder() - .withConfigurationService(DefaultConfigurationService.instance()) - .withReconciler(new DeploymentReconciler()) - .build(); + OperatorExtension.builder().withReconciler(new DeploymentReconciler()).build(); @Test void testReconciliationOfNonCustomResourceAndStatusUpdate() { diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/MaxIntervalIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/MaxIntervalIT.java index f284dbf407..69585340db 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/MaxIntervalIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/MaxIntervalIT.java @@ -6,7 +6,6 @@ import org.junit.jupiter.api.extension.RegisterExtension; import io.fabric8.kubernetes.api.model.ObjectMeta; -import io.javaoperatorsdk.operator.config.runtime.DefaultConfigurationService; import io.javaoperatorsdk.operator.junit.OperatorExtension; import io.javaoperatorsdk.operator.sample.maxinterval.MaxIntervalTestCustomResource; import io.javaoperatorsdk.operator.sample.maxinterval.MaxIntervalTestReconciler; @@ -17,10 +16,7 @@ class MaxIntervalIT { @RegisterExtension OperatorExtension operator = - OperatorExtension.builder() - .withConfigurationService(DefaultConfigurationService.instance()) - .withReconciler(new MaxIntervalTestReconciler()) - .build(); + OperatorExtension.builder().withReconciler(new MaxIntervalTestReconciler()).build(); @Test void reconciliationTriggeredBasedOnMaxInterval() { diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/MultiVersionCRDIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/MultiVersionCRDIT.java index 930fc210b3..a6cf2d5591 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/MultiVersionCRDIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/MultiVersionCRDIT.java @@ -7,7 +7,6 @@ import org.junit.jupiter.api.extension.RegisterExtension; import io.fabric8.kubernetes.api.model.ObjectMeta; -import io.javaoperatorsdk.operator.config.runtime.DefaultConfigurationService; import io.javaoperatorsdk.operator.junit.OperatorExtension; import io.javaoperatorsdk.operator.sample.multiversioncrd.*; @@ -22,7 +21,6 @@ class MultiVersionCRDIT { @RegisterExtension OperatorExtension operator = OperatorExtension.builder() - .withConfigurationService(DefaultConfigurationService.instance()) .withReconciler(MultiVersionCRDTestReconciler1.class) .withReconciler(MultiVersionCRDTestReconciler2.class) .build(); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/ObservedGenerationHandlingIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/ObservedGenerationHandlingIT.java index 5f86336a1c..4c57a977e3 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/ObservedGenerationHandlingIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/ObservedGenerationHandlingIT.java @@ -6,7 +6,6 @@ import org.junit.jupiter.api.extension.RegisterExtension; import io.fabric8.kubernetes.api.model.ObjectMeta; -import io.javaoperatorsdk.operator.config.runtime.DefaultConfigurationService; import io.javaoperatorsdk.operator.junit.OperatorExtension; import io.javaoperatorsdk.operator.sample.observedgeneration.ObservedGenerationTestCustomResource; import io.javaoperatorsdk.operator.sample.observedgeneration.ObservedGenerationTestReconciler; @@ -17,10 +16,7 @@ class ObservedGenerationHandlingIT { @RegisterExtension OperatorExtension operator = - OperatorExtension.builder() - .withConfigurationService(DefaultConfigurationService.instance()) - .withReconciler(new ObservedGenerationTestReconciler()) - .build(); + OperatorExtension.builder().withReconciler(new ObservedGenerationTestReconciler()).build(); @Test void testReconciliationOfNonCustomResourceAndStatusUpdate() { diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/RetryIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/RetryIT.java index c4ab025ac5..02b7b7af9e 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/RetryIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/RetryIT.java @@ -6,7 +6,6 @@ import org.junit.jupiter.api.extension.RegisterExtension; import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; -import io.javaoperatorsdk.operator.config.runtime.DefaultConfigurationService; import io.javaoperatorsdk.operator.junit.OperatorExtension; import io.javaoperatorsdk.operator.processing.retry.GenericRetry; import io.javaoperatorsdk.operator.sample.retry.RetryTestCustomReconciler; @@ -27,7 +26,6 @@ class RetryIT { @RegisterExtension OperatorExtension operator = OperatorExtension.builder() - .withConfigurationService(DefaultConfigurationService.instance()) .withReconciler( new RetryTestCustomReconciler(NUMBER_FAILED_EXECUTIONS), new GenericRetry().setInitialInterval(RETRY_INTERVAL).withLinearRetry() diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/RetryMaxAttemptIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/RetryMaxAttemptIT.java index 5ebb05eb0c..56a36cf6f9 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/RetryMaxAttemptIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/RetryMaxAttemptIT.java @@ -3,7 +3,6 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; -import io.javaoperatorsdk.operator.config.runtime.DefaultConfigurationService; import io.javaoperatorsdk.operator.junit.OperatorExtension; import io.javaoperatorsdk.operator.processing.retry.GenericRetry; import io.javaoperatorsdk.operator.sample.retry.RetryTestCustomReconciler; @@ -23,7 +22,6 @@ class RetryMaxAttemptIT { @RegisterExtension OperatorExtension operator = OperatorExtension.builder() - .withConfigurationService(DefaultConfigurationService.instance()) .withReconciler(reconciler, new GenericRetry().setInitialInterval(RETRY_INTERVAL).withLinearRetry() .setMaxAttempts(MAX_RETRY_ATTEMPTS)) diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneDependentResourceIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneDependentResourceIT.java index b7d88159e5..1d5f7e9d12 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneDependentResourceIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneDependentResourceIT.java @@ -8,7 +8,6 @@ import io.fabric8.kubernetes.api.model.ObjectMeta; import io.fabric8.kubernetes.api.model.apps.Deployment; import io.javaoperatorsdk.operator.api.config.ConfigurationService; -import io.javaoperatorsdk.operator.config.runtime.DefaultConfigurationService; import io.javaoperatorsdk.operator.junit.OperatorExtension; import io.javaoperatorsdk.operator.sample.standalonedependent.StandaloneDependentTestCustomResource; import io.javaoperatorsdk.operator.sample.standalonedependent.StandaloneDependentTestCustomResourceSpec; @@ -23,10 +22,7 @@ class StandaloneDependentResourceIT { @RegisterExtension OperatorExtension operator = - OperatorExtension.builder() - .withConfigurationService(DefaultConfigurationService.instance()) - .withReconciler(new StandaloneDependentTestReconciler()) - .build(); + OperatorExtension.builder().withReconciler(new StandaloneDependentTestReconciler()).build(); @Test void dependentResourceManagesDeployment() { diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/SubResourceUpdateIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/SubResourceUpdateIT.java index 75597643bf..06728063d4 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/SubResourceUpdateIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/SubResourceUpdateIT.java @@ -7,7 +7,6 @@ import org.junit.jupiter.api.extension.RegisterExtension; import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; -import io.javaoperatorsdk.operator.config.runtime.DefaultConfigurationService; import io.javaoperatorsdk.operator.junit.OperatorExtension; import io.javaoperatorsdk.operator.sample.subresource.SubResourceTestCustomReconciler; import io.javaoperatorsdk.operator.sample.subresource.SubResourceTestCustomResource; @@ -22,10 +21,7 @@ class SubResourceUpdateIT { @RegisterExtension OperatorExtension operator = - OperatorExtension.builder() - .withConfigurationService(DefaultConfigurationService.instance()) - .withReconciler(SubResourceTestCustomReconciler.class) - .build(); + OperatorExtension.builder().withReconciler(SubResourceTestCustomReconciler.class).build(); @Test void updatesSubResourceStatus() { diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/UpdatingResAndSubResIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/UpdatingResAndSubResIT.java index 6bad954400..d86ad51d60 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/UpdatingResAndSubResIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/UpdatingResAndSubResIT.java @@ -6,7 +6,6 @@ import org.junit.jupiter.api.extension.RegisterExtension; import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; -import io.javaoperatorsdk.operator.config.runtime.DefaultConfigurationService; import io.javaoperatorsdk.operator.junit.OperatorExtension; import io.javaoperatorsdk.operator.sample.doubleupdate.DoubleUpdateTestCustomReconciler; import io.javaoperatorsdk.operator.sample.doubleupdate.DoubleUpdateTestCustomResource; @@ -20,10 +19,7 @@ class UpdatingResAndSubResIT { @RegisterExtension OperatorExtension operator = - OperatorExtension.builder() - .withConfigurationService(DefaultConfigurationService.instance()) - .withReconciler(DoubleUpdateTestCustomReconciler.class) - .build(); + OperatorExtension.builder().withReconciler(DoubleUpdateTestCustomReconciler.class).build(); @Test void updatesSubResourceStatus() { diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/DefaultConfigurationServiceTest.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/DefaultConfigurationServiceTest.java index bce957a399..a99c3d5020 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/DefaultConfigurationServiceTest.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/DefaultConfigurationServiceTest.java @@ -12,6 +12,7 @@ import io.fabric8.kubernetes.model.annotation.Group; import io.fabric8.kubernetes.model.annotation.Version; import io.javaoperatorsdk.operator.ReconcilerUtils; +import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; @@ -25,11 +26,15 @@ class DefaultConfigurationServiceTest { public static final String CUSTOM_FINALIZER_NAME = "a.custom/finalizer"; + final DefaultConfigurationService configurationService = DefaultConfigurationService.instance(); @Test - void attemptingToRetrieveAnUnknownControllerShouldLogWarning() { - final var configurationService = DefaultConfigurationService.instance(); + void defaultConfigurationServiceIsSetByDefault() { + assertEquals(DefaultConfigurationService.instance(), ConfigurationServiceProvider.instance()); + } + @Test + void attemptingToRetrieveAnUnknownControllerShouldLogWarning() { final LoggerContext context = LoggerContext.getContext(false); final PatternLayout layout = PatternLayout.createDefaultLayout(context.getConfiguration()); final ListAppender appender = new ListAppender("list", null, layout, false, false); @@ -75,8 +80,7 @@ void attemptingToRetrieveAnUnknownControllerShouldLogWarning() { @Test void returnsValuesFromControllerAnnotationFinalizer() { final var reconciler = new TestCustomReconciler(); - final var configuration = - DefaultConfigurationService.instance().getConfigurationFor(reconciler); + final var configuration = configurationService.getConfigurationFor(reconciler); assertEquals(CustomResource.getCRDName(TestCustomResource.class), configuration.getResourceTypeName()); assertEquals( @@ -89,8 +93,7 @@ void returnsValuesFromControllerAnnotationFinalizer() { @Test void returnCustomerFinalizerNameIfSet() { final var reconciler = new TestCustomFinalizerReconciler(); - final var configuration = - DefaultConfigurationService.instance().getConfigurationFor(reconciler); + final var configuration = configurationService.getConfigurationFor(reconciler); assertEquals(CUSTOM_FINALIZER_NAME, configuration.getFinalizer()); } @@ -99,9 +102,7 @@ void supportsInnerClassCustomResources() { final var reconciler = new TestCustomFinalizerReconciler(); assertDoesNotThrow( () -> { - DefaultConfigurationService.instance() - .getConfigurationFor(reconciler) - .getAssociatedReconcilerClassName(); + configurationService.getConfigurationFor(reconciler).getAssociatedReconcilerClassName(); }); } diff --git a/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperator.java b/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperator.java index 8abf73f3f6..3b6078a953 100644 --- a/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperator.java +++ b/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperator.java @@ -15,7 +15,6 @@ import io.fabric8.kubernetes.client.KubernetesClient; import io.javaoperatorsdk.operator.Operator; import io.javaoperatorsdk.operator.api.config.ConfigurationServiceOverrider; -import io.javaoperatorsdk.operator.config.runtime.DefaultConfigurationService; import io.javaoperatorsdk.operator.monitoring.micrometer.MicrometerMetrics; import io.javaoperatorsdk.operator.sample.dependent.ResourcePollerConfig; import io.javaoperatorsdk.operator.sample.dependent.SchemaDependentResource; @@ -31,7 +30,7 @@ public static void main(String[] args) throws IOException { Config config = new ConfigBuilder().withNamespace(null).build(); KubernetesClient client = new DefaultKubernetesClient(config); Operator operator = new Operator(client, - new ConfigurationServiceOverrider(DefaultConfigurationService.instance()) + ConfigurationServiceOverrider.overrideCurrent() .withMetrics(new MicrometerMetrics(new LoggingMeterRegistry())) .build()); diff --git a/sample-operators/mysql-schema/src/test/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperatorE2E.java b/sample-operators/mysql-schema/src/test/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperatorE2E.java index ed0ff40489..b8e8b0566a 100644 --- a/sample-operators/mysql-schema/src/test/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperatorE2E.java +++ b/sample-operators/mysql-schema/src/test/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperatorE2E.java @@ -17,7 +17,6 @@ import io.fabric8.kubernetes.client.DefaultKubernetesClient; import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.LocalPortForward; -import io.javaoperatorsdk.operator.config.runtime.DefaultConfigurationService; import io.javaoperatorsdk.operator.junit.AbstractOperatorExtension; import io.javaoperatorsdk.operator.junit.E2EOperatorExtension; import io.javaoperatorsdk.operator.junit.OperatorExtension; @@ -39,7 +38,7 @@ class MySQLSchemaOperatorE2E { static final String MY_SQL_NS = "mysql"; - private static List infrastructure = new ArrayList<>(); + private final static List infrastructure = new ArrayList<>(); static { infrastructure.add( @@ -60,10 +59,10 @@ boolean isLocal() { } @RegisterExtension + @SuppressWarnings("unchecked") AbstractOperatorExtension operator = isLocal() ? OperatorExtension.builder() - .withConfigurationService(DefaultConfigurationService.instance()) .withReconciler( new MySQLSchemaReconciler(), c -> { @@ -75,7 +74,6 @@ boolean isLocal() { .withInfrastructure(infrastructure) .build() : E2EOperatorExtension.builder() - .withConfigurationService(DefaultConfigurationService.instance()) .withOperatorDeployment(client.load(new FileInputStream("k8s/operator.yaml")).get()) .withInfrastructure(infrastructure) .build(); diff --git a/sample-operators/tomcat-operator/src/main/java/io/javaoperatorsdk/operator/sample/TomcatOperator.java b/sample-operators/tomcat-operator/src/main/java/io/javaoperatorsdk/operator/sample/TomcatOperator.java index 09a7394e5b..6b894a3348 100644 --- a/sample-operators/tomcat-operator/src/main/java/io/javaoperatorsdk/operator/sample/TomcatOperator.java +++ b/sample-operators/tomcat-operator/src/main/java/io/javaoperatorsdk/operator/sample/TomcatOperator.java @@ -14,7 +14,6 @@ import io.fabric8.kubernetes.client.DefaultKubernetesClient; import io.fabric8.kubernetes.client.KubernetesClient; import io.javaoperatorsdk.operator.Operator; -import io.javaoperatorsdk.operator.config.runtime.DefaultConfigurationService; public class TomcatOperator { @@ -24,7 +23,7 @@ public static void main(String[] args) throws IOException { Config config = new ConfigBuilder().withNamespace(null).build(); KubernetesClient client = new DefaultKubernetesClient(config); - Operator operator = new Operator(client, DefaultConfigurationService.instance()); + Operator operator = new Operator(client); operator.register(new TomcatReconciler()); operator.register(new WebappReconciler(client)); operator.installShutdownHook(); diff --git a/sample-operators/tomcat-operator/src/test/java/io/javaoperatorsdk/operator/sample/TomcatOperatorE2E.java b/sample-operators/tomcat-operator/src/test/java/io/javaoperatorsdk/operator/sample/TomcatOperatorE2E.java index dabcbdfa4b..2f37638655 100644 --- a/sample-operators/tomcat-operator/src/test/java/io/javaoperatorsdk/operator/sample/TomcatOperatorE2E.java +++ b/sample-operators/tomcat-operator/src/test/java/io/javaoperatorsdk/operator/sample/TomcatOperatorE2E.java @@ -10,7 +10,6 @@ import io.fabric8.kubernetes.api.model.*; import io.fabric8.kubernetes.client.*; -import io.javaoperatorsdk.operator.config.runtime.DefaultConfigurationService; import io.javaoperatorsdk.operator.junit.AbstractOperatorExtension; import io.javaoperatorsdk.operator.junit.E2EOperatorExtension; import io.javaoperatorsdk.operator.junit.InClusterCurl; @@ -43,13 +42,11 @@ boolean isLocal() { @RegisterExtension AbstractOperatorExtension operator = isLocal() ? OperatorExtension.builder() .waitForNamespaceDeletion(false) - .withConfigurationService(DefaultConfigurationService.instance()) .withReconciler(new TomcatReconciler()) .withReconciler(new WebappReconciler(client)) .build() : E2EOperatorExtension.builder() .waitForNamespaceDeletion(false) - .withConfigurationService(DefaultConfigurationService.instance()) .withOperatorDeployment( client.load(new FileInputStream("k8s/operator.yaml")).get()) .build(); diff --git a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageOperator.java b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageOperator.java index 0b643462b1..0277da8199 100644 --- a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageOperator.java +++ b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageOperator.java @@ -14,7 +14,6 @@ import io.fabric8.kubernetes.client.DefaultKubernetesClient; import io.fabric8.kubernetes.client.KubernetesClient; import io.javaoperatorsdk.operator.Operator; -import io.javaoperatorsdk.operator.config.runtime.DefaultConfigurationService; public class WebPageOperator { public static final String WEBPAGE_RECONCILER_ENV = "WEBPAGE_RECONCILER"; @@ -27,7 +26,7 @@ public static void main(String[] args) throws IOException { Config config = new ConfigBuilder().withNamespace(null).build(); KubernetesClient client = new DefaultKubernetesClient(config); - Operator operator = new Operator(client, DefaultConfigurationService.instance()); + Operator operator = new Operator(client); if (WEBPAGE_RECONCILER_ENV_VALUE.equals(System.getenv(WEBPAGE_RECONCILER_ENV))) { operator.register(new WebPageReconciler(client)); } else { diff --git a/sample-operators/webpage/src/test/java/io/javaoperatorsdk/operator/sample/WebPageOperatorDependentResourcesE2E.java b/sample-operators/webpage/src/test/java/io/javaoperatorsdk/operator/sample/WebPageOperatorDependentResourcesE2E.java index 580a2022c7..3bfdd3f45b 100644 --- a/sample-operators/webpage/src/test/java/io/javaoperatorsdk/operator/sample/WebPageOperatorDependentResourcesE2E.java +++ b/sample-operators/webpage/src/test/java/io/javaoperatorsdk/operator/sample/WebPageOperatorDependentResourcesE2E.java @@ -5,7 +5,6 @@ import org.junit.jupiter.api.extension.RegisterExtension; -import io.javaoperatorsdk.operator.config.runtime.DefaultConfigurationService; import io.javaoperatorsdk.operator.junit.AbstractOperatorExtension; import io.javaoperatorsdk.operator.junit.E2EOperatorExtension; import io.javaoperatorsdk.operator.junit.OperatorExtension; @@ -19,12 +18,10 @@ public WebPageOperatorDependentResourcesE2E() throws FileNotFoundException {} isLocal() ? OperatorExtension.builder() .waitForNamespaceDeletion(false) - .withConfigurationService(DefaultConfigurationService.instance()) .withReconciler(new WebPageReconcilerDependentResources(client)) .build() : E2EOperatorExtension.builder() .waitForNamespaceDeletion(false) - .withConfigurationService(DefaultConfigurationService.instance()) .withOperatorDeployment(client.load(new FileInputStream("k8s/operator.yaml")).get()) .build(); diff --git a/sample-operators/webpage/src/test/java/io/javaoperatorsdk/operator/sample/WebPageOperatorE2E.java b/sample-operators/webpage/src/test/java/io/javaoperatorsdk/operator/sample/WebPageOperatorE2E.java index a096b03965..b7c8dae6af 100644 --- a/sample-operators/webpage/src/test/java/io/javaoperatorsdk/operator/sample/WebPageOperatorE2E.java +++ b/sample-operators/webpage/src/test/java/io/javaoperatorsdk/operator/sample/WebPageOperatorE2E.java @@ -9,7 +9,6 @@ import io.fabric8.kubernetes.api.model.Container; import io.fabric8.kubernetes.api.model.EnvVar; import io.fabric8.kubernetes.api.model.apps.Deployment; -import io.javaoperatorsdk.operator.config.runtime.DefaultConfigurationService; import io.javaoperatorsdk.operator.junit.AbstractOperatorExtension; import io.javaoperatorsdk.operator.junit.E2EOperatorExtension; import io.javaoperatorsdk.operator.junit.OperatorExtension; @@ -27,12 +26,10 @@ public WebPageOperatorE2E() throws FileNotFoundException {} isLocal() ? OperatorExtension.builder() .waitForNamespaceDeletion(false) - .withConfigurationService(DefaultConfigurationService.instance()) .withReconciler(new WebPageReconciler(client)) .build() : E2EOperatorExtension.builder() .waitForNamespaceDeletion(false) - .withConfigurationService(DefaultConfigurationService.instance()) .withOperatorDeployment(client.load(new FileInputStream("k8s/operator.yaml")).get(), resources -> { Deployment deployment = (Deployment) resources.stream() diff --git a/smoke-test-samples/pure-java/src/main/java/io/javaoperatorsdk/operator/sample/PureJavaApplicationRunner.java b/smoke-test-samples/pure-java/src/main/java/io/javaoperatorsdk/operator/sample/PureJavaApplicationRunner.java index 19d0a471b7..e80e13151c 100644 --- a/smoke-test-samples/pure-java/src/main/java/io/javaoperatorsdk/operator/sample/PureJavaApplicationRunner.java +++ b/smoke-test-samples/pure-java/src/main/java/io/javaoperatorsdk/operator/sample/PureJavaApplicationRunner.java @@ -4,14 +4,13 @@ import io.javaoperatorsdk.operator.Operator; import io.javaoperatorsdk.operator.api.config.ConfigurationServiceOverrider; -import io.javaoperatorsdk.operator.config.runtime.DefaultConfigurationService; public class PureJavaApplicationRunner { public static void main(String[] args) { Operator operator = new Operator( - ConfigurationServiceOverrider.override(DefaultConfigurationService.instance()) + ConfigurationServiceOverrider.overrideCurrent() .withExecutorService(Executors.newCachedThreadPool()) .withConcurrentReconciliationThreads(2) .build()); diff --git a/smoke-test-samples/spring-boot-plain/src/main/java/io/javaoperatorsdk/operator/sample/Config.java b/smoke-test-samples/spring-boot-plain/src/main/java/io/javaoperatorsdk/operator/sample/Config.java index 07dd4b50c0..9b07d4d61d 100644 --- a/smoke-test-samples/spring-boot-plain/src/main/java/io/javaoperatorsdk/operator/sample/Config.java +++ b/smoke-test-samples/spring-boot-plain/src/main/java/io/javaoperatorsdk/operator/sample/Config.java @@ -6,8 +6,8 @@ import org.springframework.context.annotation.Configuration; import io.javaoperatorsdk.operator.Operator; +import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; -import io.javaoperatorsdk.operator.config.runtime.DefaultConfigurationService; @Configuration public class Config { @@ -19,8 +19,9 @@ public CustomServiceReconciler customServiceController() { // Register all controller beans @Bean(initMethod = "start", destroyMethod = "stop") + @SuppressWarnings("rawtypes") public Operator operator(List controllers) { - Operator operator = new Operator(DefaultConfigurationService.instance()); + Operator operator = new Operator(ConfigurationServiceProvider.instance()); controllers.forEach(operator::register); return operator; } From c0a80645e7835cd0f75a653a90d450478dd21e72 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 9 Mar 2022 16:55:26 +0100 Subject: [PATCH 02/19] feat: make it possible to reset ConfigurationServiceProvider --- .../operator/api/config/ConfigurationServiceProvider.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProvider.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProvider.java index 9542e491ee..3157de79ff 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProvider.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProvider.java @@ -38,4 +38,8 @@ static void set(ConfigurationService instance, boolean overriding) { public static void setDefault(ConfigurationService defaultConfigurationService) { ConfigurationServiceProvider.defaultConfigurationService = defaultConfigurationService; } + + public static void reset() { + instance = null; + } } From f7d1d42b52b9bc1271cbdb181b93993fe9af4200 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 9 Mar 2022 16:56:35 +0100 Subject: [PATCH 03/19] feat: provide a default default :) --- .../api/config/ConfigurationServiceProvider.java | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProvider.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProvider.java index 3157de79ff..b092c087a6 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProvider.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProvider.java @@ -1,16 +1,13 @@ package io.javaoperatorsdk.operator.api.config; public class ConfigurationServiceProvider { - static ConfigurationService instance; - private static ConfigurationService defaultConfigurationService; + private static ConfigurationService instance; + private static ConfigurationService defaultConfigurationService = + new BaseConfigurationService(Utils.loadFromProperties()); private static boolean alreadyConfigured = false; public static ConfigurationService instance() { if (instance == null) { - if (defaultConfigurationService == null) { - throw new IllegalStateException( - "A ConfigurationService needs to be configured using the `set` method first"); - } set(defaultConfigurationService); } return instance; From fc9093c7216b2f7c57a5c5f93dd1db0a18e40a8b Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 9 Mar 2022 17:55:24 +0100 Subject: [PATCH 04/19] fix: tests --- .../operator/OperatorTest.java | 41 ++++---------- .../source/CustomResourceSelectorTest.java | 55 ++++++------------- .../junit/AbstractOperatorExtension.java | 16 ++++-- 3 files changed, 38 insertions(+), 74 deletions(-) diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/OperatorTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/OperatorTest.java index 99bd7e64b9..43bb0b788d 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/OperatorTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/OperatorTest.java @@ -4,11 +4,9 @@ import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; -import io.fabric8.kubernetes.client.CustomResource; +import io.fabric8.kubernetes.api.model.ConfigMap; import io.fabric8.kubernetes.client.KubernetesClient; -import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; -import io.javaoperatorsdk.operator.api.config.RetryConfiguration; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; @@ -17,27 +15,23 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +@SuppressWarnings("rawtypes") class OperatorTest { - private final KubernetesClient kubernetesClient = - MockKubernetesClient.client(FooCustomResource.class); - private final ConfigurationService configurationService = mock(ConfigurationService.class); + private final KubernetesClient kubernetesClient = MockKubernetesClient.client(ConfigMap.class); private final ControllerConfiguration configuration = mock(ControllerConfiguration.class); - private final Operator operator = new Operator(kubernetesClient, configurationService); - private final FooReconciler fooReconciler = FooReconciler.create(); + private final Operator operator = new Operator(kubernetesClient); + private final FooReconciler fooReconciler = new FooReconciler(); @Test @DisplayName("should register `Reconciler` to Controller") + @SuppressWarnings("unchecked") public void shouldRegisterReconcilerToController() { // given - when(configurationService.getConfigurationFor(fooReconciler)).thenReturn(configuration); - when(configuration.watchAllNamespaces()).thenReturn(true); - when(configuration.getName()).thenReturn("FOO"); - when(configuration.getResourceClass()).thenReturn(FooCustomResource.class); - when(configuration.getRetryConfiguration()).thenReturn(RetryConfiguration.DEFAULT); + when(configuration.getResourceClass()).thenReturn(ConfigMap.class); // when - operator.register(fooReconciler); + operator.register(fooReconciler, configuration); // then assertThat(operator.getControllers().size()).isEqualTo(1); @@ -50,25 +44,10 @@ public void shouldThrowOperatorExceptionWhenConfigurationIsNull() { Assertions.assertThrows(OperatorException.class, () -> operator.register(fooReconciler)); } - private static class FooCustomResource extends CustomResource { - } - - private static class FooSpec { - } - - private static class FooStatus { - } - - private static class FooReconciler implements Reconciler { - - private FooReconciler() {} - - public static FooReconciler create() { - return new FooReconciler(); - } + private static class FooReconciler implements Reconciler { @Override - public UpdateControl reconcile(FooCustomResource resource, Context context) { + public UpdateControl reconcile(ConfigMap resource, Context context) { return UpdateControl.noUpdate(); } } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/CustomResourceSelectorTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/CustomResourceSelectorTest.java index 446e654501..a93051382b 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/CustomResourceSelectorTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/CustomResourceSelectorTest.java @@ -1,7 +1,6 @@ package io.javaoperatorsdk.operator.processing.event.source; import java.util.Date; -import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Consumer; @@ -9,7 +8,6 @@ import org.awaitility.core.ConditionTimeoutException; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import org.mockito.internal.util.collections.Sets; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -19,7 +17,10 @@ import io.fabric8.kubernetes.client.server.mock.KubernetesMockServer; import io.javaoperatorsdk.operator.Operator; import io.javaoperatorsdk.operator.api.config.ConfigurationService; +import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; +import io.javaoperatorsdk.operator.api.config.DefaultControllerConfiguration; import io.javaoperatorsdk.operator.api.config.Version; +import io.javaoperatorsdk.operator.api.reconciler.Constants; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; @@ -45,14 +46,17 @@ public class CustomResourceSelectorTest { KubernetesClient client; ConfigurationService configurationService; - @SuppressWarnings("unchecked") @BeforeEach void setUpResources() { + // need to reset the provider if we plan on changing the configuration service since it likely + // has already been set in previous tests + ConfigurationServiceProvider.reset(); + configurationService = spy(ConfigurationService.class); when(configurationService.checkCRDAndValidateLocalModel()).thenReturn(false); when(configurationService.getVersion()).thenReturn(new Version("1", "1", new Date())); - when(configurationService.getConfigurationFor(any(MyController.class))).thenReturn( - new MyConfiguration(configurationService, null)); + when(configurationService.getConfigurationFor(any(MyController.class))) + .thenReturn(new MyConfiguration()); } @Test @@ -78,7 +82,7 @@ void resourceWatchedByLabel() { c1err.incrementAndGet(); } }), - new MyConfiguration(configurationService, "app=foo")); + (overrider) -> overrider.settingNamespace(NAMESPACE).withLabelSelector("app=foo")); o1.start(); o2.register( new MyController( @@ -90,7 +94,7 @@ void resourceWatchedByLabel() { c2err.incrementAndGet(); } }), - new MyConfiguration(configurationService, "app=bar")); + (overrider) -> overrider.settingNamespace(NAMESPACE).withLabelSelector("app=bar")); o2.start(); client.resources(TestCustomResource.class).inNamespace(NAMESPACE).create(newMyResource("foo", @@ -127,40 +131,13 @@ public TestCustomResource newMyResource(String app, String namespace) { return resource; } - public static class MyConfiguration - implements - io.javaoperatorsdk.operator.api.config.ControllerConfiguration { - - private final String labelSelector; - private final ConfigurationService service; - - public MyConfiguration(ConfigurationService configurationService, String labelSelector) { - this.labelSelector = labelSelector; - this.service = configurationService; - } - - @Override - public String getLabelSelector() { - return labelSelector; - } - - @Override - public String getAssociatedReconcilerClassName() { - return MyController.class.getCanonicalName(); - } - - @Override - public Set getNamespaces() { - return Sets.newSet(NAMESPACE); - } + public static class MyConfiguration extends DefaultControllerConfiguration { - @Override - public ConfigurationService getConfigurationService() { - return service; + public MyConfiguration() { + super(MyController.class.getCanonicalName(), "mycontroller", null, Constants.NO_FINALIZER, + false, null, + null, null, null, TestCustomResource.class, null, null, null); } - - @Override - public void setConfigurationService(ConfigurationService service) {} } @ControllerConfiguration(namespaces = NAMESPACE) diff --git a/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/AbstractOperatorExtension.java b/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/AbstractOperatorExtension.java index 7b1c550ac5..d575d0875d 100644 --- a/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/AbstractOperatorExtension.java +++ b/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/AbstractOperatorExtension.java @@ -199,6 +199,18 @@ public static abstract class AbstractBuilder> { protected boolean oneNamespacePerClass; protected AbstractBuilder() { + // try to load the default configuration service if present on the class path + // loading the class will register it as the default configuration service if not set manually + try { + getClass().getClassLoader() + .loadClass("io.javaoperatorsdk.operator.config.runtime.DefaultConfigurationService"); + } catch (ClassNotFoundException e) { + if (configurationService == null) { + throw new IllegalStateException( + "Couldn't load DefaultConfigurationService class. You need to either add the appropriate dependency to your build or manually set the ConfigurationService by calling withConfigurationService"); + } + } + this.configurationService = ConfigurationServiceProvider.instance(); this.infrastructure = new ArrayList<>(); @@ -232,10 +244,6 @@ public T oneNamespacePerClass(boolean value) { return (T) this; } - /** - * @deprecated There is no need to set the ConfigurationService manually anymore - */ - @Deprecated public T withConfigurationService(ConfigurationService value) { configurationService = value; return (T) this; From 255d7f3e183627c95a883fd4fd074724738c1a73 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 9 Mar 2022 20:35:10 +0100 Subject: [PATCH 05/19] feat: start using ConfigurationServiceProvider --- .../main/java/io/javaoperatorsdk/operator/Operator.java | 7 ++----- .../operator/api/config/ExecutorServiceManager.java | 8 +++----- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java index 7f51c4d8f7..2e535b370f 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java @@ -51,6 +51,7 @@ public Operator(ConfigurationService configurationService) { public Operator(KubernetesClient kubernetesClient, ConfigurationService configurationService) { this.kubernetesClient = kubernetesClient; this.configurationService = configurationService; + ConfigurationServiceProvider.set(configurationService); } /** Adds a shutdown hook that automatically calls {@link #stop()} ()} when the app shuts down. */ @@ -62,10 +63,6 @@ public KubernetesClient getKubernetesClient() { return kubernetesClient; } - public ConfigurationService getConfigurationService() { - return configurationService; - } - public List getControllers() { return new ArrayList<>(controllers.controllers.values()); } @@ -88,7 +85,7 @@ public void start() { final var clientVersion = Version.clientVersion(); log.info("Client version: {}", clientVersion); - ExecutorServiceManager.init(configurationService); + ExecutorServiceManager.init(); controllers.start(); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ExecutorServiceManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ExecutorServiceManager.java index 254a539ac3..3442466b5c 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ExecutorServiceManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ExecutorServiceManager.java @@ -25,11 +25,9 @@ private ExecutorServiceManager(InstrumentedExecutorService executor, this.terminationTimeoutSeconds = terminationTimeoutSeconds; } - public static void init(ConfigurationService configuration) { + public static void init() { if (instance == null) { - if (configuration == null) { - configuration = new BaseConfigurationService(Version.UNKNOWN); - } + final var configuration = ConfigurationServiceProvider.instance(); instance = new ExecutorServiceManager( new InstrumentedExecutorService(configuration.getExecutorService()), configuration.getTerminationTimeoutSeconds()); @@ -56,7 +54,7 @@ public static void stop() { public static ExecutorServiceManager instance() { if (instance == null) { // provide a default configuration if none has been provided by init - init(null); + init(); } return instance; } From d27c76f38ae0a62b2792b97fe0b40cdde3e82ab0 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 9 Mar 2022 20:56:49 +0100 Subject: [PATCH 06/19] fix: remove now unneeded loading of DefaultConfigurationService --- .../operator/junit/AbstractOperatorExtension.java | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/AbstractOperatorExtension.java b/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/AbstractOperatorExtension.java index d575d0875d..2c2a14b14d 100644 --- a/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/AbstractOperatorExtension.java +++ b/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/AbstractOperatorExtension.java @@ -199,18 +199,6 @@ public static abstract class AbstractBuilder> { protected boolean oneNamespacePerClass; protected AbstractBuilder() { - // try to load the default configuration service if present on the class path - // loading the class will register it as the default configuration service if not set manually - try { - getClass().getClassLoader() - .loadClass("io.javaoperatorsdk.operator.config.runtime.DefaultConfigurationService"); - } catch (ClassNotFoundException e) { - if (configurationService == null) { - throw new IllegalStateException( - "Couldn't load DefaultConfigurationService class. You need to either add the appropriate dependency to your build or manually set the ConfigurationService by calling withConfigurationService"); - } - } - this.configurationService = ConfigurationServiceProvider.instance(); this.infrastructure = new ArrayList<>(); From dad795e54cd6fdf9db7a066a5a4cf87a74346407 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Thu, 10 Mar 2022 10:27:26 +0100 Subject: [PATCH 07/19] refactor: make it easier to debug things --- .../event/ReconciliationDispatcherTest.java | 119 +++++++++++------- 1 file changed, 74 insertions(+), 45 deletions(-) diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java index f07d88db1e..7db0672bbf 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java @@ -4,6 +4,7 @@ import java.util.ArrayList; import java.util.Optional; import java.util.concurrent.TimeUnit; +import java.util.function.BiFunction; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -35,16 +36,18 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.Mockito.any; import static org.mockito.Mockito.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; -import static org.mockito.Mockito.withSettings; +@SuppressWarnings({"unchecked", "rawtypes"}) class ReconciliationDispatcherTest { private static final String DEFAULT_FINALIZER = "javaoperatorsdk.io/finalizer"; @@ -52,26 +55,24 @@ class ReconciliationDispatcherTest { public static final long RECONCILIATION_MAX_INTERVAL = 10L; private TestCustomResource testCustomResource; private ReconciliationDispatcher reconciliationDispatcher; - private final Reconciler reconciler = mock(Reconciler.class, - withSettings().extraInterfaces(ErrorStatusHandler.class)); + private TestReconciler reconciler; private final ConfigurationService configService = mock(ConfigurationService.class); private final CustomResourceFacade customResourceFacade = mock(ReconciliationDispatcher.CustomResourceFacade.class); - private ControllerConfiguration configuration = mock(ControllerConfiguration.class); @BeforeEach void setup() { testCustomResource = TestUtils.testCustomResource(); + reconciler = spy(new TestReconciler()); reconciliationDispatcher = init(testCustomResource, reconciler, null, customResourceFacade, true); } private ReconciliationDispatcher init(R customResource, - Reconciler reconciler, ControllerConfiguration configuration, + Reconciler reconciler, ControllerConfiguration configuration, CustomResourceFacade customResourceFacade, boolean useFinalizer) { configuration = configuration == null ? mock(ControllerConfiguration.class) : configuration; - ReconciliationDispatcherTest.this.configuration = configuration; final var finalizer = useFinalizer ? DEFAULT_FINALIZER : Constants.NO_FINALIZER; when(configuration.getFinalizer()).thenReturn(finalizer); when(configuration.useFinalizer()).thenCallRealMethod(); @@ -98,8 +99,6 @@ public T clone(T object) { return object; } }); - when(reconciler.cleanup(eq(customResource), any())) - .thenReturn(DeleteControl.defaultDelete()); Controller controller = new Controller<>(reconciler, configuration, MockKubernetesClient.client(customResource.getClass())); controller.start(); @@ -131,8 +130,7 @@ void callCreateOrUpdateOnNewResourceIfFinalizerSet() { void updatesOnlyStatusSubResourceIfFinalizerSet() { testCustomResource.addFinalizer(DEFAULT_FINALIZER); - when(reconciler.reconcile(eq(testCustomResource), any())) - .thenReturn(UpdateControl.updateStatus(testCustomResource)); + reconciler.reconcile = (r, c) -> UpdateControl.updateStatus(testCustomResource); reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); @@ -144,8 +142,7 @@ void updatesOnlyStatusSubResourceIfFinalizerSet() { void updatesBothResourceAndStatusIfFinalizerSet() { testCustomResource.addFinalizer(DEFAULT_FINALIZER); - when(reconciler.reconcile(eq(testCustomResource), any())) - .thenReturn(UpdateControl.updateResourceAndStatus(testCustomResource)); + reconciler.reconcile = (r, c) -> UpdateControl.updateResourceAndStatus(testCustomResource); when(customResourceFacade.replaceWithLock(testCustomResource)).thenReturn(testCustomResource); reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); @@ -219,8 +216,7 @@ void removesDefaultFinalizerOnDeleteIfSet() { void doesNotRemovesTheSetFinalizerIfTheDeleteNotMethodInstructsIt() { testCustomResource.addFinalizer(DEFAULT_FINALIZER); - when(reconciler.cleanup(eq(testCustomResource), any())) - .thenReturn(DeleteControl.noFinalizerRemoval()); + reconciler.cleanup = (r, c) -> DeleteControl.noFinalizerRemoval(); markForDeletion(testCustomResource); reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); @@ -233,8 +229,7 @@ void doesNotRemovesTheSetFinalizerIfTheDeleteNotMethodInstructsIt() { void doesNotUpdateTheResourceIfNoUpdateUpdateControlIfFinalizerSet() { testCustomResource.addFinalizer(DEFAULT_FINALIZER); - when(reconciler.reconcile(eq(testCustomResource), any())) - .thenReturn(UpdateControl.noUpdate()); + reconciler.reconcile = (r, c) -> UpdateControl.noUpdate(); reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); verify(customResourceFacade, never()).replaceWithLock(any()); @@ -244,8 +239,8 @@ void doesNotUpdateTheResourceIfNoUpdateUpdateControlIfFinalizerSet() { @Test void addsFinalizerIfNotMarkedForDeletionAndEmptyCustomResourceReturned() { removeFinalizers(testCustomResource); - when(reconciler.reconcile(eq(testCustomResource), any())) - .thenReturn(UpdateControl.noUpdate()); + + reconciler.reconcile = (r, c) -> UpdateControl.noUpdate(); reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); @@ -297,7 +292,7 @@ public boolean isLastAttempt() { verify(reconciler, times(1)) .reconcile(any(), contextArgumentCaptor.capture()); Context context = contextArgumentCaptor.getValue(); - final var retryInfo = context.getRetryInfo().get(); + final var retryInfo = context.getRetryInfo().orElseGet(fail("Missing optional")); assertThat(retryInfo.getAttemptCount()).isEqualTo(2); assertThat(retryInfo.isLastAttempt()).isEqualTo(true); } @@ -306,14 +301,13 @@ public boolean isLastAttempt() { void setReScheduleToPostExecutionControlFromUpdateControl() { testCustomResource.addFinalizer(DEFAULT_FINALIZER); - when(reconciler.reconcile(eq(testCustomResource), any())) - .thenReturn( - UpdateControl.updateStatus(testCustomResource).rescheduleAfter(1000L)); + reconciler.reconcile = + (r, c) -> UpdateControl.updateStatus(testCustomResource).rescheduleAfter(1000L); PostExecutionControl control = reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); - assertThat(control.getReScheduleDelay().get()).isEqualTo(1000L); + assertThat(control.getReScheduleDelay().orElseGet(fail("Missing optional"))).isEqualTo(1000L); } @Test @@ -321,13 +315,13 @@ void reScheduleOnDeleteWithoutFinalizerRemoval() { testCustomResource.addFinalizer(DEFAULT_FINALIZER); markForDeletion(testCustomResource); - when(reconciler.cleanup(eq(testCustomResource), any())) - .thenReturn(DeleteControl.noFinalizerRemoval().rescheduleAfter(1, TimeUnit.SECONDS)); + reconciler.cleanup = + (r, c) -> DeleteControl.noFinalizerRemoval().rescheduleAfter(1, TimeUnit.SECONDS); PostExecutionControl control = reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); - assertThat(control.getReScheduleDelay().get()).isEqualTo(1000L); + assertThat(control.getReScheduleDelay().orElseGet(fail("Missing optional"))).isEqualTo(1000L); } @Test @@ -347,8 +341,9 @@ void setObservedGenerationForStatusIfNeeded() { PostExecutionControl control = dispatcher.handleExecution( executionScopeWithCREvent(observedGenResource)); - assertThat(control.getUpdatedCustomResource().get().getStatus().getObservedGeneration()) - .isEqualTo(1L); + assertThat(control.getUpdatedCustomResource().orElseGet(fail("Missing optional")) + .getStatus().getObservedGeneration()) + .isEqualTo(1L); } @Test @@ -367,8 +362,9 @@ void updatesObservedGenerationOnNoUpdateUpdateControl() { PostExecutionControl control = dispatcher.handleExecution( executionScopeWithCREvent(observedGenResource)); - assertThat(control.getUpdatedCustomResource().get().getStatus().getObservedGeneration()) - .isEqualTo(1L); + assertThat(control.getUpdatedCustomResource().orElseGet(fail("Missing optional")) + .getStatus().getObservedGeneration()) + .isEqualTo(1L); } @Test @@ -388,20 +384,22 @@ void updateObservedGenerationOnCustomResourceUpdate() { PostExecutionControl control = dispatcher.handleExecution( executionScopeWithCREvent(observedGenResource)); - assertThat(control.getUpdatedCustomResource().get().getStatus().getObservedGeneration()) - .isEqualTo(1L); + assertThat(control.getUpdatedCustomResource().orElseGet(fail("Missing optional")) + .getStatus().getObservedGeneration()) + .isEqualTo(1L); } @Test void callErrorStatusHandlerIfImplemented() { testCustomResource.addFinalizer(DEFAULT_FINALIZER); - when(reconciler.reconcile(any(), any())) - .thenThrow(new IllegalStateException("Error Status Test")); - when(((ErrorStatusHandler) reconciler).updateErrorStatus(any(), any(), any())).then(a -> { + reconciler.reconcile = (r, c) -> { + throw new IllegalStateException("Error Status Test"); + }; + reconciler.errorHandler = (r, ri, e) -> { testCustomResource.getStatus().setConfigMapStatus(ERROR_MESSAGE); return Optional.of(testCustomResource); - }); + }; reconciliationDispatcher.handleExecution( new ExecutionScope( @@ -427,12 +425,14 @@ public boolean isLastAttempt() { void callErrorStatusHandlerEvenOnFirstError() { testCustomResource.addFinalizer(DEFAULT_FINALIZER); - when(reconciler.reconcile(any(), any())) - .thenThrow(new IllegalStateException("Error Status Test")); - when(((ErrorStatusHandler) reconciler).updateErrorStatus(any(), any(), any())).then(a -> { + reconciler.reconcile = (r, c) -> { + throw new IllegalStateException("Error Status Test"); + }; + reconciler.errorHandler = (r, ri, e) -> { testCustomResource.getStatus().setConfigMapStatus(ERROR_MESSAGE); return Optional.of(testCustomResource); - }); + }; + reconciliationDispatcher.handleExecution( new ExecutionScope( testCustomResource, null)); @@ -445,8 +445,7 @@ void callErrorStatusHandlerEvenOnFirstError() { void schedulesReconciliationIfMaxDelayIsSet() { testCustomResource.addFinalizer(DEFAULT_FINALIZER); - when(reconciler.reconcile(eq(testCustomResource), any())) - .thenReturn(UpdateControl.noUpdate()); + reconciler.reconcile = (r, c) -> UpdateControl.noUpdate(); PostExecutionControl control = reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); @@ -459,9 +458,8 @@ void schedulesReconciliationIfMaxDelayIsSet() { void canSkipSchedulingMaxDelayIf() { testCustomResource.addFinalizer(DEFAULT_FINALIZER); - when(reconciler.reconcile(eq(testCustomResource), any())) - .thenReturn(UpdateControl.noUpdate()); - when(configuration.reconciliationMaxInterval()) + reconciler.reconcile = (r, c) -> UpdateControl.noUpdate(); + when(reconciliationDispatcher.configuration().reconciliationMaxInterval()) .thenReturn(Optional.empty()); PostExecutionControl control = @@ -490,4 +488,35 @@ private void removeFinalizers(CustomResource customResource) { public ExecutionScope executionScopeWithCREvent(T resource) { return new ExecutionScope<>(resource, null); } + + private class TestReconciler + implements Reconciler, ErrorStatusHandler { + private BiFunction> reconcile; + private BiFunction cleanup; + private ErrorStatusHandler errorHandler; + + @Override + public UpdateControl reconcile(TestCustomResource resource, + Context context) { + if (reconcile != null && resource.equals(testCustomResource)) { + return reconcile.apply(resource, context); + } + return UpdateControl.noUpdate(); + } + + @Override + public DeleteControl cleanup(TestCustomResource resource, Context context) { + if (cleanup != null && resource.equals(testCustomResource)) { + return cleanup.apply(resource, context); + } + return DeleteControl.defaultDelete(); + } + + @Override + public Optional updateErrorStatus(TestCustomResource resource, + RetryInfo retryInfo, RuntimeException e) { + return errorHandler != null ? errorHandler.updateErrorStatus(resource, retryInfo, e) + : Optional.empty(); + } + } } From 49a19ddf395daa2921914b5ae93087a50efdf287 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Thu, 10 Mar 2022 10:35:43 +0100 Subject: [PATCH 08/19] refactor: remove ConfigurationService from ResourceConfiguration --- .../config/AbstractConfigurationService.java | 3 +- .../AnnotationControllerConfiguration.java | 11 ------ .../config/ConfigurationServiceOverrider.java | 10 ------ .../api/config/ControllerConfiguration.java | 4 +++ .../ControllerConfigurationOverrider.java | 1 - .../DefaultControllerConfiguration.java | 13 +------ .../config/DefaultResourceConfiguration.java | 9 +---- .../api/config/ResourceConfiguration.java | 12 ++----- .../informer/InformerConfiguration.java | 35 ++++++++----------- .../api/reconciler/DefaultContext.java | 1 + .../operator/processing/Controller.java | 21 ++--------- .../KubernetesDependentResource.java | 14 +++----- .../processing/event/EventProcessor.java | 18 ++++------ .../event/ReconciliationDispatcher.java | 10 +++--- .../source/informer/InformerManager.java | 5 ++- .../operator/ControllerManagerTest.java | 2 +- .../config/ControllerConfigurationTest.java | 15 +------- .../operator/processing/ControllerTest.java | 5 ++- .../source/CustomResourceSelectorTest.java | 2 +- .../event/source/ResourceEventFilterTest.java | 3 +- .../ControllerResourceEventSourceTest.java | 7 ++-- .../sample/simple/TestCustomResourceSpec.java | 20 +++++++++++ .../simple/TestCustomResourceStatus.java | 19 ++++++++++ .../DefaultConfigurationServiceTest.java | 6 ---- 24 files changed, 97 insertions(+), 149 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AbstractConfigurationService.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AbstractConfigurationService.java index 48c6b73f2b..2c9e01551e 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AbstractConfigurationService.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AbstractConfigurationService.java @@ -26,6 +26,7 @@ protected void replace(ControllerConfiguration config put(config, false); } + @SuppressWarnings("unchecked") private void put( ControllerConfiguration config, boolean failIfExisting) { final var name = config.getName(); @@ -36,7 +37,6 @@ private void put( } } configurations.put(name, config); - config.setConfigurationService(this); } protected void throwExceptionOnNameCollision( @@ -50,6 +50,7 @@ protected void throwExceptionOnNameCollision( + newReconcilerClassName); } + @SuppressWarnings("unchecked") @Override public ControllerConfiguration getConfigurationFor( Reconciler reconciler) { 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 a3241edf04..0d64a16ef7 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 @@ -26,7 +26,6 @@ public class AnnotationControllerConfiguration protected final Reconciler reconciler; private final ControllerConfiguration annotation; - private ConfigurationService service; private List> specs; public AnnotationControllerConfiguration(Reconciler reconciler) { @@ -77,16 +76,6 @@ public String getLabelSelector() { return valueOrDefault(annotation, ControllerConfiguration::labelSelector, ""); } - @Override - public ConfigurationService getConfigurationService() { - return service; - } - - @Override - public void setConfigurationService(ConfigurationService service) { - this.service = service; - } - @Override public String getAssociatedReconcilerClassName() { return reconciler.getClass().getCanonicalName(); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java index b5e26d7cad..cc89f2550e 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java @@ -3,10 +3,8 @@ import java.util.Set; import java.util.concurrent.ExecutorService; -import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.client.Config; import io.javaoperatorsdk.operator.api.monitoring.Metrics; -import io.javaoperatorsdk.operator.api.reconciler.Reconciler; public class ConfigurationServiceOverrider { private final ConfigurationService original; @@ -73,14 +71,6 @@ public ConfigurationServiceOverrider withExecutorService(ExecutorService executo public ConfigurationService build() { final var overridden = new BaseConfigurationService(original.getVersion()) { - @Override - public ControllerConfiguration getConfigurationFor( - Reconciler reconciler) { - final var controllerConfiguration = original.getConfigurationFor(reconciler); - controllerConfiguration.setConfigurationService(this); - return controllerConfiguration; - } - @Override public Set getKnownReconcilerNames() { return original.getKnownReconcilerNames(); 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 5ebcaf6ed4..03485412b6 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 @@ -58,4 +58,8 @@ default ResourceEventFilter getEventFilter() { default Optional reconciliationMaxInterval() { return Optional.of(Duration.ofHours(10L)); } + + default ConfigurationService getConfigurationService() { + return ConfigurationServiceProvider.instance(); + } } 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 a4d194b876..87297b8d49 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 @@ -135,7 +135,6 @@ public ControllerConfiguration build() { customResourcePredicate, original.getResourceClass(), reconciliationMaxInterval, - original.getConfigurationService(), dependentResourceSpecs); } 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 bba8c1ba12..4545c9885e 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.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilter; +@SuppressWarnings("rawtypes") public class DefaultControllerConfiguration extends DefaultResourceConfiguration implements ControllerConfiguration { @@ -37,7 +38,6 @@ public DefaultControllerConfiguration( ResourceEventFilter resourceEventFilter, Class resourceClass, Duration reconciliationMaxInterval, - ConfigurationService service, List> dependents) { super(labelSelector, resourceClass, namespaces); this.associatedControllerClassName = associatedControllerClassName; @@ -52,7 +52,6 @@ public DefaultControllerConfiguration( : retryConfiguration; this.resourceEventFilter = resourceEventFilter; - setConfigurationService(service); this.dependents = dependents != null ? dependents : Collections.emptyList(); } @@ -86,16 +85,6 @@ public RetryConfiguration getRetryConfiguration() { return retryConfiguration; } - - @Override - public void setConfigurationService(ConfigurationService service) { - if (getConfigurationService() != null) { - throw new IllegalStateException("A ConfigurationService is already associated with '" + name - + "' ControllerConfiguration. Cannot change it once set!"); - } - super.setConfigurationService(service); - } - @Override public ResourceEventFilter getEventFilter() { return resourceEventFilter; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultResourceConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultResourceConfiguration.java index d7b5f76815..6e523c587c 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultResourceConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultResourceConfiguration.java @@ -12,7 +12,6 @@ public class DefaultResourceConfiguration private final Set namespaces; private final boolean watchAllNamespaces; private final Class resourceClass; - private ConfigurationService service; public DefaultResourceConfiguration(String labelSelector, Class resourceClass, String... namespaces) { @@ -48,18 +47,12 @@ public boolean watchAllNamespaces() { return watchAllNamespaces; } - @Override public ConfigurationService getConfigurationService() { - return service; + return ConfigurationServiceProvider.instance(); } @Override public Class getResourceClass() { return resourceClass; } - - @Override - public void setConfigurationService(ConfigurationService service) { - this.service = service; - } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResourceConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResourceConfiguration.java index 3ebc3ae546..2d947050e7 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResourceConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResourceConfiguration.java @@ -63,12 +63,8 @@ static boolean currentNamespaceWatched(Set namespaces) { default Set getEffectiveNamespaces() { var targetNamespaces = getNamespaces(); if (watchCurrentNamespace()) { - final var parent = getConfigurationService(); - if (parent == null) { - throw new IllegalStateException( - "Parent ConfigurationService must be set before calling this method"); - } - String namespace = parent.getClientConfiguration().getNamespace(); + final String namespace = + ConfigurationServiceProvider.instance().getClientConfiguration().getNamespace(); if (namespace == null) { throw new OperatorException( "Couldn't retrieve the currently connected namespace. Make sure it's correctly set in your ~/.kube/config file, using, e.g. 'kubectl config set-context --namespace='"); @@ -77,8 +73,4 @@ default Set getEffectiveNamespaces() { } return targetNamespaces; } - - ConfigurationService getConfigurationService(); - - void setConfigurationService(ConfigurationService service); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java index 50f21629b8..55c8216e96 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java @@ -5,7 +5,6 @@ import java.util.Set; import io.fabric8.kubernetes.api.model.HasMetadata; -import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.api.config.DefaultResourceConfiguration; import io.javaoperatorsdk.operator.api.config.ResourceConfiguration; import io.javaoperatorsdk.operator.api.reconciler.EventSourceContext; @@ -14,6 +13,7 @@ import io.javaoperatorsdk.operator.processing.event.source.SecondaryToPrimaryMapper; import io.javaoperatorsdk.operator.processing.event.source.informer.Mappers; +@SuppressWarnings("rawtypes") public interface InformerConfiguration extends ResourceConfiguration { @@ -23,13 +23,12 @@ class DefaultInformerConfiguration private final SecondaryToPrimaryMapper secondaryToPrimaryResourcesIdSet; private final PrimaryToSecondaryMapper

associatedWith; - protected DefaultInformerConfiguration(ConfigurationService service, String labelSelector, + protected DefaultInformerConfiguration(String labelSelector, Class resourceClass, SecondaryToPrimaryMapper secondaryToPrimaryResourcesIdSet, PrimaryToSecondaryMapper

associatedWith, Set namespaces) { super(labelSelector, resourceClass, namespaces); - setConfigurationService(service); this.secondaryToPrimaryResourcesIdSet = Objects.requireNonNullElse(secondaryToPrimaryResourcesIdSet, Mappers.fromOwnerReference()); @@ -52,6 +51,7 @@ public PrimaryToSecondaryMapper

getAssociatedResourceIdentifier() { PrimaryToSecondaryMapper

getAssociatedResourceIdentifier(); + @SuppressWarnings("unused") class InformerConfigurationBuilder { private SecondaryToPrimaryMapper secondaryToPrimaryResourcesIdSet; @@ -59,12 +59,9 @@ class InformerConfigurationBuilder private Set namespaces; private String labelSelector; private final Class resourceClass; - private final ConfigurationService configurationService; - private InformerConfigurationBuilder(Class resourceClass, - ConfigurationService configurationService) { + private InformerConfigurationBuilder(Class resourceClass) { this.resourceClass = resourceClass; - this.configurationService = configurationService; } public InformerConfigurationBuilder withPrimaryResourcesRetriever( @@ -97,7 +94,7 @@ public InformerConfigurationBuilder withLabelSelector(String labelSelector } public InformerConfiguration build() { - return new DefaultInformerConfiguration<>(configurationService, labelSelector, resourceClass, + return new DefaultInformerConfiguration<>(labelSelector, resourceClass, secondaryToPrimaryResourcesIdSet, associatedWith, namespaces); } @@ -105,23 +102,21 @@ public InformerConfiguration build() { static InformerConfigurationBuilder from( EventSourceContext

context, Class resourceClass) { - return new InformerConfigurationBuilder<>(resourceClass, context.getControllerConfiguration() - .getConfigurationService()); + return new InformerConfigurationBuilder<>(resourceClass); } - static InformerConfigurationBuilder from(ConfigurationService configurationService, - Class resourceClass) { - return new InformerConfigurationBuilder<>(resourceClass, configurationService); + @SuppressWarnings({"rawtypes", "unchecked"}) + static InformerConfigurationBuilder from(Class resourceClass) { + return new InformerConfigurationBuilder<>(resourceClass); } static InformerConfigurationBuilder from( InformerConfiguration configuration) { - return new InformerConfigurationBuilder(configuration.getResourceClass(), - configuration.getConfigurationService()) - .withNamespaces(configuration.getNamespaces()) - .withLabelSelector(configuration.getLabelSelector()) - .withAssociatedSecondaryResourceIdentifier( - configuration.getAssociatedResourceIdentifier()) - .withPrimaryResourcesRetriever(configuration.getPrimaryResourcesRetriever()); + return new InformerConfigurationBuilder(configuration.getResourceClass()) + .withNamespaces(configuration.getNamespaces()) + .withLabelSelector(configuration.getLabelSelector()) + .withAssociatedSecondaryResourceIdentifier( + configuration.getAssociatedResourceIdentifier()) + .withPrimaryResourcesRetriever(configuration.getPrimaryResourcesRetriever()); } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java index 96084ffd1f..14d737f87b 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java @@ -4,6 +4,7 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; import io.javaoperatorsdk.operator.api.reconciler.dependent.ManagedDependentResourceContext; import io.javaoperatorsdk.operator.processing.Controller; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java index 5c432bf95e..75e545f7a7 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java @@ -16,10 +16,9 @@ import io.javaoperatorsdk.operator.CustomResourceUtils; import io.javaoperatorsdk.operator.MissingCRDException; import io.javaoperatorsdk.operator.OperatorException; -import io.javaoperatorsdk.operator.api.config.BaseConfigurationService; import io.javaoperatorsdk.operator.api.config.ConfigurationService; +import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; -import io.javaoperatorsdk.operator.api.config.Version; import io.javaoperatorsdk.operator.api.monitoring.Metrics; import io.javaoperatorsdk.operator.api.monitoring.Metrics.ControllerExecution; import io.javaoperatorsdk.operator.api.reconciler.Context; @@ -47,8 +46,6 @@ public class Controller

implements Reconciler

, private final EventSourceManager

eventSourceManager; private final DependentResourceManager

dependents; - private ConfigurationService configurationService; - public Controller(Reconciler

reconciler, ControllerConfiguration

configuration, KubernetesClient kubernetesClient) { @@ -228,20 +225,7 @@ public void start() throws OperatorException { } private ConfigurationService configurationService() { - if (configurationService == null) { - configurationService = configuration.getConfigurationService(); - // make sure we always have a default configuration service - if (configurationService == null) { - // we shouldn't need to register the configuration with the default service - configurationService = new BaseConfigurationService(Version.UNKNOWN) { - @Override - public boolean checkCRDAndValidateLocalModel() { - return false; - } - }; - } - } - return configurationService; + return ConfigurationServiceProvider.instance(); } private void throwMissingCRDException(String crdName, String specVersion, String controllerName) { @@ -282,6 +266,7 @@ public void stop() { } } + @SuppressWarnings("rawtypes") public List getDependents() { return dependents.getDependents(); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java index 2bee0edf1b..a471df8c09 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java @@ -11,7 +11,6 @@ import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.dsl.NonNamespaceOperation; import io.fabric8.kubernetes.client.dsl.Resource; -import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.api.config.Utils; import io.javaoperatorsdk.operator.api.config.informer.InformerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.Context; @@ -55,13 +54,12 @@ public KubernetesDependentResource() { @Override public void configureWith(KubernetesDependentResourceConfig config) { - configureWith(config.getConfigurationService(), config.labelSelector(), - Set.of(config.namespaces()), config.addOwnerReference()); + configureWith(config.labelSelector(), Set.of(config.namespaces()), config.addOwnerReference()); } @SuppressWarnings("unchecked") - private void configureWith(ConfigurationService configService, String labelSelector, - Set namespaces, boolean addOwnerReference) { + private void configureWith(String labelSelector, Set namespaces, + boolean addOwnerReference) { final var primaryResourcesRetriever = (this instanceof SecondaryToPrimaryMapper) ? (SecondaryToPrimaryMapper) this : Mappers.fromOwnerReference(); @@ -70,7 +68,7 @@ private void configureWith(ConfigurationService configService, String labelSelec ? (PrimaryToSecondaryMapper

) this : ResourceID::fromResource; InformerConfiguration ic = - InformerConfiguration.from(configService, resourceType()) + InformerConfiguration.from(resourceType()) .withLabelSelector(labelSelector) .withNamespaces(namespaces) .withPrimaryResourcesRetriever(primaryResourcesRetriever) @@ -129,9 +127,7 @@ protected NonNamespaceOperation, Resource> prepa @Override public EventSource initEventSource(EventSourceContext

context) { if (informerEventSource == null) { - configureWith(context.getControllerConfiguration().getConfigurationService(), null, - context.getControllerConfiguration().getNamespaces(), - KubernetesDependent.ADD_OWNER_REFERENCE_DEFAULT); + configureWith(null, null, KubernetesDependent.ADD_OWNER_REFERENCE_DEFAULT); log.warn("Using default configuration for " + resourceType().getSimpleName() + " KubernetesDependentResource, call configureWith to provide configuration"); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventProcessor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventProcessor.java index 7f4cae69a4..e44df906d8 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventProcessor.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventProcessor.java @@ -15,6 +15,7 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.OperatorException; import io.javaoperatorsdk.operator.api.config.ConfigurationService; +import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; import io.javaoperatorsdk.operator.api.config.ExecutorServiceManager; import io.javaoperatorsdk.operator.api.monitoring.Metrics; import io.javaoperatorsdk.operator.api.reconciler.RetryInfo; @@ -55,13 +56,7 @@ class EventProcessor implements EventHandler, LifecycleAw new ReconciliationDispatcher<>(eventSourceManager.getController()), GenericRetry.fromConfiguration( eventSourceManager.getController().getConfiguration().getRetryConfiguration()), - eventSourceManager.getController().getConfiguration().getConfigurationService() == null - ? Metrics.NOOP - : eventSourceManager - .getController() - .getConfiguration() - .getConfigurationService() - .getMetrics(), + ConfigurationServiceProvider.instance().getMetrics(), eventSourceManager); } @@ -209,11 +204,10 @@ void eventProcessingFinished( if (eventMarker.deleteEventPresent(resourceID)) { cleanupForDeletedEvent(executionScope.getCustomResourceID()); } else { - postExecutionControl.getUpdatedCustomResource().ifPresent(r -> { - eventSourceManager.getControllerResourceEventSource().handleRecentResourceUpdate( - ResourceID.fromResource(r), r, - executionScope.getResource()); - }); + postExecutionControl.getUpdatedCustomResource().ifPresent( + r -> eventSourceManager.getControllerResourceEventSource().handleRecentResourceUpdate( + ResourceID.fromResource(r), r, + executionScope.getResource())); if (eventMarker.eventPresent(resourceID)) { submitReconciliationExecution(resourceID); } else { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java index 2a14e8a69b..088592b3a4 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java @@ -9,7 +9,7 @@ import io.fabric8.kubernetes.client.dsl.MixedOperation; import io.fabric8.kubernetes.client.dsl.Resource; import io.javaoperatorsdk.operator.api.ObservedGenerationAware; -import io.javaoperatorsdk.operator.api.config.ConfigurationService; +import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.BaseControl; import io.javaoperatorsdk.operator.api.reconciler.Context; @@ -124,9 +124,8 @@ private PostExecutionControl handleReconcile( private R cloneResourceForErrorStatusHandlerIfNeeded(R resource, Context context) { if (isErrorStatusHandlerPresent() || shouldUpdateObservedGenerationAutomatically(resource)) { - final var configurationService = configuration().getConfigurationService(); - return configurationService != null ? configurationService.getResourceCloner().clone(resource) - : ConfigurationService.DEFAULT_CLONER.clone(resource); + final var cloner = ConfigurationServiceProvider.instance().getResourceCloner(); + return cloner.clone(resource); } else { return resource; } @@ -163,6 +162,7 @@ && shouldUpdateObservedGenerationAutomatically(resourceForExecution)) { return createPostExecutionControl(updatedCustomResource, updateControl); } + @SuppressWarnings("unchecked") private void handleErrorStatusHandler(R resource, Context context, RuntimeException e) { if (isErrorStatusHandlerPresent()) { @@ -301,7 +301,7 @@ private R replace(R resource) { return customResourceFacade.replaceWithLock(resource); } - private ControllerConfiguration configuration() { + ControllerConfiguration configuration() { return controller.getConfiguration(); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java index 33f962e7ed..a50eb34441 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java @@ -17,7 +17,7 @@ import io.fabric8.kubernetes.client.informers.ResourceEventHandler; import io.javaoperatorsdk.operator.OperatorException; import io.javaoperatorsdk.operator.api.config.Cloner; -import io.javaoperatorsdk.operator.api.config.ConfigurationService; +import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; import io.javaoperatorsdk.operator.api.config.ResourceConfiguration; import io.javaoperatorsdk.operator.processing.LifecycleAware; import io.javaoperatorsdk.operator.processing.event.ResourceID; @@ -41,8 +41,7 @@ public void start() throws OperatorException { void initSources(MixedOperation, Resource> client, C configuration, ResourceEventHandler eventHandler) { - final var service = configuration.getConfigurationService(); - cloner = service == null ? ConfigurationService.DEFAULT_CLONER : service.getResourceCloner(); + cloner = ConfigurationServiceProvider.instance().getResourceCloner(); final var targetNamespaces = configuration.getEffectiveNamespaces(); final var labelSelector = configuration.getLabelSelector(); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ControllerManagerTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ControllerManagerTest.java index 910492a47f..ca02094d46 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ControllerManagerTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ControllerManagerTest.java @@ -63,7 +63,7 @@ private static class TestControllerConfiguration public TestControllerConfiguration(Reconciler controller, Class crClass) { super(null, getControllerName(controller), CustomResource.getCRDName(crClass), null, false, null, null, null, null, crClass, - null, null, null); + null, null); this.controller = controller; } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationTest.java index 65c73b6b31..a2eeacf82b 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationTest.java @@ -10,20 +10,7 @@ class ControllerConfigurationTest { @Test void getCustomResourceClass() { - final ControllerConfiguration conf = new ControllerConfiguration<>() { - @Override - public String getAssociatedReconcilerClassName() { - return null; - } - - @Override - public ConfigurationService getConfigurationService() { - return null; - } - - @Override - public void setConfigurationService(ConfigurationService service) {} - }; + final ControllerConfiguration conf = () -> null; assertEquals(TestCustomResource.class, conf.getResourceClass()); } } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java index 2954af5aa4..384925453d 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java @@ -17,6 +17,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +@SuppressWarnings("unchecked") class ControllerTest { @Test @@ -51,12 +52,10 @@ void crdShouldNotBeCheckedForCustomResourcesIfDisabled() { @Test void crdShouldBeCheckedForCustomResourcesByDefault() { final var client = MockKubernetesClient.client(TestCustomResource.class); - final var configurationService = mock(ConfigurationService.class); - when(configurationService.checkCRDAndValidateLocalModel()).thenCallRealMethod(); final var reconciler = mock(Reconciler.class); final var configuration = mock(ControllerConfiguration.class); when(configuration.getResourceClass()).thenReturn(TestCustomResource.class); - when(configuration.getConfigurationService()).thenReturn(configurationService); + when(configuration.getConfigurationService()).thenCallRealMethod(); final var controller = new Controller(reconciler, configuration, client); // since we're not really connected to a cluster and the CRD wouldn't be deployed anyway, we diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/CustomResourceSelectorTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/CustomResourceSelectorTest.java index a93051382b..e995d3b9c4 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/CustomResourceSelectorTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/CustomResourceSelectorTest.java @@ -136,7 +136,7 @@ public static class MyConfiguration extends DefaultControllerConfiguration configuration) super(null, configuration, MockKubernetesClient.client(TestCustomResource.class)); } + @SuppressWarnings("unchecked") @Override public EventSourceManager getEventSourceManager() { return mock(EventSourceManager.class); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerResourceEventSourceTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerResourceEventSourceTest.java index 9782948dba..d06cf97fe8 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerResourceEventSourceTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerResourceEventSourceTest.java @@ -26,7 +26,7 @@ class ControllerResourceEventSourceTest extends public static final String FINALIZER = "finalizer"; - private TestController testController = new TestController(true); + private final TestController testController = new TestController(true); @BeforeEach public void setup() { @@ -109,9 +109,10 @@ public void callsBroadcastsOnResourceEvents() { eq(customResource1)); } + @SuppressWarnings("unchecked") private static class TestController extends Controller { - private EventSourceManager eventSourceManager = + private final EventSourceManager eventSourceManager = mock(EventSourceManager.class); public TestController(boolean generationAware) { @@ -141,7 +142,7 @@ public TestConfiguration(boolean generationAware) { null, TestCustomResource.class, null, - null, null); + null); } } } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/simple/TestCustomResourceSpec.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/simple/TestCustomResourceSpec.java index 5fd9f49084..5c23cc4c95 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/simple/TestCustomResourceSpec.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/simple/TestCustomResourceSpec.java @@ -1,5 +1,7 @@ package io.javaoperatorsdk.operator.sample.simple; +import java.util.Objects; + public class TestCustomResourceSpec { private String configMapName; @@ -46,4 +48,22 @@ public String toString() { + '\'' + '}'; } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + TestCustomResourceSpec that = (TestCustomResourceSpec) o; + return Objects.equals(configMapName, that.configMapName) && Objects.equals( + key, that.key) && Objects.equals(value, that.value); + } + + @Override + public int hashCode() { + return Objects.hash(configMapName, key, value); + } } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/simple/TestCustomResourceStatus.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/simple/TestCustomResourceStatus.java index 620bbaabd8..ab5559d80c 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/simple/TestCustomResourceStatus.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/simple/TestCustomResourceStatus.java @@ -1,5 +1,7 @@ package io.javaoperatorsdk.operator.sample.simple; +import java.util.Objects; + public class TestCustomResourceStatus { private String configMapStatus; @@ -16,4 +18,21 @@ public void setConfigMapStatus(String configMapStatus) { public String toString() { return "TestCustomResourceStatus{" + "configMapStatus='" + configMapStatus + '\'' + '}'; } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + TestCustomResourceStatus that = (TestCustomResourceStatus) o; + return Objects.equals(configMapStatus, that.configMapStatus); + } + + @Override + public int hashCode() { + return Objects.hash(configMapStatus); + } } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/DefaultConfigurationServiceTest.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/DefaultConfigurationServiceTest.java index a99c3d5020..6fd626b94b 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/DefaultConfigurationServiceTest.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/DefaultConfigurationServiceTest.java @@ -12,7 +12,6 @@ import io.fabric8.kubernetes.model.annotation.Group; import io.fabric8.kubernetes.model.annotation.Version; import io.javaoperatorsdk.operator.ReconcilerUtils; -import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; @@ -28,11 +27,6 @@ class DefaultConfigurationServiceTest { public static final String CUSTOM_FINALIZER_NAME = "a.custom/finalizer"; final DefaultConfigurationService configurationService = DefaultConfigurationService.instance(); - @Test - void defaultConfigurationServiceIsSetByDefault() { - assertEquals(DefaultConfigurationService.instance(), ConfigurationServiceProvider.instance()); - } - @Test void attemptingToRetrieveAnUnknownControllerShouldLogWarning() { final LoggerContext context = LoggerContext.getContext(false); From 056f80de1aa008bb41a58ba9dc331230878a0f97 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Thu, 10 Mar 2022 18:20:12 +0100 Subject: [PATCH 09/19] fix: lambda form doesn't record parameter type --- .../operator/api/config/ControllerConfigurationTest.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationTest.java index a2eeacf82b..05aa637f1f 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationTest.java @@ -10,7 +10,12 @@ class ControllerConfigurationTest { @Test void getCustomResourceClass() { - final ControllerConfiguration conf = () -> null; + final ControllerConfiguration conf = new ControllerConfiguration<>() { + @Override + public String getAssociatedReconcilerClassName() { + return null; + } + }; assertEquals(TestCustomResource.class, conf.getResourceClass()); } } From 23cb8de4ab361f698252810d3128ed6fe7695e09 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Thu, 10 Mar 2022 18:22:38 +0100 Subject: [PATCH 10/19] fix: configure and properly reset ConfigurationServiceProvider --- .../event/ReconciliationDispatcherTest.java | 47 +++++++++++-------- 1 file changed, 28 insertions(+), 19 deletions(-) diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java index 7db0672bbf..119b863bfa 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java @@ -6,6 +6,8 @@ import java.util.concurrent.TimeUnit; import java.util.function.BiFunction; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; @@ -17,7 +19,7 @@ import io.javaoperatorsdk.operator.MockKubernetesClient; import io.javaoperatorsdk.operator.TestUtils; import io.javaoperatorsdk.operator.api.config.Cloner; -import io.javaoperatorsdk.operator.api.config.ConfigurationService; +import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.config.RetryConfiguration; import io.javaoperatorsdk.operator.api.reconciler.Constants; @@ -56,10 +58,34 @@ class ReconciliationDispatcherTest { private TestCustomResource testCustomResource; private ReconciliationDispatcher reconciliationDispatcher; private TestReconciler reconciler; - private final ConfigurationService configService = mock(ConfigurationService.class); private final CustomResourceFacade customResourceFacade = mock(ReconciliationDispatcher.CustomResourceFacade.class); + @BeforeAll + static void classSetup() { + /* + * We need this for mock reconcilers to properly generate the expected UpdateControl: without + * this, calls such as `when(reconciler.reconcile(eq(testCustomResource), + * any())).thenReturn(UpdateControl.updateStatus(testCustomResource))` will return null because + * equals will fail on the two equal but NOT identical TestCustomResources because equals is not + * implemented on TestCustomResourceSpec or TestCustomResourceStatus + */ + ConfigurationServiceProvider.overrideCurrent(overrider -> { + overrider.checkingCRDAndValidateLocalModel(false) + .withResourceCloner(new Cloner() { + @Override + public R clone(R object) { + return object; + } + }); + }); + } + + @AfterAll + static void tearDown() { + ConfigurationServiceProvider.reset(); + } + @BeforeEach void setup() { testCustomResource = TestUtils.testCustomResource(); @@ -82,23 +108,6 @@ private ReconciliationDispatcher init(R customResourc when(configuration.reconciliationMaxInterval()) .thenReturn(Optional.of(Duration.ofHours(RECONCILIATION_MAX_INTERVAL))); - when(configuration.getConfigurationService()).thenReturn(configService); - - - /* - * We need this for mock reconcilers to properly generate the expected UpdateControl: without - * this, calls such as `when(reconciler.reconcile(eq(testCustomResource), - * any())).thenReturn(UpdateControl.updateStatus(testCustomResource))` will return null because - * equals will fail on the two equal but NOT identical TestCustomResources because equals is not - * implemented on TestCustomResourceSpec or TestCustomResourceStatus - */ - when(configService.getResourceCloner()).thenReturn(new Cloner() { - @Override - - public T clone(T object) { - return object; - } - }); Controller controller = new Controller<>(reconciler, configuration, MockKubernetesClient.client(customResource.getClass())); controller.start(); From 5ebf42e09a2a0c93b0b9d30cf588e598f16096a9 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Thu, 10 Mar 2022 18:23:32 +0100 Subject: [PATCH 11/19] fix: suppliers were improperly defined --- .../event/ReconciliationDispatcherTest.java | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java index 119b863bfa..5e230d0418 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java @@ -301,7 +301,7 @@ public boolean isLastAttempt() { verify(reconciler, times(1)) .reconcile(any(), contextArgumentCaptor.capture()); Context context = contextArgumentCaptor.getValue(); - final var retryInfo = context.getRetryInfo().orElseGet(fail("Missing optional")); + final var retryInfo = context.getRetryInfo().orElseGet(() -> fail("Missing optional")); assertThat(retryInfo.getAttemptCount()).isEqualTo(2); assertThat(retryInfo.isLastAttempt()).isEqualTo(true); } @@ -316,7 +316,8 @@ void setReScheduleToPostExecutionControlFromUpdateControl() { PostExecutionControl control = reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); - assertThat(control.getReScheduleDelay().orElseGet(fail("Missing optional"))).isEqualTo(1000L); + assertThat(control.getReScheduleDelay().orElseGet(() -> fail("Missing optional"))) + .isEqualTo(1000L); } @Test @@ -330,7 +331,8 @@ void reScheduleOnDeleteWithoutFinalizerRemoval() { PostExecutionControl control = reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); - assertThat(control.getReScheduleDelay().orElseGet(fail("Missing optional"))).isEqualTo(1000L); + assertThat(control.getReScheduleDelay().orElseGet(() -> fail("Missing optional"))) + .isEqualTo(1000L); } @Test @@ -350,7 +352,7 @@ void setObservedGenerationForStatusIfNeeded() { PostExecutionControl control = dispatcher.handleExecution( executionScopeWithCREvent(observedGenResource)); - assertThat(control.getUpdatedCustomResource().orElseGet(fail("Missing optional")) + assertThat(control.getUpdatedCustomResource().orElseGet(() -> fail("Missing optional")) .getStatus().getObservedGeneration()) .isEqualTo(1L); } @@ -371,7 +373,7 @@ void updatesObservedGenerationOnNoUpdateUpdateControl() { PostExecutionControl control = dispatcher.handleExecution( executionScopeWithCREvent(observedGenResource)); - assertThat(control.getUpdatedCustomResource().orElseGet(fail("Missing optional")) + assertThat(control.getUpdatedCustomResource().orElseGet(() -> fail("Missing optional")) .getStatus().getObservedGeneration()) .isEqualTo(1L); } @@ -393,7 +395,7 @@ void updateObservedGenerationOnCustomResourceUpdate() { PostExecutionControl control = dispatcher.handleExecution( executionScopeWithCREvent(observedGenResource)); - assertThat(control.getUpdatedCustomResource().orElseGet(fail("Missing optional")) + assertThat(control.getUpdatedCustomResource().orElseGet(() -> fail("Missing optional")) .getStatus().getObservedGeneration()) .isEqualTo(1L); } From bd720d9edc921c9d419799439ca3563610c74735 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Thu, 10 Mar 2022 18:24:28 +0100 Subject: [PATCH 12/19] fix: properly override ConfigurationService for the test and reset after --- .../operator/processing/ControllerTest.java | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java index 384925453d..b505a51c54 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java @@ -5,7 +5,7 @@ import io.fabric8.kubernetes.api.model.Secret; import io.javaoperatorsdk.operator.MissingCRDException; import io.javaoperatorsdk.operator.MockKubernetesClient; -import io.javaoperatorsdk.operator.api.config.ConfigurationService; +import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; import io.javaoperatorsdk.operator.sample.simple.TestCustomResource; @@ -19,15 +19,12 @@ @SuppressWarnings("unchecked") class ControllerTest { - @Test void crdShouldNotBeCheckedForNativeResources() { final var client = MockKubernetesClient.client(Secret.class); - final var configurationService = mock(ConfigurationService.class); final var reconciler = mock(Reconciler.class); final var configuration = mock(ControllerConfiguration.class); when(configuration.getResourceClass()).thenReturn(Secret.class); - when(configuration.getConfigurationService()).thenReturn(configurationService); final var controller = new Controller(reconciler, configuration, client); controller.start(); @@ -37,16 +34,18 @@ void crdShouldNotBeCheckedForNativeResources() { @Test void crdShouldNotBeCheckedForCustomResourcesIfDisabled() { final var client = MockKubernetesClient.client(TestCustomResource.class); - final var configurationService = mock(ConfigurationService.class); - when(configurationService.checkCRDAndValidateLocalModel()).thenReturn(false); final var reconciler = mock(Reconciler.class); final var configuration = mock(ControllerConfiguration.class); when(configuration.getResourceClass()).thenReturn(TestCustomResource.class); - when(configuration.getConfigurationService()).thenReturn(configurationService); - final var controller = new Controller(reconciler, configuration, client); - controller.start(); - verify(client, never()).apiextensions(); + try { + ConfigurationServiceProvider.overrideCurrent(o -> o.checkingCRDAndValidateLocalModel(false)); + final var controller = new Controller(reconciler, configuration, client); + controller.start(); + verify(client, never()).apiextensions(); + } finally { + ConfigurationServiceProvider.reset(); + } } @Test From 9f22a06434ec94a55c19901d4015448e26eb0e5a Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Thu, 10 Mar 2022 18:27:59 +0100 Subject: [PATCH 13/19] feat: improve ConfigurationServiceProvider implementation, add tests --- .../io/javaoperatorsdk/operator/Operator.java | 14 ++++ .../config/ConfigurationServiceOverrider.java | 13 ++-- .../config/ConfigurationServiceProvider.java | 57 ++++++++++++----- .../ConfigurationServiceProviderTest.java | 64 +++++++++++++++++++ .../operator/sample/MySQLSchemaOperator.java | 5 +- .../sample/PureJavaApplicationRunner.java | 8 +-- .../operator/sample/Config.java | 3 +- 7 files changed, 127 insertions(+), 37 deletions(-) create mode 100644 operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProviderTest.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java index 2e535b370f..ee7503fd91 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java @@ -14,6 +14,7 @@ import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.Version; import io.javaoperatorsdk.operator.api.config.ConfigurationService; +import io.javaoperatorsdk.operator.api.config.ConfigurationServiceOverrider; import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.config.ControllerConfigurationOverrider; @@ -37,10 +38,23 @@ public Operator(KubernetesClient kubernetesClient) { this(kubernetesClient, ConfigurationServiceProvider.instance()); } + /** + * @deprecated Use {@link #Operator(Consumer)} instead + */ + @Deprecated public Operator(ConfigurationService configurationService) { this(new DefaultKubernetesClient(), configurationService); } + public Operator(Consumer overrider) { + this(new DefaultKubernetesClient(), overrider); + } + + public Operator(KubernetesClient client, Consumer overrider) { + this(client); + ConfigurationServiceProvider.overrideCurrent(overrider); + } + /** * Note that Operator by default closes the client on stop, this can be changed using * {@link ConfigurationService} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java index cc89f2550e..9beb583d76 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java @@ -2,10 +2,12 @@ import java.util.Set; import java.util.concurrent.ExecutorService; +import java.util.function.Consumer; import io.fabric8.kubernetes.client.Config; import io.javaoperatorsdk.operator.api.monitoring.Metrics; +@SuppressWarnings("unused") public class ConfigurationServiceOverrider { private final ConfigurationService original; private Metrics metrics; @@ -70,7 +72,7 @@ public ConfigurationServiceOverrider withExecutorService(ExecutorService executo } public ConfigurationService build() { - final var overridden = new BaseConfigurationService(original.getVersion()) { + return new BaseConfigurationService(original.getVersion()) { @Override public Set getKnownReconcilerNames() { return original.getKnownReconcilerNames(); @@ -120,17 +122,10 @@ public ExecutorService getExecutorService() { } } }; - // make sure we set the overridden version on the provider so that it is made available - ConfigurationServiceProvider.set(overridden, true); - return overridden; - } - - public static ConfigurationServiceOverrider overrideCurrent() { - return new ConfigurationServiceOverrider(ConfigurationServiceProvider.instance()); } /** - * @deprecated Use {@link #overrideCurrent()} instead + * @deprecated Use {@link ConfigurationServiceProvider#overrideCurrent(Consumer)} instead */ @Deprecated(since = "2.2.0") public static ConfigurationServiceOverrider override(ConfigurationService original) { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProvider.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProvider.java index b092c087a6..1e6dd660cf 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProvider.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProvider.java @@ -1,42 +1,67 @@ package io.javaoperatorsdk.operator.api.config; +import java.util.function.Consumer; + public class ConfigurationServiceProvider { - private static ConfigurationService instance; - private static ConfigurationService defaultConfigurationService = + static final ConfigurationService DEFAULT = new BaseConfigurationService(Utils.loadFromProperties()); + private static ConfigurationService instance; + private static ConfigurationService defaultConfigurationService = DEFAULT; private static boolean alreadyConfigured = false; - public static ConfigurationService instance() { + private ConfigurationServiceProvider() {} + + public synchronized static ConfigurationService instance() { if (instance == null) { set(defaultConfigurationService); } return instance; } - public static void set(ConfigurationService instance) { + public synchronized static void set(ConfigurationService instance) { set(instance, false); } - static void set(ConfigurationService instance, boolean overriding) { - if ((overriding && alreadyConfigured) || - (ConfigurationServiceProvider.instance != null - && !ConfigurationServiceProvider.instance.equals(instance))) { - throw new IllegalStateException( - "A ConfigurationService has already been set and cannot be set again. Current: " - + ConfigurationServiceProvider.instance.getClass().getCanonicalName()); + private static void set(ConfigurationService instance, boolean overriding) { + final var current = ConfigurationServiceProvider.instance; + if (!overriding) { + if (current != null && !current.equals(instance)) { + throw new IllegalStateException( + "A ConfigurationService has already been set and cannot be set again. Current: " + + current.getClass().getCanonicalName()); + } + } else { + if (alreadyConfigured) { + throw new IllegalStateException( + "The ConfigurationService has already been overridden once and cannot be changed again. Current: " + + current.getClass().getCanonicalName()); + } else { + alreadyConfigured = true; + } } - if (overriding) { - alreadyConfigured = true; - } ConfigurationServiceProvider.instance = instance; } - public static void setDefault(ConfigurationService defaultConfigurationService) { + public synchronized static void overrideCurrent( + Consumer overrider) { + final var toOverride = + new ConfigurationServiceOverrider(ConfigurationServiceProvider.instance()); + overrider.accept(toOverride); + ConfigurationServiceProvider.set(toOverride.build(), true); + } + + public synchronized static void setDefault(ConfigurationService defaultConfigurationService) { ConfigurationServiceProvider.defaultConfigurationService = defaultConfigurationService; } - public static void reset() { + synchronized static ConfigurationService getDefault() { + return defaultConfigurationService; + } + + public synchronized static void reset() { + defaultConfigurationService = DEFAULT; instance = null; + alreadyConfigured = false; } } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProviderTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProviderTest.java new file mode 100644 index 0000000000..b933ddccee --- /dev/null +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProviderTest.java @@ -0,0 +1,64 @@ +package io.javaoperatorsdk.operator.api.config; + +import java.time.Instant; +import java.util.Date; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.*; + +class ConfigurationServiceProviderTest { + @BeforeEach + void resetProvider() { + ConfigurationServiceProvider.reset(); + } + + @Test + void shouldProvideADefaultInstanceIfNoneIsSet() { + final var instance = ConfigurationServiceProvider.instance(); + assertNotNull(instance); + assertTrue(instance instanceof BaseConfigurationService); + } + + @Test + void shouldProvideTheSetDefaultInstanceIfProvided() { + final var defaultConfig = new AbstractConfigurationService(null); + ConfigurationServiceProvider.setDefault(defaultConfig); + assertEquals(defaultConfig, ConfigurationServiceProvider.instance()); + } + + @Test + void shouldProvideTheSetInstanceIfProvided() { + final var config = new AbstractConfigurationService(null); + ConfigurationServiceProvider.set(config); + assertEquals(config, ConfigurationServiceProvider.instance()); + } + + @Test + void shouldBePossibleToOverrideConfigOnce() { + final var config = new AbstractConfigurationService(null); + assertTrue(config.checkCRDAndValidateLocalModel()); + + ConfigurationServiceProvider.set(config); + var instance = ConfigurationServiceProvider.instance(); + assertEquals(config, instance); + + ConfigurationServiceProvider.overrideCurrent(o -> o.checkingCRDAndValidateLocalModel(false)); + instance = ConfigurationServiceProvider.instance(); + assertNotEquals(config, instance); + assertFalse(instance.checkCRDAndValidateLocalModel()); + + assertThrows(IllegalStateException.class, + () -> ConfigurationServiceProvider.overrideCurrent(o -> o.withCloseClientOnStop(false))); + } + + @Test + void resetShouldResetAllState() { + shouldBePossibleToOverrideConfigOnce(); + + ConfigurationServiceProvider.reset(); + assertEquals(ConfigurationServiceProvider.DEFAULT, ConfigurationServiceProvider.getDefault()); + + shouldBePossibleToOverrideConfigOnce(); + } +} diff --git a/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperator.java b/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperator.java index 3b6078a953..0a5703a676 100644 --- a/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperator.java +++ b/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperator.java @@ -14,7 +14,6 @@ import io.fabric8.kubernetes.client.DefaultKubernetesClient; import io.fabric8.kubernetes.client.KubernetesClient; import io.javaoperatorsdk.operator.Operator; -import io.javaoperatorsdk.operator.api.config.ConfigurationServiceOverrider; import io.javaoperatorsdk.operator.monitoring.micrometer.MicrometerMetrics; import io.javaoperatorsdk.operator.sample.dependent.ResourcePollerConfig; import io.javaoperatorsdk.operator.sample.dependent.SchemaDependentResource; @@ -30,9 +29,7 @@ public static void main(String[] args) throws IOException { Config config = new ConfigBuilder().withNamespace(null).build(); KubernetesClient client = new DefaultKubernetesClient(config); Operator operator = new Operator(client, - ConfigurationServiceOverrider.overrideCurrent() - .withMetrics(new MicrometerMetrics(new LoggingMeterRegistry())) - .build()); + overrider -> overrider.withMetrics(new MicrometerMetrics(new LoggingMeterRegistry()))); MySQLSchemaReconciler schemaReconciler = new MySQLSchemaReconciler(); diff --git a/smoke-test-samples/pure-java/src/main/java/io/javaoperatorsdk/operator/sample/PureJavaApplicationRunner.java b/smoke-test-samples/pure-java/src/main/java/io/javaoperatorsdk/operator/sample/PureJavaApplicationRunner.java index e80e13151c..0a3b59d290 100644 --- a/smoke-test-samples/pure-java/src/main/java/io/javaoperatorsdk/operator/sample/PureJavaApplicationRunner.java +++ b/smoke-test-samples/pure-java/src/main/java/io/javaoperatorsdk/operator/sample/PureJavaApplicationRunner.java @@ -3,17 +3,13 @@ import java.util.concurrent.Executors; import io.javaoperatorsdk.operator.Operator; -import io.javaoperatorsdk.operator.api.config.ConfigurationServiceOverrider; public class PureJavaApplicationRunner { public static void main(String[] args) { Operator operator = - new Operator( - ConfigurationServiceOverrider.overrideCurrent() - .withExecutorService(Executors.newCachedThreadPool()) - .withConcurrentReconciliationThreads(2) - .build()); + new Operator(overrider -> overrider.withExecutorService(Executors.newCachedThreadPool()) + .withConcurrentReconciliationThreads(2)); operator.register(new CustomServiceReconciler()); operator.start(); } diff --git a/smoke-test-samples/spring-boot-plain/src/main/java/io/javaoperatorsdk/operator/sample/Config.java b/smoke-test-samples/spring-boot-plain/src/main/java/io/javaoperatorsdk/operator/sample/Config.java index 9b07d4d61d..7eed511d18 100644 --- a/smoke-test-samples/spring-boot-plain/src/main/java/io/javaoperatorsdk/operator/sample/Config.java +++ b/smoke-test-samples/spring-boot-plain/src/main/java/io/javaoperatorsdk/operator/sample/Config.java @@ -6,7 +6,6 @@ import org.springframework.context.annotation.Configuration; import io.javaoperatorsdk.operator.Operator; -import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; @Configuration @@ -21,7 +20,7 @@ public CustomServiceReconciler customServiceController() { @Bean(initMethod = "start", destroyMethod = "stop") @SuppressWarnings("rawtypes") public Operator operator(List controllers) { - Operator operator = new Operator(ConfigurationServiceProvider.instance()); + Operator operator = new Operator(); controllers.forEach(operator::register); return operator; } From 6c22407594ac241844b1c5688914978c1c868829 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Thu, 10 Mar 2022 20:28:11 +0100 Subject: [PATCH 14/19] fix: format --- .../operator/api/config/ConfigurationServiceProviderTest.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProviderTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProviderTest.java index b933ddccee..f69dd0bc29 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProviderTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProviderTest.java @@ -1,7 +1,5 @@ package io.javaoperatorsdk.operator.api.config; -import java.time.Instant; -import java.util.Date; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; From 635f4919ca722f2497e85f6bb5795ac3f119b843 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Thu, 10 Mar 2022 20:44:34 +0100 Subject: [PATCH 15/19] fix: Operator should only set ConfigurationService, not hold onto it --- .../java/io/javaoperatorsdk/operator/Operator.java | 14 ++++++++------ .../io/javaoperatorsdk/operator/OperatorTest.java | 14 ++++++++++++++ 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java index ee7503fd91..bcd0d7ade0 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java @@ -27,7 +27,6 @@ public class Operator implements LifecycleAware { private static final Logger log = LoggerFactory.getLogger(Operator.class); private final KubernetesClient kubernetesClient; - private final ConfigurationService configurationService; private final ControllerManager controllers = new ControllerManager(); public Operator() { @@ -64,7 +63,6 @@ public Operator(KubernetesClient client, Consumer */ public Operator(KubernetesClient kubernetesClient, ConfigurationService configurationService) { this.kubernetesClient = kubernetesClient; - this.configurationService = configurationService; ConfigurationServiceProvider.set(configurationService); } @@ -89,7 +87,7 @@ public List getControllers() { public void start() { controllers.shouldStart(); - final var version = configurationService.getVersion(); + final var version = ConfigurationServiceProvider.instance().getVersion(); log.info( "Operator SDK {} (commit: {}) built on {} starting...", version.getSdkVersion(), @@ -105,6 +103,7 @@ public void start() { @Override public void stop() throws OperatorException { + final var configurationService = ConfigurationServiceProvider.instance(); log.info( "Operator SDK {} is shutting down...", configurationService.getVersion().getSdkVersion()); @@ -126,7 +125,8 @@ public void stop() throws OperatorException { */ public void register(Reconciler reconciler) throws OperatorException { - final var controllerConfiguration = configurationService.getConfigurationFor(reconciler); + final var controllerConfiguration = + ConfigurationServiceProvider.instance().getConfigurationFor(reconciler); register(reconciler, controllerConfiguration); } @@ -151,7 +151,8 @@ public void register(Reconciler reconciler, "Cannot register reconciler with name " + reconciler.getClass().getCanonicalName() + " reconciler named " + ReconcilerUtils.getNameFor(reconciler) + " because its configuration cannot be found.\n" + - " Known reconcilers are: " + configurationService.getKnownReconcilerNames()); + " Known reconcilers are: " + + ConfigurationServiceProvider.instance().getKnownReconcilerNames()); } final var controller = new Controller<>(reconciler, configuration, kubernetesClient); @@ -177,7 +178,8 @@ public void register(Reconciler reconciler, */ public void register(Reconciler reconciler, Consumer> configOverrider) { - final var controllerConfiguration = configurationService.getConfigurationFor(reconciler); + final var controllerConfiguration = + ConfigurationServiceProvider.instance().getConfigurationFor(reconciler); var configToOverride = ControllerConfigurationOverrider.override(controllerConfiguration); configOverrider.accept(configToOverride); register(reconciler, configToOverride.build()); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/OperatorTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/OperatorTest.java index 43bb0b788d..f4de6b2141 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/OperatorTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/OperatorTest.java @@ -1,11 +1,15 @@ package io.javaoperatorsdk.operator; +import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import io.fabric8.kubernetes.api.model.ConfigMap; import io.fabric8.kubernetes.client.KubernetesClient; +import io.javaoperatorsdk.operator.api.config.AbstractConfigurationService; +import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; @@ -23,6 +27,12 @@ class OperatorTest { private final Operator operator = new Operator(kubernetesClient); private final FooReconciler fooReconciler = new FooReconciler(); + @BeforeAll + @AfterAll + static void setUpConfigurationServiceProvider() { + ConfigurationServiceProvider.reset(); + } + @Test @DisplayName("should register `Reconciler` to Controller") @SuppressWarnings("unchecked") @@ -41,6 +51,10 @@ public void shouldRegisterReconcilerToController() { @Test @DisplayName("should throw `OperationException` when Configuration is null") public void shouldThrowOperatorExceptionWhenConfigurationIsNull() { + // use a ConfigurationService that doesn't automatically create configurations + ConfigurationServiceProvider.reset(); + ConfigurationServiceProvider.set(new AbstractConfigurationService(null)); + Assertions.assertThrows(OperatorException.class, () -> operator.register(fooReconciler)); } From b44086e4e642809a06bbd09d07a768a6f8ebe4c4 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Thu, 10 Mar 2022 21:28:05 +0100 Subject: [PATCH 16/19] fix: more ConfigurationService holding removal --- .../AnnotationControllerConfiguration.java | 4 ++-- .../api/config/DefaultResourceConfiguration.java | 4 ---- .../operator/api/reconciler/DefaultContext.java | 3 +-- .../KubernetesDependentResourceConfig.java | 16 +--------------- 4 files changed, 4 insertions(+), 23 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 0d64a16ef7..963f68d499 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 @@ -161,8 +161,8 @@ public static T valueOrDefault( kubeDependent, KubernetesDependent::addOwnerReference, KubernetesDependent.ADD_OWNER_REFERENCE_DEFAULT); - config = new KubernetesDependentResourceConfig( - addOwnerReference, namespaces, labelSelector, getConfigurationService()); + config = + new KubernetesDependentResourceConfig(addOwnerReference, namespaces, labelSelector); } specs.add(new DependentResourceSpec(dependentType, config)); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultResourceConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultResourceConfiguration.java index 6e523c587c..4959b827ad 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultResourceConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultResourceConfiguration.java @@ -47,10 +47,6 @@ public boolean watchAllNamespaces() { return watchAllNamespaces; } - public ConfigurationService getConfigurationService() { - return ConfigurationServiceProvider.instance(); - } - @Override public Class getResourceClass() { return resourceClass; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java index 14d737f87b..d3a5b7bc18 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java @@ -4,7 +4,6 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; -import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; import io.javaoperatorsdk.operator.api.reconciler.dependent.ManagedDependentResourceContext; import io.javaoperatorsdk.operator.processing.Controller; @@ -14,7 +13,7 @@ public class DefaultContext

implements Context

{ private final Controller

controller; private final P primaryResource; private final ControllerConfiguration

controllerConfiguration; - private ManagedDependentResourceContext managedDependentResourceContext; + private final ManagedDependentResourceContext managedDependentResourceContext; public DefaultContext(RetryInfo retryInfo, Controller

controller, P primaryResource) { this.retryInfo = retryInfo; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResourceConfig.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResourceConfig.java index 4464f3fd7d..3a7bd45025 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResourceConfig.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResourceConfig.java @@ -1,7 +1,5 @@ package io.javaoperatorsdk.operator.processing.dependent.kubernetes; -import io.javaoperatorsdk.operator.api.config.ConfigurationService; - import static io.javaoperatorsdk.operator.api.reconciler.Constants.EMPTY_STRING; import static io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependent.ADD_OWNER_REFERENCE_DEFAULT; @@ -10,16 +8,14 @@ public class KubernetesDependentResourceConfig { private boolean addOwnerReference = ADD_OWNER_REFERENCE_DEFAULT; private String[] namespaces = new String[0]; private String labelSelector = EMPTY_STRING; - private ConfigurationService configurationService; public KubernetesDependentResourceConfig() {} public KubernetesDependentResourceConfig(boolean addOwnerReference, String[] namespaces, - String labelSelector, ConfigurationService configurationService) { + String labelSelector) { this.addOwnerReference = addOwnerReference; this.namespaces = namespaces; this.labelSelector = labelSelector; - this.configurationService = configurationService; } public KubernetesDependentResourceConfig setAddOwnerReference( @@ -38,12 +34,6 @@ public KubernetesDependentResourceConfig setLabelSelector(String labelSelector) return this; } - public KubernetesDependentResourceConfig setConfigurationService( - ConfigurationService configurationService) { - this.configurationService = configurationService; - return this; - } - public boolean addOwnerReference() { return addOwnerReference; } @@ -55,8 +45,4 @@ public String[] namespaces() { public String labelSelector() { return labelSelector; } - - public ConfigurationService getConfigurationService() { - return configurationService; - } } From 40d9124d68b8be593783ccfedb4e9c89a7ca2fce Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Fri, 11 Mar 2022 10:50:06 +0100 Subject: [PATCH 17/19] fix: make sure that ConfigurationServiceProvider is reset If we decide to go along with this work, we will probably need a JUnit extension to always reset the ConfigurationServiceProvider before and after each test automatically. --- .../operator/api/config/AnnotationControllerConfiguration.java | 2 +- .../io/javaoperatorsdk/operator/processing/ControllerTest.java | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) 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 963f68d499..72d34e16a8 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 @@ -161,7 +161,7 @@ public static T valueOrDefault( kubeDependent, KubernetesDependent::addOwnerReference, KubernetesDependent.ADD_OWNER_REFERENCE_DEFAULT); - config = + config = new KubernetesDependentResourceConfig(addOwnerReference, namespaces, labelSelector); } specs.add(new DependentResourceSpec(dependentType, config)); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java index b505a51c54..1e889023ac 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java @@ -50,6 +50,7 @@ void crdShouldNotBeCheckedForCustomResourcesIfDisabled() { @Test void crdShouldBeCheckedForCustomResourcesByDefault() { + ConfigurationServiceProvider.reset(); final var client = MockKubernetesClient.client(TestCustomResource.class); final var reconciler = mock(Reconciler.class); final var configuration = mock(ControllerConfiguration.class); From aadba2b05afa8cd2cf4fab98919b89bcf3d1d0f0 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Fri, 11 Mar 2022 14:44:40 +0100 Subject: [PATCH 18/19] refactor: remove configurationService method --- .../javaoperatorsdk/operator/processing/Controller.java | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java index 75e545f7a7..d769324be9 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java @@ -16,7 +16,6 @@ import io.javaoperatorsdk.operator.CustomResourceUtils; import io.javaoperatorsdk.operator.MissingCRDException; import io.javaoperatorsdk.operator.OperatorException; -import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.monitoring.Metrics; @@ -122,7 +121,7 @@ public UpdateControl

execute() { private Metrics metrics() { - final var metrics = configurationService().getMetrics(); + final var metrics = ConfigurationServiceProvider.instance().getMetrics(); return metrics != null ? metrics : Metrics.NOOP; } @@ -199,7 +198,7 @@ public void start() throws OperatorException { try { // check that the custom resource is known by the cluster if configured that way final CustomResourceDefinition crd; // todo: check proper CRD spec version based on config - if (configurationService().checkCRDAndValidateLocalModel() + if (ConfigurationServiceProvider.instance().checkCRDAndValidateLocalModel() && CustomResource.class.isAssignableFrom(resClass)) { crd = kubernetesClient.apiextensions().v1().customResourceDefinitions().withName(crdName) .get(); @@ -224,10 +223,6 @@ public void start() throws OperatorException { } } - private ConfigurationService configurationService() { - return ConfigurationServiceProvider.instance(); - } - private void throwMissingCRDException(String crdName, String specVersion, String controllerName) { throw new MissingCRDException( crdName, From 64659e6e5d21ba852375e3e6182f798c611b813f Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Fri, 11 Mar 2022 15:08:37 +0100 Subject: [PATCH 19/19] refactor: more access to configuration via ConfigurationServiceProvider --- .../dependent/ResourceUpdatePreProcessor.java | 2 +- .../kubernetes/GenericKubernetesResourceMatcher.java | 4 ++-- .../GenericResourceUpdatePreProcessor.java | 6 +++--- .../operator/processing/ControllerTest.java | 1 - .../GenericKubernetesResourceMatcherTest.java | 12 +++++------- .../GenericResourceUpdatePreProcessorTest.java | 9 ++++----- 6 files changed, 15 insertions(+), 19 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/ResourceUpdatePreProcessor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/ResourceUpdatePreProcessor.java index df2541aae9..55ebcca230 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/ResourceUpdatePreProcessor.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/ResourceUpdatePreProcessor.java @@ -5,5 +5,5 @@ public interface ResourceUpdatePreProcessor { - R replaceSpecOnActual(R actual, R desired, Context context); + R replaceSpecOnActual(R actual, R desired, Context context); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcher.java index 5bde08b0aa..0bbe19837e 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcher.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcher.java @@ -5,6 +5,7 @@ import io.fabric8.kubernetes.api.model.Secret; import io.fabric8.zjsonpatch.JsonDiff; import io.javaoperatorsdk.operator.ReconcilerUtils; +import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.api.reconciler.dependent.Matcher; @@ -40,8 +41,7 @@ static Matcher matcherFor( @Override public Result match(R actualResource, P primary, Context

context) { - final var objectMapper = - context.getControllerConfiguration().getConfigurationService().getObjectMapper(); + final var objectMapper = ConfigurationServiceProvider.instance().getObjectMapper(); final var desired = dependentResource.desired(primary, context); // reflection will be replaced by this: diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericResourceUpdatePreProcessor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericResourceUpdatePreProcessor.java index ce73a409cd..0e0624d183 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericResourceUpdatePreProcessor.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericResourceUpdatePreProcessor.java @@ -4,6 +4,7 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.api.model.Secret; import io.javaoperatorsdk.operator.ReconcilerUtils; +import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.api.reconciler.dependent.ResourceUpdatePreProcessor; @@ -43,9 +44,8 @@ protected void updateClonedActual(R actual, R desired) { } } - public R replaceSpecOnActual(R actual, R desired, Context context) { - var clonedActual = context.getControllerConfiguration().getConfigurationService() - .getResourceCloner().clone(actual); + public R replaceSpecOnActual(R actual, R desired, Context context) { + var clonedActual = ConfigurationServiceProvider.instance().getResourceCloner().clone(actual); updateClonedActual(clonedActual, desired); return clonedActual; } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java index 1e889023ac..ba31d6634e 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java @@ -55,7 +55,6 @@ void crdShouldBeCheckedForCustomResourcesByDefault() { final var reconciler = mock(Reconciler.class); final var configuration = mock(ControllerConfiguration.class); when(configuration.getResourceClass()).thenReturn(TestCustomResource.class); - when(configuration.getConfigurationService()).thenCallRealMethod(); final var controller = new Controller(reconciler, configuration, client); // since we're not really connected to a cluster and the CRD wouldn't be deployed anyway, we diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcherTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcherTest.java index 4621e4700e..1eb7ea1bb4 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcherTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcherTest.java @@ -2,30 +2,28 @@ import java.util.Optional; +import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.api.model.apps.Deployment; import io.fabric8.kubernetes.api.model.apps.DeploymentBuilder; import io.javaoperatorsdk.operator.ReconcilerUtils; -import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.Context; -import com.fasterxml.jackson.databind.ObjectMapper; - import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +@SuppressWarnings({"unchecked", "rawtypes"}) class GenericKubernetesResourceMatcherTest { private static final Context context = mock(Context.class); - static { - final var configurationService = mock(ConfigurationService.class); + + @BeforeAll + static void setUp() { final var controllerConfiguration = mock(ControllerConfiguration.class); - when(configurationService.getObjectMapper()).thenReturn(new ObjectMapper()); - when(controllerConfiguration.getConfigurationService()).thenReturn(configurationService); when(context.getControllerConfiguration()).thenReturn(controllerConfiguration); } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericResourceUpdatePreProcessorTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericResourceUpdatePreProcessorTest.java index 6c763d20fe..802be9c3d9 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericResourceUpdatePreProcessorTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericResourceUpdatePreProcessorTest.java @@ -2,11 +2,11 @@ import java.util.HashMap; +import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import io.fabric8.kubernetes.api.model.apps.Deployment; import io.javaoperatorsdk.operator.ReconcilerUtils; -import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.api.reconciler.dependent.ResourceUpdatePreProcessor; @@ -15,15 +15,14 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +@SuppressWarnings("rawtypes") class GenericResourceUpdatePreProcessorTest { private static final Context context = mock(Context.class); - static { - final var configurationService = mock(ConfigurationService.class); + @BeforeAll + static void setUp() { final var controllerConfiguration = mock(ControllerConfiguration.class); - when(controllerConfiguration.getConfigurationService()).thenReturn(configurationService); - when(configurationService.getResourceCloner()).thenReturn(ConfigurationService.DEFAULT_CLONER); when(context.getControllerConfiguration()).thenReturn(controllerConfiguration); }