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.resolveNodes(nodeDescriptions, true);
+
+        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(VotingConfigExclusion::new).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;
@@ -246,6 +246,11 @@ public VotingConfigExclusion(String nodeId, String nodeName) {
             this.nodeName = nodeName;
         }
 
+        public VotingConfigExclusion(String nodeIdAndName) {
+            this.nodeId = nodeIdAndName;
+            this.nodeName = nodeIdAndName;
+        }
+
         @Override
         public void writeTo(StreamOutput out) throws IOException {
             out.writeString(nodeId);

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

@@ -317,6 +317,24 @@ 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;
+        }
+    }
+
     /**
      * 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
@@ -326,60 +344,76 @@ public DiscoveryNode resolveNode(String node) {
      *   or a generic node attribute name in which case value will be treated as a wildcard and matched against the node attribute values.
      */
     public String[] resolveNodes(String... nodes) {
+        return resolveNodes(nodes, false).getResolvedNodes();
+    }
+
+    public NodeResolutionResults resolveNodes(String[] nodes, boolean returnUnresolved) {
         if (nodes == null || nodes.length == 0) {
-            return StreamSupport.stream(this.spliterator(), false).map(DiscoveryNode::getId).toArray(String[]::new);
+            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);
-            for (String nodeId : nodes) {
-                if (nodeId.equals("_local")) {
+            ObjectHashSet<String> unresolvedNodesIds = new ObjectHashSet<>();
+            boolean nodeIdResolved = false;
+
+            for (String nodeIdToBeProcessed : nodes) {
+                if (nodeIdToBeProcessed.equals("_local")) {
                     String localNodeId = getLocalNodeId();
                     if (localNodeId != null) {
                         resolvedNodesIds.add(localNodeId);
+                        nodeIdResolved = true;
                     }
-                } else if (nodeId.equals("_master")) {
+                } else if (nodeIdToBeProcessed.equals("_master")) {
                     String masterNodeId = getMasterNodeId();
                     if (masterNodeId != null) {
                         resolvedNodesIds.add(masterNodeId);
+                        nodeIdResolved = true;
                     }
-                } else if (nodeExists(nodeId)) {
-                    resolvedNodesIds.add(nodeId);
+                } else if (nodeExists(nodeIdToBeProcessed)) {
+                    resolvedNodesIds.add(nodeIdToBeProcessed);
+                    nodeIdResolved = true;
                 } else {
                     for (DiscoveryNode node : this) {
-                        if ("_all".equals(nodeId)
-                                || Regex.simpleMatch(nodeId, node.getName())
-                                || Regex.simpleMatch(nodeId, node.getHostAddress())
-                                || Regex.simpleMatch(nodeId, node.getHostName())) {
+                        if ("_all".equals(nodeIdToBeProcessed)
+                            || Regex.simpleMatch(nodeIdToBeProcessed, node.getName())
+                            || Regex.simpleMatch(nodeIdToBeProcessed, node.getHostAddress())
+                            || Regex.simpleMatch(nodeIdToBeProcessed, node.getHostName())) {
                             resolvedNodesIds.add(node.getId());
+                            nodeIdResolved = true;
                         }
                     }
-                    int index = nodeId.indexOf(':');
+                    int index = nodeIdToBeProcessed.indexOf(':');
                     if (index != -1) {
-                        String matchAttrName = nodeId.substring(0, index);
-                        String matchAttrValue = nodeId.substring(index + 1);
+                        String matchAttrName = nodeIdToBeProcessed.substring(0, index);
+                        String matchAttrValue = nodeIdToBeProcessed.substring(index + 1);
                         if (DiscoveryNodeRole.DATA_ROLE.roleName().equals(matchAttrName)) {
                             if (Booleans.parseBoolean(matchAttrValue, true)) {
                                 resolvedNodesIds.addAll(dataNodes.keys());
                             } else {
                                 resolvedNodesIds.removeAll(dataNodes.keys());
                             }
+                            nodeIdResolved = true;
                         } else if (DiscoveryNodeRole.MASTER_ROLE.roleName().equals(matchAttrName)) {
                             if (Booleans.parseBoolean(matchAttrValue, true)) {
                                 resolvedNodesIds.addAll(masterNodes.keys());
                             } else {
                                 resolvedNodesIds.removeAll(masterNodes.keys());
                             }
+                            nodeIdResolved = true;
                         } else if (DiscoveryNodeRole.INGEST_ROLE.roleName().equals(matchAttrName)) {
                             if (Booleans.parseBoolean(matchAttrValue, true)) {
                                 resolvedNodesIds.addAll(ingestNodes.keys());
                             } else {
                                 resolvedNodesIds.removeAll(ingestNodes.keys());
                             }
+                            nodeIdResolved = true;
                         } else if (DiscoveryNode.COORDINATING_ONLY.equals(matchAttrName)) {
                             if (Booleans.parseBoolean(matchAttrValue, true)) {
                                 resolvedNodesIds.addAll(getCoordinatingOnlyNodes().keys());
                             } else {
                                 resolvedNodesIds.removeAll(getCoordinatingOnlyNodes().keys());
                             }
+                            nodeIdResolved = true;
                         } else {
                             for (DiscoveryNode node : this) {
                                 for (DiscoveryNodeRole role : Sets.difference(node.getRoles(), DiscoveryNodeRole.BUILT_IN_ROLES)) {
@@ -389,6 +423,7 @@ public String[] resolveNodes(String... nodes) {
                                         } else {
                                             resolvedNodesIds.remove(node.getId());
                                         }
+                                        nodeIdResolved = true;
                                     }
                                 }
                             }
@@ -398,14 +433,19 @@ public String[] resolveNodes(String... nodes) {
                                     String attrValue = entry.getValue();
                                     if (Regex.simpleMatch(matchAttrName, attrName) && Regex.simpleMatch(matchAttrValue, attrValue)) {
                                         resolvedNodesIds.add(node.getId());
+                                        nodeIdResolved = true;
                                     }
                                 }
                             }
                         }
                     }
                 }
+
+                if(!nodeIdResolved) {
+                    unresolvedNodesIds.add(nodeIdToBeProcessed);
+                }
             }
-            return resolvedNodesIds.toArray(String.class);
+            return new NodeResolutionResults(resolvedNodesIds.toArray(String.class), unresolvedNodesIds.toArray(String.class));
         }
     }
 

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

@@ -93,10 +93,6 @@ public void testResolve() {
                 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() {

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

@@ -243,42 +243,19 @@ public void testReturnsImmediatelyIfVoteAlreadyWithdrawn() throws InterruptedExc
                 contains(otherNode1Exclusion));
     }
 
-    public void testReturnsErrorIfNoMatchingNodes() 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[]{"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 {
-        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")),
+                    clusterService.getClusterApplierService().state().getVotingConfigExclusions());
     }
 
     public void testSucceedsEvenIfAllExclusionsAlreadyAdded() throws InterruptedException {