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

Author: metacosm

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

@@ -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);
   }
 
   /**
@@ -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;
   }
@@ -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

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

@@ -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;
@@ -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() {

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

@@ -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();
@@ -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)