[issue-687] Refactor/operator register api (#688)

Author: heesuk-ahn

Diff for: operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java

@@ -70,7 +70,7 @@ public void start() {
 
     log.info("Client version: {}", Version.clientVersion());
     try {
-      final var k8sVersion = kubernetesClient.getVersion();
+      final var k8sVersion = kubernetesClient.getKubernetesVersion();
       if (k8sVersion != null) {
         log.info("Server version: {}.{}", k8sVersion.getMajor(), k8sVersion.getMinor());
       }
@@ -110,13 +110,14 @@ public void close() {
    * Add a registration requests for the specified controller with this operator. The effective
    * registration of the controller is delayed till the operator is started.
    *
-   * @param controller the controller to register
+   * @param reconciler the controller to register
    * @param <R> the {@code CustomResource} type associated with the controller
    * @throws OperatorException if a problem occurred during the registration process
    */
-  public <R extends HasMetadata> void register(Reconciler<R> controller)
+  public <R extends HasMetadata> void register(Reconciler<R> reconciler)
       throws OperatorException {
-    register(controller, null);
+    final var defaultConfiguration = configurationService.getConfigurationFor(reconciler);
+    register(reconciler, defaultConfiguration);
   }
 
   /**
@@ -127,39 +128,34 @@ public <R extends HasMetadata> void register(Reconciler<R> controller)
    * controller is delayed till the operator is started.
    *
    * @param reconciler part of the controller to register
-   * @param configuration the configuration with which we want to register the controller, if {@code
-   *     null}, the controller's original configuration is used
+   * @param configuration the configuration with which we want to register the controller
    * @param <R> the {@code CustomResource} type associated with the controller
    * @throws OperatorException if a problem occurred during the registration process
    */
-  public <R extends HasMetadata> void register(
-      Reconciler<R> reconciler, ControllerConfiguration<R> configuration)
+  public <R extends HasMetadata> void register(Reconciler<R> reconciler,
+      ControllerConfiguration<R> configuration)
       throws OperatorException {
-    final var existing = configurationService.getConfigurationFor(reconciler);
-    if (existing == null) {
+
+    if (configuration == null) {
       throw new OperatorException(
           "Cannot register controller with name " + reconciler.getClass().getCanonicalName() +
               " controller named " + ControllerUtils.getNameFor(reconciler)
               + " because its configuration cannot be found.\n" +
               " Known controllers are: " + configurationService.getKnownControllerNames());
-    } else {
-      if (configuration == null) {
-        configuration = existing;
-      }
-      final var controller =
-          new Controller<>(reconciler, configuration, kubernetesClient);
-      controllers.add(controller);
-
-      final var watchedNS =
-          configuration.watchAllNamespaces()
-              ? "[all namespaces]"
-              : configuration.getEffectiveNamespaces();
-      log.info(
-          "Registered Controller: '{}' for CRD: '{}' for namespace(s): {}",
-          configuration.getName(),
-          configuration.getResourceClass(),
-          watchedNS);
     }
+
+    final var controller = new Controller<>(reconciler, configuration, kubernetesClient);
+
+    controllers.add(controller);
+
+    final var watchedNS = configuration.watchAllNamespaces() ? "[all namespaces]"
+        : configuration.getEffectiveNamespaces();
+
+    log.info(
+        "Registered Controller: '{}' for CRD: '{}' for namespace(s): {}",
+        configuration.getName(),
+        configuration.getResourceClass(),
+        watchedNS);
   }
 
   static class ControllerManager implements LifecycleAware {

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

@@ -0,0 +1,79 @@
+package io.javaoperatorsdk.operator;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+
+import io.fabric8.kubernetes.client.CustomResource;
+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.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();
+
+  @Test
+  @DisplayName("should register `Reconciler` to Controller")
+  public void shouldRegisterReconcilerToController() {
+    // given
+    when(configurationService.getConfigurationFor(fooReconciler)).thenReturn(configuration);
+    when(configuration.watchAllNamespaces()).thenReturn(true);
+    when(configuration.getName()).thenReturn("FOO");
+    when(configuration.getResourceClass()).thenReturn(FooReconciler.class);
+
+    // 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);
+  }
+
+  @Test
+  @DisplayName("should throw `OperationException` when Configuration is null")
+  public void shouldThrowOperatorExceptionWhenConfigurationIsNull() {
+    Assertions.assertThrows(OperatorException.class, () -> operator.register(fooReconciler, null));
+  }
+
+  private static class FooCustomResource extends CustomResource<FooSpec, FooStatus> {
+  }
+
+  private static class FooSpec {
+  }
+
+  private static class FooStatus {
+  }
+
+  private static class FooReconciler implements Reconciler<FooCustomResource> {
+
+    private FooReconciler() {}
+
+    public static FooReconciler create() {
+      return new FooReconciler();
+    }
+
+    @Override
+    public UpdateControl<FooCustomResource> reconcile(FooCustomResource resource, Context context) {
+      return UpdateControl.noUpdate();
+    }
+  }
+
+}