Skip to content

fix: update configuration service when overriding configuration #2784

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import io.javaoperatorsdk.operator.api.config.ConfigurationServiceOverrider;
import io.javaoperatorsdk.operator.api.config.ControllerConfiguration;
import io.javaoperatorsdk.operator.api.config.ControllerConfigurationOverrider;
import io.javaoperatorsdk.operator.api.config.UpdatableConfigurationService;
import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
import io.javaoperatorsdk.operator.processing.Controller;
import io.javaoperatorsdk.operator.processing.LifecycleAware;
Expand All @@ -26,7 +27,7 @@ public class Operator implements LifecycleAware {

private final ControllerManager controllerManager;
private final LeaderElectionManager leaderElectionManager;
private final ConfigurationService configurationService;
private final UpdatableConfigurationService configurationService;
private volatile boolean started = false;

public Operator() {
Expand All @@ -45,7 +46,7 @@ public Operator() {
* @param configurationService a {@link ConfigurationService} providing the configuration for the
* operator
*/
public Operator(ConfigurationService configurationService) {
public Operator(UpdatableConfigurationService configurationService) {
this.configurationService = configurationService;

final var executorServiceManager = configurationService.getExecutorServiceManager();
Expand All @@ -65,7 +66,7 @@ public Operator(Consumer<ConfigurationServiceOverrider> overrider) {
this(initConfigurationService(null, overrider));
}

private static ConfigurationService initConfigurationService(
private static UpdatableConfigurationService initConfigurationService(
KubernetesClient client, Consumer<ConfigurationServiceOverrider> overrider) {
// initialize the client if the user didn't provide one
if (client == null) {
Expand All @@ -82,7 +83,7 @@ private static ConfigurationService initConfigurationService(
overrider = o -> o.withKubernetesClient(kubernetesClient);
}

return ConfigurationService.newOverriddenConfigurationService(overrider);
return UpdatableConfigurationService.newOverriddenConfigurationService(overrider);
}

/**
Expand Down Expand Up @@ -209,6 +210,12 @@ public <P extends HasMetadata> RegisteredController<P> register(
+ configurationService.getKnownReconcilerNames());
}

// update the reconciler's configuration in the ConfigurationService if needed
final var existing = configurationService.getConfigurationFor(reconciler);
if (!configuration.equals(existing)) {
configurationService.replace(configuration);
}

final var controller = new Controller<>(reconciler, configuration, getKubernetesClient());

controllerManager.add(controller);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* An abstract implementation of {@link ConfigurationService} meant to ease custom implementations
*/
@SuppressWarnings("rawtypes")
public class AbstractConfigurationService implements ConfigurationService {
public class AbstractConfigurationService implements UpdatableConfigurationService {
private final Map<String, ControllerConfiguration> configurations = new ConcurrentHashMap<>();
private final Version version;
private KubernetesClient client;
Expand Down Expand Up @@ -71,15 +71,16 @@ public AbstractConfigurationService(
protected void init(
Cloner cloner, ExecutorServiceManager executorServiceManager, KubernetesClient client) {
this.client = client;
this.cloner = cloner != null ? cloner : ConfigurationService.super.getResourceCloner();
this.cloner = cloner != null ? cloner : UpdatableConfigurationService.super.getResourceCloner();
this.executorServiceManager = executorServiceManager;
}

protected <R extends HasMetadata> void register(ControllerConfiguration<R> config) {
put(config, true);
}

protected <R extends HasMetadata> void replace(ControllerConfiguration<R> config) {
@Override
public <R extends HasMetadata> void replace(ControllerConfiguration<R> config) {
put(config, false);
}

Expand Down Expand Up @@ -162,7 +163,7 @@ public Cloner getResourceCloner() {
public KubernetesClient getKubernetesClient() {
// lazy init to avoid needing initializing a client when not needed (in tests, in particular)
if (client == null) {
client = ConfigurationService.super.getKubernetesClient();
client = UpdatableConfigurationService.super.getKubernetesClient();
}
return client;
}
Expand All @@ -171,7 +172,7 @@ public KubernetesClient getKubernetesClient() {
public ExecutorServiceManager getExecutorServiceManager() {
// lazy init to avoid initializing thread pools for nothing in an overriding scenario
if (executorServiceManager == null) {
executorServiceManager = ConfigurationService.super.getExecutorServiceManager();
executorServiceManager = UpdatableConfigurationService.super.getExecutorServiceManager();
}
return executorServiceManager;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package io.javaoperatorsdk.operator.api.config;

import java.util.function.Consumer;

import io.fabric8.kubernetes.api.model.HasMetadata;

public interface UpdatableConfigurationService extends ConfigurationService {
<R extends HasMetadata> void replace(ControllerConfiguration<R> config);

static UpdatableConfigurationService newOverriddenConfigurationService(
Consumer<ConfigurationServiceOverrider> overrider) {
final var baseConfiguration = new BaseConfigurationService();
if (overrider != null) {
final var toOverride = new ConfigurationServiceOverrider(baseConfiguration);
overrider.accept(toOverride);
return (UpdatableConfigurationService) toOverride.build();
}
return baseConfiguration;
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package io.javaoperatorsdk.operator;

import java.util.Set;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

Expand Down Expand Up @@ -51,6 +53,15 @@ void shouldBePossibleToRetrieveRegisteredControllerByName() {
assertEquals(maybeController.get(), registeredControllers.stream().findFirst().orElseThrow());
}

@Test
void overriddenConfigurationShouldBeUpdatedInConfigurationService() {
final var reconciler = new FooReconciler();
operator.register(new FooReconciler(), override -> override.settingNamespace("test"));
final var config = operator.getConfigurationService().getConfigurationFor(reconciler);
final var namespaces = config.getInformerConfig().getNamespaces();
assertEquals(Set.of("test"), namespaces);
}

@ControllerConfiguration
private static class FooReconciler implements Reconciler<ConfigMap> {

Expand Down