From 90ea84e31623a5c74decdc9d94a5a364da09287f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Mon, 15 Apr 2024 13:10:57 +0200 Subject: [PATCH] fix: change namespace starts processor on namespace change even if not leader (#2344) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../operator/processing/Controller.java | 14 ++- .../processing/event/EventProcessor.java | 4 + .../LeaderElectionChangeNamespaceIT.java | 98 +++++++++++++++++++ ...ElectionChangeNamespaceCustomResource.java | 15 +++ ...aderElectionChangeNamespaceReconciler.java | 29 ++++++ 5 files changed, 157 insertions(+), 3 deletions(-) create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/LeaderElectionChangeNamespaceIT.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/leaderelectionchangenamespace/LeaderElectionChangeNamespaceCustomResource.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/leaderelectionchangenamespace/LeaderElectionChangeNamespaceReconciler.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java index 70069148c3..bc2d3f9eec 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java @@ -368,7 +368,7 @@ private void validateCRDWithLocalModelIfRequired(Class

resClass, String contr } } - public void changeNamespaces(Set namespaces) { + public synchronized void changeNamespaces(Set namespaces) { if (namespaces.contains(WATCH_CURRENT_NAMESPACE)) { throw new OperatorException("Unexpected value in target namespaces: " + namespaces); } @@ -376,9 +376,17 @@ public void changeNamespaces(Set namespaces) { throw new OperatorException( "Watching all namespaces, but additional specific namespace is present"); } - eventProcessor.stop(); + // if the processor was not running, for example because the controller + // was not leading in a HA setup, we don't want to stop and + // mainly start the processor on namespace change. + boolean eventProcessorWasRunning = eventProcessor.isRunning(); + if (eventProcessorWasRunning) { + eventProcessor.stop(); + } eventSourceManager.changeNamespaces(namespaces); - eventProcessor.start(); + if (eventProcessorWasRunning) { + eventProcessor.start(); + } } public synchronized void startEventProcessing() { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventProcessor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventProcessor.java index f5d7d5ba83..2809efde8a 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventProcessor.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventProcessor.java @@ -473,4 +473,8 @@ private String controllerName() { public synchronized boolean isUnderProcessing(ResourceID resourceID) { return isControllerUnderExecution(resourceStateManager.getOrCreate(resourceID)); } + + public synchronized boolean isRunning() { + return running; + } } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/LeaderElectionChangeNamespaceIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/LeaderElectionChangeNamespaceIT.java new file mode 100644 index 0000000000..1a3a450b90 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/LeaderElectionChangeNamespaceIT.java @@ -0,0 +1,98 @@ +package io.javaoperatorsdk.operator; + +import java.time.Duration; +import java.time.ZonedDateTime; + +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.fabric8.kubernetes.api.model.coordination.v1.Lease; +import io.fabric8.kubernetes.api.model.coordination.v1.LeaseSpecBuilder; +import io.fabric8.kubernetes.client.KubernetesClient; +import io.fabric8.kubernetes.client.KubernetesClientBuilder; +import io.javaoperatorsdk.operator.api.config.LeaderElectionConfiguration; +import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; +import io.javaoperatorsdk.operator.sample.leaderelectionchangenamespace.LeaderElectionChangeNamespaceCustomResource; +import io.javaoperatorsdk.operator.sample.leaderelectionchangenamespace.LeaderElectionChangeNamespaceReconciler; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + +public class LeaderElectionChangeNamespaceIT { + + public static final String LEASE_NAME = "nschangelease"; + + @RegisterExtension + LocallyRunOperatorExtension extension = + LocallyRunOperatorExtension.builder() + .withConfigurationService(o -> o.withLeaderElectionConfiguration( + new LeaderElectionConfiguration(LEASE_NAME))) + .withReconciler(new LeaderElectionChangeNamespaceReconciler()) + .build(); + + private static KubernetesClient client = new KubernetesClientBuilder().build(); + + @BeforeAll + static void createLeaseManually() { + client.resource(lease()).create(); + } + + @AfterAll + static void deleteLeaseManually() { + client.resource(lease()).delete(); + } + + @Test + @DisplayName("If operator is not a leader, namespace change should not start processor") + void noReconcileOnChangeNamespace() { + extension.create(testResource()); + + var reconciler = extension.getReconcilerOfType(LeaderElectionChangeNamespaceReconciler.class); + await().pollDelay(Duration.ofSeconds(1)) + .timeout(Duration.ofSeconds(3)) + .untilAsserted(() -> { + assertThat(reconciler.getNumberOfExecutions()).isEqualTo(0); + }); + + extension.getRegisteredControllerForReconcile(LeaderElectionChangeNamespaceReconciler.class) + .changeNamespaces("default", extension.getNamespace()); + + await().pollDelay(Duration.ofSeconds(1)) + .timeout(Duration.ofSeconds(3)) + .untilAsserted(() -> { + assertThat(reconciler.getNumberOfExecutions()).isEqualTo(0); + }); + } + + + LeaderElectionChangeNamespaceCustomResource testResource() { + var resource = new LeaderElectionChangeNamespaceCustomResource(); + resource.setMetadata(new ObjectMetaBuilder() + .withName("test1") + .build()); + return resource; + } + + static Lease lease() { + var lease = new Lease(); + lease.setMetadata(new ObjectMetaBuilder() + .withName(LEASE_NAME) + .withNamespace("default") + .build()); + var time = ZonedDateTime.now(); + lease.setSpec(new LeaseSpecBuilder() + .withAcquireTime(ZonedDateTime.now()) + .withRenewTime(time) + .withAcquireTime(time) + .withHolderIdentity("non-operator-identity") + .withLeaseTransitions(0) + .withLeaseDurationSeconds(30) + .build()); + + return lease; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/leaderelectionchangenamespace/LeaderElectionChangeNamespaceCustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/leaderelectionchangenamespace/LeaderElectionChangeNamespaceCustomResource.java new file mode 100644 index 0000000000..8a37720955 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/leaderelectionchangenamespace/LeaderElectionChangeNamespaceCustomResource.java @@ -0,0 +1,15 @@ +package io.javaoperatorsdk.operator.sample.leaderelectionchangenamespace; + +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("lcn") +public class LeaderElectionChangeNamespaceCustomResource + extends CustomResource + implements Namespaced { +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/leaderelectionchangenamespace/LeaderElectionChangeNamespaceReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/leaderelectionchangenamespace/LeaderElectionChangeNamespaceReconciler.java new file mode 100644 index 0000000000..8651a4774e --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/leaderelectionchangenamespace/LeaderElectionChangeNamespaceReconciler.java @@ -0,0 +1,29 @@ +package io.javaoperatorsdk.operator.sample.leaderelectionchangenamespace; + +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() +public class LeaderElectionChangeNamespaceReconciler + implements Reconciler, TestExecutionInfoProvider { + + private final AtomicInteger numberOfExecutions = new AtomicInteger(0); + + @Override + public UpdateControl reconcile( + LeaderElectionChangeNamespaceCustomResource resource, + Context context) { + numberOfExecutions.addAndGet(1); + return UpdateControl.noUpdate(); + } + + public int getNumberOfExecutions() { + return numberOfExecutions.get(); + } + +}