Try to get lease namespace if unspecified (#1450)

Author: honnix

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,24 +30,43 @@ 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() {
     return leaderElector != null;
   }
 
   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();
     }

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<String> getLeaseNamespace() {
+    return Optional.ofNullable(leaseNamespace);
   }
 
   public String getLeaseName() {

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));
+  }
+}

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

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:
+      - '*'

sample-operators/leader-election/pom.xml

@@ -51,6 +51,11 @@
             <version>${project.version}</version>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.junit.jupiter</groupId>
+            <artifactId>junit-jupiter-params</artifactId>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
     <build>
         <plugins>
@@ -75,4 +80,4 @@
         </plugins>
     </build>
 
-</project>
+</project>

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();
   }
-
 }

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();