Reject duplicate requirements early

DaveCTurner · DaveCTurner · commit eadc79797854 · 2019-01-21T15:55:27.000Z
diff --git a/server/src/main/java/org/elasticsearch/cluster/coordination/ClusterBootstrapService.java b/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.LinkedHashSet;
 import java.util.List;
 import java.util.Random;
 import java.util.Set;
@@ -61,7 +62,7 @@ public class ClusterBootstrapService {
     static final String BOOTSTRAP_PLACEHOLDER_PREFIX = "{bootstrap-placeholder}";
 
     private static final Logger logger = LogManager.getLogger(ClusterBootstrapService.class);
-    private final List<String> initialMasterNodes;
+    private final Set<String> initialMasterNodes;
     @Nullable // null if discoveryIsConfigured()
     private final TimeValue unconfiguredBootstrapTimeout;
     private final TransportService transportService;
@@ -74,7 +75,13 @@ public class ClusterBootstrapService {
     public ClusterBootstrapService(Settings settings, TransportService transportService, Random random,
                                    Supplier<Iterable<DiscoveryNode>> discoveredNodesSupplier, BooleanSupplier isBootstrappedSupplier,
                                    Consumer<VotingConfiguration> votingConfigurationConsumer) {
-        initialMasterNodes = INITIAL_MASTER_NODES_SETTING.get(settings);
+        final List<String> initialMasterNodesList = INITIAL_MASTER_NODES_SETTING.get(settings);
+        this.initialMasterNodes = new LinkedHashSet<>(initialMasterNodesList);
+        if (this.initialMasterNodes.size() != initialMasterNodesList.size()) {
+            throw new IllegalArgumentException(
+                "setting [" + INITIAL_MASTER_NODES_SETTING.getKey() + "] contains duplicates: " + initialMasterNodesList);
+        }
+
         unconfiguredBootstrapTimeout = discoveryIsConfigured(settings) ? null : UNCONFIGURED_BOOTSTRAP_TIMEOUT_SETTING.get(settings);
         this.transportService = transportService;
         this.random = random;
diff --git a/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java b/server/src/test/java/org/elasticsearch/cluster/coordination/ClusterBootstrapServiceTests.java
@@ -164,6 +164,20 @@ public void testDoesNothingByDefaultIfZen1NodesDiscovered() {
         deterministicTaskQueue.runAllTasks();
     }
 
+
+    public void testThrowsExceptionOnDuplicates() {
+        final IllegalArgumentException illegalArgumentException = expectThrows(IllegalArgumentException.class, () -> {
+            new ClusterBootstrapService(builder().putList(
+                INITIAL_MASTER_NODES_SETTING.getKey(), "duplicate-requirement", "duplicate-requirement").build(),
+                transportService, random(), Collections::emptyList, () -> false, vc -> {
+                throw new AssertionError("should not be called");
+            });
+        });
+
+        assertThat(illegalArgumentException.getMessage(), containsString(INITIAL_MASTER_NODES_SETTING.getKey()));
+        assertThat(illegalArgumentException.getMessage(), containsString("duplicate-requirement"));
+    }
+
     public void testBootstrapsOnDiscoveryOfAllRequiredNodes() {
         final AtomicBoolean bootstrapped = new AtomicBoolean();
 
