feat: different reconciler same type (#2229)

Signed-off-by: Attila Mészáros <csviri@gmail.com>
Signed-off-by: Chris Laprun <claprun@redhat.com>
Co-authored-by: Chris Laprun <claprun@redhat.com>

Author: csviri

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ControllerManager.java

@@ -17,6 +17,8 @@
  */
 class ControllerManager {
 
+  public static final String CANNOT_REGISTER_MULTIPLE_CONTROLLERS_WITH_SAME_NAME_MESSAGE =
+      "Cannot register multiple controllers with same name: ";
   private static final Logger log = LoggerFactory.getLogger(ControllerManager.class);
 
   @SuppressWarnings("rawtypes")
@@ -62,25 +64,20 @@ public synchronized void startEventProcessing() {
     }, c -> "Event processor starter for: " + c.getConfiguration().getName());
   }
 
-  @SuppressWarnings({"unchecked", "rawtypes"})
+  @SuppressWarnings("rawtypes")
   synchronized void add(Controller controller) {
     final var configuration = controller.getConfiguration();
-    final var resourceTypeName = ReconcilerUtils
-        .getResourceTypeNameWithVersion(configuration.getResourceClass());
-    final var existing = controllers.get(resourceTypeName);
-    if (existing != null) {
-      throw new OperatorException("Cannot register controller '" + configuration.getName()
-          + "': another controller named '" + existing.getConfiguration().getName()
-          + "' is already registered for resource '" + resourceTypeName + "'");
+    final var name = configuration.getName();
+    if (controllers.containsKey(name)) {
+      throw new OperatorException(
+          CANNOT_REGISTER_MULTIPLE_CONTROLLERS_WITH_SAME_NAME_MESSAGE + name);
     }
-    controllers.put(resourceTypeName, controller);
+    controllers.put(name, controller);
   }
 
   @SuppressWarnings("rawtypes")
   synchronized Optional<Controller> get(String name) {
-    return controllers().stream()
-        .filter(c -> name.equals(c.getConfiguration().getName()))
-        .findFirst();
+    return Optional.ofNullable(controllers.get(name));
   }
 
   @SuppressWarnings("rawtypes")

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

@@ -8,66 +8,42 @@
 import io.javaoperatorsdk.operator.api.config.ResolvedControllerConfiguration;
 import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
 import io.javaoperatorsdk.operator.processing.Controller;
-import io.javaoperatorsdk.operator.sample.simple.DuplicateCRController;
 import io.javaoperatorsdk.operator.sample.simple.TestCustomReconciler;
-import io.javaoperatorsdk.operator.sample.simple.TestCustomReconcilerOtherV1;
 import io.javaoperatorsdk.operator.sample.simple.TestCustomResource;
-import io.javaoperatorsdk.operator.sample.simple.TestCustomResourceOtherV1;
 
+import static io.javaoperatorsdk.operator.ControllerManager.CANNOT_REGISTER_MULTIPLE_CONTROLLERS_WITH_SAME_NAME_MESSAGE;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 class ControllerManagerTest {
 
   @Test
-  void shouldNotAddMultipleControllersForSameCustomResource() {
-    final var registered = new TestControllerConfiguration<>(new TestCustomReconciler(null),
-        TestCustomResource.class);
-    final var duplicated =
-        new TestControllerConfiguration<>(new DuplicateCRController(), TestCustomResource.class);
-
-    checkException(registered, duplicated);
-  }
-
-  @Test
-  void addingMultipleControllersForCustomResourcesWithSameVersionsShouldNotWork() {
-    final var registered = new TestControllerConfiguration<>(new TestCustomReconciler(null),
-        TestCustomResource.class);
-    final var duplicated = new TestControllerConfiguration<>(new TestCustomReconcilerOtherV1(),
-        TestCustomResourceOtherV1.class);
-
-    checkException(registered, duplicated);
-  }
-
-  private <T extends HasMetadata, U extends HasMetadata> void checkException(
-      TestControllerConfiguration<T> registered,
-      TestControllerConfiguration<U> duplicated) {
-
+  void addingReconcilerWithSameNameShouldNotWork() {
+    final var controllerConfiguration =
+        new TestControllerConfiguration<>(new TestCustomReconciler(null),
+            TestCustomResource.class);
+    var controller = new Controller<>(controllerConfiguration.reconciler, controllerConfiguration,
+        MockKubernetesClient.client(controllerConfiguration.getResourceClass()));
     ConfigurationService configurationService = new BaseConfigurationService();
+    final var controllerManager =
+        new ControllerManager(configurationService.getExecutorServiceManager());
+    controllerManager.add(controller);
 
-    final var exception = assertThrows(OperatorException.class, () -> {
-      final var controllerManager =
-          new ControllerManager(configurationService.getExecutorServiceManager());
-      controllerManager.add(new Controller<>(registered.controller, registered,
-          MockKubernetesClient.client(registered.getResourceClass())));
-      controllerManager.add(new Controller<>(duplicated.controller, duplicated,
-          MockKubernetesClient.client(duplicated.getResourceClass())));
+    var ex = assertThrows(OperatorException.class, () -> {
+      controllerManager.add(controller);
     });
-    final var msg = exception.getMessage();
     assertTrue(
-        msg.contains("Cannot register controller '" + duplicated.getName() + "'")
-            && msg.contains(registered.getName())
-            && msg.contains(registered.getResourceTypeName()));
+        ex.getMessage().contains(CANNOT_REGISTER_MULTIPLE_CONTROLLERS_WITH_SAME_NAME_MESSAGE));
   }
 
   private static class TestControllerConfiguration<R extends HasMetadata>
       extends ResolvedControllerConfiguration<R> {
-    private final Reconciler<R> controller;
+    private final Reconciler<R> reconciler;
 
-    public TestControllerConfiguration(Reconciler<R> controller, Class<R> crClass) {
-      super(crClass, getControllerName(controller), controller.getClass(),
+    public TestControllerConfiguration(Reconciler<R> reconciler, Class<R> crClass) {
+      super(crClass, getControllerName(reconciler), reconciler.getClass(),
           new BaseConfigurationService());
-      this.controller = controller;
+      this.reconciler = reconciler;
     }
 
     static <R extends HasMetadata> String getControllerName(

operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/simple/DuplicateCRController.java

@@ -0,0 +1,61 @@
+package io.javaoperatorsdk.operator;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
+import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension;
+import io.javaoperatorsdk.operator.sample.multiplereconcilersametype.MultipleReconcilerSameTypeCustomResource;
+import io.javaoperatorsdk.operator.sample.multiplereconcilersametype.MultipleReconcilerSameTypeReconciler1;
+import io.javaoperatorsdk.operator.sample.multiplereconcilersametype.MultipleReconcilerSameTypeReconciler2;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.awaitility.Awaitility.await;
+
+public class MultipleReconcilerSameTypeIT {
+
+  public static final String TEST_RESOURCE_1 = "test1";
+  public static final String TEST_RESOURCE_2 = "test2";
+  @RegisterExtension
+  LocallyRunOperatorExtension extension =
+      LocallyRunOperatorExtension.builder()
+          .withReconciler(MultipleReconcilerSameTypeReconciler1.class)
+          .withReconciler(MultipleReconcilerSameTypeReconciler2.class)
+          .build();
+
+
+  @Test
+  void multipleReconcilersBasedOnLeaderElection() {
+    extension.create(testResource(TEST_RESOURCE_1, true));
+    extension.create(testResource(TEST_RESOURCE_2, false));
+
+
+    await().untilAsserted(() -> {
+      assertThat(extension.getReconcilerOfType(MultipleReconcilerSameTypeReconciler1.class)
+          .getNumberOfExecutions()).isEqualTo(1);
+      assertThat(extension.getReconcilerOfType(MultipleReconcilerSameTypeReconciler2.class)
+          .getNumberOfExecutions()).isEqualTo(1);
+
+      var res1 = extension.get(MultipleReconcilerSameTypeCustomResource.class, TEST_RESOURCE_1);
+      var res2 = extension.get(MultipleReconcilerSameTypeCustomResource.class, TEST_RESOURCE_2);
+      assertThat(res1).isNotNull();
+      assertThat(res2).isNotNull();
+      assertThat(res1.getStatus().getReconciledBy())
+          .isEqualTo(MultipleReconcilerSameTypeReconciler1.class.getSimpleName());
+      assertThat(res2.getStatus().getReconciledBy())
+          .isEqualTo(MultipleReconcilerSameTypeReconciler2.class.getSimpleName());
+    });
+  }
+
+  MultipleReconcilerSameTypeCustomResource testResource(String name, boolean type1) {
+    var res = new MultipleReconcilerSameTypeCustomResource();
+    res.setMetadata(new ObjectMetaBuilder()
+        .withName(name)
+        .build());
+    if (type1) {
+      res.getMetadata().getLabels().put("reconciler", "1");
+    }
+    return res;
+  }
+
+}

operator-framework/src/test/java/io/javaoperatorsdk/operator/MultipleReconcilerSameTypeIT.java

@@ -0,0 +1,15 @@
+package io.javaoperatorsdk.operator.sample.multiplereconcilersametype;
+
+import io.fabric8.kubernetes.api.model.Namespaced;
+import io.fabric8.kubernetes.client.CustomResource;
+import io.fabric8.kubernetes.model.annotation.Group;
+import io.fabric8.kubernetes.model.annotation.ShortNames;
+import io.fabric8.kubernetes.model.annotation.Version;
+
+@Group("sample.javaoperatorsdk")
+@Version("v1")
+@ShortNames("mrst")
+public class MultipleReconcilerSameTypeCustomResource
+    extends CustomResource<Void, MultipleReconcilerSameTypeStatus>
+    implements Namespaced {
+}

operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/multiplereconcilersametype/MultipleReconcilerSameTypeCustomResource.java

@@ -0,0 +1,29 @@
+package io.javaoperatorsdk.operator.sample.multiplereconcilersametype;
+
+import java.util.concurrent.atomic.AtomicInteger;
+
+import io.javaoperatorsdk.operator.api.reconciler.*;
+import io.javaoperatorsdk.operator.support.TestExecutionInfoProvider;
+
+@ControllerConfiguration(labelSelector = "reconciler = 1")
+public class MultipleReconcilerSameTypeReconciler1
+    implements Reconciler<MultipleReconcilerSameTypeCustomResource>, TestExecutionInfoProvider {
+
+  private final AtomicInteger numberOfExecutions = new AtomicInteger(0);
+
+  @Override
+  public UpdateControl<MultipleReconcilerSameTypeCustomResource> reconcile(
+      MultipleReconcilerSameTypeCustomResource resource,
+      Context<MultipleReconcilerSameTypeCustomResource> context) {
+    numberOfExecutions.addAndGet(1);
+
+    resource.setStatus(new MultipleReconcilerSameTypeStatus());
+    resource.getStatus().setReconciledBy(getClass().getSimpleName());
+    return UpdateControl.patchStatus(resource);
+  }
+
+  public int getNumberOfExecutions() {
+    return numberOfExecutions.get();
+  }
+
+}

operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/multiplereconcilersametype/MultipleReconcilerSameTypeReconciler1.java

@@ -0,0 +1,32 @@
+package io.javaoperatorsdk.operator.sample.multiplereconcilersametype;
+
+import java.util.concurrent.atomic.AtomicInteger;
+
+import io.javaoperatorsdk.operator.api.reconciler.Context;
+import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration;
+import io.javaoperatorsdk.operator.api.reconciler.Reconciler;
+import io.javaoperatorsdk.operator.api.reconciler.UpdateControl;
+import io.javaoperatorsdk.operator.support.TestExecutionInfoProvider;
+
+@ControllerConfiguration(labelSelector = "reconciler != 1")
+public class MultipleReconcilerSameTypeReconciler2
+    implements Reconciler<MultipleReconcilerSameTypeCustomResource>, TestExecutionInfoProvider {
+
+  private final AtomicInteger numberOfExecutions = new AtomicInteger(0);
+
+  @Override
+  public UpdateControl<MultipleReconcilerSameTypeCustomResource> reconcile(
+      MultipleReconcilerSameTypeCustomResource resource,
+      Context<MultipleReconcilerSameTypeCustomResource> context) {
+    numberOfExecutions.addAndGet(1);
+
+    resource.setStatus(new MultipleReconcilerSameTypeStatus());
+    resource.getStatus().setReconciledBy(getClass().getSimpleName());
+    return UpdateControl.patchStatus(resource);
+  }
+
+  public int getNumberOfExecutions() {
+    return numberOfExecutions.get();
+  }
+
+}

operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/multiplereconcilersametype/MultipleReconcilerSameTypeReconciler2.java

@@ -0,0 +1,14 @@
+package io.javaoperatorsdk.operator.sample.multiplereconcilersametype;
+
+public class MultipleReconcilerSameTypeStatus {
+
+  private String reconciledBy;
+
+  public String getReconciledBy() {
+    return reconciledBy;
+  }
+
+  public void setReconciledBy(String reconciledBy) {
+    this.reconciledBy = reconciledBy;
+  }
+}