From b0d8146d3e61feb0f972a480b0174475bbbc3c22 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 17 Sep 2025 20:02:02 +0200 Subject: [PATCH 1/2] refactor: avoid unneeded initializations in injection contexts Signed-off-by: Chris Laprun --- .../io/javaoperatorsdk/operator/Operator.java | 37 +++++++++++++------ 1 file changed, 26 insertions(+), 11 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 22072bb696..a56cfb7110 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 @@ -24,17 +24,17 @@ public class Operator implements LifecycleAware { private static final Logger log = LoggerFactory.getLogger(Operator.class); - private final ControllerManager controllerManager; - private final LeaderElectionManager leaderElectionManager; - private final ConfigurationService configurationService; + private ControllerManager controllerManager; + private LeaderElectionManager leaderElectionManager; + private ConfigurationService configurationService; private volatile boolean started = false; public Operator() { - this((KubernetesClient) null); + init(initConfigurationService(null, null), true); } Operator(KubernetesClient kubernetesClient) { - this(initConfigurationService(kubernetesClient, null)); + init(initConfigurationService(kubernetesClient, null), false); } /** @@ -46,12 +46,23 @@ public Operator() { * operator */ public Operator(ConfigurationService configurationService) { - this.configurationService = configurationService; + init(configurationService, false); + } + + private void init(ConfigurationService configurationService, boolean allowDeferredInit) { + if (configurationService == null) { + if (!allowDeferredInit) { + throw new IllegalStateException( + "Deferred initialization of ConfigurationService is not allowed"); + } + } else { + this.configurationService = configurationService; - final var executorServiceManager = configurationService.getExecutorServiceManager(); - controllerManager = new ControllerManager(executorServiceManager); + final var executorServiceManager = configurationService.getExecutorServiceManager(); + controllerManager = new ControllerManager(executorServiceManager); - leaderElectionManager = new LeaderElectionManager(controllerManager, configurationService); + leaderElectionManager = new LeaderElectionManager(controllerManager, configurationService); + } } /** @@ -62,10 +73,14 @@ public Operator(ConfigurationService configurationService) { * {@link ConfigurationService} values */ public Operator(Consumer overrider) { - this(initConfigurationService(null, overrider)); + init(initConfigurationService(null, overrider), false); } - private static ConfigurationService initConfigurationService( + /** + * Overridable by subclasses to enable deferred configuration, useful to avoid unneeded processing + * in injection scenarios + */ + protected ConfigurationService initConfigurationService( KubernetesClient client, Consumer overrider) { // initialize the client if the user didn't provide one if (client == null) { From 26e72bfb948d923eae07a9fe6d1b10e1080e470a Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Fri, 19 Sep 2025 14:41:59 +0200 Subject: [PATCH 2/2] docs: added javadoc and tests Signed-off-by: Chris Laprun --- .../io/javaoperatorsdk/operator/Operator.java | 61 +++++++++++++++---- .../operator/OperatorTest.java | 54 +++++++++++----- 2 files changed, 87 insertions(+), 28 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 a56cfb7110..f65f6ae022 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 @@ -49,6 +49,32 @@ public Operator(ConfigurationService configurationService) { init(configurationService, false); } + /** + * Creates an Operator overriding the default configuration with the values provided by the + * specified {@link ConfigurationServiceOverrider}. + * + * @param overrider a {@link ConfigurationServiceOverrider} consumer used to override the default + * {@link ConfigurationService} values + */ + public Operator(Consumer overrider) { + init(initConfigurationService(null, overrider), false); + } + + /** + * In a deferred initialization scenario, the default constructor will typically be called to + * create a proxy instance, usually to be replaced at some later time when the dependents (in this + * case the ConfigurationService instance) are available. In this situation, we want to make it + * possible to not perform the initialization steps directly so this implementation makes it + * possible to not crash when a null ConfigurationService is passed only if deferred + * initialization is allowed + * + * @param configurationService the potentially {@code null} {@link ConfigurationService} to use + * for this operator + * @param allowDeferredInit whether or not deferred initialization of the configuration service is + * allowed + * @throws IllegalStateException if the specified configuration service is {@code null} but + * deferred initialization is not allowed + */ private void init(ConfigurationService configurationService, boolean allowDeferredInit) { if (configurationService == null) { if (!allowDeferredInit) { @@ -65,20 +91,19 @@ private void init(ConfigurationService configurationService, boolean allowDeferr } } - /** - * Creates an Operator overriding the default configuration with the values provided by the - * specified {@link ConfigurationServiceOverrider}. - * - * @param overrider a {@link ConfigurationServiceOverrider} consumer used to override the default - * {@link ConfigurationService} values - */ - public Operator(Consumer overrider) { - init(initConfigurationService(null, overrider), false); - } - /** * Overridable by subclasses to enable deferred configuration, useful to avoid unneeded processing - * in injection scenarios + * in injection scenarios, typically returning {@code null} here instead of performing any + * configuration + * + * @param client a potentially {@code null} {@link KubernetesClient} to initialize the operator's + * {@link ConfigurationService} with + * @param overrider a potentially {@code null} {@link ConfigurationServiceOverrider} consumer to + * override the default {@link ConfigurationService} with + * @return a ready to use {@link ConfigurationService} using values provided by the specified + * overrides and kubernetes client, if provided or {@code null} in case deferred + * initialization is possible, in which case it is up to the extension to ensure that the + * {@link ConfigurationService} is properly set before the operator instance is used */ protected ConfigurationService initConfigurationService( KubernetesClient client, Consumer overrider) { @@ -247,8 +272,8 @@ public

RegisteredController

register( * * @param reconciler part of the reconciler to register * @param configOverrider consumer to use to change config values - * @return registered controller * @param

the {@code HasMetadata} type associated with the reconciler + * @return registered controller */ public

RegisteredController

register( Reconciler

reconciler, Consumer> configOverrider) { @@ -281,4 +306,14 @@ boolean isStarted() { public ConfigurationService getConfigurationService() { return configurationService; } + + /** + * Make it possible for extensions to set the {@link ConfigurationService} after the operator has + * been initialized + * + * @param configurationService the {@link ConfigurationService} to use for this operator + */ + protected void setConfigurationService(ConfigurationService configurationService) { + init(configurationService, false); + } } 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 9bdd54ca8d..39fc98f6b0 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,31 +1,24 @@ package io.javaoperatorsdk.operator; -import org.junit.jupiter.api.BeforeEach; +import java.util.function.Consumer; + 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.ConfigurationService; +import io.javaoperatorsdk.operator.api.config.ConfigurationServiceOverrider; import io.javaoperatorsdk.operator.api.reconciler.Context; -import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.*; @SuppressWarnings("rawtypes") class OperatorTest { - - private final KubernetesClient kubernetesClient = MockKubernetesClient.client(ConfigMap.class); - private Operator operator; - - @BeforeEach - void initOperator() { - operator = new Operator(kubernetesClient); - } - @Test void shouldBePossibleToRetrieveNumberOfRegisteredControllers() { + final var operator = new Operator(); assertEquals(0, operator.getRegisteredControllersNumber()); operator.register(new FooReconciler()); @@ -34,6 +27,7 @@ void shouldBePossibleToRetrieveNumberOfRegisteredControllers() { @Test void shouldBePossibleToRetrieveRegisteredControllerByName() { + final var operator = new Operator(); final var reconciler = new FooReconciler(); final var name = ReconcilerUtils.getNameFor(reconciler); @@ -51,12 +45,42 @@ void shouldBePossibleToRetrieveRegisteredControllerByName() { assertEquals(maybeController.get(), registeredControllers.stream().findFirst().orElseThrow()); } - @ControllerConfiguration - private static class FooReconciler implements Reconciler { + @Test + void shouldThrowExceptionIf() { + final var operator = new OperatorExtension(); + assertNotNull(operator); + operator.setConfigurationService(ConfigurationService.newOverriddenConfigurationService(null)); + assertNotNull(operator.getConfigurationService()); + + // should fail because the implementation is not providing a valid configuration service when + // constructing the operator + assertThrows( + IllegalStateException.class, + () -> new OperatorExtension(MockKubernetesClient.client(ConfigMap.class))); + } + private static class FooReconciler implements Reconciler { @Override public UpdateControl reconcile(ConfigMap resource, Context context) { return UpdateControl.noUpdate(); } } + + private static class OperatorExtension extends Operator { + public OperatorExtension() {} + + public OperatorExtension(KubernetesClient client) { + super(client); + } + + /** + * Overridden to mimic deferred initialization (or rather the fact that we don't want to do that + * processing at this time so return null). + */ + @Override + protected ConfigurationService initConfigurationService( + KubernetesClient client, Consumer overrider) { + return null; + } + } }