From c70678e95b0e6af85194682780b28bae53e383e8 Mon Sep 17 00:00:00 2001 From: csviri Date: Thu, 13 Apr 2023 12:58:54 +0200 Subject: [PATCH 1/7] parent cc124e56586751af047fcc3f90dda3bfc69d7e04 author csviri 1681383534 +0200 committer csviri 1683271037 +0200 gpgsig -----BEGIN PGP SIGNATURE----- iQIzBAABCgAdFiEEx4CKPTleRaFiFbIenxwva5o3BHIFAmRUrX0ACgkQnxwva5o3 BHK1Jw/+PNwivEbJp3+A/F5rJeMBFsgPayBZAxuVqAlqd5U9uo5JlLkGddpMBf9q tFJyXaw/gYTO3B6XIEi8qg75l3hPJxo9rPauDT+s1t+zWpuQqrvpAatMi2cBeoaa Aluw2LYaKX75IEUW14ooGPwjzjnaOFs0rVPcBUgWnmQqPDFez/QC2Xz3rfGXhU2b P87fi/A5lZbXDXHaaWzWfJ4XoGMG4152KIo6EgM5QQGd+BD+qYm9qTATM3vVxjkQ PG9By9CC8u24mDVtYI9YAGLNINLk1poTC1/reVzn8cX2lVENLRWPB/mF2qc/2edu Z5LYkJ/7fp6qv4SS03QUC2WGycVXf9cLM/SHJiX7s87glE/rBocRVBHA1iBNbz2A AaiacSyQkkMdBbiVWfG7Xd//gPhAFqwFSpp6myWaqcsLEWhJ6F8J6uDTT1qjbF9W XviGSXugs/8U4BTpPTnh71w553/StnHHw24g9jnOGnpyPGvC5uqrDgyL4cgO4RfK lYvUy1APi/GAMEr7iCLXP19CFuT/QhnRzcyzU6hEFk9Pz/l2U2HHwvct7c+Snx/f jZ2bxVOZuZMz26w+lvlmhrZWmOdRM0CQYd6/0CzyLuTZ24LgBqlujfqYC0J9zTrZ NtUTT7Up6cymBbqpwyrbITEdM+3o+EmKccXwF4Qrpl/Zj0MaJCM= =c5ON -----END PGP SIGNATURE----- chore: update version to 4.4.0-SNAPSHOT fix: bom version improve: configuration service no static access wip compiles wip unit tests passing IT fixes Fixes comment improvements fix workflow execution issue leader election fix rebase next, test fix improve: jetty log level to info (#1882) chore: update version to 4.4.0-SNAPSHOT fix: bom version fix: avoid potential NPE refactor: rename more appropriately refactor: access configuration service from Operator when possible fix: now outdated method reference fix: format fix: workflows don't need to record the configuration service improve: dependent resource matcher API and handling (#1881) refactor: get ExecutorServiceManager from ConfigurationService feat: re-add parameter-less getEffectiveNamespaces method --- .../AbstractMicrometerMetricsTestFixture.java | 24 ++---- .../operator/ControllerManager.java | 12 ++- .../operator/LeaderElectionManager.java | 69 +++++++++-------- .../io/javaoperatorsdk/operator/Operator.java | 45 ++++++----- .../api/config/BaseConfigurationService.java | 4 +- .../api/config/ConfigurationService.java | 21 +++++- .../config/ConfigurationServiceOverrider.java | 6 +- .../config/ConfigurationServiceProvider.java | 75 ------------------- .../api/config/ControllerConfiguration.java | 10 ++- .../ControllerConfigurationOverrider.java | 3 +- .../api/config/ExecutorServiceManager.java | 57 ++------------ .../ResolvedControllerConfiguration.java | 21 ++++-- .../api/config/ResourceConfiguration.java | 4 +- .../informer/InformerConfiguration.java | 3 +- .../operator/api/reconciler/Context.java | 6 ++ .../api/reconciler/DefaultContext.java | 8 ++ .../operator/processing/Controller.java | 35 +++++++-- .../GenericKubernetesResourceMatcher.java | 19 +++-- .../GenericResourceUpdatePreProcessor.java | 5 +- .../KubernetesDependentResource.java | 3 +- .../workflow/AbstractWorkflowExecutor.java | 9 ++- .../workflow/DefaultManagedWorkflow.java | 6 +- .../dependent/workflow/DefaultWorkflow.java | 1 - .../workflow/ManagedWorkflowFactory.java | 21 +----- .../workflow/ManagedWorkflowSupport.java | 8 -- .../workflow/WorkflowCleanupExecutor.java | 13 ++-- .../workflow/WorkflowReconcileExecutor.java | 36 ++++----- .../processing/event/EventProcessor.java | 25 +++---- .../processing/event/EventSourceManager.java | 28 +++++-- .../processing/event/EventSources.java | 5 +- .../event/ReconciliationDispatcher.java | 5 +- .../ControllerResourceEventSource.java | 15 ++++ .../source/informer/InformerEventSource.java | 26 ++++++- .../source/informer/InformerManager.java | 26 ++++--- .../source/informer/InformerWrapper.java | 18 +++-- .../informer/ManagedInformerEventSource.java | 26 ++++--- .../operator/ControllerManagerTest.java | 17 +++-- .../operator/LeaderElectionManagerTest.java | 24 +++--- .../operator/OperatorTest.java | 53 ------------- .../ConfigurationServiceProviderTest.java | 70 ----------------- .../config/MockControllerConfiguration.java | 12 ++- .../operator/processing/ControllerTest.java | 41 +++++----- .../GenericKubernetesResourceMatcherTest.java | 3 + ...GenericResourceUpdatePreProcessorTest.java | 3 + .../workflow/ManagedWorkflowSupportTest.java | 2 +- .../workflow/ManagedWorkflowTest.java | 4 +- .../workflow/WorkflowCleanupExecutorTest.java | 16 +++- .../WorkflowReconcileExecutorTest.java | 13 ++++ .../dependent/workflow/WorkflowTest.java | 4 + .../processing/event/EventProcessorTest.java | 29 +++++-- .../event/EventSourceManagerTest.java | 8 ++ .../processing/event/EventSourcesTest.java | 4 +- .../event/ReconciliationDispatcherTest.java | 36 +++++---- .../source/AbstractEventSourceTestBase.java | 21 ++++++ .../event/source/ResourceEventFilterTest.java | 3 +- .../ControllerResourceEventSourceTest.java | 6 +- .../informer/InformerEventSourceTest.java | 41 +++++----- .../junit/AbstractOperatorExtension.java | 18 +++-- .../junit/LocallyRunOperatorExtension.java | 13 ++-- .../runtime/DefaultConfigurationService.java | 37 --------- .../operator/DependentResourceCrossRefIT.java | 2 + .../operator/InformerRelatedBehaviorITS.java | 3 +- .../StandaloneDependentResourceIT.java | 28 ++++++- .../DefaultConfigurationServiceTest.java | 2 +- 64 files changed, 596 insertions(+), 615 deletions(-) delete mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProvider.java delete mode 100644 operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProviderTest.java diff --git a/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/AbstractMicrometerMetricsTestFixture.java b/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/AbstractMicrometerMetricsTestFixture.java index aa67c75f76..8575f07243 100644 --- a/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/AbstractMicrometerMetricsTestFixture.java +++ b/micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/AbstractMicrometerMetricsTestFixture.java @@ -3,15 +3,12 @@ import java.util.HashSet; import java.util.Set; -import org.junit.jupiter.api.AfterAll; -import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInstance; import org.junit.jupiter.api.extension.RegisterExtension; import io.fabric8.kubernetes.api.model.ConfigMap; import io.fabric8.kubernetes.api.model.ConfigMapBuilder; -import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; import io.javaoperatorsdk.operator.api.reconciler.*; import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; import io.javaoperatorsdk.operator.processing.event.ResourceID; @@ -23,26 +20,21 @@ @TestInstance(TestInstance.Lifecycle.PER_CLASS) public abstract class AbstractMicrometerMetricsTestFixture { - @RegisterExtension - LocallyRunOperatorExtension operator = - LocallyRunOperatorExtension.builder().withReconciler(new MetricsCleaningTestReconciler()) - .build(); protected final TestSimpleMeterRegistry registry = new TestSimpleMeterRegistry(); protected final MicrometerMetrics metrics = getMetrics(); protected static final String testResourceName = "micrometer-metrics-cr"; - protected abstract MicrometerMetrics getMetrics(); - @BeforeAll - void setup() { - ConfigurationServiceProvider.overrideCurrent(overrider -> overrider.withMetrics(metrics)); - } + @RegisterExtension + LocallyRunOperatorExtension operator = + LocallyRunOperatorExtension.builder() + .withConfigurationService(overrider -> overrider.withMetrics(metrics)) + .withReconciler(new MetricsCleaningTestReconciler()) + .build(); - @AfterAll - void reset() { - ConfigurationServiceProvider.reset(); - } + + protected abstract MicrometerMetrics getMetrics(); @Test void properlyHandlesResourceDeletion() throws Exception { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ControllerManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ControllerManager.java index c9732892b9..5504e543f5 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ControllerManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ControllerManager.java @@ -22,6 +22,12 @@ class ControllerManager { @SuppressWarnings("rawtypes") private final Map controllers = new HashMap<>(); private boolean started = false; + private final ExecutorServiceManager executorServiceManager; + + public ControllerManager(ExecutorServiceManager executorServiceManager) { + this.executorServiceManager = executorServiceManager; + } + public synchronized void shouldStart() { if (started) { @@ -33,7 +39,7 @@ public synchronized void shouldStart() { } public synchronized void start(boolean startEventProcessor) { - ExecutorServiceManager.boundedExecuteAndWaitForAllToComplete(controllers().stream(), c -> { + executorServiceManager.boundedExecuteAndWaitForAllToComplete(controllers().stream(), c -> { c.start(startEventProcessor); return null; }, c -> "Controller Starter for: " + c.getConfiguration().getName()); @@ -41,7 +47,7 @@ public synchronized void start(boolean startEventProcessor) { } public synchronized void stop() { - ExecutorServiceManager.boundedExecuteAndWaitForAllToComplete(controllers().stream(), c -> { + executorServiceManager.boundedExecuteAndWaitForAllToComplete(controllers().stream(), c -> { log.debug("closing {}", c); c.stop(); return null; @@ -50,7 +56,7 @@ public synchronized void stop() { } public synchronized void startEventProcessing() { - ExecutorServiceManager.boundedExecuteAndWaitForAllToComplete(controllers().stream(), c -> { + executorServiceManager.boundedExecuteAndWaitForAllToComplete(controllers().stream(), c -> { c.startEventProcessing(); return null; }, c -> "Event processor starter for: " + c.getConfiguration().getName()); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java index 26f26afd1b..76f638331c 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java @@ -12,8 +12,7 @@ import io.fabric8.kubernetes.client.extended.leaderelection.LeaderElector; import io.fabric8.kubernetes.client.extended.leaderelection.LeaderElectorBuilder; import io.fabric8.kubernetes.client.extended.leaderelection.resourcelock.LeaseLock; -import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; -import io.javaoperatorsdk.operator.api.config.ExecutorServiceManager; +import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.api.config.LeaderElectionConfiguration; public class LeaderElectionManager { @@ -24,41 +23,19 @@ public class LeaderElectionManager { private final ControllerManager controllerManager; private String identity; private CompletableFuture leaderElectionFuture; + private final ConfigurationService configurationService; + private final KubernetesClient kubernetesClient; - public LeaderElectionManager(ControllerManager controllerManager) { + public LeaderElectionManager(KubernetesClient kubernetesClient, + ControllerManager controllerManager, + ConfigurationService configurationService) { + this.kubernetesClient = kubernetesClient; this.controllerManager = controllerManager; - } - - public void init(LeaderElectionConfiguration config, KubernetesClient client) { - this.identity = identity(config); - final var leaseNamespace = - config.getLeaseNamespace().orElseGet( - () -> ConfigurationServiceProvider.instance().getClientConfiguration().getNamespace()); - if (leaseNamespace == null) { - final var message = - "Lease namespace is not set and cannot be inferred. Leader election cannot continue."; - log.error(message); - throw new IllegalArgumentException(message); - } - final var lock = new LeaseLock(leaseNamespace, config.getLeaseName(), identity); - // releaseOnCancel is not used in the underlying implementation - leaderElector = - new LeaderElectorBuilder( - client, ExecutorServiceManager.instance().executorService()) - .withConfig( - new LeaderElectionConfig( - lock, - config.getLeaseDuration(), - config.getRenewDeadline(), - config.getRetryPeriod(), - leaderCallbacks(), - true, - config.getLeaseName())) - .build(); + this.configurationService = configurationService; } public boolean isLeaderElectionEnabled() { - return leaderElector != null; + return configurationService.getLeaderElectionConfiguration().isPresent(); } private LeaderCallbacks leaderCallbacks() { @@ -90,6 +67,7 @@ private String identity(LeaderElectionConfiguration config) { public void start() { if (isLeaderElectionEnabled()) { + init(configurationService.getLeaderElectionConfiguration().orElseThrow()); leaderElectionFuture = leaderElector.start(); } } @@ -99,4 +77,31 @@ public void stop() { leaderElectionFuture.cancel(false); } } + + private void init(LeaderElectionConfiguration config) { + this.identity = identity(config); + final var leaseNamespace = + config.getLeaseNamespace().orElseGet( + () -> configurationService.getClientConfiguration().getNamespace()); + if (leaseNamespace == null) { + final var message = + "Lease namespace is not set and cannot be inferred. Leader election cannot continue."; + log.error(message); + throw new IllegalArgumentException(message); + } + final var lock = new LeaseLock(leaseNamespace, config.getLeaseName(), identity); + // releaseOnCancel is not used in the underlying implementation + leaderElector = new LeaderElectorBuilder( + kubernetesClient, configurationService.getExecutorServiceManager().cachingExecutorService()) + .withConfig( + new LeaderElectionConfig( + lock, + config.getLeaseDuration(), + config.getRenewDeadline(), + config.getRetryPeriod(), + leaderCallbacks(), + true, + config.getLeaseName())) + .build(); + } } 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 b40d8d6dac..517383d79c 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,12 +14,11 @@ import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.KubernetesClientBuilder; import io.fabric8.kubernetes.client.Version; +import io.javaoperatorsdk.operator.api.config.BaseConfigurationService; 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; -import io.javaoperatorsdk.operator.api.config.ExecutorServiceManager; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; import io.javaoperatorsdk.operator.processing.Controller; import io.javaoperatorsdk.operator.processing.LifecycleAware; @@ -29,17 +28,18 @@ public class Operator implements LifecycleAware { private static final Logger log = LoggerFactory.getLogger(Operator.class); private static final int DEFAULT_MAX_CONCURRENT_REQUEST = 512; private final KubernetesClient kubernetesClient; - private final ControllerManager controllerManager = new ControllerManager(); - private final LeaderElectionManager leaderElectionManager = - new LeaderElectionManager(controllerManager); + private final ControllerManager controllerManager; + private final LeaderElectionManager leaderElectionManager; + private final ConfigurationService configurationService; private volatile boolean started = false; + public Operator() { this((KubernetesClient) null); } public Operator(KubernetesClient kubernetesClient) { - this(kubernetesClient, ConfigurationServiceProvider.instance()); + this(kubernetesClient, new BaseConfigurationService()); } /** @@ -56,7 +56,8 @@ public Operator(Consumer overrider) { } public Operator(KubernetesClient client, Consumer overrider) { - this(client, ConfigurationServiceProvider.overrideCurrent(overrider)); + this(client, ConfigurationService + .newOverriddenConfigurationService(new BaseConfigurationService(), overrider)); } /** @@ -67,15 +68,19 @@ public Operator(KubernetesClient client, Consumer * @param configurationService provides configuration */ public Operator(KubernetesClient kubernetesClient, ConfigurationService configurationService) { + this.configurationService = configurationService; + final var executorServiceManager = configurationService.getExecutorServiceManager(); + controllerManager = new ControllerManager(executorServiceManager); this.kubernetesClient = kubernetesClient != null ? kubernetesClient : new KubernetesClientBuilder() .withConfig(new ConfigBuilder() .withMaxConcurrentRequests(DEFAULT_MAX_CONCURRENT_REQUEST).build()) .build(); - ConfigurationServiceProvider.set(configurationService); - configurationService.getLeaderElectionConfiguration() - .ifPresent(c -> leaderElectionManager.init(c, this.kubernetesClient)); + + + leaderElectionManager = + new LeaderElectionManager(kubernetesClient, controllerManager, configurationService); } /** @@ -87,7 +92,7 @@ public Operator(KubernetesClient kubernetesClient, ConfigurationService configur @Deprecated(forRemoval = true) public void installShutdownHook() { installShutdownHook( - Duration.ofSeconds(ConfigurationServiceProvider.instance().getTerminationTimeoutSeconds())); + Duration.ofSeconds(configurationService.getTerminationTimeoutSeconds())); } /** @@ -123,9 +128,8 @@ public synchronized void start() { if (started) { return; } - ExecutorServiceManager.init(); controllerManager.shouldStart(); - final var version = ConfigurationServiceProvider.instance().getVersion(); + final var version = configurationService.getVersion(); log.info( "Operator SDK {} (commit: {}) built on {} starting...", version.getSdkVersion(), @@ -149,12 +153,11 @@ public void stop(Duration gracefulShutdownTimeout) throws OperatorException { if (!started) { return; } - final var configurationService = ConfigurationServiceProvider.instance(); log.info( "Operator SDK {} is shutting down...", configurationService.getVersion().getSdkVersion()); controllerManager.stop(); - ExecutorServiceManager.stop(gracefulShutdownTimeout); + configurationService.getExecutorServiceManager().stop(gracefulShutdownTimeout); leaderElectionManager.stop(); if (configurationService.closeClientOnStop()) { kubernetesClient.close(); @@ -180,7 +183,7 @@ public void stop() throws OperatorException { public

