Skip to content

Commit 0dbca4b

Browse files
csviricsviri
committed
improve: configuration service no static access
1 parent 0ea7794 commit 0dbca4b

33 files changed

+235
-286
lines changed

‎micrometer-support/src/test/java/io/javaoperatorsdk/operator/monitoring/micrometer/AbstractMicrometerMetricsTestFixture.java

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,12 @@
33
import java.util.HashSet;
44
import java.util.Set;
55

6-
import org.junit.jupiter.api.AfterAll;
7-
import org.junit.jupiter.api.BeforeAll;
86
import org.junit.jupiter.api.Test;
97
import org.junit.jupiter.api.TestInstance;
108
import org.junit.jupiter.api.extension.RegisterExtension;
119

1210
import io.fabric8.kubernetes.api.model.ConfigMap;
1311
import io.fabric8.kubernetes.api.model.ConfigMapBuilder;
14-
import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider;
1512
import io.javaoperatorsdk.operator.api.reconciler.*;
1613
import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension;
1714
import io.javaoperatorsdk.operator.processing.event.ResourceID;
@@ -23,26 +20,21 @@
2320

2421
@TestInstance(TestInstance.Lifecycle.PER_CLASS)
2522
public abstract class AbstractMicrometerMetricsTestFixture {
26-
@RegisterExtension
27-
LocallyRunOperatorExtension operator =
28-
LocallyRunOperatorExtension.builder().withReconciler(new MetricsCleaningTestReconciler())
29-
.build();
3023

3124
protected final TestSimpleMeterRegistry registry = new TestSimpleMeterRegistry();
3225
protected final MicrometerMetrics metrics = getMetrics();
3326
protected static final String testResourceName = "micrometer-metrics-cr";
3427

35-
protected abstract MicrometerMetrics getMetrics();
3628

37-
@BeforeAll
38-
void setup() {
39-
ConfigurationServiceProvider.overrideCurrent(overrider -> overrider.withMetrics(metrics));
40-
}
29+
@RegisterExtension
30+
LocallyRunOperatorExtension operator =
31+
LocallyRunOperatorExtension.builder()
32+
.withConfigurationService(overrider -> overrider.withMetrics(metrics))
33+
.withReconciler(new MetricsCleaningTestReconciler())
34+
.build();
4135

42-
@AfterAll
43-
void reset() {
44-
ConfigurationServiceProvider.reset();
45-
}
36+
37+
protected abstract MicrometerMetrics getMetrics();
4638

4739
@Test
4840
void properlyHandlesResourceDeletion() throws Exception {

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ControllerManager.java

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,12 @@ class ControllerManager {
2222
@SuppressWarnings("rawtypes")
2323
private final Map<String, Controller> controllers = new HashMap<>();
2424
private boolean started = false;
25+
private ExecutorServiceManager executorServiceManager;
26+
27+
public ControllerManager(ExecutorServiceManager executorServiceManager) {
28+
this.executorServiceManager = executorServiceManager;
29+
}
30+
2531

2632
public synchronized void shouldStart() {
2733
if (started) {
@@ -33,15 +39,15 @@ public synchronized void shouldStart() {
3339
}
3440

3541
public synchronized void start(boolean startEventProcessor) {
36-
ExecutorServiceManager.boundedExecuteAndWaitForAllToComplete(controllers().stream(), c -> {
42+
executorServiceManager.boundedExecuteAndWaitForAllToComplete(controllers().stream(), c -> {
3743
c.start(startEventProcessor);
3844
return null;
3945
}, c -> "Controller Starter for: " + c.getConfiguration().getName());
4046
started = true;
4147
}
4248

4349
public synchronized void stop() {
44-
ExecutorServiceManager.boundedExecuteAndWaitForAllToComplete(controllers().stream(), c -> {
50+
executorServiceManager.boundedExecuteAndWaitForAllToComplete(controllers().stream(), c -> {
4551
log.debug("closing {}", c);
4652
c.stop();
4753
return null;
@@ -50,7 +56,7 @@ public synchronized void stop() {
5056
}
5157

5258
public synchronized void startEventProcessing() {
53-
ExecutorServiceManager.boundedExecuteAndWaitForAllToComplete(controllers().stream(), c -> {
59+
executorServiceManager.boundedExecuteAndWaitForAllToComplete(controllers().stream(), c -> {
5460
c.startEventProcessing();
5561
return null;
5662
}, c -> "Event processor starter for: " + c.getConfiguration().getName());

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
import io.fabric8.kubernetes.client.extended.leaderelection.LeaderElector;
1313
import io.fabric8.kubernetes.client.extended.leaderelection.LeaderElectorBuilder;
1414
import io.fabric8.kubernetes.client.extended.leaderelection.resourcelock.LeaseLock;
15-
import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider;
15+
import io.javaoperatorsdk.operator.api.config.ConfigurationService;
1616
import io.javaoperatorsdk.operator.api.config.ExecutorServiceManager;
1717
import io.javaoperatorsdk.operator.api.config.LeaderElectionConfiguration;
1818

@@ -24,16 +24,21 @@ public class LeaderElectionManager {
2424
private final ControllerManager controllerManager;
2525
private String identity;
2626
private CompletableFuture<?> leaderElectionFuture;
27+
private ConfigurationService configurationService;
28+
private ExecutorServiceManager executorServiceManager;
2729

28-
public LeaderElectionManager(ControllerManager controllerManager) {
30+
public LeaderElectionManager(ControllerManager controllerManager,
31+
ConfigurationService configurationService, ExecutorServiceManager executorServiceManager) {
2932
this.controllerManager = controllerManager;
33+
this.configurationService = configurationService;
34+
this.executorServiceManager = executorServiceManager;
3035
}
3136

3237
public void init(LeaderElectionConfiguration config, KubernetesClient client) {
3338
this.identity = identity(config);
3439
final var leaseNamespace =
3540
config.getLeaseNamespace().orElseGet(
36-
() -> ConfigurationServiceProvider.instance().getClientConfiguration().getNamespace());
41+
() -> configurationService.getClientConfiguration().getNamespace());
3742
if (leaseNamespace == null) {
3843
final var message =
3944
"Lease namespace is not set and cannot be inferred. Leader election cannot continue.";
@@ -44,7 +49,7 @@ public void init(LeaderElectionConfiguration config, KubernetesClient client) {
4449
// releaseOnCancel is not used in the underlying implementation
4550
leaderElector =
4651
new LeaderElectorBuilder(
47-
client, ExecutorServiceManager.instance().executorService())
52+
client, executorServiceManager.executorService())
4853
.withConfig(
4954
new LeaderElectionConfig(
5055
lock,

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java

Lines changed: 24 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,7 @@
1414
import io.fabric8.kubernetes.client.KubernetesClient;
1515
import io.fabric8.kubernetes.client.KubernetesClientBuilder;
1616
import io.fabric8.kubernetes.client.Version;
17-
import io.javaoperatorsdk.operator.api.config.ConfigurationService;
18-
import io.javaoperatorsdk.operator.api.config.ConfigurationServiceOverrider;
19-
import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider;
20-
import io.javaoperatorsdk.operator.api.config.ControllerConfiguration;
21-
import io.javaoperatorsdk.operator.api.config.ControllerConfigurationOverrider;
22-
import io.javaoperatorsdk.operator.api.config.ExecutorServiceManager;
17+
import io.javaoperatorsdk.operator.api.config.*;
2318
import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
2419
import io.javaoperatorsdk.operator.processing.Controller;
2520
import io.javaoperatorsdk.operator.processing.LifecycleAware;
@@ -29,17 +24,19 @@ public class Operator implements LifecycleAware {
2924
private static final Logger log = LoggerFactory.getLogger(Operator.class);
3025
private static final int DEFAULT_MAX_CONCURRENT_REQUEST = 512;
3126
private final KubernetesClient kubernetesClient;
32-
private final ControllerManager controllerManager = new ControllerManager();
33-
private final LeaderElectionManager leaderElectionManager =
34-
new LeaderElectionManager(controllerManager);
27+
private final ControllerManager controllerManager;
28+
private final LeaderElectionManager leaderElectionManager;
29+
private final ConfigurationService configurationService;
30+
private final ExecutorServiceManager executorServiceManager;
3531
private volatile boolean started = false;
3632

33+
3734
public Operator() {
3835
this((KubernetesClient) null);
3936
}
4037

4138
public Operator(KubernetesClient kubernetesClient) {
42-
this(kubernetesClient, ConfigurationServiceProvider.instance());
39+
this(kubernetesClient, new BaseConfigurationService());
4340
}
4441

4542
/**
@@ -56,7 +53,7 @@ public Operator(Consumer<ConfigurationServiceOverrider> overrider) {
5653
}
5754

5855
public Operator(KubernetesClient client, Consumer<ConfigurationServiceOverrider> overrider) {
59-
this(client, ConfigurationServiceProvider.overrideCurrent(overrider));
56+
this(client, ConfigurationService.overrideCurrent(new BaseConfigurationService(), overrider));
6057
}
6158

6259
/**
@@ -67,13 +64,19 @@ public Operator(KubernetesClient client, Consumer<ConfigurationServiceOverrider>
6764
* @param configurationService provides configuration
6865
*/
6966
public Operator(KubernetesClient kubernetesClient, ConfigurationService configurationService) {
67+
this.configurationService = configurationService;
68+
this.executorServiceManager = new ExecutorServiceManager(configurationService);
69+
controllerManager = new ControllerManager(executorServiceManager);
7070
this.kubernetesClient =
7171
kubernetesClient != null ? kubernetesClient
7272
: new KubernetesClientBuilder()
7373
.withConfig(new ConfigBuilder()
7474
.withMaxConcurrentRequests(DEFAULT_MAX_CONCURRENT_REQUEST).build())
7575
.build();
76-
ConfigurationServiceProvider.set(configurationService);
76+
77+
78+
leaderElectionManager =
79+
new LeaderElectionManager(controllerManager, configurationService, executorServiceManager);
7780
configurationService.getLeaderElectionConfiguration()
7881
.ifPresent(c -> leaderElectionManager.init(c, this.kubernetesClient));
7982
}
@@ -87,7 +90,7 @@ public Operator(KubernetesClient kubernetesClient, ConfigurationService configur
8790
@Deprecated(forRemoval = true)
8891
public void installShutdownHook() {
8992
installShutdownHook(
90-
Duration.ofSeconds(ConfigurationServiceProvider.instance().getTerminationTimeoutSeconds()));
93+
Duration.ofSeconds(configurationService.getTerminationTimeoutSeconds()));
9194
}
9295

9396
/**
@@ -123,9 +126,9 @@ public synchronized void start() {
123126
if (started) {
124127
return;
125128
}
126-
ExecutorServiceManager.init();
129+
executorServiceManager.init();
127130
controllerManager.shouldStart();
128-
final var version = ConfigurationServiceProvider.instance().getVersion();
131+
final var version = configurationService.getVersion();
129132
log.info(
130133
"Operator SDK {} (commit: {}) built on {} starting...",
131134
version.getSdkVersion(),
@@ -149,12 +152,11 @@ public void stop(Duration gracefulShutdownTimeout) throws OperatorException {
149152
if (!started) {
150153
return;
151154
}
152-
final var configurationService = ConfigurationServiceProvider.instance();
153155
log.info(
154156
"Operator SDK {} is shutting down...", configurationService.getVersion().getSdkVersion());
155157
controllerManager.stop();
156158

157-
ExecutorServiceManager.stop(gracefulShutdownTimeout);
159+
executorServiceManager.stop(gracefulShutdownTimeout);
158160
leaderElectionManager.stop();
159161
if (configurationService.closeClientOnStop()) {
160162
kubernetesClient.close();
@@ -180,7 +182,7 @@ public void stop() throws OperatorException {
180182
public <P extends HasMetadata> RegisteredController<P> register(Reconciler<P> reconciler)
181183
throws OperatorException {
182184
final var controllerConfiguration =
183-
ConfigurationServiceProvider.instance().getConfigurationFor(reconciler);
185+
configurationService.getConfigurationFor(reconciler);
184186
return register(reconciler, controllerConfiguration);
185187
}
186188

@@ -210,10 +212,11 @@ public <P extends HasMetadata> RegisteredController<P> register(Reconciler<P> re
210212
" reconciler named " + ReconcilerUtils.getNameFor(reconciler)
211213
+ " because its configuration cannot be found.\n" +
212214
" Known reconcilers are: "
213-
+ ConfigurationServiceProvider.instance().getKnownReconcilerNames());
215+
+ configurationService.getKnownReconcilerNames());
214216
}
215217

216-
final var controller = new Controller<>(reconciler, configuration, kubernetesClient);
218+
final var controller =
219+
new Controller<>(reconciler, configuration, kubernetesClient, executorServiceManager);
217220

218221
controllerManager.add(controller);
219222

@@ -239,7 +242,7 @@ public <P extends HasMetadata> RegisteredController<P> register(Reconciler<P> re
239242
public <P extends HasMetadata> RegisteredController<P> register(Reconciler<P> reconciler,
240243
Consumer<ControllerConfigurationOverrider<P>> configOverrider) {
241244
final var controllerConfiguration =
242-
ConfigurationServiceProvider.instance().getConfigurationFor(reconciler);
245+
configurationService.getConfigurationFor(reconciler);
243246
var configToOverride = ControllerConfigurationOverrider.override(controllerConfiguration);
244247
configOverrider.accept(configToOverride);
245248
return register(reconciler, configToOverride.build());

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/BaseConfigurationService.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ protected <P extends HasMetadata> ControllerConfiguration<P> configFor(Reconcile
107107
" annotation for reconciler: " + reconciler);
108108
}
109109
Class<Reconciler<P>> reconcilerClass = (Class<Reconciler<P>>) reconciler.getClass();
110-
final var resourceClass = ConfigurationServiceProvider.instance().getResourceClassResolver()
110+
final var resourceClass = getResourceClassResolver()
111111
.getResourceClass(reconcilerClass);
112112

113113
final var name = ReconcilerUtils.getNameFor(reconciler);
@@ -152,7 +152,7 @@ protected <P extends HasMetadata> ControllerConfiguration<P> configFor(Reconcile
152152
io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration::labelSelector,
153153
Constants.NO_VALUE_SET),
154154
null,
155-
Utils.instantiate(annotation.itemStore(), ItemStore.class, context));
155+
Utils.instantiate(annotation.itemStore(), ItemStore.class, context), this);
156156

157157
ResourceEventFilter<P> answer = deprecatedEventFilter(annotation);
158158
config.setEventFilter(answer != null ? answer : ResourceEventFilters.passthrough());

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import java.util.Optional;
55
import java.util.Set;
66
import java.util.concurrent.*;
7+
import java.util.function.Consumer;
78

89
import org.slf4j.Logger;
910
import org.slf4j.LoggerFactory;
@@ -248,4 +249,11 @@ default ManagedWorkflowFactory getWorkflowFactory() {
248249
default ResourceClassResolver getResourceClassResolver() {
249250
return new DefaultResourceClassResolver();
250251
}
252+
253+
static ConfigurationService overrideCurrent(ConfigurationService service,
254+
Consumer<ConfigurationServiceOverrider> overrider) {
255+
final var toOverride = new ConfigurationServiceOverrider(service);
256+
overrider.accept(toOverride);
257+
return toOverride.build();
258+
}
251259
}

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceProvider.java

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,12 @@ public class ConfigurationServiceProvider {
1313

1414
private ConfigurationServiceProvider() {}
1515

16-
public synchronized static ConfigurationService instance() {
17-
if (instance == null) {
18-
set(defaultConfigurationService);
19-
}
20-
return instance;
21-
}
16+
// public synchronized static ConfigurationService instance() {
17+
// if (instance == null) {
18+
// set(defaultConfigurationService);
19+
// }
20+
// return instance;
21+
// }
2222

2323
public synchronized static void set(ConfigurationService instance) {
2424
set(instance, false);
@@ -45,15 +45,15 @@ private static void set(ConfigurationService instance, boolean overriding) {
4545
ConfigurationServiceProvider.instance = instance;
4646
}
4747

48-
public synchronized static ConfigurationService overrideCurrent(
49-
Consumer<ConfigurationServiceOverrider> overrider) {
50-
if (overrider != null) {
51-
final var toOverride = new ConfigurationServiceOverrider(instance());
52-
overrider.accept(toOverride);
53-
set(toOverride.build(), true);
54-
}
55-
return instance();
56-
}
48+
// public synchronized static ConfigurationService overrideCurrent(
49+
// Consumer<ConfigurationServiceOverrider> overrider) {
50+
// if (overrider != null) {
51+
// final var toOverride = new ConfigurationServiceOverrider(instance());
52+
// overrider.accept(toOverride);
53+
// set(toOverride.build(), true);
54+
// }
55+
// return instance();
56+
// }
5757

5858
public synchronized static void setDefault(ConfigurationService defaultConfigurationService) {
5959
ConfigurationServiceProvider.defaultConfigurationService = defaultConfigurationService;

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -107,9 +107,7 @@ default Optional<Duration> maxReconciliationInterval() {
107107
}
108108

109109
@SuppressWarnings("unused")
110-
default ConfigurationService getConfigurationService() {
111-
return ConfigurationServiceProvider.instance();
112-
}
110+
ConfigurationService getConfigurationService();
113111

114112
@SuppressWarnings("unchecked")
115113
@Override

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,8 @@ public ControllerConfiguration<R> build() {
190190
generationAware, original.getAssociatedReconcilerClassName(), retry, rateLimiter,
191191
reconciliationMaxInterval, onAddFilter, onUpdateFilter, genericFilter,
192192
original.getDependentResources(),
193-
namespaces, finalizer, labelSelector, configurations, itemStore);
193+
namespaces, finalizer, labelSelector, configurations, itemStore,
194+
original.getConfigurationService());
194195
overridden.setEventFilter(customResourcePredicate);
195196
return overridden;
196197
}

0 commit comments

Comments
 (0)