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 57faef696a..3f09fe77ed 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 @@ -31,15 +31,15 @@ public Operator() { } public Operator(KubernetesClient kubernetesClient) { - this(kubernetesClient, ConfigurationServiceProvider.instance(), null); + this(kubernetesClient, ConfigurationServiceProvider.instance()); } /** * @deprecated Use {@link #Operator(Consumer)} instead */ - @Deprecated + @Deprecated(forRemoval = true) public Operator(ConfigurationService configurationService) { - this(null, configurationService, null); + this(null, configurationService); } public Operator(Consumer overrider) { @@ -47,7 +47,7 @@ public Operator(Consumer overrider) { } public Operator(KubernetesClient client, Consumer overrider) { - this(client, ConfigurationServiceProvider.instance(), overrider); + this(client, ConfigurationServiceProvider.overrideCurrent(overrider)); } /** @@ -58,19 +58,11 @@ public Operator(KubernetesClient client, Consumer * @param configurationService provides configuration */ public Operator(KubernetesClient kubernetesClient, ConfigurationService configurationService) { - this(kubernetesClient, configurationService, null); - } - - private Operator(KubernetesClient kubernetesClient, ConfigurationService configurationService, - Consumer overrider) { this.kubernetesClient = kubernetesClient != null ? kubernetesClient : new KubernetesClientBuilder().build(); - ConfigurationServiceProvider.set(configurationService); - if (overrider != null) { - ConfigurationServiceProvider.overrideCurrent(overrider); - } - ConfigurationServiceProvider.instance().getLeaderElectionConfiguration() + configurationService.getLeaderElectionConfiguration() .ifPresent(c -> leaderElectionManager.init(c, this.kubernetesClient)); + ConfigurationServiceProvider.set(configurationService); } /** Adds a shutdown hook that automatically calls {@link #stop()} when the app shuts down. */ 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 d222024115..918bbf8d2f 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 @@ -15,28 +15,19 @@ public class ConfigurationServiceOverrider { private final ConfigurationService original; private Metrics metrics; private Config clientConfig; - private boolean checkCR; - private int threadNumber; + private Boolean checkCR; + private Integer threadNumber; private Cloner cloner; - private int timeoutSeconds; - private boolean closeClientOnStop; + private Integer timeoutSeconds; + private Boolean closeClientOnStop; private ObjectMapper objectMapper; - private ExecutorService executorService = null; + private ExecutorService executorService; private LeaderElectionConfiguration leaderElectionConfiguration; ConfigurationServiceOverrider(ConfigurationService original) { this.original = original; - this.clientConfig = original.getClientConfiguration(); - this.checkCR = original.checkCRDAndValidateLocalModel(); - this.threadNumber = original.concurrentReconciliationThreads(); - this.cloner = original.getResourceCloner(); - this.timeoutSeconds = original.getTerminationTimeoutSeconds(); - this.metrics = original.getMetrics(); - this.closeClientOnStop = original.closeClientOnStop(); - this.objectMapper = original.getObjectMapper(); } - public ConfigurationServiceOverrider withClientConfiguration(Config configuration) { this.clientConfig = configuration; return this; @@ -97,51 +88,48 @@ public Set getKnownReconcilerNames() { @Override public Config getClientConfiguration() { - return clientConfig; + return clientConfig != null ? clientConfig : original.getClientConfiguration(); } @Override public boolean checkCRDAndValidateLocalModel() { - return checkCR; + return checkCR != null ? checkCR : original.checkCRDAndValidateLocalModel(); } @Override public int concurrentReconciliationThreads() { - return threadNumber; + return threadNumber != null ? threadNumber : original.concurrentReconciliationThreads(); } @Override public int getTerminationTimeoutSeconds() { - return timeoutSeconds; + return timeoutSeconds != null ? timeoutSeconds : original.getTerminationTimeoutSeconds(); } @Override public Metrics getMetrics() { - return metrics; + return metrics != null ? metrics : original.getMetrics(); } @Override public boolean closeClientOnStop() { - return closeClientOnStop; + return closeClientOnStop != null ? closeClientOnStop : original.closeClientOnStop(); } @Override public ExecutorService getExecutorService() { - if (executorService != null) { - return executorService; - } else { - return super.getExecutorService(); - } + return executorService != null ? executorService : original.getExecutorService(); } @Override public ObjectMapper getObjectMapper() { - return objectMapper; + return objectMapper != null ? objectMapper : original.getObjectMapper(); } @Override public Optional getLeaderElectionConfiguration() { - return Optional.ofNullable(leaderElectionConfiguration); + return leaderElectionConfiguration != null ? Optional.of(leaderElectionConfiguration) + : original.getLeaderElectionConfiguration(); } }; } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProvider.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProvider.java index 0a8503ae05..25bfebe1d1 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProvider.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProvider.java @@ -45,12 +45,14 @@ private static void set(ConfigurationService instance, boolean overriding) { ConfigurationServiceProvider.instance = instance; } - public synchronized static void overrideCurrent( + public synchronized static ConfigurationService overrideCurrent( Consumer overrider) { - final var toOverride = - new ConfigurationServiceOverrider(ConfigurationServiceProvider.instance()); - overrider.accept(toOverride); - ConfigurationServiceProvider.set(toOverride.build(), true); + 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) { 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 2e3e8918c1..1b30d94c69 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 @@ -11,6 +11,7 @@ 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.reconciler.Context; import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; @@ -74,6 +75,15 @@ 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()); + } + @ControllerConfiguration private static class FooReconciler implements Reconciler { diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverriderTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverriderTest.java new file mode 100644 index 0000000000..8b973b8159 --- /dev/null +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverriderTest.java @@ -0,0 +1,132 @@ +package io.javaoperatorsdk.operator.api.config; + +import java.util.Optional; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; + +import org.junit.jupiter.api.Test; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.client.Config; +import io.fabric8.kubernetes.client.ConfigBuilder; +import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; +import io.javaoperatorsdk.operator.api.monitoring.Metrics; +import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; +import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResourceFactory; + +import com.fasterxml.jackson.databind.ObjectMapper; + +import static org.junit.jupiter.api.Assertions.*; + +class ConfigurationServiceOverriderTest { + + private static final Metrics METRICS = new Metrics() {}; + private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); + private static final LeaderElectionConfiguration LEADER_ELECTION_CONFIGURATION = + new LeaderElectionConfiguration("foo", "fooNS"); + private static final DependentResourceFactory FACTORY = new DependentResourceFactory() { + @Override + public > T createFrom(DependentResourceSpec spec) { + return DependentResourceFactory.super.createFrom(spec); + } + }; + + private static final Cloner CLONER = new Cloner() { + @Override + public R clone(R object) { + return null; + } + }; + + @Test + void overrideShouldWork() { + final var config = new BaseConfigurationService(null) { + @Override + public boolean checkCRDAndValidateLocalModel() { + return false; + } + + @Override + public Config getClientConfiguration() { + return new ConfigBuilder().withNamespace("namespace").build(); + } + + @Override + public int concurrentReconciliationThreads() { + return -1; + } + + @Override + public int getTerminationTimeoutSeconds() { + return -1; + } + + @Override + public Metrics getMetrics() { + return METRICS; + } + + @Override + public ExecutorService getExecutorService() { + return null; + } + + @Override + public boolean closeClientOnStop() { + return true; + } + + @Override + public ObjectMapper getObjectMapper() { + return OBJECT_MAPPER; + } + + @Override + public Cloner getResourceCloner() { + return CLONER; + } + + @Override + public DependentResourceFactory dependentResourceFactory() { + return FACTORY; + } + + @Override + public Optional getLeaderElectionConfiguration() { + return Optional.of(LEADER_ELECTION_CONFIGURATION); + } + }; + final var overridden = new ConfigurationServiceOverrider(config) + .withClientConfiguration(new ConfigBuilder().withNamespace("newNS").build()) + .checkingCRDAndValidateLocalModel(true) + .withExecutorService(Executors.newSingleThreadExecutor()) + .withCloseClientOnStop(false) + .withObjectMapper(new ObjectMapper()) + .withResourceCloner(new Cloner() { + @Override + public R clone(R object) { + return null; + } + }) + .withConcurrentReconciliationThreads(25) + .withTerminationTimeoutSeconds(100) + .withMetrics(new Metrics() {}) + .withLeaderElectionConfiguration(new LeaderElectionConfiguration("newLease", "newLeaseNS")) + .build(); + + assertNotEquals(config.closeClientOnStop(), overridden.closeClientOnStop()); + assertNotEquals(config.checkCRDAndValidateLocalModel(), + overridden.checkCRDAndValidateLocalModel()); + assertNotEquals(config.concurrentReconciliationThreads(), + overridden.concurrentReconciliationThreads()); + assertNotEquals(config.getTerminationTimeoutSeconds(), + overridden.getTerminationTimeoutSeconds()); + assertNotEquals(config.dependentResourceFactory(), overridden.dependentResourceFactory()); + assertNotEquals(config.getClientConfiguration(), overridden.getClientConfiguration()); + assertNotEquals(config.getExecutorService(), overridden.getExecutorService()); + assertNotEquals(config.getMetrics(), overridden.getMetrics()); + assertNotEquals(config.getObjectMapper(), overridden.getObjectMapper()); + assertNotEquals(config.getLeaderElectionConfiguration(), + overridden.getLeaderElectionConfiguration()); + } +} diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProviderTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProviderTest.java index fbe5a3542e..33ff4bd214 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProviderTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProviderTest.java @@ -34,17 +34,23 @@ void shouldProvideTheSetInstanceIfProvided() { @Test void shouldBePossibleToOverrideConfigOnce() { - final var config = new AbstractConfigurationService(null); + 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); - ConfigurationServiceProvider.overrideCurrent(o -> o.checkingCRDAndValidateLocalModel(true)); + 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))); diff --git a/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java b/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java index 60b3ad639a..4c78ce5d01 100644 --- a/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java +++ b/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java @@ -50,10 +50,10 @@ class LeaderElectionE2E { // not for local mode by design @EnabledIfSystemProperty(named = "test.deployment", matches = "remote") void otherInstancesTakesOverWhenSteppingDown() { - log.info("Deploying operator"); - deployOperatorsInOrder(); log.info("Applying custom resource"); applyCustomResource(); + log.info("Deploying operator instances"); + deployOperatorsInOrder(); log.info("Awaiting custom resource reconciliations"); await().pollDelay(Duration.ofSeconds(MINIMAL_SECONDS_FOR_RENEWAL)) @@ -131,12 +131,14 @@ void tearDown() { } private void deployOperatorsInOrder() { + log.info("Installing 1st instance"); applyResources("k8s/operator.yaml"); await().atMost(Duration.ofSeconds(POD_STARTUP_TIMEOUT)).untilAsserted(() -> { var pod = client.pods().inNamespace(namespace).withName(OPERATOR_1_POD_NAME).get(); assertThat(pod.getStatus().getContainerStatuses().get(0).getReady()).isTrue(); }); + log.info("Installing 2nd instance"); applyResources("k8s/operator-instance-2.yaml"); await().atMost(Duration.ofSeconds(POD_STARTUP_TIMEOUT)).untilAsserted(() -> { var pod = client.pods().inNamespace(namespace).withName(OPERATOR_2_POD_NAME).get();