From 32b8b926c36fa1272d269807fd3d33bc2cfa55b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Tue, 6 Feb 2024 12:19:20 +0100 Subject: [PATCH] feat: different reconciler same type (#2229) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros Signed-off-by: Chris Laprun Co-authored-by: Chris Laprun --- .../operator/ControllerManager.java | 21 +++---- .../operator/ControllerManagerTest.java | 58 ++++++------------ .../sample/simple/DuplicateCRController.java | 16 ----- .../MultipleReconcilerSameTypeIT.java | 61 +++++++++++++++++++ ...tipleReconcilerSameTypeCustomResource.java | 15 +++++ ...MultipleReconcilerSameTypeReconciler1.java | 29 +++++++++ ...MultipleReconcilerSameTypeReconciler2.java | 32 ++++++++++ .../MultipleReconcilerSameTypeStatus.java | 14 +++++ 8 files changed, 177 insertions(+), 69 deletions(-) delete mode 100644 operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/simple/DuplicateCRController.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/MultipleReconcilerSameTypeIT.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/multiplereconcilersametype/MultipleReconcilerSameTypeCustomResource.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/multiplereconcilersametype/MultipleReconcilerSameTypeReconciler1.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/multiplereconcilersametype/MultipleReconcilerSameTypeReconciler2.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/multiplereconcilersametype/MultipleReconcilerSameTypeStatus.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ControllerManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ControllerManager.java index 5504e543f5..e90070d3c2 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ControllerManager.java +++ b/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 get(String name) { - return controllers().stream() - .filter(c -> name.equals(c.getConfiguration().getName())) - .findFirst(); + return Optional.ofNullable(controllers.get(name)); } @SuppressWarnings("rawtypes") diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ControllerManagerTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ControllerManagerTest.java index 7b80aeed8b..c8c4cb5008 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ControllerManagerTest.java +++ b/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 void checkException( - TestControllerConfiguration registered, - TestControllerConfiguration 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 extends ResolvedControllerConfiguration { - private final Reconciler controller; + private final Reconciler reconciler; - public TestControllerConfiguration(Reconciler controller, Class crClass) { - super(crClass, getControllerName(controller), controller.getClass(), + public TestControllerConfiguration(Reconciler reconciler, Class crClass) { + super(crClass, getControllerName(reconciler), reconciler.getClass(), new BaseConfigurationService()); - this.controller = controller; + this.reconciler = reconciler; } static String getControllerName( diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/simple/DuplicateCRController.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/simple/DuplicateCRController.java deleted file mode 100644 index 6b5b6ab08d..0000000000 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/sample/simple/DuplicateCRController.java +++ /dev/null @@ -1,16 +0,0 @@ -package io.javaoperatorsdk.operator.sample.simple; - -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; - -@ControllerConfiguration -public class DuplicateCRController implements Reconciler { - - @Override - public UpdateControl reconcile(TestCustomResource resource, - Context context) { - return UpdateControl.noUpdate(); - } -} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/MultipleReconcilerSameTypeIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/MultipleReconcilerSameTypeIT.java new file mode 100644 index 0000000000..b0be12bd91 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/MultipleReconcilerSameTypeIT.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; + } + +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/multiplereconcilersametype/MultipleReconcilerSameTypeCustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/multiplereconcilersametype/MultipleReconcilerSameTypeCustomResource.java new file mode 100644 index 0000000000..53c731e5e7 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/multiplereconcilersametype/MultipleReconcilerSameTypeCustomResource.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 + implements Namespaced { +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/multiplereconcilersametype/MultipleReconcilerSameTypeReconciler1.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/multiplereconcilersametype/MultipleReconcilerSameTypeReconciler1.java new file mode 100644 index 0000000000..e32a6ad7e2 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/multiplereconcilersametype/MultipleReconcilerSameTypeReconciler1.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, TestExecutionInfoProvider { + + private final AtomicInteger numberOfExecutions = new AtomicInteger(0); + + @Override + public UpdateControl reconcile( + MultipleReconcilerSameTypeCustomResource resource, + Context context) { + numberOfExecutions.addAndGet(1); + + resource.setStatus(new MultipleReconcilerSameTypeStatus()); + resource.getStatus().setReconciledBy(getClass().getSimpleName()); + return UpdateControl.patchStatus(resource); + } + + public int getNumberOfExecutions() { + return numberOfExecutions.get(); + } + +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/multiplereconcilersametype/MultipleReconcilerSameTypeReconciler2.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/multiplereconcilersametype/MultipleReconcilerSameTypeReconciler2.java new file mode 100644 index 0000000000..73b082f3b0 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/multiplereconcilersametype/MultipleReconcilerSameTypeReconciler2.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, TestExecutionInfoProvider { + + private final AtomicInteger numberOfExecutions = new AtomicInteger(0); + + @Override + public UpdateControl reconcile( + MultipleReconcilerSameTypeCustomResource resource, + Context context) { + numberOfExecutions.addAndGet(1); + + resource.setStatus(new MultipleReconcilerSameTypeStatus()); + resource.getStatus().setReconciledBy(getClass().getSimpleName()); + return UpdateControl.patchStatus(resource); + } + + public int getNumberOfExecutions() { + return numberOfExecutions.get(); + } + +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/multiplereconcilersametype/MultipleReconcilerSameTypeStatus.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/multiplereconcilersametype/MultipleReconcilerSameTypeStatus.java new file mode 100644 index 0000000000..9335d89752 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/multiplereconcilersametype/MultipleReconcilerSameTypeStatus.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; + } +}