Voting config exclusions should work with absent nodes

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

@@ -18,15 +18,19 @@
  */
 package org.elasticsearch.action.admin.cluster.configuration;
 
+import org.elasticsearch.Version;
 import org.elasticsearch.action.ActionRequestValidationException;
 import org.elasticsearch.action.support.master.MasterNodeRequest;
 import org.elasticsearch.cluster.ClusterState;
 import org.elasticsearch.cluster.coordination.CoordinationMetaData.VotingConfigExclusion;
 import org.elasticsearch.cluster.node.DiscoveryNode;
 import org.elasticsearch.cluster.node.DiscoveryNodes;
+import org.elasticsearch.common.Strings;
 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;
@@ -39,6 +43,8 @@
  */
 public class AddVotingConfigExclusionsRequest extends MasterNodeRequest<AddVotingConfigExclusionsRequest> {
     private final String[] nodeDescriptions;
+    private String[] nodeIds = null;
+    private String[] nodeNames = null;
     private final TimeValue timeout;
 
     /**
@@ -47,7 +53,7 @@ public class AddVotingConfigExclusionsRequest extends MasterNodeRequest<AddVotin
      * @param nodeDescriptions Descriptions of the nodes to add - see {@link DiscoveryNodes#resolveNodes(String...)}
      */
     public AddVotingConfigExclusionsRequest(String[] nodeDescriptions) {
-        this(nodeDescriptions, TimeValue.timeValueSeconds(30));
+        this(nodeDescriptions, Strings.EMPTY_ARRAY, Strings.EMPTY_ARRAY, TimeValue.timeValueSeconds(30));
     }
 
     /**
@@ -56,32 +62,72 @@ public AddVotingConfigExclusionsRequest(String[] nodeDescriptions) {
      * @param nodeDescriptions Descriptions of the nodes whose exclusions to add - see {@link DiscoveryNodes#resolveNodes(String...)}.
      * @param timeout How long to wait for the added exclusions to take effect and be removed from the voting configuration.
      */
-    public AddVotingConfigExclusionsRequest(String[] nodeDescriptions, TimeValue timeout) {
+    public AddVotingConfigExclusionsRequest(String[] nodeDescriptions, String[] nodeIds, String[] nodeNames, TimeValue timeout) {
         if (timeout.compareTo(TimeValue.ZERO) < 0) {
             throw new IllegalArgumentException("timeout [" + timeout + "] must be non-negative");
         }
         this.nodeDescriptions = nodeDescriptions;
+        this.nodeIds = nodeIds;
+        this.nodeNames = nodeNames;
         this.timeout = timeout;
     }
 
     public AddVotingConfigExclusionsRequest(StreamInput in) throws IOException {
         super(in);
+        // TODO should this be removed in the latest version where nodeIds and nodeNames are used?
         nodeDescriptions = in.readStringArray();
+        // TODO which version to use here?
+        if (in.getVersion() == Version.V_EMPTY) {
+            nodeIds = in.readStringArray();
+            nodeNames = in.readStringArray();
+        }
         timeout = in.readTimeValue();
     }
 
     Set<VotingConfigExclusion> resolveVotingConfigExclusions(ClusterState currentState) {
         final DiscoveryNodes allNodes = currentState.nodes();
-        final Set<VotingConfigExclusion> resolvedNodes = Arrays.stream(allNodes.resolveNodes(nodeDescriptions))
+        Set<VotingConfigExclusion> allProcessedNodes = null;
+
+        if (nodeDescriptions.length >= 1) {
+            allProcessedNodes = Arrays.stream(allNodes.resolveNodes(nodeDescriptions)).map(allNodes::get)
+                                    .filter(DiscoveryNode::isMasterNode).map(VotingConfigExclusion::new).collect(Collectors.toSet());
+
+            if (allProcessedNodes.isEmpty()) {
+                throw new IllegalArgumentException("add voting config exclusions request for " + Arrays.asList(nodeDescriptions)
+                    + " matched no master-eligible nodes");
+            }
+        }
+        else {
+            DiscoveryNodes.NodeResolutionResults nodeResolutionResults;
+            Set<VotingConfigExclusion> resolvedNodes;
+            Set<VotingConfigExclusion> unresolvedNodes;
+
+            if (nodeIds.length >= 1) {
+                nodeResolutionResults = allNodes.resolveNodesExact(true, nodeIds);
+
+                unresolvedNodes = Arrays.stream(nodeResolutionResults.getUnresolvedNodes())
+                    .map(nodeId -> new VotingConfigExclusion(nodeId, "")).collect(Collectors.toSet());
+            }
+            else {
+                nodeResolutionResults = allNodes.resolveNodesExact(false, nodeNames);
+
+                unresolvedNodes = Arrays.stream(nodeResolutionResults.getUnresolvedNodes())
+                    .map(nodeName -> new VotingConfigExclusion("", nodeName)).collect(Collectors.toSet());
+            }
+
+            resolvedNodes = Arrays.stream(nodeResolutionResults.getResolvedNodes())
                 .map(allNodes::get).filter(DiscoveryNode::isMasterNode).map(VotingConfigExclusion::new).collect(Collectors.toSet());
 
-        if (resolvedNodes.isEmpty()) {
-            throw new IllegalArgumentException("add voting config exclusions request for " + Arrays.asList(nodeDescriptions)
-                + " matched no master-eligible nodes");
+            allProcessedNodes = Sets.newHashSet(Iterables.concat(resolvedNodes, unresolvedNodes));
+
+            if (allProcessedNodes.isEmpty()) {
+                throw new IllegalArgumentException("add voting config exclusions request for nodeIds " + Arrays.asList(nodeIds) +
+                    " or nodeNames " + Arrays.asList(nodeNames) + " 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,
@@ -106,6 +152,20 @@ public String[] getNodeDescriptions() {
         return nodeDescriptions;
     }
 
+    /**
+     * @return ids of the nodes for whom to add voting config exclusions.
+     */
+    public String[] getNodeIds() {
+        return nodeIds;
+    }
+
+    /**
+     * @return names of the nodes for whom to add voting config exclusions.
+     */
+    public String[] getNodeNames() {
+        return nodeNames;
+    }
+
     /**
      * @return how long to wait after adding the exclusions for the nodes to be removed from the voting configuration.
      */
@@ -121,15 +181,24 @@ public ActionRequestValidationException validate() {
     @Override
     public void writeTo(StreamOutput out) throws IOException {
         super.writeTo(out);
+        // TODO should this be removed in the latest version where nodeIds and nodeNames are used?
         out.writeStringArray(nodeDescriptions);
+        // TODO which version to use here?
+        if (out.getVersion() == Version.V_EMPTY) {
+            out.writeStringArray(nodeIds);
+            out.writeStringArray(nodeNames);
+
+        }
         out.writeTimeValue(timeout);
     }
 
     @Override
     public String toString() {
         return "AddVotingConfigExclusionsRequest{" +
-            "nodeDescriptions=" + Arrays.asList(nodeDescriptions) +
-            ", timeout=" + timeout +
+            "nodeDescriptions=" + Arrays.asList(nodeDescriptions) + ", " +
+            "nodeIds=" + Arrays.asList(nodeIds) + ", " +
+            "nodeNames=" + Arrays.asList(nodeNames) + ", " +
+            "timeout=" + timeout +
             '}';
     }
 }

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,63 @@ 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(boolean isNodeIds, 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> resolvedNodes = new ObjectHashSet<>(nodes.length);
+            ObjectHashSet<String> unresolvedNodes = new ObjectHashSet<>();
+
+            if (isNodeIds) {
+                for (String nodeId : nodes) {
+                    if (nodeExists(nodeId)) {
+                        resolvedNodes.add(nodeId);
+                    }
+                    else {
+                        unresolvedNodes.add(nodeId);
+                    }
+                }
+            }
+            else {
+                Map<String, String> existingNodesNameId = new HashMap<>();
+                for (DiscoveryNode node : this) {
+                    existingNodesNameId.put(node.getName(), node.getId());
+                }
+
+                for (String nodeName : nodes) {
+                    if (existingNodesNameId.containsKey(nodeName)){
+                        resolvedNodes.add(existingNodesNameId.get(nodeName));
+                    }
+                    else {
+                        unresolvedNodes.add(nodeName);
+                    }
+                }
+            }
+
+            return new NodeResolutionResults(resolvedNodes.toArray(String.class), unresolvedNodes.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 +388,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 +405,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

@@ -23,20 +23,28 @@
 import org.elasticsearch.action.admin.cluster.configuration.AddVotingConfigExclusionsRequest;
 import org.elasticsearch.client.node.NodeClient;
 import org.elasticsearch.common.Strings;
+import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.unit.TimeValue;
 import org.elasticsearch.rest.BaseRestHandler;
 import org.elasticsearch.rest.RestController;
 import org.elasticsearch.rest.RestRequest;
 import org.elasticsearch.rest.action.RestToXContentListener;
 
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 import java.io.IOException;
 
 public class RestAddVotingConfigExclusionAction extends BaseRestHandler {
 
     private static final TimeValue DEFAULT_TIMEOUT = TimeValue.timeValueSeconds(30L);
+    private static final Logger logger = LogManager.getLogger(RestAddVotingConfigExclusionAction.class);
+    private static final DeprecationLogger DEPRECATION_LOGGER = new DeprecationLogger(logger);
 
     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", this);
     }
 
     @Override
@@ -55,10 +63,53 @@ protected RestChannelConsumer prepareRequest(final RestRequest request, final No
     }
 
     AddVotingConfigExclusionsRequest resolveVotingConfigExclusionsRequest(final RestRequest request) {
-        String nodeName = request.param("node_name");
+        String deprecatedNodeDescription = null;
+        String nodeIds = null;
+        String nodeNames = null;
+
+        if (request.hasParam("node_name")) {
+            DEPRECATION_LOGGER.deprecatedAndMaybeLog("add_voting_config_exclusion",
+                "Using [node_name] for adding voting config exclustion will be removed in a future version. " +
+                    "Please use [node_ids] or [node_names] instead");
+            deprecatedNodeDescription = request.param("node_name");
+        }
+
+        if (request.hasParam("node_ids")){
+            nodeIds = request.param("node_ids");
+        }
+
+        if (request.hasParam("node_names")){
+            nodeNames = request.param("node_names");
+        }
+
+        if(!oneAndonlyOneIsSet(deprecatedNodeDescription, nodeIds, nodeNames)) {
+            throw new IllegalArgumentException("Please set node identifiers correctly. " +
+                "One and only one of [node_name], [node_names] and [node_ids] has to be set");
+        }
+
         return new AddVotingConfigExclusionsRequest(
-            Strings.splitStringByCommaToArray(nodeName),
+            Strings.splitStringByCommaToArray(deprecatedNodeDescription),
+            Strings.splitStringByCommaToArray(nodeIds),
+            Strings.splitStringByCommaToArray(nodeNames),
             TimeValue.parseTimeValue(request.param("timeout"), DEFAULT_TIMEOUT, getClass().getSimpleName() + ".timeout")
         );
     }
+
+    private boolean oneAndonlyOneIsSet(String deprecatedNodeDescription, String nodeIds, String nodeNames) {
+        if(Strings.hasText(deprecatedNodeDescription)) {
+            return Strings.isNullOrEmpty(nodeIds) && Strings.isNullOrEmpty(nodeNames);
+        }
+        else if (Strings.hasText(nodeIds)) {
+            return Strings.isNullOrEmpty(nodeNames);
+        }
+        else if (Strings.hasText(nodeNames)) {
+            return true;
+        }
+        else {
+            // none of the node identifiers are set
+            return false;
+        }
+
+    }
+
 }