RegisteredController

register(Reconciler

reconciler) throws OperatorException { final var controllerConfiguration = - ConfigurationServiceProvider.instance().getConfigurationFor(reconciler); + configurationService.getConfigurationFor(reconciler); return register(reconciler, controllerConfiguration); } @@ -210,7 +213,7 @@ public

RegisteredController

register(Reconciler

re " reconciler named " + ReconcilerUtils.getNameFor(reconciler) + " because its configuration cannot be found.\n" + " Known reconcilers are: " - + ConfigurationServiceProvider.instance().getKnownReconcilerNames()); + + configurationService.getKnownReconcilerNames()); } final var controller = new Controller<>(reconciler, configuration, kubernetesClient); @@ -218,7 +221,7 @@ public

RegisteredController

register(Reconciler

re controllerManager.add(controller); final var watchedNS = configuration.watchAllNamespaces() ? "[all namespaces]" - : configuration.getEffectiveNamespaces(); + : configuration.getEffectiveNamespaces(configurationService); log.info( "Registered reconciler: '{}' for resource: '{}' for namespace(s): {}", @@ -239,7 +242,7 @@ public

RegisteredController

register(Reconciler

re public

RegisteredController

register(Reconciler

reconciler, Consumer> configOverrider) { final var controllerConfiguration = - ConfigurationServiceProvider.instance().getConfigurationFor(reconciler); + configurationService.getConfigurationFor(reconciler); var configToOverride = ControllerConfigurationOverrider.override(controllerConfiguration); configOverrider.accept(configToOverride); return register(reconciler, configToOverride.build()); @@ -264,4 +267,8 @@ public RuntimeInfo getRuntimeInfo() { boolean isStarted() { return started; } + + public ConfigurationService getConfigurationService() { + return configurationService; + } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/BaseConfigurationService.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/BaseConfigurationService.java index 3575093ebb..6bd436998d 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/BaseConfigurationService.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/BaseConfigurationService.java @@ -107,7 +107,7 @@ protected

ControllerConfiguration

configFor(Reconcile " annotation for reconciler: " + reconciler); } Class> reconcilerClass = (Class>) reconciler.getClass(); - final var resourceClass = ConfigurationServiceProvider.instance().getResourceClassResolver() + final var resourceClass = getResourceClassResolver() .getResourceClass(reconcilerClass); final var name = ReconcilerUtils.getNameFor(reconciler); @@ -152,7 +152,7 @@ protected

ControllerConfiguration

configFor(Reconcile io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration::labelSelector, Constants.NO_VALUE_SET), null, - Utils.instantiate(annotation.itemStore(), ItemStore.class, context)); + Utils.instantiate(annotation.itemStore(), ItemStore.class, context), this); ResourceEventFilter

answer = deprecatedEventFilter(annotation); config.setEventFilter(answer != null ? answer : ResourceEventFilters.passthrough()); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java index 809a6ee359..dce6033fd0 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java @@ -3,7 +3,11 @@ import java.time.Duration; import java.util.Optional; import java.util.Set; -import java.util.concurrent.*; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.LinkedBlockingDeque; +import java.util.concurrent.ThreadPoolExecutor; +import java.util.concurrent.TimeUnit; +import java.util.function.Consumer; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -248,4 +252,19 @@ default ManagedWorkflowFactory getWorkflowFactory() { default ResourceClassResolver getResourceClassResolver() { return new DefaultResourceClassResolver(); } + + static ConfigurationService newOverriddenConfigurationService( + ConfigurationService baseConfiguration, + Consumer overrider) { + if (overrider != null) { + final var toOverride = new ConfigurationServiceOverrider(baseConfiguration); + overrider.accept(toOverride); + return toOverride.build(); + } + return baseConfiguration; + } + + default ExecutorServiceManager getExecutorServiceManager() { + return new ExecutorServiceManager(this); + } } 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 c094b2c884..e8fa2a1577 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 @@ -236,8 +236,10 @@ public ResourceClassResolver getResourceClassResolver() { } /** - * @deprecated Use {@link ConfigurationServiceProvider#overrideCurrent(Consumer)} instead - * @param original that will be overriding + * @deprecated Use + * {@link ConfigurationService#newOverriddenConfigurationService(ConfigurationService, Consumer)} + * instead + * @param original that will be overridden * @return current overrider */ @Deprecated(since = "2.2.0") 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 deleted file mode 100644 index 2ba60d1fca..0000000000 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProvider.java +++ /dev/null @@ -1,75 +0,0 @@ -package io.javaoperatorsdk.operator.api.config; - -import java.util.function.Consumer; - -/** - * For internal usage only, to avoid passing the operator configuration around. Preferred way to get - * to the ConfigurationService is via the reconciliation context. - */ -public class ConfigurationServiceProvider { - private static ConfigurationService instance; - private static ConfigurationService defaultConfigurationService = createDefault(); - private static boolean alreadyConfigured = false; - - private ConfigurationServiceProvider() {} - - public synchronized static ConfigurationService instance() { - if (instance == null) { - set(defaultConfigurationService); - } - return instance; - } - - public synchronized static void set(ConfigurationService instance) { - set(instance, false); - } - - 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; - } - } - - ConfigurationServiceProvider.instance = instance; - } - - public synchronized static ConfigurationService overrideCurrent( - Consumer overrider) { - if (overrider != null) { - final var toOverride = new ConfigurationServiceOverrider(instance()); - overrider.accept(toOverride); - set(toOverride.build(), true); - } - return instance(); - } - - public synchronized static void setDefault(ConfigurationService defaultConfigurationService) { - ConfigurationServiceProvider.defaultConfigurationService = defaultConfigurationService; - } - - synchronized static ConfigurationService getDefault() { - return defaultConfigurationService; - } - - public synchronized static void reset() { - defaultConfigurationService = createDefault(); - instance = null; - alreadyConfigured = false; - } - - static ConfigurationService createDefault() { - return new BaseConfigurationService(Utils.VERSION); - } -} 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 1363ef0101..60a053d0a8 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 @@ -4,6 +4,7 @@ import java.util.Collections; import java.util.List; import java.util.Optional; +import java.util.Set; import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.ReconcilerUtils; @@ -107,9 +108,7 @@ default Optional maxReconciliationInterval() { } @SuppressWarnings("unused") - default ConfigurationService getConfigurationService() { - return ConfigurationServiceProvider.instance(); - } + ConfigurationService getConfigurationService(); @SuppressWarnings("unchecked") @Override @@ -120,4 +119,9 @@ default Class

