fix: make things more resilient in the absence of explicit configuration (#789)

Author: metacosm

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

@@ -19,16 +19,23 @@ public class ExecutorServiceManager {
   private final ExecutorService executor;
   private final int terminationTimeoutSeconds;
 
-  private ExecutorServiceManager(ExecutorService executor, int terminationTimeoutSeconds) {
+  private ExecutorServiceManager(InstrumentedExecutorService executor,
+      int terminationTimeoutSeconds) {
     this.executor = executor;
     this.terminationTimeoutSeconds = terminationTimeoutSeconds;
   }
 
   public static void init(ConfigurationService configuration) {
     if (instance == null) {
+      if (configuration == null) {
+        configuration = new BaseConfigurationService(Version.UNKNOWN);
+      }
       instance = new ExecutorServiceManager(
           new InstrumentedExecutorService(configuration.getExecutorService()),
           configuration.getTerminationTimeoutSeconds());
+      log.debug("Initialized ExecutorServiceManager executor: {}, timeout: {}",
+          configuration.getExecutorService().getClass(),
+          configuration.getTerminationTimeoutSeconds());
     } else {
       log.debug("Already started, reusing already setup instance!");
     }
@@ -45,8 +52,8 @@ public static void stop() {
 
   public static ExecutorServiceManager instance() {
     if (instance == null) {
-      throw new IllegalStateException(
-          "ExecutorServiceManager hasn't been started. Call start method before using!");
+      // provide a default configuration if none has been provided by init
+      init(null);
     }
     return instance;
   }
@@ -72,6 +79,9 @@ private static class InstrumentedExecutorService implements ExecutorService {
     private final ExecutorService executor;
 
     private InstrumentedExecutorService(ExecutorService executor) {
+      if (executor == null) {
+        throw new NullPointerException();
+      }
       this.executor = executor;
       debug = Utils.debugThreadPool();
     }

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java

@@ -12,7 +12,11 @@
 import io.javaoperatorsdk.operator.CustomResourceUtils;
 import io.javaoperatorsdk.operator.MissingCRDException;
 import io.javaoperatorsdk.operator.OperatorException;
+import io.javaoperatorsdk.operator.api.config.BaseConfigurationService;
+import io.javaoperatorsdk.operator.api.config.ConfigurationService;
 import io.javaoperatorsdk.operator.api.config.ControllerConfiguration;
+import io.javaoperatorsdk.operator.api.config.Version;
+import io.javaoperatorsdk.operator.api.monitoring.Metrics;
 import io.javaoperatorsdk.operator.api.monitoring.Metrics.ControllerExecution;
 import io.javaoperatorsdk.operator.api.reconciler.Context;
 import io.javaoperatorsdk.operator.api.reconciler.DeleteControl;
@@ -29,6 +33,7 @@ public class Controller<R extends HasMetadata> implements Reconciler<R>,
   private final ControllerConfiguration<R> configuration;
   private final KubernetesClient kubernetesClient;
   private EventSourceManager<R> eventSourceManager;
+  private volatile ConfigurationService configurationService;
 
   public Controller(Reconciler<R> reconciler,
       ControllerConfiguration<R> configuration,
@@ -40,7 +45,7 @@ public Controller(Reconciler<R> reconciler,
 
   @Override
   public DeleteControl cleanup(R resource, Context context) {
-    return configuration.getConfigurationService().getMetrics().timeControllerExecution(
+    return metrics().timeControllerExecution(
         new ControllerExecution<>() {
           @Override
           public String name() {
@@ -66,7 +71,7 @@ public DeleteControl execute() {
 
   @Override
   public UpdateControl<R> reconcile(R resource, Context context) {
-    return configuration.getConfigurationService().getMetrics().timeControllerExecution(
+    return metrics().timeControllerExecution(
         new ControllerExecution<>() {
           @Override
           public String name() {
@@ -97,6 +102,11 @@ public UpdateControl<R> execute() {
         });
   }
 
+  private Metrics metrics() {
+    final var metrics = configurationService().getMetrics();
+    return metrics != null ? metrics : Metrics.NOOP;
+  }
+
   @Override
   public List<EventSource> prepareEventSources(EventSourceInitializationContext<R> context) {
     throw new UnsupportedOperationException("This method should never be called directly");
@@ -157,7 +167,7 @@ public void start() throws OperatorException {
     try {
       // check that the custom resource is known by the cluster if configured that way
       final CustomResourceDefinition crd; // todo: check proper CRD spec version based on config
-      if (configuration.getConfigurationService().checkCRDAndValidateLocalModel()) {
+      if (configurationService().checkCRDAndValidateLocalModel()) {
         crd =
             kubernetesClient.apiextensions().v1().customResourceDefinitions().withName(crdName)
                 .get();
@@ -174,7 +184,7 @@ public void start() throws OperatorException {
         ((EventSourceInitializer<R>) reconciler)
             .prepareEventSources(new EventSourceInitializationContext<>(
                 eventSourceManager.getControllerResourceEventSource().getResourceCache(),
-                configuration.getConfigurationService()))
+                configurationService()))
             .forEach(eventSourceManager::registerEventSource);
       }
       if (failOnMissingCurrentNS()) {
@@ -189,6 +199,23 @@ public void start() throws OperatorException {
     }
   }
 
+  private ConfigurationService configurationService() {
+    if (configurationService == null) {
+      configurationService = configuration.getConfigurationService();
+      // make sure we always have a default configuration service
+      if (configurationService == null) {
+        // we shouldn't need to register the configuration with the default service
+        configurationService = new BaseConfigurationService(Version.UNKNOWN) {
+          @Override
+          public boolean checkCRDAndValidateLocalModel() {
+            return false;
+          }
+        };
+      }
+    }
+    return configurationService;
+  }
+
   private void throwMissingCRDException(String crdName, String specVersion, String controllerName) {
     throw new MissingCRDException(
         crdName,

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventProcessor.java

@@ -56,8 +56,10 @@ class EventProcessor<R extends HasMetadata> implements EventHandler, LifecycleAw
         new ReconciliationDispatcher<>(eventSourceManager.getController()),
         GenericRetry.fromConfiguration(
             eventSourceManager.getController().getConfiguration().getRetryConfiguration()),
-        eventSourceManager.getController().getConfiguration().getConfigurationService()
-            .getMetrics(),
+        eventSourceManager.getController().getConfiguration().getConfigurationService() == null
+            ? Metrics.NOOP
+            : eventSourceManager.getController().getConfiguration().getConfigurationService()
+                .getMetrics(),
         eventSourceManager);
   }
 

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java

@@ -10,6 +10,7 @@
 import io.fabric8.kubernetes.client.dsl.MixedOperation;
 import io.fabric8.kubernetes.client.dsl.Resource;
 import io.javaoperatorsdk.operator.api.ObservedGenerationAware;
+import io.javaoperatorsdk.operator.api.config.ConfigurationService;
 import io.javaoperatorsdk.operator.api.config.ControllerConfiguration;
 import io.javaoperatorsdk.operator.api.reconciler.BaseControl;
 import io.javaoperatorsdk.operator.api.reconciler.Context;
@@ -131,7 +132,9 @@ private PostExecutionControl<R> handleReconcile(
   private R cloneResourceForErrorStatusHandlerIfNeeded(R resource, Context context) {
     if (isErrorStatusHandlerPresent() ||
         shouldUpdateObservedGenerationAutomatically(resource)) {
-      return configuration().getConfigurationService().getResourceCloner().clone(resource);
+      final var configurationService = configuration().getConfigurationService();
+      return configurationService != null ? configurationService.getResourceCloner().clone(resource)
+          : ConfigurationService.DEFAULT_CLONER.clone(resource);
     } else {
       return resource;
     }

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerResourceEventSource.java

@@ -16,6 +16,7 @@
 import io.fabric8.kubernetes.client.informers.ResourceEventHandler;
 import io.fabric8.kubernetes.client.informers.SharedIndexInformer;
 import io.javaoperatorsdk.operator.MissingCRDException;
+import io.javaoperatorsdk.operator.api.config.ConfigurationService;
 import io.javaoperatorsdk.operator.api.config.ControllerConfiguration;
 import io.javaoperatorsdk.operator.processing.Controller;
 import io.javaoperatorsdk.operator.processing.MDCUtils;
@@ -45,7 +46,9 @@ public class ControllerResourceEventSource<T extends HasMetadata>
   public ControllerResourceEventSource(Controller<T> controller) {
     super(controller.getConfiguration().getResourceClass());
     this.controller = controller;
-    var cloner = controller.getConfiguration().getConfigurationService().getResourceCloner();
+    final var configurationService = controller.getConfiguration().getConfigurationService();
+    var cloner = configurationService != null ? configurationService.getResourceCloner()
+        : ConfigurationService.DEFAULT_CLONER;
     this.cache = new ControllerResourceCache<>(sharedIndexInformers, cloner);
 
     var filters = new ResourceEventFilter[] {

operator-framework-core/src/test/java/io/javaoperatorsdk/operator/OperatorTest.java

@@ -8,21 +8,20 @@
 import io.fabric8.kubernetes.client.KubernetesClient;
 import io.javaoperatorsdk.operator.api.config.ConfigurationService;
 import io.javaoperatorsdk.operator.api.config.ControllerConfiguration;
+import io.javaoperatorsdk.operator.api.config.RetryConfiguration;
 import io.javaoperatorsdk.operator.api.reconciler.Context;
 import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
 import io.javaoperatorsdk.operator.api.reconciler.UpdateControl;
 
 import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
 import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 class OperatorTest {
 
   private final KubernetesClient kubernetesClient = mock(KubernetesClient.class);
   private final ConfigurationService configurationService = mock(ConfigurationService.class);
   private final ControllerConfiguration configuration = mock(ControllerConfiguration.class);
-
   private final Operator operator = new Operator(kubernetesClient, configurationService);
   private final FooReconciler fooReconciler = FooReconciler.create();
 
@@ -33,16 +32,13 @@ public void shouldRegisterReconcilerToController() {
     when(configurationService.getConfigurationFor(fooReconciler)).thenReturn(configuration);
     when(configuration.watchAllNamespaces()).thenReturn(true);
     when(configuration.getName()).thenReturn("FOO");
-    when(configuration.getResourceClass()).thenReturn(FooReconciler.class);
+    when(configuration.getResourceClass()).thenReturn(FooCustomResource.class);
+    when(configuration.getRetryConfiguration()).thenReturn(RetryConfiguration.DEFAULT);
 
     // when
     operator.register(fooReconciler);
 
     // then
-    verify(configuration).watchAllNamespaces();
-    verify(configuration).getName();
-    verify(configuration).getResourceClass();
-
     assertThat(operator.getControllers().size()).isEqualTo(1);
     assertThat(operator.getControllers().get(0).getReconciler()).isEqualTo(fooReconciler);
   }

operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.java

@@ -9,9 +9,7 @@
 
 import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.javaoperatorsdk.operator.OperatorException;
-import io.javaoperatorsdk.operator.api.config.ConfigurationService;
 import io.javaoperatorsdk.operator.api.config.ControllerConfiguration;
-import io.javaoperatorsdk.operator.api.config.ExecutorServiceManager;
 import io.javaoperatorsdk.operator.processing.Controller;
 import io.javaoperatorsdk.operator.processing.event.source.CachingEventSource;
 import io.javaoperatorsdk.operator.processing.event.source.EventSource;
@@ -174,10 +172,7 @@ private EventSourceManager initManager() {
     final Controller controller = mock(Controller.class);
     final ControllerConfiguration configuration = mock(ControllerConfiguration.class);
     when(configuration.getResourceClass()).thenReturn(HasMetadata.class);
-    when(configuration.getConfigurationService()).thenReturn(mock(ConfigurationService.class));
     when(controller.getConfiguration()).thenReturn(configuration);
-    ExecutorServiceManager.init(configuration.getConfigurationService());
-    var manager = new EventSourceManager(controller);
-    return manager;
+    return new EventSourceManager(controller);
   }
 }