Do not include any extra nodes in the bootstrap configuration

Author: DaveCTurner

server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java

@@ -34,6 +34,7 @@
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.function.Consumer;
@@ -84,18 +85,16 @@ void onFoundPeersUpdated() {
         if (transportService.getLocalNode().isMasterNode() && initialMasterNodes.isEmpty() == false
             && nodes.stream().noneMatch(Coordinator::isZen1Node)) {
 
-            final boolean waitRequirementsPassed;
+            final Optional<Set<DiscoveryNode>> nodesMatchingRequirements;
             try {
-                waitRequirementsPassed = checkWaitRequirements(nodes);
+                nodesMatchingRequirements = getNodesMatchingRequirements(nodes);
             } catch (IllegalStateException e) {
                 logger.warn("bootstrapping cancelled", e);
                 bootstrappingPermitted.set(false);
                 return;
             }
 
-            if (waitRequirementsPassed) {
-                startBootstrap(nodes);
-            }
+            nodesMatchingRequirements.ifPresent(this::startBootstrap);
         }
     }
 
@@ -160,14 +159,14 @@ private static boolean matchesRequirement(DiscoveryNode discoveryNode, String re
             || discoveryNode.getAddress().getAddress().equals(requirement);
     }
 
-    private boolean checkWaitRequirements(Set<DiscoveryNode> nodes) {
+    private Optional<Set<DiscoveryNode>> getNodesMatchingRequirements(Set<DiscoveryNode> nodes) {
         final Set<DiscoveryNode> selectedNodes = new HashSet<>();
         for (final String requirement : initialMasterNodes) {
             final Set<DiscoveryNode> matchingNodes
                 = nodes.stream().filter(n -> matchesRequirement(n, requirement)).collect(Collectors.toSet());
 
             if (matchingNodes.isEmpty()) {
-                return false;
+                return Optional.empty();
             }
             if (matchingNodes.size() > 1) {
                 throw new IllegalStateException("requirement [" + requirement + "] matches multiple nodes: " + matchingNodes);
@@ -181,6 +180,6 @@ private boolean checkWaitRequirements(Set<DiscoveryNode> nodes) {
             }
         }
 
-        return true;
+        return Optional.of(Collections.unmodifiableSet(selectedNodes));
     }
 }

server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java

@@ -48,7 +48,9 @@
 import static org.elasticsearch.node.Node.NODE_NAME_SETTING;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.greaterThanOrEqualTo;
+import static org.hamcrest.Matchers.hasItem;
 import static org.hamcrest.Matchers.hasSize;
+import static org.hamcrest.Matchers.not;
 
 public class ClusterBootstrapServiceTests extends ESTestCase {
 
@@ -329,4 +331,19 @@ public void testDoesNotJustMatchEverything() {
         clusterBootstrapService.onFoundPeersUpdated();
         deterministicTaskQueue.runAllTasks();
     }
-}
+
+    public void testDoesNotIncludeExtraNodes() {
+        final DiscoveryNode extraNode = newDiscoveryNode("extra-node");
+        final AtomicBoolean bootstrapped = new AtomicBoolean();
+        ClusterBootstrapService clusterBootstrapService = new ClusterBootstrapService(Settings.builder().putList(
+            INITIAL_MASTER_NODES_SETTING.getKey(), localNode.getName(), otherNode1.getName(), otherNode2.getName()).build(),
+            transportService, () -> Stream.of(otherNode1, otherNode2, extraNode).collect(Collectors.toList()), vc -> {
+            assertTrue(bootstrapped.compareAndSet(false, true));
+            assertThat(vc.getNodeIds(), not(hasItem(extraNode.getId())));
+        });
+
+        transportService.start();
+        clusterBootstrapService.onFoundPeersUpdated();
+        deterministicTaskQueue.runAllTasks();
+        assertTrue(bootstrapped.get());
+    }}