diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java index 94d201dce9..931a7ddf27 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java @@ -12,7 +12,6 @@ import io.fabric8.kubernetes.client.extended.leaderelection.LeaderElector; import io.fabric8.kubernetes.client.extended.leaderelection.LeaderElectorBuilder; import io.fabric8.kubernetes.client.extended.leaderelection.resourcelock.LeaseLock; -import io.fabric8.kubernetes.client.extended.leaderelection.resourcelock.Lock; import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; import io.javaoperatorsdk.operator.api.config.LeaderElectionConfiguration; @@ -31,14 +30,30 @@ public LeaderElectionManager(ControllerManager controllerManager) { public void init(LeaderElectionConfiguration config, KubernetesClient client) { this.identity = identity(config); - Lock lock = new LeaseLock(config.getLeaseNamespace(), config.getLeaseName(), identity); + final var leaseNamespace = + config.getLeaseNamespace().orElseGet( + () -> ConfigurationServiceProvider.instance().getClientConfiguration().getNamespace()); + if (leaseNamespace == null) { + final var message = + "Lease namespace is not set and cannot be inferred. Leader election cannot continue."; + log.error(message); + throw new IllegalArgumentException(message); + } + final var lock = new LeaseLock(leaseNamespace, config.getLeaseName(), identity); // releaseOnCancel is not used in the underlying implementation - leaderElector = new LeaderElectorBuilder(client, - ConfigurationServiceProvider.instance().getExecutorService()) - .withConfig( - new LeaderElectionConfig(lock, config.getLeaseDuration(), config.getRenewDeadline(), - config.getRetryPeriod(), leaderCallbacks(), true, config.getLeaseName())) - .build(); + leaderElector = + new LeaderElectorBuilder( + client, ConfigurationServiceProvider.instance().getExecutorService()) + .withConfig( + new LeaderElectionConfig( + lock, + config.getLeaseDuration(), + config.getRenewDeadline(), + config.getRetryPeriod(), + leaderCallbacks(), + true, + config.getLeaseName())) + .build(); } public boolean isLeaderElectionEnabled() { @@ -46,9 +61,12 @@ public boolean isLeaderElectionEnabled() { } private LeaderCallbacks leaderCallbacks() { - return new LeaderCallbacks(this::startLeading, this::stopLeading, leader -> { - log.info("New leader with identity: {}", leader); - }); + return new LeaderCallbacks( + this::startLeading, + this::stopLeading, + leader -> { + log.info("New leader with identity: {}", leader); + }); } private void startLeading() { @@ -64,7 +82,7 @@ private void stopLeading() { } private String identity(LeaderElectionConfiguration config) { - String id = config.getIdentity().orElse(System.getenv("HOSTNAME")); + var id = config.getIdentity().orElse(System.getenv("HOSTNAME")); if (id == null || id.isBlank()) { id = UUID.randomUUID().toString(); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfiguration.java index 5146fa6a1e..5a2c322657 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfiguration.java @@ -35,6 +35,15 @@ public LeaderElectionConfiguration(String leaseName, String leaseNamespace) { RETRY_PERIOD_DEFAULT_VALUE, null); } + public LeaderElectionConfiguration(String leaseName) { + this( + leaseName, + null, + LEASE_DURATION_DEFAULT_VALUE, + RENEW_DEADLINE_DEFAULT_VALUE, + RETRY_PERIOD_DEFAULT_VALUE, null); + } + public LeaderElectionConfiguration( String leaseName, String leaseNamespace, @@ -59,8 +68,8 @@ public LeaderElectionConfiguration( this.identity = identity; } - public String getLeaseNamespace() { - return leaseNamespace; + public Optional getLeaseNamespace() { + return Optional.ofNullable(leaseNamespace); } public String getLeaseName() { diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/LeaderElectionManagerTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/LeaderElectionManagerTest.java new file mode 100644 index 0000000000..65d78e02b3 --- /dev/null +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/LeaderElectionManagerTest.java @@ -0,0 +1,77 @@ +package io.javaoperatorsdk.operator; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import io.fabric8.kubernetes.client.KubernetesClient; +import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; +import io.javaoperatorsdk.operator.api.config.LeaderElectionConfiguration; + +import static io.fabric8.kubernetes.client.Config.KUBERNETES_AUTH_TRYKUBECONFIG_SYSTEM_PROPERTY; +import static io.fabric8.kubernetes.client.Config.KUBERNETES_NAMESPACE_FILE; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; + +class LeaderElectionManagerTest { + + private ControllerManager controllerManager; + private KubernetesClient kubernetesClient; + private LeaderElectionManager leaderElectionManager; + + @BeforeEach + void setUp() { + controllerManager = mock(ControllerManager.class); + kubernetesClient = mock(KubernetesClient.class); + leaderElectionManager = new LeaderElectionManager(controllerManager); + } + + @AfterEach + void tearDown() { + ConfigurationServiceProvider.reset(); + System.getProperties().remove(KUBERNETES_NAMESPACE_FILE); + System.getProperties().remove(KUBERNETES_AUTH_TRYKUBECONFIG_SYSTEM_PROPERTY); + } + + @Test + void testInit() { + leaderElectionManager.init(new LeaderElectionConfiguration("test", "testns"), kubernetesClient); + assertTrue(leaderElectionManager.isLeaderElectionEnabled()); + } + + @Test + void testInitInferLeaseNamespace(@TempDir Path tempDir) throws IOException { + var namespace = "foo"; + var namespacePath = tempDir.resolve("namespace"); + Files.writeString(namespacePath, namespace); + + System.setProperty(KUBERNETES_AUTH_TRYKUBECONFIG_SYSTEM_PROPERTY, "false"); + System.setProperty(KUBERNETES_NAMESPACE_FILE, namespacePath.toString()); + + leaderElectionManager.init(new LeaderElectionConfiguration("test"), kubernetesClient); + assertTrue(leaderElectionManager.isLeaderElectionEnabled()); + } + + @Test + void testFailedToInitInferLeaseNamespace() { + System.setProperty(KUBERNETES_AUTH_TRYKUBECONFIG_SYSTEM_PROPERTY, "false"); + assertThrows( + IllegalArgumentException.class, + () -> leaderElectionManager.init(new LeaderElectionConfiguration("test"), + kubernetesClient)); + } + + @Test + void testFailedToInitInferLeaseNamespaceProbablyUsingKubeConfig() { + assertThrows( + IllegalArgumentException.class, + () -> leaderElectionManager.init(new LeaderElectionConfiguration("test"), + kubernetesClient)); + } +} diff --git a/sample-operators/leader-election/k8s/namespace-inferred-operator-instance-2.yaml b/sample-operators/leader-election/k8s/namespace-inferred-operator-instance-2.yaml new file mode 100644 index 0000000000..b8f09681e7 --- /dev/null +++ b/sample-operators/leader-election/k8s/namespace-inferred-operator-instance-2.yaml @@ -0,0 +1,15 @@ +apiVersion: v1 +kind: Pod +metadata: + name: leader-election-operator-2 +spec: + serviceAccountName: leader-election-operator + containers: + - name: operator + image: leader-election-operator + imagePullPolicy: Never + env: + - name: POD_NAME + valueFrom: + fieldRef: + fieldPath: metadata.name diff --git a/sample-operators/leader-election/k8s/namespace-inferred-operator.yaml b/sample-operators/leader-election/k8s/namespace-inferred-operator.yaml new file mode 100644 index 0000000000..68e00d81ad --- /dev/null +++ b/sample-operators/leader-election/k8s/namespace-inferred-operator.yaml @@ -0,0 +1,60 @@ +apiVersion: v1 +kind: ServiceAccount +metadata: + name: leader-election-operator + +--- +apiVersion: v1 +kind: Pod +metadata: + name: leader-election-operator-1 +spec: + serviceAccountName: leader-election-operator + containers: + - name: operator + image: leader-election-operator + imagePullPolicy: Never + env: + - name: POD_NAME + valueFrom: + fieldRef: + fieldPath: metadata.name + +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + name: operator-admin +subjects: + - kind: ServiceAccount + name: leader-election-operator +roleRef: + kind: ClusterRole + name: leader-election-operator + apiGroup: "" + +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: leader-election-operator +rules: + - apiGroups: + - "apiextensions.k8s.io" + resources: + - customresourcedefinitions + verbs: + - '*' + - apiGroups: + - "sample.javaoperatorsdk" + resources: + - leaderelectiontestcustomresources + - leaderelectiontestcustomresources/status + verbs: + - '*' + - apiGroups: + - "coordination.k8s.io" + resources: + - "leases" + verbs: + - '*' diff --git a/sample-operators/leader-election/pom.xml b/sample-operators/leader-election/pom.xml index 1e8fadf645..99b0bef044 100644 --- a/sample-operators/leader-election/pom.xml +++ b/sample-operators/leader-election/pom.xml @@ -51,6 +51,11 @@ ${project.version} test + + org.junit.jupiter + junit-jupiter-params + test + @@ -75,4 +80,4 @@ - \ No newline at end of file + diff --git a/sample-operators/leader-election/src/main/java/io/javaoperatorsdk/operator/sample/LeaderElectionTestOperator.java b/sample-operators/leader-election/src/main/java/io/javaoperatorsdk/operator/sample/LeaderElectionTestOperator.java index 818fbe7ee1..6bb171cfd5 100644 --- a/sample-operators/leader-election/src/main/java/io/javaoperatorsdk/operator/sample/LeaderElectionTestOperator.java +++ b/sample-operators/leader-election/src/main/java/io/javaoperatorsdk/operator/sample/LeaderElectionTestOperator.java @@ -17,14 +17,17 @@ public static void main(String[] args) { log.info("Starting operator with identity: {}", identity); + LeaderElectionConfiguration leaderElectionConfiguration = + namespace == null + ? new LeaderElectionConfiguration("leader-election-test") + : new LeaderElectionConfiguration("leader-election-test", namespace, identity); + var client = new KubernetesClientBuilder().build(); - Operator operator = new Operator(client, - c -> c.withLeaderElectionConfiguration( - new LeaderElectionConfiguration("leader-election-test", namespace, identity))); + Operator operator = + new Operator(client, c -> c.withLeaderElectionConfiguration(leaderElectionConfiguration)); operator.register(new LeaderElectionTestReconciler(identity)); operator.installShutdownHook(); operator.start(); } - } diff --git a/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java b/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java index 4c78ce5d01..9b4eb736db 100644 --- a/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java +++ b/sample-operators/leader-election/src/test/java/io/javaoperatorsdk/operator/sample/LeaderElectionE2E.java @@ -13,8 +13,9 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; import org.junit.jupiter.api.condition.EnabledIfSystemProperty; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -46,14 +47,15 @@ class LeaderElectionE2E { private String namespace; private KubernetesClient client; - @Test + @ParameterizedTest + @ValueSource(strings = {"namespace-inferred-", ""}) // not for local mode by design @EnabledIfSystemProperty(named = "test.deployment", matches = "remote") - void otherInstancesTakesOverWhenSteppingDown() { + void otherInstancesTakesOverWhenSteppingDown(String yamlFilePrefix) { log.info("Applying custom resource"); applyCustomResource(); log.info("Deploying operator instances"); - deployOperatorsInOrder(); + deployOperatorsInOrder(yamlFilePrefix); log.info("Awaiting custom resource reconciliations"); await().pollDelay(Duration.ofSeconds(MINIMAL_SECONDS_FOR_RENEWAL)) @@ -130,16 +132,16 @@ void tearDown() { .untilAsserted(() -> assertThat(client.namespaces().withName(namespace).get()).isNull()); } - private void deployOperatorsInOrder() { + private void deployOperatorsInOrder(String yamlFilePrefix) { log.info("Installing 1st instance"); - applyResources("k8s/operator.yaml"); + applyResources("k8s/" + yamlFilePrefix + "operator.yaml"); await().atMost(Duration.ofSeconds(POD_STARTUP_TIMEOUT)).untilAsserted(() -> { var pod = client.pods().inNamespace(namespace).withName(OPERATOR_1_POD_NAME).get(); assertThat(pod.getStatus().getContainerStatuses().get(0).getReady()).isTrue(); }); log.info("Installing 2nd instance"); - applyResources("k8s/operator-instance-2.yaml"); + applyResources("k8s/" + yamlFilePrefix + "operator-instance-2.yaml"); await().atMost(Duration.ofSeconds(POD_STARTUP_TIMEOUT)).untilAsserted(() -> { var pod = client.pods().inNamespace(namespace).withName(OPERATOR_2_POD_NAME).get(); assertThat(pod.getStatus().getContainerStatuses().get(0).getReady()).isTrue();