getResourceClass() { return (Class

) Utils.getFirstTypeArgumentFromSuperClassOrInterface(getClass(), ControllerConfiguration.class); } + + @SuppressWarnings("unused") + default Set getEffectiveNamespaces() { + return ResourceConfiguration.super.getEffectiveNamespaces(getConfigurationService()); + } } 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 241c7d3271..5d96c2abf6 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 @@ -190,7 +190,8 @@ public ControllerConfiguration build() { generationAware, original.getAssociatedReconcilerClassName(), retry, rateLimiter, reconciliationMaxInterval, onAddFilter, onUpdateFilter, genericFilter, original.getDependentResources(), - namespaces, finalizer, labelSelector, configurations, itemStore); + namespaces, finalizer, labelSelector, configurations, itemStore, + original.getConfigurationService()); overridden.setEventFilter(customResourcePredicate); return overridden; } 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 54e4586aa0..f57af9fc30 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 @@ -21,56 +21,15 @@ public class ExecutorServiceManager { private static final Logger log = LoggerFactory.getLogger(ExecutorServiceManager.class); - private static ExecutorServiceManager instance; private final ExecutorService executor; private final ExecutorService workflowExecutor; private final ExecutorService cachingExecutorService; - private ExecutorServiceManager(ExecutorService executor, ExecutorService workflowExecutor) { + ExecutorServiceManager(ConfigurationService configurationService) { this.cachingExecutorService = Executors.newCachedThreadPool(); - this.executor = new InstrumentedExecutorService(executor); - this.workflowExecutor = new InstrumentedExecutorService(workflowExecutor); - - } - - public static synchronized void init() { - if (instance == null) { - final var configuration = ConfigurationServiceProvider.instance(); - final var executorService = configuration.getExecutorService(); - final var workflowExecutorService = configuration.getWorkflowExecutorService(); - instance = new ExecutorServiceManager(executorService, workflowExecutorService); - log.debug( - "Initialized ExecutorServiceManager executor: {}, workflow executor: {}, timeout: {}", - executorService.getClass(), - workflowExecutorService.getClass(), - configuration.getTerminationTimeoutSeconds()); - } else { - log.debug("Already started, reusing already setup instance!"); - } - } - - /** For testing purposes only */ - public static synchronized void reset() { - instance().doStop(Duration.ZERO); - instance = null; - init(); - } - - public static synchronized void stop(Duration gracefulShutdownTimeout) { - if (instance != null) { - instance.doStop(gracefulShutdownTimeout); - } - // make sure that we remove the singleton so that the thread pool is re-created on next call to - // start - instance = null; - } - - public static synchronized ExecutorServiceManager instance() { - if (instance == null) { - // provide a default configuration if none has been provided by init - init(); - } - return instance; + this.executor = new InstrumentedExecutorService(configurationService.getExecutorService()); + this.workflowExecutor = + new InstrumentedExecutorService(configurationService.getWorkflowExecutorService()); } /** @@ -82,9 +41,9 @@ public static synchronized ExecutorServiceManager instance() { * @param threadNamer for naming thread * @param type */ - public static void boundedExecuteAndWaitForAllToComplete(Stream stream, + public void boundedExecuteAndWaitForAllToComplete(Stream stream, Function task, Function threadNamer) { - executeAndWaitForAllToComplete(stream, task, threadNamer, instance().cachingExecutorService()); + executeAndWaitForAllToComplete(stream, task, threadNamer, cachingExecutorService()); } public static void executeAndWaitForAllToComplete(Stream stream, @@ -121,7 +80,7 @@ public static void executeAndWaitForAllToComplete(Stream stream, } } - public ExecutorService executorService() { + public ExecutorService reconcileExecutorService() { return executor; } @@ -133,7 +92,7 @@ public ExecutorService cachingExecutorService() { return cachingExecutorService; } - private void doStop(Duration gracefulShutdownTimeout) { + public void stop(Duration gracefulShutdownTimeout) { try { var parallelExec = Executors.newFixedThreadPool(3); log.debug("Closing executor"); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResolvedControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResolvedControllerConfiguration.java index 827d8bb81a..d527850fd3 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResolvedControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResolvedControllerConfiguration.java @@ -31,6 +31,7 @@ public class ResolvedControllerConfiguration

private final String finalizer; private final Map configurations; private final ItemStore

itemStore; + private final ConfigurationService configurationService; private ResourceEventFilter

eventFilter; private List dependentResources; @@ -43,7 +44,7 @@ public ResolvedControllerConfiguration(Class

resourceClass, ControllerConfigu other.genericFilter().orElse(null), other.getDependentResources(), other.getNamespaces(), other.getFinalizerName(), other.getLabelSelector(), Collections.emptyMap(), - other.getItemStore().orElse(null)); + other.getItemStore().orElse(null), other.getConfigurationService()); } public static Duration getMaxReconciliationInterval(long interval, TimeUnit timeUnit) { @@ -70,10 +71,11 @@ public ResolvedControllerConfiguration(Class

resourceClass, String name, GenericFilter

genericFilter, List dependentResources, Set namespaces, String finalizer, String labelSelector, - Map configurations, ItemStore

itemStore) { + Map configurations, ItemStore

itemStore, + ConfigurationService configurationService) { this(resourceClass, name, generationAware, associatedReconcilerClassName, retry, rateLimiter, maxReconciliationInterval, onAddFilter, onUpdateFilter, genericFilter, - namespaces, finalizer, labelSelector, configurations, itemStore); + namespaces, finalizer, labelSelector, configurations, itemStore, configurationService); setDependentResources(dependentResources); } @@ -82,9 +84,11 @@ protected ResolvedControllerConfiguration(Class

resourceClass, String name, RateLimiter rateLimiter, Duration maxReconciliationInterval, OnAddFilter

onAddFilter, OnUpdateFilter

onUpdateFilter, GenericFilter

genericFilter, Set namespaces, String finalizer, String labelSelector, - Map configurations, ItemStore

itemStore) { + Map configurations, ItemStore

itemStore, + ConfigurationService configurationService) { super(resourceClass, namespaces, labelSelector, onAddFilter, onUpdateFilter, genericFilter, itemStore); + this.configurationService = configurationService; this.name = ControllerConfiguration.ensureValidName(name, associatedReconcilerClassName); this.generationAware = generationAware; this.associatedReconcilerClassName = associatedReconcilerClassName; @@ -98,10 +102,10 @@ protected ResolvedControllerConfiguration(Class

resourceClass, String name, } protected ResolvedControllerConfiguration(Class

resourceClass, String name, - Class reconcilerClas) { + Class reconcilerClas, ConfigurationService configurationService) { this(resourceClass, name, false, getAssociatedReconcilerClassName(reconcilerClas), null, null, null, null, null, null, null, - null, null, null, null); + null, null, null, null, configurationService); } @Override @@ -149,6 +153,11 @@ public Optional maxReconciliationInterval() { return Optional.ofNullable(maxReconciliationInterval); } + @Override + public ConfigurationService getConfigurationService() { + return configurationService; + } + @Override public ResourceEventFilter

getEventFilter() { return eventFilter; 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 d3a8379d46..1d7e10c5d5 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 @@ -110,11 +110,11 @@ static Set ensureValidNamespaces(Collection namespaces) { * * @return a Set of namespace names the associated controller will watch */ - default Set getEffectiveNamespaces() { + default Set getEffectiveNamespaces(ConfigurationService configurationService) { var targetNamespaces = getNamespaces(); if (watchCurrentNamespace()) { final String namespace = - ConfigurationServiceProvider.instance().getClientConfiguration().getNamespace(); + configurationService.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='"); 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 f883799cc9..2b7c7ff461 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 @@ -175,7 +175,8 @@ public InformerConfigurationBuilder withNamespaces(Set namespaces, */ public

InformerConfigurationBuilder withNamespacesInheritedFromController( EventSourceContext

context) { - namespaces = context.getControllerConfiguration().getEffectiveNamespaces(); + namespaces = context.getControllerConfiguration().getEffectiveNamespaces( + context.getControllerConfiguration().getConfigurationService()); this.inheritControllerNamespacesOnChange = true; return this; } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Context.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Context.java index 031b930775..e0b01fb9c0 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Context.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/Context.java @@ -2,6 +2,7 @@ import java.util.Optional; import java.util.Set; +import java.util.concurrent.ExecutorService; import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.client.KubernetesClient; @@ -32,4 +33,9 @@ Optional getSecondaryResource(Class expectedType, EventSourceRetriever

eventSourceRetriever(); KubernetesClient getClient(); + + /** + * ExecutorService initialized by framework for workflows. Used for workflow standalone mode. + */ + ExecutorService getWorkflowExecutorService(); } 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 8e17b09fc6..0553bc4b6c 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 @@ -2,6 +2,7 @@ import java.util.Optional; import java.util.Set; +import java.util.concurrent.ExecutorService; import java.util.stream.Collectors; import io.fabric8.kubernetes.api.model.HasMetadata; @@ -75,6 +76,13 @@ public KubernetesClient getClient() { return controller.getClient(); } + @Override + public ExecutorService getWorkflowExecutorService() { + // not that this should be always received from executor service manager, so we are able to do + // restarts. + return controller.getExecutorServiceManager().workflowExecutorService(); + } + public DefaultContext

setRetryInfo(RetryInfo retryInfo) { this.retryInfo = retryInfo; return this; 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 d004ab1209..1966b8f695 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 @@ -1,6 +1,11 @@ package io.javaoperatorsdk.operator.processing; -import java.util.*; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -16,11 +21,20 @@ import io.javaoperatorsdk.operator.MissingCRDException; import io.javaoperatorsdk.operator.OperatorException; import io.javaoperatorsdk.operator.RegisteredController; -import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.config.ExecutorServiceManager; import io.javaoperatorsdk.operator.api.monitoring.Metrics; import io.javaoperatorsdk.operator.api.monitoring.Metrics.ControllerExecution; -import io.javaoperatorsdk.operator.api.reconciler.*; +import io.javaoperatorsdk.operator.api.reconciler.Cleaner; +import io.javaoperatorsdk.operator.api.reconciler.Constants; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.ContextInitializer; +import io.javaoperatorsdk.operator.api.reconciler.DeleteControl; +import io.javaoperatorsdk.operator.api.reconciler.EventSourceContext; +import io.javaoperatorsdk.operator.api.reconciler.EventSourceInitializer; +import io.javaoperatorsdk.operator.api.reconciler.Ignore; +import io.javaoperatorsdk.operator.api.reconciler.Reconciler; +import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; import io.javaoperatorsdk.operator.api.reconciler.dependent.EventSourceNotFoundException; import io.javaoperatorsdk.operator.api.reconciler.dependent.EventSourceProvider; import io.javaoperatorsdk.operator.api.reconciler.dependent.EventSourceReferencer; @@ -69,10 +83,11 @@ public Controller(Reconciler

reconciler, // needs to be initialized early since it's used in other downstream classes associatedGVK = GroupVersionKind.gvkFor(configuration.getResourceClass()); + final var configurationService = configuration.getConfigurationService(); + final var executorServiceManager = configurationService.getExecutorServiceManager(); this.reconciler = reconciler; this.configuration = configuration; this.kubernetesClient = kubernetesClient; - final var configurationService = ConfigurationServiceProvider.instance(); this.metrics = Optional.ofNullable(configurationService.getMetrics()).orElse(Metrics.NOOP); contextInitializer = reconciler instanceof ContextInitializer; isCleaner = reconciler instanceof Cleaner; @@ -81,13 +96,13 @@ public Controller(Reconciler

reconciler, managedWorkflow = managed.resolve(kubernetesClient, configuration); eventSourceManager = new EventSourceManager<>(this); - eventProcessor = new EventProcessor<>(eventSourceManager); + eventProcessor = new EventProcessor<>(eventSourceManager, configurationService); eventSourceManager.postProcessDefaultEventSourcesAfterProcessorInitializer(); controllerHealthInfo = new ControllerHealthInfo(eventSourceManager); final var context = new EventSourceContext<>( eventSourceManager.getControllerResourceEventSource(), configuration, kubernetesClient); initAndRegisterEventSources(context); - ConfigurationServiceProvider.instance().getMetrics().controllerRegistered(this); + configurationService.getMetrics().controllerRegistered(this); } @Override @@ -340,7 +355,7 @@ public synchronized void start(boolean startEventProcessor) throws OperatorExcep private void validateCRDWithLocalModelIfRequired(Class

resClass, String controllerName, String crdName, String specVersion) { final CustomResourceDefinition crd; - if (ConfigurationServiceProvider.instance().checkCRDAndValidateLocalModel() + if (getConfiguration().getConfigurationService().checkCRDAndValidateLocalModel() && CustomResource.class.isAssignableFrom(resClass)) { crd = kubernetesClient.apiextensions().v1().customResourceDefinitions().withName(crdName) .get(); @@ -389,7 +404,7 @@ private void throwMissingCRDException(String crdName, String specVersion, String */ private void failOnMissingCurrentNS() { try { - configuration.getEffectiveNamespaces(); + configuration.getEffectiveNamespaces(getConfiguration().getConfigurationService()); } catch (OperatorException e) { throw new OperatorException( "Controller '" @@ -422,4 +437,8 @@ public GroupVersionKind getAssociatedGroupVersionKind() { public EventProcessor

getEventProcessor() { return eventProcessor; } + + public ExecutorServiceManager getExecutorServiceManager() { + return getConfiguration().getConfigurationService().getExecutorServiceManager(); + } } 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 3c2d3e8401..e708f2be43 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 @@ -57,7 +57,8 @@ static Matcher matcherFor( @Override public Result match(R actualResource, P primary, Context

context) { var desired = dependentResource.desired(primary, context); - return match(desired, actualResource, false, false, false); + return match(desired, actualResource, false, false, false, + context.getControllerConfiguration().getConfigurationService().getObjectMapper()); } /** @@ -90,9 +91,9 @@ public Result match(R actualResource, P primary, Context

context) { */ public static Result match(R desired, R actualResource, boolean considerLabelsAndAnnotations, boolean labelsAndAnnotationsEquality, - boolean specEquality) { + boolean specEquality,ObjectMapper objectMapper) { return match(desired, actualResource, considerLabelsAndAnnotations, - labelsAndAnnotationsEquality, specEquality, EMPTY_ARRAY); + labelsAndAnnotationsEquality, specEquality,objectMapper, EMPTY_ARRAY); } /** @@ -117,9 +118,9 @@ public static Result match(R desired, R actualResourc */ public static Result match(R desired, R actualResource, boolean considerLabelsAndAnnotations, boolean labelsAndAnnotationsEquality, - String... ignorePaths) { + ObjectMapper objectMapper, String... ignorePaths) { return match(desired, actualResource, considerLabelsAndAnnotations, - labelsAndAnnotationsEquality, false, ignorePaths); + labelsAndAnnotationsEquality, false,objectMapper, ignorePaths); } /** @@ -151,7 +152,7 @@ public static Result match(R desired, R actualResourc public static Result match( KubernetesDependentResource dependentResource, R actualResource, P primary, Context

context, boolean considerLabelsAndAnnotations, - boolean labelsAndAnnotationsEquality, + boolean labelsAndAnnotationsEquality,ObjectMapper objectMapper, String... ignorePaths) { final var desired = dependentResource.desired(primary, context); return match(desired, actualResource, considerLabelsAndAnnotations, @@ -180,8 +181,6 @@ private static Result match(R desired, R actualResour "Equality should be false in case of ignore list provided"); } - final var objectMapper = ConfigurationServiceProvider.instance().getObjectMapper(); - var desiredNode = objectMapper.valueToTree(desired); var actualNode = objectMapper.valueToTree(actualResource); var wholeDiffJsonPatch = JsonDiff.asJson(desiredNode, actualNode); @@ -299,7 +298,7 @@ public static Result match( KubernetesDependentResource dependentResource, R actualResource, P primary, Context

context, boolean considerLabelsAndAnnotations, boolean specEquality) { final var desired = dependentResource.desired(primary, context); - return match(desired, actualResource, considerLabelsAndAnnotations, specEquality); + return match(desired, actualResource, considerLabelsAndAnnotations, specEquality, context.getControllerConfiguration().getConfigurationService().getObjectMapper()); } @Deprecated(forRemoval = true) @@ -307,7 +306,7 @@ public static Result match( KubernetesDependentResource dependentResource, R actualResource, P primary, Context

context, boolean considerLabelsAndAnnotations, String... ignorePaths) { final var desired = dependentResource.desired(primary, context); - return match(desired, actualResource, considerLabelsAndAnnotations, true, ignorePaths); + return match(desired, actualResource, considerLabelsAndAnnotations, true, context.getControllerConfiguration().getConfigurationService().getObjectMapper(), ignorePaths); } } 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 e35913af68..9cd63a2871 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,7 +4,6 @@ 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; public abstract class GenericResourceUpdatePreProcessor implements @@ -44,7 +43,9 @@ protected void updateClonedActual(R actual, R desired) { } public R replaceSpecOnActual(R actual, R desired, Context context) { - var clonedActual = ConfigurationServiceProvider.instance().getResourceCloner().clone(actual); + var clonedActual = context.getControllerConfiguration().getConfigurationService() + .getResourceCloner() + .clone(actual); updateClonedActual(clonedActual, desired); return clonedActual; } 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 986b0337b1..c4500a6e50 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 @@ -143,7 +143,7 @@ public Result match(R actualResource, P primary, Context

context) { @SuppressWarnings("unused") public Result match(R actualResource, R desired, P primary, Context

context) { return GenericKubernetesResourceMatcher.match(desired, actualResource, false, - false, false); + false, false,context.getControllerConfiguration().getConfigurationService().getObjectMapper()); } protected void handleDelete(P primary, R secondary, Context

context) { @@ -258,4 +258,5 @@ public Optional> configuration() { public boolean isDeletable() { return super.isDeletable() && !garbageCollected; } + } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutor.java index e8c129126d..f5774ed42f 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutor.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutor.java @@ -21,6 +21,7 @@ public abstract class AbstractWorkflowExecutor

{ protected final Workflow

workflow; protected final P primary; + protected final ResourceID primaryID; protected final Context

context; /** * Covers both deleted and reconciled @@ -34,6 +35,7 @@ public AbstractWorkflowExecutor(Workflow

workflow, P primary, Context

cont this.workflow = workflow; this.primary = primary; this.context = context; + this.primaryID = ResourceID.fromResource(primary); } protected abstract Logger logger(); @@ -49,11 +51,10 @@ protected synchronized void waitForScheduledExecutionsToRun() { } } catch (InterruptedException e) { if (noMoreExecutionsScheduled()) { - logger().debug("interrupted, no more executions for: {}", - ResourceID.fromResource(primary)); + logger().debug("interrupted, no more executions for: {}", primaryID); return; } else { - logger().error("Thread interrupted for primary: {}", ResourceID.fromResource(primary), e); + logger().error("Thread interrupted for primary: {}", primaryID, e); throw new OperatorException(e); } } @@ -99,7 +100,7 @@ protected Map getErroredDependents() { protected synchronized void handleNodeExecutionFinish( DependentResourceNode dependentResourceNode) { - logger().debug("Finished execution for: {}", dependentResourceNode); + logger().trace("Finished execution for: {} primary: {}", dependentResourceNode, primaryID); actualExecutions.remove(dependentResourceNode); if (noMoreExecutionsScheduled()) { this.notifyAll(); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultManagedWorkflow.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultManagedWorkflow.java index 4d46293722..e5b89f6c80 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultManagedWorkflow.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultManagedWorkflow.java @@ -8,7 +8,6 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.client.KubernetesClient; -import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; @@ -25,8 +24,7 @@ public class DefaultManagedWorkflow

implements ManagedWor private final List orderedSpecs; private final boolean hasCleaner; - protected DefaultManagedWorkflow(List orderedSpecs, - boolean hasCleaner) { + protected DefaultManagedWorkflow(List orderedSpecs, boolean hasCleaner) { this.hasCleaner = hasCleaner; topLevelResources = new HashSet<>(orderedSpecs.size()); bottomLevelResources = orderedSpecs.stream() @@ -102,7 +100,7 @@ private DependentResource resolve(DependentResourceSpec spec, KubernetesClient client, ControllerConfiguration

configuration) { final DependentResource dependentResource = - ConfigurationServiceProvider.instance().dependentResourceFactory() + configuration.getConfigurationService().dependentResourceFactory() .createFrom(spec, configuration); if (dependentResource instanceof KubernetesClientAware) { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultWorkflow.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultWorkflow.java index 3a5291c8b8..b25de43bb7 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultWorkflow.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultWorkflow.java @@ -56,7 +56,6 @@ protected DefaultWorkflow(Map dependentResourceNo boolean hasCleaner) { this.throwExceptionAutomatically = throwExceptionAutomatically; this.hasCleaner = hasCleaner; - this.topLevelResources = topLevelResources; this.bottomLevelResource = bottomLevelResource; this.dependentResourceNodes = dependentResourceNodes; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowFactory.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowFactory.java index 4923084de8..eb01dcd3f4 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowFactory.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowFactory.java @@ -1,6 +1,5 @@ package io.javaoperatorsdk.operator.processing.dependent.workflow; -import io.fabric8.kubernetes.client.KubernetesClient; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; public interface ManagedWorkflowFactory> { @@ -9,24 +8,10 @@ public interface ManagedWorkflowFactory> { ManagedWorkflowFactory DEFAULT = (configuration) -> { final var dependentResourceSpecs = configuration.getDependentResources(); if (dependentResourceSpecs == null || dependentResourceSpecs.isEmpty()) { - return new ManagedWorkflow() { - @Override - public boolean hasCleaner() { - return false; - } - - @Override - public boolean isEmpty() { - return true; - } - - @Override - public Workflow resolve(KubernetesClient client, ControllerConfiguration configuration) { - return new DefaultWorkflow(null); - } - }; + return (ManagedWorkflow) (client, configuration1) -> new DefaultWorkflow(null); } - return ManagedWorkflowSupport.instance().createWorkflow(dependentResourceSpecs); + ManagedWorkflowSupport support = new ManagedWorkflowSupport(); + return support.createWorkflow(dependentResourceSpecs); }; @SuppressWarnings("rawtypes") diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupport.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupport.java index 8ec337d2f0..b5a6fd26b2 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupport.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupport.java @@ -16,14 +16,6 @@ @SuppressWarnings({"rawtypes", "unchecked"}) class ManagedWorkflowSupport { - private final static ManagedWorkflowSupport instance = new ManagedWorkflowSupport(); - - static ManagedWorkflowSupport instance() { - return instance; - } - - private ManagedWorkflowSupport() {} - public void checkForNameDuplication(List dependentResourceSpecs) { if (dependentResourceSpecs == null) { return; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutor.java index 3a65d8b112..b604b47edb 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutor.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutor.java @@ -3,6 +3,7 @@ import java.util.List; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ExecutorService; import java.util.concurrent.Future; import java.util.stream.Collectors; @@ -10,7 +11,6 @@ import org.slf4j.LoggerFactory; import io.fabric8.kubernetes.api.model.HasMetadata; -import io.javaoperatorsdk.operator.api.config.ExecutorServiceManager; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.api.reconciler.dependent.Deleter; import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; @@ -23,9 +23,11 @@ public class WorkflowCleanupExecutor

extends AbstractWork private final Set postDeleteConditionNotMet = ConcurrentHashMap.newKeySet(); private final Set deleteCalled = ConcurrentHashMap.newKeySet(); + private final ExecutorService executorService; public WorkflowCleanupExecutor(Workflow

workflow, P primary, Context

context) { super(workflow, primary, context); + this.executorService = context.getWorkflowExecutorService(); } public synchronized WorkflowCleanupResult cleanup() { @@ -43,17 +45,17 @@ protected Logger logger() { @SuppressWarnings({"rawtypes", "unchecked"}) private synchronized void handleCleanup(DependentResourceNode dependentResourceNode) { - log.debug("Submitting for cleanup: {}", dependentResourceNode); + log.debug("Submitting for cleanup: {} primaryID: {}", dependentResourceNode, primaryID); if (alreadyVisited(dependentResourceNode) || isExecutingNow(dependentResourceNode) || !allDependentsCleaned(dependentResourceNode) || hasErroredDependent(dependentResourceNode)) { - log.debug("Skipping submit of: {}, ", dependentResourceNode); + log.debug("Skipping submit of: {} primaryID: {}", dependentResourceNode, primaryID); return; } - Future nodeFuture = ExecutorServiceManager.instance().workflowExecutorService() + Future nodeFuture = executorService .submit(new CleanupExecutor<>(dependentResourceNode)); markAsExecuting(dependentResourceNode, nodeFuture); log.debug("Submitted for cleanup: {}", dependentResourceNode); @@ -94,7 +96,8 @@ private synchronized void handleDependentCleaned( var dependOns = dependentResourceNode.getDependsOn(); if (dependOns != null) { dependOns.forEach(d -> { - log.debug("Handle cleanup for dependent: {} of parent:{}", d, dependentResourceNode); + log.debug("Handle cleanup for dependent: {} of parent: {} primaryID: {}", d, + dependentResourceNode, primaryID); handleCleanup(d); }); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java index e437d58a27..38f263667f 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java @@ -4,6 +4,7 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ExecutorService; import java.util.concurrent.Future; import java.util.stream.Collectors; @@ -11,12 +12,10 @@ import org.slf4j.LoggerFactory; import io.fabric8.kubernetes.api.model.HasMetadata; -import io.javaoperatorsdk.operator.api.config.ExecutorServiceManager; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.api.reconciler.dependent.Deleter; import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; import io.javaoperatorsdk.operator.api.reconciler.dependent.ReconcileResult; -import io.javaoperatorsdk.operator.processing.event.ResourceID; @SuppressWarnings({"rawtypes", "unchecked"}) public class WorkflowReconcileExecutor

extends AbstractWorkflowExecutor

{ @@ -33,9 +32,11 @@ public class WorkflowReconcileExecutor

extends AbstractWo private final Set reconciled = ConcurrentHashMap.newKeySet(); private final Map reconcileResults = new ConcurrentHashMap<>(); + private final ExecutorService executorService; public WorkflowReconcileExecutor(Workflow

workflow, P primary, Context

context) { super(workflow, primary, context); + this.executorService = context.getWorkflowExecutorService(); } public synchronized WorkflowReconcileResult reconcile() { @@ -52,14 +53,14 @@ protected Logger logger() { } private synchronized void handleReconcile(DependentResourceNode dependentResourceNode) { - log.debug("Submitting for reconcile: {}", dependentResourceNode); + log.debug("Submitting for reconcile: {} primaryID: {}", dependentResourceNode, primaryID); if (alreadyVisited(dependentResourceNode) || isExecutingNow(dependentResourceNode) || !allParentsReconciledAndReady(dependentResourceNode) || markedForDelete.contains(dependentResourceNode) || hasErroredParent(dependentResourceNode)) { - log.debug("Skipping submit of: {}, ", dependentResourceNode); + log.debug("Skipping submit of: {}, primaryID: {}", dependentResourceNode, primaryID); return; } @@ -68,10 +69,10 @@ private synchronized void handleReconcile(DependentResourceNode depend if (!reconcileConditionMet) { handleReconcileConditionNotMet(dependentResourceNode); } else { - Future nodeFuture = ExecutorServiceManager.instance().workflowExecutorService() + Future nodeFuture = executorService .submit(new NodeReconcileExecutor(dependentResourceNode)); markAsExecuting(dependentResourceNode, nodeFuture); - log.debug("Submitted to reconcile: {}", dependentResourceNode); + log.debug("Submitted to reconcile: {} primaryID: {}", dependentResourceNode, primaryID); } } @@ -82,11 +83,12 @@ private synchronized void handleDelete(DependentResourceNode dependentResourceNo || isExecutingNow(dependentResourceNode) || !markedForDelete.contains(dependentResourceNode) || !allDependentsDeletedAlready(dependentResourceNode)) { - log.debug("Skipping submit for delete of: {}, ", dependentResourceNode); + log.debug("Skipping submit for delete of: {} primaryID: {} ", dependentResourceNode, + primaryID); return; } - Future nodeFuture = ExecutorServiceManager.instance().workflowExecutorService() + Future nodeFuture = executorService .submit(new NodeDeleteExecutor(dependentResourceNode)); markAsExecuting(dependentResourceNode, nodeFuture); log.debug("Submitted to delete: {}", dependentResourceNode); @@ -115,12 +117,9 @@ private NodeReconcileExecutor(DependentResourceNode dependentResourceNode) @Override protected void doRun(DependentResourceNode dependentResourceNode, DependentResource dependentResource) { - if (log.isDebugEnabled()) { - log.debug( - "Reconciling {} for primary: {}", - dependentResourceNode, - ResourceID.fromResource(primary)); - } + + log.debug( + "Reconciling for primary: {} node: {} ", primaryID, dependentResourceNode); ReconcileResult reconcileResult = dependentResource.reconcile(primary, context); reconcileResults.put(dependentResource, reconcileResult); reconciled.add(dependentResourceNode); @@ -128,7 +127,8 @@ protected void doRun(DependentResourceNode dependentResourceNode, boolean ready = isConditionMet(dependentResourceNode.getReadyPostcondition(), dependentResource); if (ready) { - log.debug("Setting already reconciled for: {}", dependentResourceNode); + log.debug("Setting already reconciled for: {} primaryID: {}", + dependentResourceNode, primaryID); markAsVisited(dependentResourceNode); handleDependentsReconcile(dependentResourceNode); } else { @@ -171,7 +171,8 @@ protected void doRun(DependentResourceNode dependentResourceNode, private synchronized void handleDependentDeleted( DependentResourceNode dependentResourceNode) { dependentResourceNode.getDependsOn().forEach(dr -> { - log.debug("Handle deleted for: {} with dependent: {}", dr, dependentResourceNode); + log.debug("Handle deleted for: {} with dependent: {} primaryID: {}", dr, + dependentResourceNode, primaryID); handleDelete(dr); }); } @@ -180,7 +181,8 @@ private synchronized void handleDependentsReconcile( DependentResourceNode dependentResourceNode) { var dependents = dependentResourceNode.getParents(); dependents.forEach(d -> { - log.debug("Handle reconcile for dependent: {} of parent:{}", d, dependentResourceNode); + log.debug("Handle reconcile for dependent: {} of parent:{} primaryID: {}", d, + dependentResourceNode, primaryID); handleReconcile(d); }); } 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 7bf499c1c3..cef5e0ac45 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 @@ -11,9 +11,8 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.OperatorException; -import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; +import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; -import io.javaoperatorsdk.operator.api.config.ExecutorServiceManager; import io.javaoperatorsdk.operator.api.monitoring.Metrics; import io.javaoperatorsdk.operator.api.reconciler.Constants; import io.javaoperatorsdk.operator.processing.LifecycleAware; @@ -46,13 +45,12 @@ public class EventProcessor

implements EventHandler, Life private final Map metricsMetadata; private ExecutorService executor; - public EventProcessor(EventSourceManager

eventSourceManager) { + public EventProcessor(EventSourceManager

eventSourceManager, + ConfigurationService configurationService) { this( eventSourceManager.getController().getConfiguration(), - eventSourceManager.getControllerResourceEventSource(), - new ReconciliationDispatcher<>(eventSourceManager.getController()), - ConfigurationServiceProvider.instance().getMetrics(), - eventSourceManager); + new ReconciliationDispatcher<>(eventSourceManager.getController()), eventSourceManager, + configurationService.getMetrics(), eventSourceManager.getControllerResourceEventSource()); } @SuppressWarnings("rawtypes") @@ -63,19 +61,15 @@ public EventProcessor(EventSourceManager

eventSourceManager) { Metrics metrics) { this( controllerConfiguration, - eventSourceManager.getControllerResourceEventSource(), - reconciliationDispatcher, - metrics, - eventSourceManager); + reconciliationDispatcher, eventSourceManager, metrics, + eventSourceManager.getControllerResourceEventSource()); } @SuppressWarnings({"rawtypes", "unchecked"}) private EventProcessor( ControllerConfiguration controllerConfiguration, - Cache

cache, ReconciliationDispatcher

reconciliationDispatcher, - Metrics metrics, - EventSourceManager

eventSourceManager) { + EventSourceManager

eventSourceManager, Metrics metrics, Cache

cache) { this.controllerConfiguration = controllerConfiguration; this.running = false; this.reconciliationDispatcher = reconciliationDispatcher; @@ -367,7 +361,8 @@ public synchronized void stop() { @Override public void start() throws OperatorException { // on restart new executor service is created and needs to be set here - executor = ExecutorServiceManager.instance().executorService(); + executor = controllerConfiguration.getConfigurationService().getExecutorServiceManager() + .reconcileExecutorService(); this.running = true; handleAlreadyMarkedEvents(); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java index 41ce4c1826..96c7f1140c 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java @@ -1,6 +1,10 @@ package io.javaoperatorsdk.operator.processing.event; -import java.util.*; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -22,6 +26,7 @@ import io.javaoperatorsdk.operator.processing.event.source.ResourceEventSource; import io.javaoperatorsdk.operator.processing.event.source.controller.ControllerResourceEventSource; import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceAction; +import io.javaoperatorsdk.operator.processing.event.source.informer.ManagedInformerEventSource; import io.javaoperatorsdk.operator.processing.event.source.timer.TimerEventSource; public class EventSourceManager

@@ -31,6 +36,7 @@ public class EventSourceManager

private final EventSources

eventSources; private final Controller

controller; + private final ExecutorServiceManager executorServiceManager; public EventSourceManager(Controller

controller) { this(controller, new EventSources<>()); @@ -39,10 +45,9 @@ public EventSourceManager(Controller

controller) { EventSourceManager(Controller

controller, EventSources

eventSources) { this.eventSources = eventSources; this.controller = controller; + this.executorServiceManager = controller.getExecutorServiceManager(); // controller event source needs to be available before we create the event processor - eventSources.initControllerEventSource(controller); - - + eventSources.createControllerEventSource(controller); postProcessDefaultEventSourcesAfterProcessorInitializer(); } @@ -65,13 +70,13 @@ public void postProcessDefaultEventSourcesAfterProcessorInitializer() { public synchronized void start() { startEventSource(eventSources.namedControllerResourceEventSource()); - ExecutorServiceManager.boundedExecuteAndWaitForAllToComplete( + executorServiceManager.boundedExecuteAndWaitForAllToComplete( eventSources.additionalNamedEventSources() .filter(es -> es.priority().equals(EventSourceStartPriority.RESOURCE_STATE_LOADER)), this::startEventSource, getThreadNamer("start")); - ExecutorServiceManager.boundedExecuteAndWaitForAllToComplete( + executorServiceManager.boundedExecuteAndWaitForAllToComplete( eventSources.additionalNamedEventSources() .filter(es -> es.priority().equals(EventSourceStartPriority.DEFAULT)), this::startEventSource, @@ -93,7 +98,7 @@ private static Function getEventSourceThreadNamer(S @Override public synchronized void stop() { stopEventSource(eventSources.namedControllerResourceEventSource()); - ExecutorServiceManager.boundedExecuteAndWaitForAllToComplete( + executorServiceManager.boundedExecuteAndWaitForAllToComplete( eventSources.additionalNamedEventSources(), this::stopEventSource, getThreadNamer("stop")); @@ -142,6 +147,7 @@ public final void registerEventSource(EventSource eventSource) throws OperatorEx registerEventSource(null, eventSource); } + @SuppressWarnings("rawtypes") public final synchronized void registerEventSource(String name, EventSource eventSource) throws OperatorException { Objects.requireNonNull(eventSource, "EventSource must not be null"); @@ -149,6 +155,12 @@ public final synchronized void registerEventSource(String name, EventSource even if (name == null || name.isBlank()) { name = EventSourceInitializer.generateNameFor(eventSource); } + if (eventSource instanceof ManagedInformerEventSource) { + var managedInformerEventSource = ((ManagedInformerEventSource) eventSource); + managedInformerEventSource.setConfigurationService(controller + .getConfiguration().getConfigurationService()); + managedInformerEventSource.setExecutorServiceManager(executorServiceManager); + } final var named = new NamedEventSource(eventSource, name); eventSources.add(named); named.setEventHandler(controller.getEventProcessor()); @@ -185,7 +197,7 @@ public void broadcastOnResourceEvent(ResourceAction action, P resource, P oldRes public void changeNamespaces(Set namespaces) { eventSources.controllerResourceEventSource() .changeNamespaces(namespaces); - ExecutorServiceManager.boundedExecuteAndWaitForAllToComplete(eventSources + executorServiceManager.boundedExecuteAndWaitForAllToComplete(eventSources .additionalEventSources() .filter(NamespaceChangeable.class::isInstance) .map(NamespaceChangeable.class::cast) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSources.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSources.java index e140544586..22dfc04b80 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSources.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSources.java @@ -30,8 +30,11 @@ class EventSources { private ControllerResourceEventSource controllerResourceEventSource; - void initControllerEventSource(Controller controller) { + void createControllerEventSource(Controller controller) { controllerResourceEventSource = new ControllerResourceEventSource<>(controller); + controllerResourceEventSource + .setConfigurationService(controller.getConfiguration().getConfigurationService()); + controllerResourceEventSource.setExecutorServiceManager(controller.getExecutorServiceManager()); } ControllerResourceEventSource controllerResourceEventSource() { 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 6c7e7bc9e7..a5ea74de87 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 @@ -14,7 +14,7 @@ import io.fabric8.kubernetes.client.dsl.Resource; import io.javaoperatorsdk.operator.OperatorException; import io.javaoperatorsdk.operator.api.ObservedGenerationAware; -import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; +import io.javaoperatorsdk.operator.api.config.Cloner; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.BaseControl; import io.javaoperatorsdk.operator.api.reconciler.Context; @@ -43,11 +43,13 @@ class ReconciliationDispatcher

{ // this is to handle corner case, when there is a retry, but it is actually limited to 0. // Usually for testing purposes. private final boolean retryConfigurationHasZeroAttempts; + private final Cloner cloner; ReconciliationDispatcher(Controller

controller, CustomResourceFacade

customResourceFacade) { this.controller = controller; this.customResourceFacade = customResourceFacade; + this.cloner = controller.getConfiguration().getConfigurationService().getResourceCloner(); var retry = controller.getConfiguration().getRetry(); retryConfigurationHasZeroAttempts = retry == null || retry.initExecution().isLastAttempt(); @@ -124,7 +126,6 @@ private PostExecutionControl

handleReconcile( } private P cloneResource(P resource) { - final var cloner = ConfigurationServiceProvider.instance().getResourceCloner(); return cloner.clone(resource); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerResourceEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerResourceEventSource.java index 89bf745ded..5affaac0b7 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerResourceEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerResourceEventSource.java @@ -1,7 +1,10 @@ package io.javaoperatorsdk.operator.processing.event.source.controller; +import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.function.Function; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -9,12 +12,14 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.client.KubernetesClientException; import io.fabric8.kubernetes.client.informers.ResourceEventHandler; +import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.processing.Controller; import io.javaoperatorsdk.operator.processing.MDCUtils; import io.javaoperatorsdk.operator.processing.event.ResourceID; import io.javaoperatorsdk.operator.processing.event.source.filter.OnDeleteFilter; import io.javaoperatorsdk.operator.processing.event.source.filter.OnUpdateFilter; +import io.javaoperatorsdk.operator.processing.event.source.informer.InformerManager; import io.javaoperatorsdk.operator.processing.event.source.informer.ManagedInformerEventSource; import static io.javaoperatorsdk.operator.ReconcilerUtils.handleKubernetesClientException; @@ -131,4 +136,14 @@ public void setOnDeleteFilter(OnDeleteFilter onDeleteFilter) { throw new IllegalStateException( "onDeleteFilter is not supported for controller resource event source"); } + + @Override + public void addIndexers(Map>> indexers) { + manager().addIndexers(indexers); + } + + public void setConfigurationService(ConfigurationService configurationService) { + cache = new InformerManager<>(client, configuration, configurationService, this); + this.configurationService = configurationService; + } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java index b6a8567126..83405743d4 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java @@ -1,7 +1,7 @@ package io.javaoperatorsdk.operator.processing.event.source.informer; -import java.util.Optional; -import java.util.Set; +import java.util.*; +import java.util.function.Function; import java.util.stream.Collectors; import org.slf4j.Logger; @@ -10,6 +10,8 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.informers.ResourceEventHandler; +import io.javaoperatorsdk.operator.OperatorException; +import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.api.config.informer.InformerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.EventSourceContext; import io.javaoperatorsdk.operator.api.reconciler.dependent.RecentOperationEventFilter; @@ -76,6 +78,7 @@ public class InformerEventSource // we need direct control for the indexer to propagate the just update resource also to the index private final PrimaryToSecondaryIndex primaryToSecondaryIndex; private final PrimaryToSecondaryMapper

primaryToSecondaryMapper; + private Map>> indexerBuffer = new HashMap<>(); public InformerEventSource( InformerConfiguration configuration, EventSourceContext

context) { @@ -338,4 +341,23 @@ private boolean acceptedByDeleteFilters(R resource, boolean b) { return (onDeleteFilter == null || onDeleteFilter.accept(resource, b)) && (genericFilter == null || genericFilter.accept(resource)); } + + + // Since this event source instance is created by the user, the ConfigService and + // ExecutorManagerService is actually injected after it is registered. Some of the subcomponents + // are created that time here. + public void setConfigurationService(ConfigurationService configurationService) { + this.configurationService = configurationService; + cache = new InformerManager<>(client, configuration, configurationService, this); + cache.addIndexers(indexerBuffer); + indexerBuffer = null; + } + + public void addIndexers(Map>> indexers) { + if (indexerBuffer == null) { + throw new OperatorException("Cannot add indexers after InformerEventSource started."); + } + indexerBuffer.putAll(indexers); + } + } 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 db4ce49076..9c7e283b5b 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 @@ -23,10 +23,7 @@ import io.fabric8.kubernetes.client.informers.ResourceEventHandler; import io.javaoperatorsdk.operator.OperatorException; import io.javaoperatorsdk.operator.ReconcilerUtils; -import io.javaoperatorsdk.operator.api.config.Cloner; -import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; -import io.javaoperatorsdk.operator.api.config.ExecutorServiceManager; -import io.javaoperatorsdk.operator.api.config.ResourceConfiguration; +import io.javaoperatorsdk.operator.api.config.*; import io.javaoperatorsdk.operator.health.InformerHealthIndicator; import io.javaoperatorsdk.operator.processing.LifecycleAware; import io.javaoperatorsdk.operator.processing.event.ResourceID; @@ -46,20 +43,23 @@ public class InformerManager, Resource> client; private final ResourceEventHandler eventHandler; private final Map>> indexers = new HashMap<>(); + private ExecutorServiceManager executorServiceManager; + private final ConfigurationService configurationService; public InformerManager(MixedOperation, Resource> client, - C configuration, + C configuration, ConfigurationService configurationService, ResourceEventHandler eventHandler) { this.client = client; this.configuration = configuration; this.eventHandler = eventHandler; + this.configurationService = configurationService; } @Override public void start() throws OperatorException { initSources(); // make sure informers are all started before proceeding further - ExecutorServiceManager.boundedExecuteAndWaitForAllToComplete(sources.values().stream(), + executorServiceManager.boundedExecuteAndWaitForAllToComplete(sources.values().stream(), iw -> { iw.start(); return null; @@ -72,8 +72,8 @@ private void initSources() { if (!sources.isEmpty()) { throw new IllegalStateException("Some sources already initialized."); } - cloner = ConfigurationServiceProvider.instance().getResourceCloner(); - final var targetNamespaces = configuration.getEffectiveNamespaces(); + cloner = configurationService.getResourceCloner(); + final var targetNamespaces = configuration.getEffectiveNamespaces(configurationService); if (ResourceConfiguration.allNamespacesWatched(targetNamespaces)) { var source = createEventSourceForNamespace(WATCH_ALL_NAMESPACES); log.debug("Registered {} -> {} for any namespace", this, source); @@ -129,7 +129,7 @@ private InformerWrapper createEventSource( var informer = filteredBySelectorClient.runnableInformer(0); configuration.getItemStore().ifPresent(informer::itemStore); var source = - new InformerWrapper<>(informer, namespaceIdentifier); + new InformerWrapper<>(informer, configurationService, namespaceIdentifier); source.addEventHandler(eventHandler); sources.put(namespaceIdentifier, source); return source; @@ -207,11 +207,17 @@ public String toString() { return "InformerManager [" + ReconcilerUtils.getResourceTypeNameWithVersion(configuration.getResourceClass()) + "] watching: " - + configuration.getEffectiveNamespaces() + + configuration.getEffectiveNamespaces(configurationService) + (selector != null ? " selector: " + selector : ""); } public Map informerHealthIndicators() { return Collections.unmodifiableMap(sources); } + + public InformerManager setExecutorServiceManager( + ExecutorServiceManager executorServiceManager) { + this.executorServiceManager = executorServiceManager; + return this; + } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.java index 1ea93adb70..5a55bc7d7c 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.java @@ -20,7 +20,7 @@ import io.fabric8.kubernetes.client.informers.cache.Cache; import io.javaoperatorsdk.operator.OperatorException; import io.javaoperatorsdk.operator.ReconcilerUtils; -import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; +import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.health.InformerHealthIndicator; import io.javaoperatorsdk.operator.health.Status; import io.javaoperatorsdk.operator.processing.LifecycleAware; @@ -35,19 +35,23 @@ class InformerWrapper private final SharedIndexInformer informer; private final Cache cache; private final String namespaceIdentifier; + private ConfigurationService configurationService; - public InformerWrapper(SharedIndexInformer informer, String namespaceIdentifier) { + public InformerWrapper(SharedIndexInformer informer, ConfigurationService configurationService, + String namespaceIdentifier) { this.informer = informer; this.namespaceIdentifier = namespaceIdentifier; this.cache = (Cache) informer.getStore(); + this.configurationService = configurationService; + } @Override public void start() throws OperatorException { try { - var configService = ConfigurationServiceProvider.instance(); + // register stopped handler if we have one defined - configService.getInformerStoppedHandler() + configurationService.getInformerStoppedHandler() .ifPresent(ish -> { final var stopped = informer.stopped(); if (stopped != null) { @@ -65,7 +69,7 @@ public void start() throws OperatorException { + fullResourceName + "/" + version); } }); - if (!configService.stopOnInformerErrorDuringStartup()) { + if (!configurationService.stopOnInformerErrorDuringStartup()) { informer.exceptionHandler((b, t) -> !ExceptionHandler.isDeserializationException(t)); } // change thread name for easier debugging @@ -82,12 +86,12 @@ public void start() throws OperatorException { // starts log.trace("Waiting informer to start namespace: {} resource: {}", namespaceIdentifier, resourceName); - start.toCompletableFuture().get(configService.cacheSyncTimeout().toMillis(), + start.toCompletableFuture().get(configurationService.cacheSyncTimeout().toMillis(), TimeUnit.MILLISECONDS); log.debug("Started informer for namespace: {} resource: {}", namespaceIdentifier, resourceName); } catch (TimeoutException | ExecutionException e) { - if (configService.stopOnInformerErrorDuringStartup()) { + if (configurationService.stopOnInformerErrorDuringStartup()) { log.error("Informer startup error. Operator will be stopped. Informer: {}", informer, e); throw new OperatorException(e); } else { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java index b0b5b6313f..356a71bc14 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java @@ -1,9 +1,6 @@ package io.javaoperatorsdk.operator.processing.event.source.informer; -import java.util.List; -import java.util.Map; -import java.util.Optional; -import java.util.Set; +import java.util.*; import java.util.function.Function; import java.util.function.Predicate; import java.util.stream.Stream; @@ -16,6 +13,8 @@ import io.fabric8.kubernetes.client.dsl.MixedOperation; import io.fabric8.kubernetes.client.dsl.Resource; import io.fabric8.kubernetes.client.informers.ResourceEventHandler; +import io.javaoperatorsdk.operator.api.config.ConfigurationService; +import io.javaoperatorsdk.operator.api.config.ExecutorServiceManager; import io.javaoperatorsdk.operator.api.config.NamespaceChangeable; import io.javaoperatorsdk.operator.api.config.ResourceConfiguration; import io.javaoperatorsdk.operator.api.reconciler.dependent.RecentOperationCacheFiller; @@ -31,20 +30,22 @@ public abstract class ManagedInformerEventSource> extends AbstractResourceEventSource implements ResourceEventHandler, Cache, IndexerResourceCache, - RecentOperationCacheFiller, - NamespaceChangeable, InformerWrappingEventSourceHealthIndicator, Configurable { + RecentOperationCacheFiller, NamespaceChangeable, + InformerWrappingEventSourceHealthIndicator, Configurable { private static final Logger log = LoggerFactory.getLogger(ManagedInformerEventSource.class); protected TemporaryResourceCache temporaryResourceCache; protected InformerManager cache; protected C configuration; + protected MixedOperation, Resource> client; + protected ConfigurationService configurationService; protected ManagedInformerEventSource( MixedOperation, Resource> client, C configuration) { super(configuration.getResourceClass()); + this.client = client; temporaryResourceCache = new TemporaryResourceCache<>(this); - cache = new InformerManager<>(client, configuration, this); this.configuration = configuration; } @@ -126,9 +127,7 @@ void setTemporalResourceCache(TemporaryResourceCache temporaryResourceCache) this.temporaryResourceCache = temporaryResourceCache; } - public void addIndexers(Map>> indexers) { - manager().addIndexers(indexers); - } + public abstract void addIndexers(Map>> indexers); public List byIndex(String indexName, String indexKey) { return manager().byIndex(indexName, indexKey); @@ -170,4 +169,11 @@ public String toString() { "resourceClass: " + configuration.getResourceClass().getSimpleName() + "}"; } + + public abstract void setConfigurationService(ConfigurationService configurationService); + + public void setExecutorServiceManager( + ExecutorServiceManager executorServiceManager) { + cache.setExecutorServiceManager(executorServiceManager); + } } 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 1cc98403f8..7b80aeed8b 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 @@ -3,6 +3,8 @@ import org.junit.jupiter.api.Test; import io.fabric8.kubernetes.api.model.HasMetadata; +import io.javaoperatorsdk.operator.api.config.BaseConfigurationService; +import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.api.config.ResolvedControllerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; import io.javaoperatorsdk.operator.processing.Controller; @@ -15,10 +17,10 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; -public class ControllerManagerTest { +class ControllerManagerTest { @Test - public void shouldNotAddMultipleControllersForSameCustomResource() { + void shouldNotAddMultipleControllersForSameCustomResource() { final var registered = new TestControllerConfiguration<>(new TestCustomReconciler(null), TestCustomResource.class); final var duplicated = @@ -28,7 +30,7 @@ public void shouldNotAddMultipleControllersForSameCustomResource() { } @Test - public void addingMultipleControllersForCustomResourcesWithSameVersionsShouldNotWork() { + void addingMultipleControllersForCustomResourcesWithSameVersionsShouldNotWork() { final var registered = new TestControllerConfiguration<>(new TestCustomReconciler(null), TestCustomResource.class); final var duplicated = new TestControllerConfiguration<>(new TestCustomReconcilerOtherV1(), @@ -40,8 +42,12 @@ public void addingMultipleControllersForCustomResourcesWithSameVersionsShouldNot private void checkException( TestControllerConfiguration registered, TestControllerConfiguration duplicated) { + + ConfigurationService configurationService = new BaseConfigurationService(); + final var exception = assertThrows(OperatorException.class, () -> { - final var controllerManager = new ControllerManager(); + final var controllerManager = + new ControllerManager(configurationService.getExecutorServiceManager()); controllerManager.add(new Controller<>(registered.controller, registered, MockKubernetesClient.client(registered.getResourceClass()))); controllerManager.add(new Controller<>(duplicated.controller, duplicated, @@ -59,7 +65,8 @@ private static class TestControllerConfiguration private final Reconciler controller; public TestControllerConfiguration(Reconciler controller, Class crClass) { - super(crClass, getControllerName(controller), controller.getClass()); + super(crClass, getControllerName(controller), controller.getClass(), + new BaseConfigurationService()); this.controller = controller; } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/LeaderElectionManagerTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/LeaderElectionManagerTest.java index 66b0030b5e..5fd1aee206 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/LeaderElectionManagerTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/LeaderElectionManagerTest.java @@ -10,7 +10,8 @@ import org.junit.jupiter.api.io.TempDir; import io.fabric8.kubernetes.client.KubernetesClient; -import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; +import io.javaoperatorsdk.operator.api.config.BaseConfigurationService; +import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.api.config.LeaderElectionConfiguration; import static io.fabric8.kubernetes.client.Config.KUBERNETES_AUTH_TRYKUBECONFIG_SYSTEM_PROPERTY; @@ -21,29 +22,25 @@ class LeaderElectionManagerTest { - private KubernetesClient kubernetesClient; private LeaderElectionManager leaderElectionManager; @BeforeEach void setUp() { ControllerManager controllerManager = mock(ControllerManager.class); - kubernetesClient = mock(KubernetesClient.class); - leaderElectionManager = new LeaderElectionManager(controllerManager); + final var kubernetesClient = mock(KubernetesClient.class); + var configurationService = + ConfigurationService.newOverriddenConfigurationService(new BaseConfigurationService(), + o -> o.withLeaderElectionConfiguration(new LeaderElectionConfiguration("test"))); + leaderElectionManager = + new LeaderElectionManager(kubernetesClient, controllerManager, configurationService); } @AfterEach void tearDown() { - ConfigurationServiceProvider.reset(); System.getProperties().remove(KUBERNETES_NAMESPACE_FILE); System.getProperties().remove(KUBERNETES_AUTH_TRYKUBECONFIG_SYSTEM_PROPERTY); } - @Test - void testInit() { - leaderElectionManager.init(new LeaderElectionConfiguration("test", "testns"), kubernetesClient); - assertTrue(leaderElectionManager.isLeaderElectionEnabled()); - } - @Test void testInitInferLeaseNamespace(@TempDir Path tempDir) throws IOException { var namespace = "foo"; @@ -53,7 +50,7 @@ void testInitInferLeaseNamespace(@TempDir Path tempDir) throws IOException { System.setProperty(KUBERNETES_AUTH_TRYKUBECONFIG_SYSTEM_PROPERTY, "false"); System.setProperty(KUBERNETES_NAMESPACE_FILE, namespacePath.toString()); - leaderElectionManager.init(new LeaderElectionConfiguration("test"), kubernetesClient); + leaderElectionManager.start(); assertTrue(leaderElectionManager.isLeaderElectionEnabled()); } @@ -62,7 +59,6 @@ void testFailedToInitInferLeaseNamespace() { System.setProperty(KUBERNETES_AUTH_TRYKUBECONFIG_SYSTEM_PROPERTY, "false"); assertThrows( IllegalArgumentException.class, - () -> leaderElectionManager.init(new LeaderElectionConfiguration("test"), - kubernetesClient)); + () -> leaderElectionManager.start()); } } 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 8a95431885..d5646c476a 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,20 +1,10 @@ package io.javaoperatorsdk.operator; -import java.util.Optional; - -import org.junit.jupiter.api.AfterAll; -import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; -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.LeaderElectionConfiguration; -import io.javaoperatorsdk.operator.api.config.Version; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; @@ -29,28 +19,11 @@ class OperatorTest { private final KubernetesClient kubernetesClient = MockKubernetesClient.client(ConfigMap.class); private Operator operator; - @BeforeAll - @AfterAll - static void setUpConfigurationServiceProvider() { - ConfigurationServiceProvider.reset(); - } - @BeforeEach void initOperator() { - ConfigurationServiceProvider.reset(); operator = new Operator(kubernetesClient); } - @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(new FooReconciler())); - } - @Test void shouldBePossibleToRetrieveNumberOfRegisteredControllers() { assertEquals(0, operator.getRegisteredControllersNumber()); @@ -78,32 +51,6 @@ void shouldBePossibleToRetrieveRegisteredControllerByName() { assertEquals(maybeController.get(), registeredControllers.stream().findFirst().orElseThrow()); } - @Test - void shouldBeAbleToProvideLeaderElectionConfiguration() { - assertTrue(ConfigurationServiceProvider.instance().getLeaderElectionConfiguration().isEmpty()); - new Operator(kubernetesClient, c -> c.withLeaderElectionConfiguration( - new LeaderElectionConfiguration("leader-election-test", "namespace", "identity"))); - assertEquals("identity", ConfigurationServiceProvider.instance() - .getLeaderElectionConfiguration().orElseThrow().getIdentity().orElseThrow()); - } - - @Test - void shouldBeAbleToInitLeaderElectionManagerWithoutOverrider() { - ConfigurationServiceProvider.reset(); - final LeaderElectionConfiguration leaderElectionConfiguration = - new LeaderElectionConfiguration("leader-election-test", "namespace", "identity"); - final AbstractConfigurationService configurationService = - new AbstractConfigurationService(Version.UNKNOWN) { - @Override - public Optional getLeaderElectionConfiguration() { - return Optional.of(leaderElectionConfiguration); - } - }; - new Operator(kubernetesClient, configurationService); - assertEquals("identity", ConfigurationServiceProvider.instance() - .getLeaderElectionConfiguration().orElseThrow().getIdentity().orElseThrow()); - } - @ControllerConfiguration private static class FooReconciler implements Reconciler { 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 deleted file mode 100644 index 33ff4bd214..0000000000 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProviderTest.java +++ /dev/null @@ -1,70 +0,0 @@ -package io.javaoperatorsdk.operator.api.config; - -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 ConfigurationServiceOverrider(new AbstractConfigurationService(null)) - .withLeaderElectionConfiguration(new LeaderElectionConfiguration("bar", "barNS")) - .build(); - assertFalse(config.checkCRDAndValidateLocalModel()); - assertEquals("bar", config.getLeaderElectionConfiguration().orElseThrow().getLeaseName()); - - ConfigurationServiceProvider.set(config); - var instance = ConfigurationServiceProvider.instance(); - assertEquals(config, instance); - - final ConfigurationService overridden = ConfigurationServiceProvider.overrideCurrent( - o -> o.checkingCRDAndValidateLocalModel(true)); - instance = ConfigurationServiceProvider.instance(); - assertEquals(overridden, instance); - assertNotEquals(config, instance); - assertTrue(instance.checkCRDAndValidateLocalModel()); - assertEquals("bar", instance.getLeaderElectionConfiguration().orElseThrow().getLeaseName()); - - assertThrows(IllegalStateException.class, - () -> ConfigurationServiceProvider.overrideCurrent(o -> o.withCloseClientOnStop(false))); - } - - @Test - void resetShouldResetAllState() { - shouldBePossibleToOverrideConfigOnce(); - - ConfigurationServiceProvider.reset(); - // makes sure createDefault creates a new instance - assertNotEquals(ConfigurationServiceProvider.getDefault(), - ConfigurationServiceProvider.createDefault()); - - shouldBePossibleToOverrideConfigOnce(); - } -} diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/MockControllerConfiguration.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/MockControllerConfiguration.java index baf680b677..6ae83294bb 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/MockControllerConfiguration.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/MockControllerConfiguration.java @@ -3,18 +3,26 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import static io.javaoperatorsdk.operator.api.reconciler.Constants.DEFAULT_NAMESPACES_SET; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; public class MockControllerConfiguration { - @SuppressWarnings({"unchecked", "rawtypes"}) + public static ControllerConfiguration forResource( Class resourceType) { + return forResource(resourceType, null); + } + + @SuppressWarnings({"unchecked", "rawtypes"}) + public static ControllerConfiguration forResource( + Class resourceType, ConfigurationService configurationService) { final ControllerConfiguration configuration = mock(ControllerConfiguration.class); when(configuration.getResourceClass()).thenReturn(resourceType); when(configuration.getNamespaces()).thenReturn(DEFAULT_NAMESPACES_SET); - when(configuration.getEffectiveNamespaces()).thenCallRealMethod(); + when(configuration.getEffectiveNamespaces(any())).thenCallRealMethod(); when(configuration.getName()).thenReturn(resourceType.getSimpleName()); + when(configuration.getConfigurationService()).thenReturn(configurationService); return configuration; } } 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 a9031ffcf6..e0afa909fe 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 @@ -4,51 +4,58 @@ import io.fabric8.kubernetes.api.model.Secret; import io.javaoperatorsdk.operator.MockKubernetesClient; -import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; +import io.javaoperatorsdk.operator.api.config.BaseConfigurationService; +import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.api.config.MockControllerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.Cleaner; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; import io.javaoperatorsdk.operator.sample.simple.TestCustomResource; import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.*; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import static org.mockito.Mockito.withSettings; @SuppressWarnings({"unchecked", "rawtypes"}) class ControllerTest { final Reconciler reconciler = mock(Reconciler.class); + ConfigurationService configurationService = new BaseConfigurationService(); + @Test void crdShouldNotBeCheckedForNativeResources() { final var client = MockKubernetesClient.client(Secret.class); - final var configuration = MockControllerConfiguration.forResource(Secret.class); - - final var controller = new Controller(reconciler, configuration, client); + final var configuration = + MockControllerConfiguration.forResource(Secret.class, configurationService); + final var controller = + new Controller(reconciler, configuration, client); controller.start(); verify(client, never()).apiextensions(); } @Test void crdShouldNotBeCheckedForCustomResourcesIfDisabled() { - ConfigurationServiceProvider.reset(); final var client = MockKubernetesClient.client(TestCustomResource.class); - final var configuration = MockControllerConfiguration.forResource(TestCustomResource.class); - - try { - ConfigurationServiceProvider.overrideCurrent(o -> o.checkingCRDAndValidateLocalModel(false)); - final var controller = new Controller(reconciler, configuration, client); - controller.start(); - verify(client, never()).apiextensions(); - } finally { - ConfigurationServiceProvider.reset(); - } - } + ConfigurationService configurationService = + ConfigurationService.newOverriddenConfigurationService(new BaseConfigurationService(), + o -> o.checkingCRDAndValidateLocalModel(false)); + final var configuration = + MockControllerConfiguration.forResource(TestCustomResource.class, configurationService); + final var controller = new Controller(reconciler, configuration, client); + controller.start(); + verify(client, never()).apiextensions(); + + } @Test void usesFinalizerIfThereIfReconcilerImplementsCleaner() { Reconciler reconciler = mock(Reconciler.class, withSettings().extraInterfaces(Cleaner.class)); final var configuration = MockControllerConfiguration.forResource(Secret.class); + when(configuration.getConfigurationService()).thenReturn(new BaseConfigurationService()); final var controller = new Controller(reconciler, configuration, MockKubernetesClient.client(Secret.class)); 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 a23af75d9c..cefb855b43 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 @@ -9,6 +9,7 @@ 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.BaseConfigurationService; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.processing.dependent.Matcher; @@ -31,6 +32,8 @@ class GenericKubernetesResourceMatcherTest { @BeforeAll static void setUp() { final var controllerConfiguration = mock(ControllerConfiguration.class); + when(controllerConfiguration.getConfigurationService()) + .thenReturn(new BaseConfigurationService()); 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 e67d579adc..049b86a559 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 @@ -7,6 +7,7 @@ import io.fabric8.kubernetes.api.model.apps.Deployment; import io.javaoperatorsdk.operator.ReconcilerUtils; +import io.javaoperatorsdk.operator.api.config.BaseConfigurationService; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.Context; @@ -22,6 +23,8 @@ class GenericResourceUpdatePreProcessorTest { @BeforeAll static void setUp() { final var controllerConfiguration = mock(ControllerConfiguration.class); + when(controllerConfiguration.getConfigurationService()) + .thenReturn(new BaseConfigurationService()); when(context.getControllerConfiguration()).thenReturn(controllerConfiguration); } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupportTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupportTest.java index 0f6fea2449..ca73c8cae1 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupportTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowSupportTest.java @@ -21,7 +21,7 @@ class ManagedWorkflowSupportTest { public static final String NAME_3 = "name3"; public static final String NAME_4 = "name4"; - ManagedWorkflowSupport managedWorkflowSupport = ManagedWorkflowSupport.instance(); + ManagedWorkflowSupport managedWorkflowSupport = new ManagedWorkflowSupport(); @Test void trivialCasesNameDuplicates() { diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowTest.java index cfe6a7cfed..649e0c3050 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/ManagedWorkflowTest.java @@ -4,7 +4,7 @@ import org.junit.jupiter.api.Test; -import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; +import io.javaoperatorsdk.operator.api.config.BaseConfigurationService; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; import io.javaoperatorsdk.operator.api.reconciler.dependent.Deleter; @@ -63,7 +63,7 @@ ManagedWorkflow managedWorkflow(DependentResourceSpec... specs) { final var specList = List.of(specs); when(configuration.getDependentResources()).thenReturn(specList); - return ConfigurationServiceProvider.instance().getWorkflowFactory() + return new BaseConfigurationService().getWorkflowFactory() .workflowFor(configuration); } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutorTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutorTest.java index a8e46fc258..64fdc6e6f7 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutorTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutorTest.java @@ -1,6 +1,10 @@ package io.javaoperatorsdk.operator.processing.dependent.workflow; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; + import org.assertj.core.api.Assertions; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import io.javaoperatorsdk.operator.AggregatedOperatorException; @@ -10,6 +14,7 @@ import static io.javaoperatorsdk.operator.processing.dependent.workflow.ExecutionAssert.assertThat; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; class WorkflowCleanupExecutorTest extends AbstractWorkflowExecutorTest { @@ -17,7 +22,14 @@ class WorkflowCleanupExecutorTest extends AbstractWorkflowExecutorTest { protected TestDeleterDependent dd2 = new TestDeleterDependent("DR_DELETER_2"); protected TestDeleterDependent dd3 = new TestDeleterDependent("DR_DELETER_3"); @SuppressWarnings("unchecked") + Context mockContext = mock(Context.class); + ExecutorService executorService = Executors.newCachedThreadPool(); + + @BeforeEach + void setup() { + when(mockContext.getWorkflowExecutorService()).thenReturn(executorService); + } @Test void cleanUpDiamondWorkflow() { @@ -28,7 +40,7 @@ void cleanUpDiamondWorkflow() { .addDependentResource(dd3).dependsOn(dr1, dd2) .build(); - var res = workflow.cleanup(new TestCustomResource(), null); + var res = workflow.cleanup(new TestCustomResource(), mockContext); assertThat(executionHistory).reconciledInOrder(dd3, dd2, dd1).notReconciled(dr1); @@ -121,7 +133,7 @@ void dontDeleteIfGarbageCollected() { .addDependentResource(gcDeleter) .build(); - var res = workflow.cleanup(new TestCustomResource(), null); + var res = workflow.cleanup(new TestCustomResource(), mockContext); assertThat(executionHistory) .notReconciled(gcDeleter); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutorTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutorTest.java index 96de59ce5b..b3c4ea35e9 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutorTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutorTest.java @@ -1,6 +1,10 @@ package io.javaoperatorsdk.operator.processing.dependent.workflow; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; + import org.assertj.core.api.Assertions; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import io.javaoperatorsdk.operator.AggregatedOperatorException; @@ -10,10 +14,13 @@ import static io.javaoperatorsdk.operator.processing.dependent.workflow.ExecutionAssert.assertThat; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; @SuppressWarnings("rawtypes") class WorkflowReconcileExecutorTest extends AbstractWorkflowExecutorTest { + + private final Condition met_reconcile_condition = (primary, secondary, context) -> true; private final Condition not_met_reconcile_condition = (primary, secondary, context) -> false; @@ -24,6 +31,12 @@ class WorkflowReconcileExecutorTest extends AbstractWorkflowExecutorTest { @SuppressWarnings("unchecked") Context mockContext = mock(Context.class); + ExecutorService executorService = Executors.newCachedThreadPool(); + + @BeforeEach + void setup() { + when(mockContext.getWorkflowExecutorService()).thenReturn(executorService); + } @Test void reconcileTopLevelResources() { diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowTest.java index 4e6cc5c329..49f1274fa8 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowTest.java @@ -1,6 +1,8 @@ package io.javaoperatorsdk.operator.processing.dependent.workflow; import java.util.Set; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; import java.util.stream.Collectors; import org.junit.jupiter.api.Test; @@ -20,6 +22,8 @@ @SuppressWarnings("rawtypes") class WorkflowTest { + ExecutorService executorService = Executors.newCachedThreadPool(); + @Test void calculatesTopLevelResources() { var dr1 = mock(DependentResource.class); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventProcessorTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventProcessorTest.java index a7e431b88c..880b4a8e14 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventProcessorTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventProcessorTest.java @@ -15,7 +15,10 @@ import org.slf4j.LoggerFactory; import io.fabric8.kubernetes.api.model.HasMetadata; -import io.javaoperatorsdk.operator.api.config.*; +import io.javaoperatorsdk.operator.api.config.BaseConfigurationService; +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.monitoring.Metrics; import io.javaoperatorsdk.operator.processing.event.rate.LinearRateLimiter; import io.javaoperatorsdk.operator.processing.event.rate.RateLimiter; @@ -426,12 +429,17 @@ void executionOfReconciliationShouldNotStartIfProcessorStopped() throws Interrup Thread.sleep(DISPATCHING_DELAY); return PostExecutionControl.defaultDispatch(); }); - // one event will lock the thread / executor - ConfigurationServiceProvider.overrideCurrent(o -> { - o.withConcurrentReconciliationThreads(1); - o.withMinConcurrentReconciliationThreads(1); - }); - ExecutorServiceManager.reset(); + + final var configurationService = ConfigurationService.newOverriddenConfigurationService( + new BaseConfigurationService(), + o -> { + o.withConcurrentReconciliationThreads(1); + o.withMinConcurrentReconciliationThreads(1); + }); + eventProcessor = + spy(new EventProcessor(controllerConfiguration(null, rateLimiterMock, configurationService), + reconciliationDispatcherMock, + eventSourceManagerMock, null)); eventProcessor.start(); eventProcessor.handleEvent(prepareCREvent()); @@ -485,12 +493,17 @@ private void overrideData(ResourceID id, HasMetadata applyTo) { } ControllerConfiguration controllerConfiguration(Retry retry, RateLimiter rateLimiter) { + return controllerConfiguration(retry, rateLimiter, new BaseConfigurationService()); + } + + ControllerConfiguration controllerConfiguration(Retry retry, RateLimiter rateLimiter, + ConfigurationService configurationService) { ControllerConfiguration res = mock(ControllerConfiguration.class); when(res.getName()).thenReturn("Test"); when(res.getRetry()).thenReturn(retry); when(res.getRateLimiter()).thenReturn(rateLimiter); when(res.maxReconciliationInterval()).thenReturn(Optional.of(Duration.ofMillis(1000))); - when(res.getConfigurationService()).thenReturn(new BaseConfigurationService()); + when(res.getConfigurationService()).thenReturn(configurationService); return res; } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.java index 51a4b597f7..65d2ec7413 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.java @@ -8,6 +8,7 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.MockKubernetesClient; import io.javaoperatorsdk.operator.OperatorException; +import io.javaoperatorsdk.operator.api.config.BaseConfigurationService; import io.javaoperatorsdk.operator.api.config.MockControllerConfiguration; import io.javaoperatorsdk.operator.api.config.informer.InformerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; @@ -148,6 +149,10 @@ void changesNamespacesOnControllerAndInformerEventSources() { String newNamespaces = "new-namespace"; final var configuration = MockControllerConfiguration.forResource(HasMetadata.class); + + final var configService = new BaseConfigurationService(); + when(configuration.getConfigurationService()).thenReturn(configService); + final Controller controller = new Controller(mock(Reconciler.class), configuration, MockKubernetesClient.client(HasMetadata.class)); @@ -173,6 +178,9 @@ void changesNamespacesOnControllerAndInformerEventSources() { private EventSourceManager initManager() { final var configuration = MockControllerConfiguration.forResource(ConfigMap.class); + final var configService = new BaseConfigurationService(); + when(configuration.getConfigurationService()).thenReturn(configService); + final Controller controller = new Controller(mock(Reconciler.class), configuration, MockKubernetesClient.client(ConfigMap.class)); return new EventSourceManager(controller); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourcesTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourcesTest.java index b5438cdd04..db2a69609d 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourcesTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourcesTest.java @@ -6,6 +6,7 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.api.model.Service; import io.javaoperatorsdk.operator.MockKubernetesClient; +import io.javaoperatorsdk.operator.api.config.BaseConfigurationService; import io.javaoperatorsdk.operator.api.config.MockControllerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; import io.javaoperatorsdk.operator.processing.Controller; @@ -73,9 +74,10 @@ void additionalEventSourcesShouldNotContainNamedEventSources() { void checkControllerResourceEventSource() { final var eventSources = new EventSources(); final var configuration = MockControllerConfiguration.forResource(HasMetadata.class); + when(configuration.getConfigurationService()).thenReturn(new BaseConfigurationService()); final var controller = new Controller(mock(Reconciler.class), configuration, MockKubernetesClient.client(HasMetadata.class)); - eventSources.initControllerEventSource(controller); + eventSources.createControllerEventSource(controller); final var controllerResourceEventSource = eventSources.controllerResourceEventSource(); assertNotNull(controllerResourceEventSource); assertEquals(HasMetadata.class, controllerResourceEventSource.resourceType()); 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 e7f9331d5c..0094af313e 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,7 +6,6 @@ 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; @@ -22,7 +21,12 @@ import io.javaoperatorsdk.operator.MockKubernetesClient; import io.javaoperatorsdk.operator.OperatorException; import io.javaoperatorsdk.operator.TestUtils; -import io.javaoperatorsdk.operator.api.config.*; +import io.javaoperatorsdk.operator.api.config.BaseConfigurationService; +import io.javaoperatorsdk.operator.api.config.Cloner; +import io.javaoperatorsdk.operator.api.config.ConfigurationService; +import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.config.ExecutorServiceManager; +import io.javaoperatorsdk.operator.api.config.MockControllerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.Cleaner; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.api.reconciler.DeleteControl; @@ -65,6 +69,8 @@ class ReconciliationDispatcherTest { private TestReconciler reconciler; private final CustomResourceFacade customResourceFacade = mock(ReconciliationDispatcher.CustomResourceFacade.class); + private static ConfigurationService configurationService; + private static ExecutorServiceManager executorServiceManager; @BeforeAll static void classSetup() { @@ -75,19 +81,15 @@ static void classSetup() { * equals will fail on the two equal but NOT identical TestCustomResources because equals is not * implemented on TestCustomResourceSpec or TestCustomResourceStatus */ - ConfigurationServiceProvider.reset(); - ConfigurationServiceProvider.overrideCurrent(overrider -> overrider - .checkingCRDAndValidateLocalModel(false).withResourceCloner(new Cloner() { - @Override - public R clone(R object) { - return object; - } - })); - } - - @AfterAll - static void tearDown() { - ConfigurationServiceProvider.reset(); + configurationService = + ConfigurationService.newOverriddenConfigurationService(new BaseConfigurationService(), + overrider -> overrider.checkingCRDAndValidateLocalModel(false) + .withResourceCloner(new Cloner() { + @Override + public R clone(R object) { + return object; + } + })); } @BeforeEach @@ -103,9 +105,11 @@ private ReconciliationDispatcher init(R customResourc CustomResourceFacade customResourceFacade, boolean useFinalizer) { final Class resourceClass = (Class) customResource.getClass(); - configuration = configuration == null ? MockControllerConfiguration.forResource(resourceClass) + configuration = configuration == null + ? MockControllerConfiguration.forResource(resourceClass, configurationService) : configuration; + when(configuration.getConfigurationService()).thenReturn(configurationService); when(configuration.getFinalizerName()).thenReturn(DEFAULT_FINALIZER); when(configuration.getName()).thenReturn("EventDispatcherTestController"); when(configuration.getResourceClass()).thenReturn(resourceClass); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/AbstractEventSourceTestBase.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/AbstractEventSourceTestBase.java index 79483e2865..373c071258 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/AbstractEventSourceTestBase.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/AbstractEventSourceTestBase.java @@ -2,7 +2,10 @@ import org.junit.jupiter.api.AfterEach; +import io.javaoperatorsdk.operator.api.config.BaseConfigurationService; +import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.processing.event.EventHandler; +import io.javaoperatorsdk.operator.processing.event.source.informer.ManagedInformerEventSource; import static org.mockito.Mockito.mock; @@ -19,6 +22,11 @@ public void setUpSource(S source) { setUpSource(source, true); } + + public void setUpSource(S source, boolean start, ConfigurationService configurationService) { + setUpSource(source, (T) mock(EventHandler.class), start, configurationService); + } + @SuppressWarnings("unchecked") public void setUpSource(S source, boolean start) { setUpSource(source, (T) mock(EventHandler.class), start); @@ -29,9 +37,22 @@ public void setUpSource(S source, T eventHandler) { } public void setUpSource(S source, T eventHandler, boolean start) { + setUpSource(source, eventHandler, start, new BaseConfigurationService()); + } + + public void setUpSource(S source, T eventHandler, boolean start, + ConfigurationService configurationService) { this.eventHandler = eventHandler; this.source = source; + + if (source instanceof ManagedInformerEventSource) { + ((ManagedInformerEventSource) source).setConfigurationService(configurationService); + ((ManagedInformerEventSource) source).setExecutorServiceManager( + configurationService.getExecutorServiceManager()); + } + source.setEventHandler(eventHandler); + if (start) { source.start(); } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/ResourceEventFilterTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/ResourceEventFilterTest.java index 26994cee1a..9e58a93ddf 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/ResourceEventFilterTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/ResourceEventFilterTest.java @@ -10,6 +10,7 @@ import io.javaoperatorsdk.operator.MockKubernetesClient; import io.javaoperatorsdk.operator.ReconcilerUtils; import io.javaoperatorsdk.operator.TestUtils; +import io.javaoperatorsdk.operator.api.config.BaseConfigurationService; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.config.ResolvedControllerConfiguration; import io.javaoperatorsdk.operator.processing.Controller; @@ -144,7 +145,7 @@ public ControllerConfig(String finalizer, boolean generationAware, null, null, null, - null, null, null, finalizer, null, null); + null, null, null, finalizer, null, null, new BaseConfigurationService()); setEventFilter(eventFilter); } } 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 dc8cf8698b..a22d2f85b6 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 @@ -9,6 +9,7 @@ import io.javaoperatorsdk.operator.MockKubernetesClient; import io.javaoperatorsdk.operator.ReconcilerUtils; import io.javaoperatorsdk.operator.TestUtils; +import io.javaoperatorsdk.operator.api.config.BaseConfigurationService; import io.javaoperatorsdk.operator.api.config.ResolvedControllerConfiguration; import io.javaoperatorsdk.operator.processing.Controller; import io.javaoperatorsdk.operator.processing.event.EventHandler; @@ -36,7 +37,8 @@ class ControllerResourceEventSourceTest extends @BeforeEach public void setup() { - setUpSource(new ControllerResourceEventSource<>(testController), false); + setUpSource(new ControllerResourceEventSource<>(testController), false, + new BaseConfigurationService()); } @Test @@ -196,7 +198,7 @@ public TestConfiguration(boolean generationAware, OnAddFilter overrider.withInformerStoppedHandler(informerStoppedHandler)); - informerEventSource = new InformerEventSource<>(informerConfiguration, - MockKubernetesClient.client(Deployment.class, unused -> { - throw exception; - })); - - // by default informer fails to start if there is an exception in the client on start. - // Throws the exception further. - assertThrows(OperatorException.class, () -> informerEventSource.start()); - verify(informerStoppedHandler, atLeastOnce()).onStop(any(), eq(exception)); - } finally { - ConfigurationServiceProvider.reset(); - } + final var exception = new RuntimeException("Informer stopped exceptionally!"); + final var informerStoppedHandler = mock(InformerStoppedHandler.class); + var configuration = + ConfigurationService.newOverriddenConfigurationService(new BaseConfigurationService(), + o -> o.withInformerStoppedHandler(informerStoppedHandler)); + + informerEventSource = new InformerEventSource<>(informerConfiguration, + MockKubernetesClient.client(Deployment.class, unused -> { + throw exception; + })); + informerEventSource.setConfigurationService(configuration); + informerEventSource.setExecutorServiceManager(configuration.getExecutorServiceManager()); + + // by default informer fails to start if there is an exception in the client on start. + // Throws the exception further. + assertThrows(OperatorException.class, () -> informerEventSource.start()); + verify(informerStoppedHandler, atLeastOnce()).onStop(any(), eq(exception)); } Deployment testDeployment() { 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 1da3a1af95..6a9e363210 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 @@ -10,7 +10,11 @@ import java.util.function.Consumer; import org.awaitility.Awaitility; -import org.junit.jupiter.api.extension.*; +import org.junit.jupiter.api.extension.AfterAllCallback; +import org.junit.jupiter.api.extension.AfterEachCallback; +import org.junit.jupiter.api.extension.BeforeAllCallback; +import org.junit.jupiter.api.extension.BeforeEachCallback; +import org.junit.jupiter.api.extension.ExtensionContext; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -23,8 +27,9 @@ 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.ConfigurationServiceOverrider; -import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; public abstract class AbstractOperatorExtension implements HasKubernetesClient, BeforeAllCallback, @@ -54,8 +59,7 @@ protected AbstractOperatorExtension( boolean waitForNamespaceDeletion, KubernetesClient kubernetesClient) { this.kubernetesClient = kubernetesClient != null ? kubernetesClient - : new KubernetesClientBuilder() - .withConfig(ConfigurationServiceProvider.instance().getClientConfiguration()).build(); + : new KubernetesClientBuilder().build(); this.infrastructure = infrastructure; this.infrastructureTimeout = infrastructureTimeout; this.oneNamespacePerClass = oneNamespacePerClass; @@ -180,8 +184,6 @@ protected void afterAllImpl(ExtensionContext context) { } protected void afterEachImpl(ExtensionContext context) { - // resets the config service provider so the controller configs are reconstructed always - ConfigurationServiceProvider.reset(); if (!oneNamespacePerClass) { after(context); } @@ -219,6 +221,7 @@ public static abstract class AbstractBuilder> { protected boolean waitForNamespaceDeletion; protected boolean oneNamespacePerClass; protected int namespaceDeleteTimeout; + protected ConfigurationService configurationService = new BaseConfigurationService(); protected AbstractBuilder() { this.infrastructure = new ArrayList<>(); @@ -257,7 +260,8 @@ public T oneNamespacePerClass(boolean value) { } public T withConfigurationService(Consumer overrider) { - ConfigurationServiceProvider.overrideCurrent(overrider); + configurationService = + ConfigurationService.newOverriddenConfigurationService(configurationService, overrider); return (T) this; } diff --git a/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/LocallyRunOperatorExtension.java b/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/LocallyRunOperatorExtension.java index e852455ebf..ec334a2ac4 100644 --- a/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/LocallyRunOperatorExtension.java +++ b/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/LocallyRunOperatorExtension.java @@ -22,7 +22,7 @@ import io.javaoperatorsdk.operator.Operator; import io.javaoperatorsdk.operator.ReconcilerUtils; import io.javaoperatorsdk.operator.RegisteredController; -import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; +import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.api.config.ControllerConfigurationOverrider; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; import io.javaoperatorsdk.operator.processing.retry.Retry; @@ -50,7 +50,8 @@ private LocallyRunOperatorExtension( boolean preserveNamespaceOnError, boolean waitForNamespaceDeletion, boolean oneNamespacePerClass, - KubernetesClient kubernetesClient) { + KubernetesClient kubernetesClient, + ConfigurationService configurationService) { super( infrastructure, infrastructureTimeout, @@ -62,7 +63,7 @@ private LocallyRunOperatorExtension( this.portForwards = portForwards; this.localPortForwards = new ArrayList<>(portForwards.size()); this.additionalCustomResourceDefinitions = additionalCustomResourceDefinitions; - this.operator = new Operator(getKubernetesClient()); + this.operator = new Operator(getKubernetesClient(), configurationService); this.registeredControllers = new HashMap<>(); } @@ -129,9 +130,8 @@ protected void before(ExtensionContext context) { additionalCustomResourceDefinitions .forEach(cr -> applyCrd(ReconcilerUtils.getResourceTypeName(cr))); - final var configurationService = ConfigurationServiceProvider.instance(); for (var ref : reconcilers) { - final var config = configurationService.getConfigurationFor(ref.reconciler); + final var config = operator.getConfigurationService().getConfigurationFor(ref.reconciler); final var oconfig = override(config); if (Namespaced.class.isAssignableFrom(config.getResourceClass())) { @@ -284,7 +284,8 @@ public LocallyRunOperatorExtension build() { preserveNamespaceOnError, waitForNamespaceDeletion, oneNamespacePerClass, - kubernetesClient); + kubernetesClient, + configurationService); } } 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 49f0ed2b67..12573b50f7 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,49 +2,12 @@ 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.config.ResolvedControllerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; 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(); - } - - static DefaultConfigurationService instance() { - return instance; - } - - ControllerConfiguration getConfigurationFor( - Reconciler reconciler, boolean createIfNeeded) { - final var previous = createIfNeeded(); - setCreateIfNeeded(createIfNeeded); - try { - return super.getConfigurationFor(reconciler); - } finally { - setCreateIfNeeded(previous); - } - } - - @Override - protected boolean createIfNeeded() { - return createIfNeeded; - } - - public void setCreateIfNeeded(boolean createIfNeeded) { - this.createIfNeeded = createIfNeeded; - } - @Override protected ControllerConfiguration configFor(Reconciler reconciler) { final var other = super.configFor(reconciler); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/DependentResourceCrossRefIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/DependentResourceCrossRefIT.java index 912b5b8ce7..deaa3c7493 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/DependentResourceCrossRefIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/DependentResourceCrossRefIT.java @@ -28,6 +28,7 @@ class DependentResourceCrossRefIT { @Test void dependentResourceCanReferenceEachOther() { + for (int i = 0; i < EXECUTION_NUMBER; i++) { operator.create(testResource(i)); } @@ -43,6 +44,7 @@ void dependentResourceCanReferenceEachOther() { assertThat(operator.get(Secret.class, TEST_RESOURCE_NAME + i)).isNotNull(); } }); + } DependentResourceCrossRefResource testResource(int n) { diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/InformerRelatedBehaviorITS.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/InformerRelatedBehaviorITS.java index 60d9f8f076..c2ce0b6ecd 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/InformerRelatedBehaviorITS.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/InformerRelatedBehaviorITS.java @@ -19,7 +19,6 @@ import io.fabric8.kubernetes.client.KubernetesClientBuilder; import io.fabric8.kubernetes.client.utils.KubernetesResourceUtil; import io.javaoperatorsdk.jenvtest.junit.EnableKubeAPIServer; -import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; import io.javaoperatorsdk.operator.health.InformerHealthIndicator; import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; import io.javaoperatorsdk.operator.processing.event.source.controller.ControllerResourceEventSource; @@ -315,7 +314,7 @@ Operator startOperator(boolean stopOnInformerErrorDuringStartup) { Operator startOperator(boolean stopOnInformerErrorDuringStartup, boolean addStopHandler, String... namespaces) { - ConfigurationServiceProvider.reset(); + reconciler = new InformerRelatedBehaviorTestReconciler(); Operator operator = new Operator(clientUsingServiceAccount(), 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 f9082ed481..5d1c570410 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneDependentResourceIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneDependentResourceIT.java @@ -1,13 +1,16 @@ package io.javaoperatorsdk.operator; import java.time.Duration; +import java.util.Set; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; +import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.api.model.ObjectMeta; import io.fabric8.kubernetes.api.model.apps.Deployment; -import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; +import io.javaoperatorsdk.operator.api.config.*; +import io.javaoperatorsdk.operator.api.reconciler.Reconciler; import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; import io.javaoperatorsdk.operator.sample.standalonedependent.StandaloneDependentTestCustomResource; import io.javaoperatorsdk.operator.sample.standalonedependent.StandaloneDependentTestCustomResourceSpec; @@ -52,7 +55,7 @@ void executeUpdateForTestingCacheUpdateForGetResource() { awaitForDeploymentReadyReplicas(1); - var clonedCr = ConfigurationServiceProvider.instance().getResourceCloner().clone(createdCR); + var clonedCr = cloner().clone(createdCR); clonedCr.getSpec().setReplicaCount(2); operator.replace(clonedCr); @@ -81,4 +84,25 @@ void awaitForDeploymentReadyReplicas(int expectedReplicaCount) { && deployment.getStatus().getReadyReplicas() == expectedReplicaCount; }); } + + Cloner cloner() { + return new ConfigurationService() { + @Override + public ControllerConfiguration getConfigurationFor( + Reconciler reconciler) { + return null; + } + + @Override + public Set getKnownReconcilerNames() { + return null; + } + + @Override + public Version getVersion() { + return null; + } + }.getResourceCloner(); + } + } 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 fc3c94ab6d..88fd33e946 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 @@ -16,7 +16,7 @@ class DefaultConfigurationServiceTest { public static final String CUSTOM_FINALIZER_NAME = "a.custom/finalizer"; - final DefaultConfigurationService configurationService = DefaultConfigurationService.instance(); + final DefaultConfigurationService configurationService = new DefaultConfigurationService(); @Test void returnsValuesFromControllerAnnotationFinalizer() { From 3d8f7f392cbbb05b9f6f0e033f7e914d6dc63cf2 Mon Sep 17 00:00:00 2001 From: csviri Date: Fri, 5 May 2023 09:38:27 +0200 Subject: [PATCH 2/7] rebase on next --- .../GenericKubernetesResourceMatcher.java | 27 ++++++++++++------- .../KubernetesDependentResource.java | 3 ++- 2 files changed, 19 insertions(+), 11 deletions(-) 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 e708f2be43..069b25f541 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 @@ -11,11 +11,11 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.api.model.Secret; import io.fabric8.zjsonpatch.JsonDiff; -import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.processing.dependent.Matcher; import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; public class GenericKubernetesResourceMatcher implements Matcher { @@ -58,7 +58,7 @@ static Matcher matcherFor( public Result match(R actualResource, P primary, Context

context) { var desired = dependentResource.desired(primary, context); return match(desired, actualResource, false, false, false, - context.getControllerConfiguration().getConfigurationService().getObjectMapper()); + context.getControllerConfiguration().getConfigurationService().getObjectMapper()); } /** @@ -91,9 +91,9 @@ public Result match(R actualResource, P primary, Context

context) { */ public static Result match(R desired, R actualResource, boolean considerLabelsAndAnnotations, boolean labelsAndAnnotationsEquality, - boolean specEquality,ObjectMapper objectMapper) { + boolean specEquality, ObjectMapper objectMapper) { return match(desired, actualResource, considerLabelsAndAnnotations, - labelsAndAnnotationsEquality, specEquality,objectMapper, EMPTY_ARRAY); + labelsAndAnnotationsEquality, specEquality, objectMapper, EMPTY_ARRAY); } /** @@ -120,7 +120,7 @@ public static Result match(R desired, R actualResourc boolean considerLabelsAndAnnotations, boolean labelsAndAnnotationsEquality, ObjectMapper objectMapper, String... ignorePaths) { return match(desired, actualResource, considerLabelsAndAnnotations, - labelsAndAnnotationsEquality, false,objectMapper, ignorePaths); + labelsAndAnnotationsEquality, false, objectMapper, ignorePaths); } /** @@ -152,11 +152,13 @@ public static Result match(R desired, R actualResourc public static Result match( KubernetesDependentResource dependentResource, R actualResource, P primary, Context

context, boolean considerLabelsAndAnnotations, - boolean labelsAndAnnotationsEquality,ObjectMapper objectMapper, + boolean labelsAndAnnotationsEquality, String... ignorePaths) { final var desired = dependentResource.desired(primary, context); return match(desired, actualResource, considerLabelsAndAnnotations, - labelsAndAnnotationsEquality, ignorePaths); + labelsAndAnnotationsEquality, context.getControllerConfiguration() + .getConfigurationService().getObjectMapper(), + ignorePaths); } public static Result match( @@ -166,11 +168,13 @@ public static Result match( boolean specEquality) { final var desired = dependentResource.desired(primary, context); return match(desired, actualResource, considerLabelsAndAnnotations, - labelsAndAnnotationsEquality, specEquality); + labelsAndAnnotationsEquality, specEquality, context.getControllerConfiguration() + .getConfigurationService().getObjectMapper()); } private static Result match(R desired, R actualResource, boolean considerMetadata, boolean labelsAndAnnotationsEquality, boolean specEquality, + ObjectMapper objectMapper, String... ignoredPaths) { final List ignoreList = ignoredPaths != null && ignoredPaths.length > 0 ? Arrays.asList(ignoredPaths) @@ -298,7 +302,8 @@ public static Result match( KubernetesDependentResource dependentResource, R actualResource, P primary, Context

context, boolean considerLabelsAndAnnotations, boolean specEquality) { final var desired = dependentResource.desired(primary, context); - return match(desired, actualResource, considerLabelsAndAnnotations, specEquality, context.getControllerConfiguration().getConfigurationService().getObjectMapper()); + return match(desired, actualResource, considerLabelsAndAnnotations, specEquality, + context.getControllerConfiguration().getConfigurationService().getObjectMapper()); } @Deprecated(forRemoval = true) @@ -306,7 +311,9 @@ public static Result match( KubernetesDependentResource dependentResource, R actualResource, P primary, Context

context, boolean considerLabelsAndAnnotations, String... ignorePaths) { final var desired = dependentResource.desired(primary, context); - return match(desired, actualResource, considerLabelsAndAnnotations, true, context.getControllerConfiguration().getConfigurationService().getObjectMapper(), ignorePaths); + return match(desired, actualResource, considerLabelsAndAnnotations, true, + context.getControllerConfiguration().getConfigurationService().getObjectMapper(), + ignorePaths); } } 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 c4500a6e50..ee98bf5f3f 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 @@ -143,7 +143,8 @@ public Result match(R actualResource, P primary, Context

context) { @SuppressWarnings("unused") public Result match(R actualResource, R desired, P primary, Context

context) { return GenericKubernetesResourceMatcher.match(desired, actualResource, false, - false, false,context.getControllerConfiguration().getConfigurationService().getObjectMapper()); + false, false, + context.getControllerConfiguration().getConfigurationService().getObjectMapper()); } protected void handleDelete(P primary, R secondary, Context

context) { From c67f79153b407085b1c914f7cce6b529d0e7d484 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Fri, 5 May 2023 14:24:11 +0200 Subject: [PATCH 3/7] refactor: use ControllerConfiguration.getEffectiveNamespaces --- .../operator/api/config/BaseConfigurationService.java | 3 +-- .../operator/api/config/informer/InformerConfiguration.java | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/BaseConfigurationService.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/BaseConfigurationService.java index 6bd436998d..46343a13d0 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/BaseConfigurationService.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/BaseConfigurationService.java @@ -107,8 +107,7 @@ protected

ControllerConfiguration

configFor(Reconcile " annotation for reconciler: " + reconciler); } Class> reconcilerClass = (Class>) reconciler.getClass(); - final var resourceClass = getResourceClassResolver() - .getResourceClass(reconcilerClass); + final var resourceClass = getResourceClassResolver().getResourceClass(reconcilerClass); final var name = ReconcilerUtils.getNameFor(reconciler); final var generationAware = valueOrDefault( 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 2b7c7ff461..f883799cc9 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 @@ -175,8 +175,7 @@ public InformerConfigurationBuilder withNamespaces(Set namespaces, */ public

InformerConfigurationBuilder withNamespacesInheritedFromController( EventSourceContext

context) { - namespaces = context.getControllerConfiguration().getEffectiveNamespaces( - context.getControllerConfiguration().getConfigurationService()); + namespaces = context.getControllerConfiguration().getEffectiveNamespaces(); this.inheritControllerNamespacesOnChange = true; return this; } From 330a558ceef69b1dbcb8e35e2ca5bc3ce8e6f2bc Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Fri, 5 May 2023 15:00:14 +0200 Subject: [PATCH 4/7] fix: typo, format --- .../operator/api/reconciler/DefaultContext.java | 2 +- .../dependent/workflow/WorkflowCleanupExecutor.java | 3 +-- .../dependent/workflow/WorkflowReconcileExecutor.java | 3 +-- .../event/source/informer/ManagedInformerEventSource.java | 8 +++++--- 4 files changed, 8 insertions(+), 8 deletions(-) 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 0553bc4b6c..4ad2c334ef 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 @@ -78,7 +78,7 @@ public KubernetesClient getClient() { @Override public ExecutorService getWorkflowExecutorService() { - // not that this should be always received from executor service manager, so we are able to do + // note that this should be always received from executor service manager, so we are able to do // restarts. return controller.getExecutorServiceManager().workflowExecutorService(); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutor.java index b604b47edb..56262ed6af 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutor.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutor.java @@ -55,8 +55,7 @@ private synchronized void handleCleanup(DependentResourceNode dependentResourceN return; } - Future nodeFuture = executorService - .submit(new CleanupExecutor<>(dependentResourceNode)); + Future nodeFuture = executorService.submit(new CleanupExecutor<>(dependentResourceNode)); markAsExecuting(dependentResourceNode, nodeFuture); log.debug("Submitted for cleanup: {}", dependentResourceNode); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java index 38f263667f..3e44cb3461 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowReconcileExecutor.java @@ -69,8 +69,7 @@ private synchronized void handleReconcile(DependentResourceNode depend if (!reconcileConditionMet) { handleReconcileConditionNotMet(dependentResourceNode); } else { - Future nodeFuture = executorService - .submit(new NodeReconcileExecutor(dependentResourceNode)); + var nodeFuture = executorService.submit(new NodeReconcileExecutor(dependentResourceNode)); markAsExecuting(dependentResourceNode, nodeFuture); log.debug("Submitted to reconcile: {} primaryID: {}", dependentResourceNode, primaryID); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java index 356a71bc14..c490d464a2 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java @@ -1,6 +1,9 @@ package io.javaoperatorsdk.operator.processing.event.source.informer; -import java.util.*; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; import java.util.function.Function; import java.util.function.Predicate; import java.util.stream.Stream; @@ -172,8 +175,7 @@ public String toString() { public abstract void setConfigurationService(ConfigurationService configurationService); - public void setExecutorServiceManager( - ExecutorServiceManager executorServiceManager) { + public void setExecutorServiceManager(ExecutorServiceManager executorServiceManager) { cache.setExecutorServiceManager(executorServiceManager); } } From d8a12c30b573b63d35d5f9b0cc3fc41c585e5551 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Fri, 5 May 2023 15:21:28 +0200 Subject: [PATCH 5/7] refactor: clean setting ConfigurationService in managed event sources --- .../processing/event/EventSourceManager.java | 5 ++- .../processing/event/EventSources.java | 3 -- .../ControllerResourceEventSource.java | 22 +++++------- .../source/informer/InformerEventSource.java | 15 ++++---- .../source/informer/InformerManager.java | 17 ++++----- .../source/informer/InformerWrapper.java | 36 +++++++++---------- .../informer/ManagedInformerEventSource.java | 8 ++--- .../source/AbstractEventSourceTestBase.java | 2 -- .../informer/InformerEventSourceTest.java | 1 - 9 files changed, 45 insertions(+), 64 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java index 96c7f1140c..29785eda01 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java @@ -157,9 +157,8 @@ public final synchronized void registerEventSource(String name, EventSource even } if (eventSource instanceof ManagedInformerEventSource) { var managedInformerEventSource = ((ManagedInformerEventSource) eventSource); - managedInformerEventSource.setConfigurationService(controller - .getConfiguration().getConfigurationService()); - managedInformerEventSource.setExecutorServiceManager(executorServiceManager); + managedInformerEventSource.setConfigurationService( + controller.getConfiguration().getConfigurationService()); } final var named = new NamedEventSource(eventSource, name); eventSources.add(named); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSources.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSources.java index 22dfc04b80..82b14dfacf 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSources.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSources.java @@ -32,9 +32,6 @@ class EventSources { void createControllerEventSource(Controller controller) { controllerResourceEventSource = new ControllerResourceEventSource<>(controller); - controllerResourceEventSource - .setConfigurationService(controller.getConfiguration().getConfigurationService()); - controllerResourceEventSource.setExecutorServiceManager(controller.getExecutorServiceManager()); } ControllerResourceEventSource controllerResourceEventSource() { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerResourceEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerResourceEventSource.java index 5affaac0b7..bd01d71a94 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerResourceEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerResourceEventSource.java @@ -12,14 +12,12 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.client.KubernetesClientException; import io.fabric8.kubernetes.client.informers.ResourceEventHandler; -import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.processing.Controller; import io.javaoperatorsdk.operator.processing.MDCUtils; import io.javaoperatorsdk.operator.processing.event.ResourceID; import io.javaoperatorsdk.operator.processing.event.source.filter.OnDeleteFilter; import io.javaoperatorsdk.operator.processing.event.source.filter.OnUpdateFilter; -import io.javaoperatorsdk.operator.processing.event.source.informer.InformerManager; import io.javaoperatorsdk.operator.processing.event.source.informer.ManagedInformerEventSource; import static io.javaoperatorsdk.operator.ReconcilerUtils.handleKubernetesClientException; @@ -44,20 +42,23 @@ public ControllerResourceEventSource(Controller controller) { super(controller.getCRClient(), controller.getConfiguration()); this.controller = controller; + final var config = controller.getConfiguration(); OnUpdateFilter internalOnUpdateFilter = (OnUpdateFilter) onUpdateFinalizerNeededAndApplied(controller.useFinalizer(), - controller.getConfiguration().getFinalizerName()) - .or(onUpdateGenerationAware(controller.getConfiguration().isGenerationAware())) + config.getFinalizerName()) + .or(onUpdateGenerationAware(config.isGenerationAware())) .or(onUpdateMarkedForDeletion()); - legacyFilters = controller.getConfiguration().getEventFilter(); + legacyFilters = config.getEventFilter(); // by default the on add should be processed in all cases regarding internal filters - controller.getConfiguration().onAddFilter().ifPresent(this::setOnAddFilter); - controller.getConfiguration().onUpdateFilter() + config.onAddFilter().ifPresent(this::setOnAddFilter); + config.onUpdateFilter() .ifPresentOrElse(filter -> setOnUpdateFilter(filter.and(internalOnUpdateFilter)), () -> setOnUpdateFilter(internalOnUpdateFilter)); - controller.getConfiguration().genericFilter().ifPresent(this::setGenericFilter); + config.genericFilter().ifPresent(this::setGenericFilter); + + setConfigurationService(config.getConfigurationService()); } @Override @@ -141,9 +142,4 @@ public void setOnDeleteFilter(OnDeleteFilter onDeleteFilter) { public void addIndexers(Map>> indexers) { manager().addIndexers(indexers); } - - public void setConfigurationService(ConfigurationService configurationService) { - cache = new InformerManager<>(client, configuration, configurationService, this); - this.configurationService = configurationService; - } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java index 83405743d4..fdd8312670 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java @@ -1,6 +1,10 @@ package io.javaoperatorsdk.operator.processing.event.source.informer; -import java.util.*; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; import java.util.function.Function; import java.util.stream.Collectors; @@ -343,12 +347,11 @@ private boolean acceptedByDeleteFilters(R resource, boolean b) { } - // Since this event source instance is created by the user, the ConfigService and - // ExecutorManagerService is actually injected after it is registered. Some of the subcomponents - // are created that time here. + // Since this event source instance is created by the user, the ConfigurationService is actually + // injected after it is registered. Some of the subcomponents are initialized at that time here. public void setConfigurationService(ConfigurationService configurationService) { - this.configurationService = configurationService; - cache = new InformerManager<>(client, configuration, configurationService, this); + super.setConfigurationService(configurationService); + cache.addIndexers(indexerBuffer); indexerBuffer = null; } 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 9c7e283b5b..a1b58b8c66 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 @@ -23,7 +23,9 @@ import io.fabric8.kubernetes.client.informers.ResourceEventHandler; import io.javaoperatorsdk.operator.OperatorException; import io.javaoperatorsdk.operator.ReconcilerUtils; -import io.javaoperatorsdk.operator.api.config.*; +import io.javaoperatorsdk.operator.api.config.Cloner; +import io.javaoperatorsdk.operator.api.config.ConfigurationService; +import io.javaoperatorsdk.operator.api.config.ResourceConfiguration; import io.javaoperatorsdk.operator.health.InformerHealthIndicator; import io.javaoperatorsdk.operator.processing.LifecycleAware; import io.javaoperatorsdk.operator.processing.event.ResourceID; @@ -43,7 +45,6 @@ public class InformerManager, Resource> client; private final ResourceEventHandler eventHandler; private final Map>> indexers = new HashMap<>(); - private ExecutorServiceManager executorServiceManager; private final ConfigurationService configurationService; public InformerManager(MixedOperation, Resource> client, @@ -59,7 +60,8 @@ public InformerManager(MixedOperation, Resource> public void start() throws OperatorException { initSources(); // make sure informers are all started before proceeding further - executorServiceManager.boundedExecuteAndWaitForAllToComplete(sources.values().stream(), + configurationService.getExecutorServiceManager().boundedExecuteAndWaitForAllToComplete( + sources.values().stream(), iw -> { iw.start(); return null; @@ -128,8 +130,7 @@ private InformerWrapper createEventSource( ResourceEventHandler eventHandler, String namespaceIdentifier) { var informer = filteredBySelectorClient.runnableInformer(0); configuration.getItemStore().ifPresent(informer::itemStore); - var source = - new InformerWrapper<>(informer, configurationService, namespaceIdentifier); + var source = new InformerWrapper<>(informer, configurationService, namespaceIdentifier); source.addEventHandler(eventHandler); sources.put(namespaceIdentifier, source); return source; @@ -214,10 +215,4 @@ public String toString() { public Map informerHealthIndicators() { return Collections.unmodifiableMap(sources); } - - public InformerManager setExecutorServiceManager( - ExecutorServiceManager executorServiceManager) { - this.executorServiceManager = executorServiceManager; - return this; - } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.java index 5a55bc7d7c..639eac1a59 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.java @@ -35,7 +35,7 @@ class InformerWrapper private final SharedIndexInformer informer; private final Cache cache; private final String namespaceIdentifier; - private ConfigurationService configurationService; + private final ConfigurationService configurationService; public InformerWrapper(SharedIndexInformer informer, ConfigurationService configurationService, String namespaceIdentifier) { @@ -43,7 +43,6 @@ public InformerWrapper(SharedIndexInformer informer, ConfigurationService con this.namespaceIdentifier = namespaceIdentifier; this.cache = (Cache) informer.getStore(); this.configurationService = configurationService; - } @Override @@ -51,24 +50,23 @@ public void start() throws OperatorException { try { // register stopped handler if we have one defined - configurationService.getInformerStoppedHandler() - .ifPresent(ish -> { - final var stopped = informer.stopped(); - if (stopped != null) { - stopped.handle((res, ex) -> { - ish.onStop(informer, ex); - return null; - }); - } else { - final var apiTypeClass = informer.getApiTypeClass(); - final var fullResourceName = - HasMetadata.getFullResourceName(apiTypeClass); - final var version = HasMetadata.getVersion(apiTypeClass); - throw new IllegalStateException( - "Cannot retrieve 'stopped' callback to listen to informer stopping for informer for " - + fullResourceName + "/" + version); - } + configurationService.getInformerStoppedHandler().ifPresent(ish -> { + final var stopped = informer.stopped(); + if (stopped != null) { + stopped.handle((res, ex) -> { + ish.onStop(informer, ex); + return null; }); + } else { + final var apiTypeClass = informer.getApiTypeClass(); + final var fullResourceName = + HasMetadata.getFullResourceName(apiTypeClass); + final var version = HasMetadata.getVersion(apiTypeClass); + throw new IllegalStateException( + "Cannot retrieve 'stopped' callback to listen to informer stopping for informer for " + + fullResourceName + "/" + version); + } + }); if (!configurationService.stopOnInformerErrorDuringStartup()) { informer.exceptionHandler((b, t) -> !ExceptionHandler.isDeserializationException(t)); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java index c490d464a2..92a317096c 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java @@ -17,7 +17,6 @@ import io.fabric8.kubernetes.client.dsl.Resource; import io.fabric8.kubernetes.client.informers.ResourceEventHandler; import io.javaoperatorsdk.operator.api.config.ConfigurationService; -import io.javaoperatorsdk.operator.api.config.ExecutorServiceManager; import io.javaoperatorsdk.operator.api.config.NamespaceChangeable; import io.javaoperatorsdk.operator.api.config.ResourceConfiguration; import io.javaoperatorsdk.operator.api.reconciler.dependent.RecentOperationCacheFiller; @@ -42,7 +41,6 @@ public abstract class ManagedInformerEventSource cache; protected C configuration; protected MixedOperation, Resource> client; - protected ConfigurationService configurationService; protected ManagedInformerEventSource( MixedOperation, Resource> client, C configuration) { @@ -173,9 +171,7 @@ public String toString() { "}"; } - public abstract void setConfigurationService(ConfigurationService configurationService); - - public void setExecutorServiceManager(ExecutorServiceManager executorServiceManager) { - cache.setExecutorServiceManager(executorServiceManager); + public void setConfigurationService(ConfigurationService configurationService) { + cache = new InformerManager<>(client, configuration, configurationService, this); } } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/AbstractEventSourceTestBase.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/AbstractEventSourceTestBase.java index 373c071258..de5df68319 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/AbstractEventSourceTestBase.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/AbstractEventSourceTestBase.java @@ -47,8 +47,6 @@ public void setUpSource(S source, T eventHandler, boolean start, if (source instanceof ManagedInformerEventSource) { ((ManagedInformerEventSource) source).setConfigurationService(configurationService); - ((ManagedInformerEventSource) source).setExecutorServiceManager( - configurationService.getExecutorServiceManager()); } source.setEventHandler(eventHandler); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSourceTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSourceTest.java index 41c88565fc..69e26f0b35 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSourceTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSourceTest.java @@ -259,7 +259,6 @@ void informerStoppedHandlerShouldBeCalledWhenInformerStops() { throw exception; })); informerEventSource.setConfigurationService(configuration); - informerEventSource.setExecutorServiceManager(configuration.getExecutorServiceManager()); // by default informer fails to start if there is an exception in the client on start. // Throws the exception further. From 8b3f76985002f64326e4c78f16b42c4b95ae9c99 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Fri, 5 May 2023 15:43:16 +0200 Subject: [PATCH 6/7] refactor: minor --- .../src/main/java/io/javaoperatorsdk/operator/Operator.java | 2 +- .../java/io/javaoperatorsdk/operator/processing/Controller.java | 2 +- 2 files changed, 2 insertions(+), 2 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 517383d79c..ee713d28a2 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 @@ -221,7 +221,7 @@ public

RegisteredController

register(Reconciler

re controllerManager.add(controller); final var watchedNS = configuration.watchAllNamespaces() ? "[all namespaces]" - : configuration.getEffectiveNamespaces(configurationService); + : configuration.getEffectiveNamespaces(); log.info( "Registered reconciler: '{}' for resource: '{}' for namespace(s): {}", 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 1966b8f695..e63ccf2d22 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 @@ -404,7 +404,7 @@ private void throwMissingCRDException(String crdName, String specVersion, String */ private void failOnMissingCurrentNS() { try { - configuration.getEffectiveNamespaces(getConfiguration().getConfigurationService()); + configuration.getEffectiveNamespaces(); } catch (OperatorException e) { throw new OperatorException( "Controller '" From 1c40dfc9d54a449804e0ba20db8c28074aa81e3b Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Fri, 5 May 2023 16:07:08 +0200 Subject: [PATCH 7/7] chore: clean-up --- .../src/main/java/io/javaoperatorsdk/operator/Operator.java | 6 ++---- .../io/javaoperatorsdk/operator/processing/Controller.java | 1 - .../javaoperatorsdk/operator/LeaderElectionManagerTest.java | 4 +--- .../javaoperatorsdk/operator/processing/ControllerTest.java | 3 +-- .../processing/event/ReconciliationDispatcherTest.java | 2 -- 5 files changed, 4 insertions(+), 12 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 ee713d28a2..295b1c8ffb 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 @@ -91,8 +91,7 @@ public Operator(KubernetesClient kubernetesClient, ConfigurationService configur */ @Deprecated(forRemoval = true) public void installShutdownHook() { - installShutdownHook( - Duration.ofSeconds(configurationService.getTerminationTimeoutSeconds())); + installShutdownHook(Duration.ofSeconds(configurationService.getTerminationTimeoutSeconds())); } /** @@ -182,8 +181,7 @@ public void stop() throws OperatorException { */ public

RegisteredController

register(Reconciler

reconciler) throws OperatorException { - final var controllerConfiguration = - configurationService.getConfigurationFor(reconciler); + final var controllerConfiguration = configurationService.getConfigurationFor(reconciler); return register(reconciler, controllerConfiguration); } 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 e63ccf2d22..e8d15671bf 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 @@ -84,7 +84,6 @@ public Controller(Reconciler

reconciler, associatedGVK = GroupVersionKind.gvkFor(configuration.getResourceClass()); final var configurationService = configuration.getConfigurationService(); - final var executorServiceManager = configurationService.getExecutorServiceManager(); this.reconciler = reconciler; this.configuration = configuration; this.kubernetesClient = kubernetesClient; diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/LeaderElectionManagerTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/LeaderElectionManagerTest.java index 5fd1aee206..f40e0630a2 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/LeaderElectionManagerTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/LeaderElectionManagerTest.java @@ -57,8 +57,6 @@ void testInitInferLeaseNamespace(@TempDir Path tempDir) throws IOException { @Test void testFailedToInitInferLeaseNamespace() { System.setProperty(KUBERNETES_AUTH_TRYKUBECONFIG_SYSTEM_PROPERTY, "false"); - assertThrows( - IllegalArgumentException.class, - () -> leaderElectionManager.start()); + assertThrows(IllegalArgumentException.class, () -> leaderElectionManager.start()); } } 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 e0afa909fe..36fdc2dee8 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 @@ -30,8 +30,7 @@ void crdShouldNotBeCheckedForNativeResources() { final var client = MockKubernetesClient.client(Secret.class); final var configuration = MockControllerConfiguration.forResource(Secret.class, configurationService); - final var controller = - new Controller(reconciler, configuration, client); + final var controller = new Controller(reconciler, configuration, client); controller.start(); verify(client, never()).apiextensions(); } 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 0094af313e..f6f46daf19 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 @@ -25,7 +25,6 @@ import io.javaoperatorsdk.operator.api.config.Cloner; import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; -import io.javaoperatorsdk.operator.api.config.ExecutorServiceManager; import io.javaoperatorsdk.operator.api.config.MockControllerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.Cleaner; import io.javaoperatorsdk.operator.api.reconciler.Context; @@ -70,7 +69,6 @@ class ReconciliationDispatcherTest { private final CustomResourceFacade customResourceFacade = mock(ReconciliationDispatcher.CustomResourceFacade.class); private static ConfigurationService configurationService; - private static ExecutorServiceManager executorServiceManager; @BeforeAll static void classSetup() {