feat: log when a configuration is not found or created automatically

Author: metacosm

operator-framework-core/pom.xml

@@ -85,7 +85,6 @@
     <dependency>
       <groupId>org.apache.logging.log4j</groupId>
       <artifactId>log4j-slf4j-impl</artifactId>
-      <version>2.13.3</version>
       <scope>test</scope>
     </dependency>
     <dependency>

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

@@ -7,9 +7,14 @@
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.stream.Stream;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public abstract class AbstractConfigurationService implements ConfigurationService {
 
+  public static final String LOGGER_NAME = "Default ConfigurationService implementation";
+  protected static final Logger log = LoggerFactory.getLogger(LOGGER_NAME);
+
   private final Map<String, ControllerConfiguration> configurations = new ConcurrentHashMap<>();
   private final Version version;
 
@@ -52,7 +57,19 @@ protected void throwExceptionOnNameCollision(
   @Override
   public <R extends CustomResource> ControllerConfiguration<R> getConfigurationFor(
       ResourceController<R> controller) {
-    return configurations.get(keyFor(controller));
+    final var key = keyFor(controller);
+    final var configuration = configurations.get(key);
+    if (configuration == null) {
+      log.warn(
+          "Configuration for controller '{}' was not found. {}", key, getControllersNameMessage());
+    }
+    return configuration;
+  }
+
+  private String getControllersNameMessage() {
+    return "Known controllers: "
+        + getKnownControllerNames().stream().reduce((s, s2) -> s + ", " + s2).orElse("None")
+        + ".";
   }
 
   protected String keyFor(ResourceController controller) {

operator-framework/pom.xml

@@ -55,6 +55,12 @@
       <artifactId>junit-jupiter-engine</artifactId>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>ch.qos.logback</groupId>
+      <artifactId>logback-classic</artifactId>
+      <version>1.2.3</version>
+      <scope>test</scope>
+    </dependency>
     <dependency>
       <groupId>org.assertj</groupId>
       <artifactId>assertj-core</artifactId>

operator-framework/src/main/java/io/javaoperatorsdk/operator/config/runtime/DefaultConfigurationService.java

@@ -3,30 +3,40 @@
 import io.fabric8.kubernetes.client.CustomResource;
 import io.javaoperatorsdk.operator.api.ResourceController;
 import io.javaoperatorsdk.operator.api.config.AbstractConfigurationService;
-import io.javaoperatorsdk.operator.api.config.ConfigurationService;
 import io.javaoperatorsdk.operator.api.config.ControllerConfiguration;
 import io.javaoperatorsdk.operator.api.config.Utils;
 
 public class DefaultConfigurationService extends AbstractConfigurationService {
 
-  private static final ConfigurationService instance = new DefaultConfigurationService();
+  private static final DefaultConfigurationService instance = new DefaultConfigurationService();
 
   private DefaultConfigurationService() {
     super(Utils.loadFromProperties());
   }
 
-  public static ConfigurationService instance() {
+  public static DefaultConfigurationService instance() {
     return instance;
   }
 
   @Override
   public <R extends CustomResource> ControllerConfiguration<R> getConfigurationFor(
       ResourceController<R> controller) {
+    return getConfigurationFor(controller, true);
+  }
+
+  <R extends CustomResource> ControllerConfiguration<R> getConfigurationFor(
+      ResourceController<R> controller, boolean createIfNeeded) {
     var config = super.getConfigurationFor(controller);
     if (config == null) {
-      // create the the configuration on demand and register it
-      config = new AnnotationConfiguration(controller);
-      register(config);
+      if (createIfNeeded) {
+        // create the the configuration on demand and register it
+        config = new AnnotationConfiguration(controller);
+        register(config);
+        log.info(
+            "Created configuration for controller {} with name {}",
+            controller.getClass().getName(),
+            config.getName());
+      }
     } else {
       // check that we don't have a controller name collision
       final var newControllerClassName = controller.getClass().getCanonicalName();

operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/DefaultConfigurationServiceTest.java

@@ -3,7 +3,12 @@
 import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
+import ch.qos.logback.classic.Logger;
+import ch.qos.logback.classic.spi.ILoggingEvent;
+import ch.qos.logback.core.read.ListAppender;
 import io.fabric8.kubernetes.client.CustomResource;
 import io.fabric8.kubernetes.model.annotation.Group;
 import io.fabric8.kubernetes.model.annotation.Version;
@@ -12,12 +17,39 @@
 import io.javaoperatorsdk.operator.api.Controller;
 import io.javaoperatorsdk.operator.api.ResourceController;
 import io.javaoperatorsdk.operator.api.UpdateControl;
+import io.javaoperatorsdk.operator.api.config.AbstractConfigurationService;
 import org.junit.jupiter.api.Test;
+import org.slf4j.LoggerFactory;
 
 public class DefaultConfigurationServiceTest {
 
   public static final String CUSTOM_FINALIZER_NAME = "a.custom/finalizer";
 
+  @Test
+  void attemptingToRetrieveAnUnknownControllerShouldLogWarning() {
+    final var logger = (Logger) LoggerFactory.getLogger(AbstractConfigurationService.LOGGER_NAME);
+    final var appender = new ListAppender<ILoggingEvent>();
+    logger.addAppender(appender);
+    appender.start();
+    try {
+      final var config =
+          DefaultConfigurationService.instance()
+              .getConfigurationFor(new NotAutomaticallyCreated(), false);
+      assertNull(config);
+      assertEquals(1, appender.list.size());
+      assertTrue(
+          appender.list.stream()
+              .allMatch(
+                  e -> {
+                    final var message = e.getFormattedMessage();
+                    return message.contains(NotAutomaticallyCreated.NAME)
+                        && message.contains("not found");
+                  }));
+    } finally {
+      logger.detachAndStopAllAppenders();
+    }
+  }
+
   @Test
   public void returnsValuesFromControllerAnnotationFinalizer() {
     final var controller = new TestCustomResourceController();
@@ -65,6 +97,18 @@ public UpdateControl<TestCustomFinalizerController.InnerCustomResource> createOr
     }
   }
 
+  @Controller(name = NotAutomaticallyCreated.NAME)
+  static class NotAutomaticallyCreated implements ResourceController<TestCustomResource> {
+
+    public static final String NAME = "should-be-logged";
+
+    @Override
+    public UpdateControl<TestCustomResource> createOrUpdateResource(
+        TestCustomResource resource, Context<TestCustomResource> context) {
+      return null;
+    }
+  }
+
   @Controller(generationAwareEventProcessing = false, name = "test")
   static class TestCustomResourceController implements ResourceController<TestCustomResource> {