Voting config exclusions should work with absent nodes

server/src/main/java/org/elasticsearch/action/admin/cluster/configuration/AddVotingConfigExclusionsRequest.java

@@ -27,6 +27,8 @@
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.unit.TimeValue;
+import org.elasticsearch.common.util.iterable.Iterables;
+import org.elasticsearch.common.util.set.Sets;
 
 import java.io.IOException;
 import java.util.Arrays;
@@ -72,16 +74,22 @@ public AddVotingConfigExclusionsRequest(StreamInput in) throws IOException {
 
     Set<VotingConfigExclusion> resolveVotingConfigExclusions(ClusterState currentState) {
         final DiscoveryNodes allNodes = currentState.nodes();
-        final Set<VotingConfigExclusion> resolvedNodes = Arrays.stream(allNodes.resolveNodes(nodeDescriptions))
+        final DiscoveryNodes.NodeResolutionResults nodeResolutionResults = allNodes.resolveNodesExact(nodeDescriptions);
+
+        final Set<VotingConfigExclusion> resolvedNodes = Arrays.stream(nodeResolutionResults.getResolvedNodes())
                 .map(allNodes::get).filter(DiscoveryNode::isMasterNode).map(VotingConfigExclusion::new).collect(Collectors.toSet());
+        final Set<VotingConfigExclusion> unresolvedNodes = Arrays.stream(nodeResolutionResults.getUnresolvedNodes())
+                .map(nodeIdOrName -> new VotingConfigExclusion(nodeIdOrName, nodeIdOrName)).collect(Collectors.toSet());
+
+        Set<VotingConfigExclusion> allProcessedNodes = Sets.newHashSet(Iterables.concat(resolvedNodes, unresolvedNodes));
 
-        if (resolvedNodes.isEmpty()) {
+        if (allProcessedNodes.isEmpty()) {
             throw new IllegalArgumentException("add voting config exclusions request for " + Arrays.asList(nodeDescriptions)
-                + " matched no master-eligible nodes");
+                + " matched no master-eligible nodes or absent nodes");
         }
 
-        resolvedNodes.removeIf(n -> currentState.getVotingConfigExclusions().contains(n));
-        return resolvedNodes;
+        allProcessedNodes.removeIf(n -> currentState.getVotingConfigExclusions().contains(n));
+        return allProcessedNodes;
     }
 
     Set<VotingConfigExclusion> resolveVotingConfigExclusionsAndCheckMaximum(ClusterState currentState, int maxExclusionsCount,

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

@@ -190,7 +190,7 @@ public static class Builder {
         public Builder() {
 
         }
-        
+
         public Builder(CoordinationMetaData state) {
             this.term = state.term;
             this.lastCommittedConfiguration = state.lastCommittedConfiguration;
@@ -233,7 +233,8 @@ public static class VotingConfigExclusion implements Writeable, ToXContentFragme
         private final String nodeName;
 
         public VotingConfigExclusion(DiscoveryNode node) {
-            this(node.getId(), node.getName());
+            this.nodeId = node.getId();
+            this.nodeName = node.getName();
         }
 
         public VotingConfigExclusion(StreamInput in) throws IOException {

server/src/main/java/org/elasticsearch/cluster/node/DiscoveryNodes.java

@@ -38,6 +38,7 @@
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
@@ -317,6 +318,54 @@ public DiscoveryNode resolveNode(String node) {
         return nodes.get(resolvedNodeIds[0]);
     }
 
+    public static class NodeResolutionResults {
+        private String[] resolvedNodes;
+        private String[] unresolvedNodes;
+
+        public NodeResolutionResults(String[] resolvedNodes, String[] unresolvedNodes) {
+            this.resolvedNodes = resolvedNodes;
+            this.unresolvedNodes = unresolvedNodes;
+        }
+
+        public String[] getResolvedNodes() {
+            return resolvedNodes;
+        }
+
+        public String[] getUnresolvedNodes() {
+            return unresolvedNodes;
+        }
+    }
+
+    public NodeResolutionResults resolveNodesExact(String... nodes) {
+        if (nodes == null || nodes.length == 0) {
+            return new NodeResolutionResults(StreamSupport.stream(this.spliterator(), false)
+                                            .map(DiscoveryNode::getId).toArray(String[]::new), new String[0]);
+        } else {
+            ObjectHashSet<String> resolvedNodesIds = new ObjectHashSet<>(nodes.length);
+            ObjectHashSet<String> unresolvedNodesIds = new ObjectHashSet<>();
+
+            Map<String, String> existingNodesNameId = new HashMap<>();
+            for (DiscoveryNode node : this) {
+                existingNodesNameId.put(node.getName(), node.getId());
+            }
+
+            for (String nodeToBeProcessed : nodes) {
+                if (nodeExists(nodeToBeProcessed)) {
+                    resolvedNodesIds.add(nodeToBeProcessed);
+                }
+                else if (existingNodesNameId.containsKey(nodeToBeProcessed)){
+                    resolvedNodesIds.add(existingNodesNameId.get(nodeToBeProcessed));
+                }
+                else {
+                    unresolvedNodesIds.add(nodeToBeProcessed);
+                }
+            }
+
+            return new NodeResolutionResults(resolvedNodesIds.toArray(String.class), unresolvedNodesIds.toArray(String.class));
+        }
+    }
+
+
     /**
      * resolves a set of node "descriptions" to concrete and existing node ids. "descriptions" can be (resolved in this order):
      * - "_local" or "_master" for the relevant nodes
@@ -330,6 +379,7 @@ public String[] resolveNodes(String... nodes) {
             return StreamSupport.stream(this.spliterator(), false).map(DiscoveryNode::getId).toArray(String[]::new);
         } else {
             ObjectHashSet<String> resolvedNodesIds = new ObjectHashSet<>(nodes.length);
+
             for (String nodeId : nodes) {
                 if (nodeId.equals("_local")) {
                     String localNodeId = getLocalNodeId();
@@ -346,9 +396,9 @@ public String[] resolveNodes(String... nodes) {
                 } else {
                     for (DiscoveryNode node : this) {
                         if ("_all".equals(nodeId)
-                                || Regex.simpleMatch(nodeId, node.getName())
-                                || Regex.simpleMatch(nodeId, node.getHostAddress())
-                                || Regex.simpleMatch(nodeId, node.getHostName())) {
+                            || Regex.simpleMatch(nodeId, node.getName())
+                            || Regex.simpleMatch(nodeId, node.getHostAddress())
+                            || Regex.simpleMatch(nodeId, node.getHostName())) {
                             resolvedNodesIds.add(node.getId());
                         }
                     }

server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestAddVotingConfigExclusionAction.java

@@ -36,7 +36,11 @@ public class RestAddVotingConfigExclusionAction extends BaseRestHandler {
     private static final TimeValue DEFAULT_TIMEOUT = TimeValue.timeValueSeconds(30L);
 
     public RestAddVotingConfigExclusionAction(RestController controller) {
+        // TODO This API is being deprecated.
         controller.registerHandler(RestRequest.Method.POST, "/_cluster/voting_config_exclusions/{node_name}", this);
+
+        controller.registerHandler(RestRequest.Method.POST,
+            "/_cluster/voting_config_exclusions/node_ids_or_names/{node_id_or_names}", this);
     }
 
     @Override
@@ -55,9 +59,20 @@ protected RestChannelConsumer prepareRequest(final RestRequest request, final No
     }
 
     AddVotingConfigExclusionsRequest resolveVotingConfigExclusionsRequest(final RestRequest request) {
-        String nodeName = request.param("node_name");
+        String nodeDescriptions;
+
+        // TODO This request param is being deprecated
+        if (request.hasParam("node_name")) {
+            nodeDescriptions = request.param("node_name");
+        }
+        else {
+            nodeDescriptions = request.param("node_ids_or_names");
+        }
+
+        assert !Strings.isNullOrEmpty(nodeDescriptions);
+
         return new AddVotingConfigExclusionsRequest(
-            Strings.splitStringByCommaToArray(nodeName),
+            Strings.splitStringByCommaToArray(nodeDescriptions),
             TimeValue.parseTimeValue(request.param("timeout"), DEFAULT_TIMEOUT, getClass().getSimpleName() + ".timeout")
         );
     }

server/src/test/java/org/elasticsearch/action/admin/cluster/configuration/AddVotingConfigExclusionsRequestTests.java

@@ -35,7 +35,6 @@
 
 import static java.util.Collections.emptyMap;
 import static java.util.Collections.emptySet;
-import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.equalTo;
 
@@ -87,16 +86,6 @@ public void testResolve() {
 
         assertThat(makeRequest().resolveVotingConfigExclusions(clusterState),
                 containsInAnyOrder(localNodeExclusion, otherNode1Exclusion, otherNode2Exclusion));
-        assertThat(makeRequest("_all").resolveVotingConfigExclusions(clusterState),
-                containsInAnyOrder(localNodeExclusion, otherNode1Exclusion, otherNode2Exclusion));
-        assertThat(makeRequest("_local").resolveVotingConfigExclusions(clusterState),
-                contains(localNodeExclusion));
-        assertThat(makeRequest("other*").resolveVotingConfigExclusions(clusterState),
-                containsInAnyOrder(otherNode1Exclusion, otherNode2Exclusion));
-
-        assertThat(expectThrows(IllegalArgumentException.class,
-                () -> makeRequest("not-a-node").resolveVotingConfigExclusions(clusterState)).getMessage(),
-                    equalTo("add voting config exclusions request for [not-a-node] matched no master-eligible nodes"));
     }
 
     public void testResolveAndCheckMaximum() {
@@ -133,8 +122,6 @@ public void testResolveAndCheckMaximum() {
 
         assertThat(makeRequest().resolveVotingConfigExclusionsAndCheckMaximum(clusterState, 3, "setting.name"),
                 containsInAnyOrder(localNodeExclusion, otherNode2Exclusion));
-        assertThat(makeRequest("_local").resolveVotingConfigExclusionsAndCheckMaximum(clusterState, 2, "setting.name"),
-                contains(localNodeExclusion));
 
         assertThat(expectThrows(IllegalArgumentException.class,
             () -> makeRequest().resolveVotingConfigExclusionsAndCheckMaximum(clusterState, 2, "setting.name")).getMessage(),

server/src/test/java/org/elasticsearch/action/admin/cluster/configuration/TransportAddVotingConfigExclusionsActionTests.java

@@ -166,58 +166,6 @@ public void testWithdrawsVotesFromMultipleNodes() throws InterruptedException {
         assertThat(clusterService.getClusterApplierService().state().getVotingConfigExclusions(),
                 containsInAnyOrder(otherNode1Exclusion, otherNode2Exclusion));
     }
-
-    public void testWithdrawsVotesFromNodesMatchingWildcard() throws InterruptedException {
-        final CountDownLatch countDownLatch = new CountDownLatch(1);
-
-        clusterStateObserver.waitForNextChange(new AdjustConfigurationForExclusions());
-        transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME,
-            new AddVotingConfigExclusionsRequest(new String[]{"other*"}),
-            expectSuccess(r -> {
-                assertNotNull(r);
-                countDownLatch.countDown();
-            })
-        );
-
-        assertTrue(countDownLatch.await(30, TimeUnit.SECONDS));
-        assertThat(clusterService.getClusterApplierService().state().getVotingConfigExclusions(),
-                containsInAnyOrder(otherNode1Exclusion, otherNode2Exclusion));
-    }
-
-    public void testWithdrawsVotesFromAllMasterEligibleNodes() throws InterruptedException {
-        final CountDownLatch countDownLatch = new CountDownLatch(1);
-
-        clusterStateObserver.waitForNextChange(new AdjustConfigurationForExclusions());
-        transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME,
-            new AddVotingConfigExclusionsRequest(new String[]{"_all"}),
-            expectSuccess(r -> {
-                assertNotNull(r);
-                countDownLatch.countDown();
-            })
-        );
-
-        assertTrue(countDownLatch.await(30, TimeUnit.SECONDS));
-        assertThat(clusterService.getClusterApplierService().state().getVotingConfigExclusions(),
-                containsInAnyOrder(localNodeExclusion, otherNode1Exclusion, otherNode2Exclusion));
-    }
-
-    public void testWithdrawsVoteFromLocalNode() throws InterruptedException {
-        final CountDownLatch countDownLatch = new CountDownLatch(1);
-
-        clusterStateObserver.waitForNextChange(new AdjustConfigurationForExclusions());
-        transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME,
-            new AddVotingConfigExclusionsRequest(new String[]{"_local"}),
-            expectSuccess(r -> {
-                assertNotNull(r);
-                countDownLatch.countDown();
-            })
-        );
-
-        assertTrue(countDownLatch.await(30, TimeUnit.SECONDS));
-        assertThat(clusterService.getClusterApplierService().state().getVotingConfigExclusions(),
-                contains(localNodeExclusion));
-    }
-
     public void testReturnsImmediatelyIfVoteAlreadyWithdrawn() throws InterruptedException {
         final ClusterState state = clusterService.state();
         setState(clusterService, builder(state)
@@ -243,42 +191,19 @@ public void testReturnsImmediatelyIfVoteAlreadyWithdrawn() throws InterruptedExc
                 contains(otherNode1Exclusion));
     }
 
-    public void testReturnsErrorIfNoMatchingNodes() throws InterruptedException {
-        final CountDownLatch countDownLatch = new CountDownLatch(1);
-        final SetOnce<TransportException> exceptionHolder = new SetOnce<>();
-
-        transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME,
-            new AddVotingConfigExclusionsRequest(new String[]{"not-a-node"}),
-            expectError(e -> {
-                exceptionHolder.set(e);
-                countDownLatch.countDown();
-            })
-        );
-
-        assertTrue(countDownLatch.await(30, TimeUnit.SECONDS));
-        final Throwable rootCause = exceptionHolder.get().getRootCause();
-        assertThat(rootCause, instanceOf(IllegalArgumentException.class));
-        assertThat(rootCause.getMessage(),
-            equalTo("add voting config exclusions request for [not-a-node] matched no master-eligible nodes"));
-    }
-
-    public void testOnlyMatchesMasterEligibleNodes() throws InterruptedException {
+    public void testMatchesAbsentNodes() throws InterruptedException {
         final CountDownLatch countDownLatch = new CountDownLatch(1);
-        final SetOnce<TransportException> exceptionHolder = new SetOnce<>();
 
         transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME,
-            new AddVotingConfigExclusionsRequest(new String[]{"_all", "master:false"}),
-            expectError(e -> {
-                exceptionHolder.set(e);
+            new AddVotingConfigExclusionsRequest(new String[]{"absent_node"}),
+            expectSuccess(e -> {
                 countDownLatch.countDown();
             })
         );
 
         assertTrue(countDownLatch.await(30, TimeUnit.SECONDS));
-        final Throwable rootCause = exceptionHolder.get().getRootCause();
-        assertThat(rootCause, instanceOf(IllegalArgumentException.class));
-        assertThat(rootCause.getMessage(),
-            equalTo("add voting config exclusions request for [_all, master:false] matched no master-eligible nodes"));
+        assertEquals(Set.of(new VotingConfigExclusion("absent_node", "absent_node")),
+                    clusterService.getClusterApplierService().state().getVotingConfigExclusions());
     }
 
     public void testSucceedsEvenIfAllExclusionsAlreadyAdded() throws InterruptedException {

server/src/test/java/org/elasticsearch/rest/action/admin/cluster/RestAddVotingConfigExclusionActionTests.java

@@ -50,4 +50,19 @@ public void testResolveVotingConfigExclusionsRequest() {
         String[] expected = {"node-1","node-2", "node-3"};
         assertArrayEquals(expected, addVotingConfigExclusionsRequest.getNodeDescriptions());
     }
+
+    public void testResolveVotingConfigExclusionsRequestNodeIdsOrNames() {
+        Map<String, String> params = new HashMap<>();
+        params.put("node_ids_or_names", "node-1,node-2,node-3");
+        RestRequest deprecatedRequest = new FakeRestRequest.Builder(xContentRegistry())
+            .withMethod(RestRequest.Method.PUT)
+            .withPath("/_cluster/voting_config_exclusions/node_ids_or_names")
+            .withParams(params)
+            .build();
+
+        AddVotingConfigExclusionsRequest addVotingConfigExclusionsRequest = action.resolveVotingConfigExclusionsRequest(deprecatedRequest);
+        String[] expected = {"node-1","node-2", "node-3"};
+        assertArrayEquals(expected, addVotingConfigExclusionsRequest.getNodeDescriptions());
+    }
+
 }