Skip to content

fix: default implementations shouldn't always recreate a client #1959

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

Merged
merged 1 commit into from
Jun 21, 2023
Merged
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 @@ -6,28 +6,44 @@
import java.util.stream.Stream;

import io.fabric8.kubernetes.api.model.HasMetadata;
import io.fabric8.kubernetes.client.KubernetesClient;
import io.javaoperatorsdk.operator.ReconcilerUtils;
import io.javaoperatorsdk.operator.api.reconciler.Reconciler;

@SuppressWarnings("rawtypes")
public class AbstractConfigurationService implements ConfigurationService {
private final Map<String, ControllerConfiguration> configurations = new ConcurrentHashMap<>();
private final Version version;
private KubernetesClient client;
private Cloner cloner;
private ExecutorServiceManager executorServiceManager;

public AbstractConfigurationService(Version version) {
this(version, null, null);
this(version, null);
}

public AbstractConfigurationService(Version version, Cloner cloner) {
this(version, cloner, null);
this(version, cloner, null, null);
}

/**
* Creates a new {@link AbstractConfigurationService} with the specified parameters.
*
* @param client the {@link KubernetesClient} instance to use to connect to the cluster, if let
* {@code null}, the client will be lazily instantiated with the default configuration
* provided by {@link ConfigurationService#getKubernetesClient()} the first time
* {@link #getKubernetesClient()} is called
* @param version the version information
* @param cloner the {@link Cloner} to use, if {@code null} the default provided by
* {@link ConfigurationService#getResourceCloner()} will be used
* @param executorServiceManager the {@link ExecutorServiceManager} instance to be used, can be
* {@code null} to lazily initialize one by default when
* {@link #getExecutorServiceManager()} is called
*/
public AbstractConfigurationService(Version version, Cloner cloner,
ExecutorServiceManager executorServiceManager) {
ExecutorServiceManager executorServiceManager, KubernetesClient client) {
this.version = version;
init(cloner, executorServiceManager);
init(cloner, executorServiceManager, client);
}

/**
Expand All @@ -36,10 +52,19 @@ public AbstractConfigurationService(Version version, Cloner cloner,
* is useful in situations where the cloner depends on a mapper that might require additional
* configuration steps before it's ready to be used.
*
* @param cloner the {@link Cloner} instance to be used
* @param executorServiceManager the {@link ExecutorServiceManager} instance to be used
* @param cloner the {@link Cloner} instance to be used, if {@code null}, the default provided by
* {@link ConfigurationService#getResourceCloner()} will be used
* @param executorServiceManager the {@link ExecutorServiceManager} instance to be used, can be
* {@code null} to lazily initialize one by default when
* {@link #getExecutorServiceManager()} is called
* @param client the {@link KubernetesClient} instance to use to connect to the cluster, if let
* {@code null}, the client will be lazily instantiated with the default configuration
* provided by {@link ConfigurationService#getKubernetesClient()} the first time
* {@link #getKubernetesClient()} is called
*/
protected void init(Cloner cloner, ExecutorServiceManager executorServiceManager) {
protected void init(Cloner cloner, ExecutorServiceManager executorServiceManager,
KubernetesClient client) {
this.client = client;
this.cloner = cloner != null ? cloner : ConfigurationService.super.getResourceCloner();
this.executorServiceManager = executorServiceManager;
}
Expand Down Expand Up @@ -127,6 +152,15 @@ public Cloner getResourceCloner() {
return cloner;
}

@Override
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();
}
return client;
}

@Override
public ExecutorServiceManager getExecutorServiceManager() {
// lazy init to avoid initializing thread pools for nothing in an overriding scenario
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import org.slf4j.LoggerFactory;

import io.fabric8.kubernetes.api.model.HasMetadata;
import io.fabric8.kubernetes.client.KubernetesClient;
import io.fabric8.kubernetes.client.informers.cache.ItemStore;
import io.javaoperatorsdk.operator.OperatorException;
import io.javaoperatorsdk.operator.ReconcilerUtils;
Expand Down Expand Up @@ -40,11 +41,15 @@ public class BaseConfigurationService extends AbstractConfigurationService {
private static final Logger logger = LoggerFactory.getLogger(LOGGER_NAME);

public BaseConfigurationService(Version version) {
super(version);
this(version, null);
}

public BaseConfigurationService(Version version, Cloner cloner) {
super(version, cloner);
this(version, cloner, null);
}

public BaseConfigurationService(Version version, Cloner cloner, KubernetesClient client) {
super(version, cloner, null, client);
}

public BaseConfigurationService() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ public ConfigurationServiceOverrider withSSABasedDefaultMatchingForDependentReso
}

public ConfigurationService build() {
return new BaseConfigurationService(original.getVersion(), cloner) {
return new BaseConfigurationService(original.getVersion(), cloner, client) {
@Override
public Set<String> getKnownReconcilerNames() {
return original.getKnownReconcilerNames();
Expand Down Expand Up @@ -228,11 +228,6 @@ public ExecutorService getWorkflowExecutorService() {
: super.getWorkflowExecutorService();
}

@Override
public KubernetesClient getKubernetesClient() {
return client != null ? client : original.getKubernetesClient();
}

@Override
public Optional<LeaderElectionConfiguration> getLeaderElectionConfiguration() {
return leaderElectionConfiguration != null ? Optional.of(leaderElectionConfiguration)
Expand Down