SOLR-17550: preferred Overseer might not work

changelog/unreleased/SOLR-17550.yml

@@ -0,0 +1,9 @@
+title: "SOLR-17550: Fixed preferred Overseer ADDROLE message to use the correct node key"
+type: fixed
+authors:
+  - name: r4mercur
+links:
+  - name: SOLR-17550
+    url: https://issues.apache.org/jira/browse/SOLR-17550
+  - name: PR#4175
+    url: https://github.com/apache/solr/pull/4175

solr/core/src/java/org/apache/solr/cloud/ZkController.java

@@ -2609,7 +2609,7 @@ public void setPreferredOverseer() throws KeeperException, InterruptedException
     MapWriter props =
         ew ->
             ew.put(Overseer.QUEUE_OPERATION, ADDROLE.toString().toLowerCase(Locale.ROOT))
-                .put(getNodeName(), getNodeName())
+                .put("node", getNodeName())
                 .put("role", "overseer")
                 .put("persist", "false");
     log.warn(

solr/core/src/test/org/apache/solr/cloud/OverseerRolesTest.java

@@ -224,4 +224,43 @@ public void testDesignatedOverseerRestarts() throws Exception {
     // assert that there is just a single leadership transition
     waitForNewOverseer(15, overseer1, true);
   }
+
+  @Test
+  public void testSetPreferredOverseerIntegration() throws Exception {
+    final String collection = "preferred_overseer_" + System.nanoTime();
+    ZkController zkController = cluster.getJettySolrRunner(0).getCoreContainer().getZkController();
+
+    String oldLeader = OverseerCollectionConfigSetProcessor.getLeaderNode(zkClient());
+    assertNotNull("Expected current overseer leader", oldLeader);
+
+    zkController.setPreferredOverseer();
+
+    String leaderId = OverseerCollectionConfigSetProcessor.getLeaderId(zkClient());
+    assertNotNull("Expected current overseer leader id", leaderId);
+
+    Objects.requireNonNull(getOverseerJetty())
+        .getCoreContainer()
+        .getZkController()
+        .getOverseer()
+        .sendQuitToOverseer(leaderId);
+
+    waitForNewOverseer(15, s -> s != null && !Objects.equals(s, oldLeader), false);
+
+    try {
+      CollectionAdminRequest.createCollection(collection, "conf", 1, 1)
+          .process(cluster.getSolrClient());
+      cluster.waitForActiveCollection(collection, 1, 1);
+
+      assertTrue(
+          "Collection should exist after setPreferredOverseer() was called",
+          zkController.getZkStateReader().getClusterState().hasCollection(collection));
+    } finally {
+      try {
+        if (zkController.getZkStateReader().getClusterState().hasCollection(collection)) {
+          CollectionAdminRequest.deleteCollection(collection).process(cluster.getSolrClient());
+        }
+      } catch (Exception ignored) {
+      }
+    }
+  }
 }

solr/core/src/test/org/apache/solr/cloud/ZkControllerTest.java

@@ -32,6 +32,7 @@
 import java.util.Map;
 import java.util.Optional;
 import java.util.Properties;
+import java.util.Set;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.TimeUnit;
@@ -767,6 +768,35 @@ public void testOverseerEnabledClusterPropertyTrue() throws Exception {
     }
   }
 
+  @Test
+  public void testSetPreferredOverseerHasRightKeysSet() throws Exception {
+    Path zkDir = createTempDir("testSetPreferredOverseerHasRightKeysSet");
+    ZkTestServer server = new ZkTestServer(zkDir);
+    try {
+      server.run();
+      CoreContainer cc = getCoreContainer();
+      CloudConfig cloudConfig = new CloudConfig.CloudConfigBuilder("127.0.0.1", 8983).build();
+      try (ZkController zkController =
+          new ZkController(cc, server.getZkAddress(), TIMEOUT, cloudConfig)) {
+        zkController.setPreferredOverseer();
+        byte[] messageBytes = zkController.getOverseerCollectionQueue().peek();
+
+        @SuppressWarnings("unchecked")
+        Map<String, Object> message = (Map<String, Object>) Utils.fromJSON(messageBytes);
+
+        assertEquals(Set.of(Overseer.QUEUE_OPERATION, "node", "role", "persist"), message.keySet());
+        assertEquals("addrole", message.get(Overseer.QUEUE_OPERATION));
+        assertEquals(zkController.getNodeName(), message.get("node"));
+        assertEquals("overseer", message.get("role"));
+        assertEquals("false", message.get("persist"));
+      } finally {
+        cc.shutdown();
+      }
+    } finally {
+      server.shutdown();
+    }
+  }
+
   private CoreContainer getCoreContainer() {
     return new MockCoreContainer();
   }