Add master_timeout support to voting config exclusions APIs (#86670)

Today the add/clear voting config exclusions APIs route a request to the
master node but do not expose the usual `?master_timeout` parameter
allowing to change the timeout for this phase of execution. This commit
adds the missing parameter.

docs/changelog/86670.yaml

@@ -0,0 +1,5 @@
+pr: 86670
+summary: Add `master_timeout` support to voting config exclusions APIs
+area: Cluster Coordination
+type: bug
+issues: []

docs/reference/cluster/voting-exclusions.asciidoc

@@ -57,11 +57,13 @@ For more information, see <<modules-discovery-removing-nodes>>.
 
 `node_names`::
 A comma-separated list of the names of the nodes to exclude from the voting
-configuration. If specified, you may not also specify `?node_ids`.
+configuration. If specified, you may not also specify `?node_ids`. Only applies
+to the `POST` form of this API.
 
 `node_ids`::
 A comma-separated list of the persistent ids of the nodes to exclude from the
 voting configuration. If specified, you may not also specify `?node_names`.
+Only applies to the `POST` form of this API.
 
 `timeout`::
 (Optional, <<time-units, time units>>) When adding a voting configuration
@@ -69,7 +71,12 @@ exclusion, the API waits for the specified nodes to be excluded from the voting
 configuration before returning. The period of time to wait is specified by the
 `?timeout` query parameter. If the timeout expires before the appropriate
 condition is satisfied, the request fails and returns an error. Defaults to
-`30s`.
+`30s`. Only applies to the `POST` form of this API.
+
+`master_timeout`::
+(Optional, <<time-units, time units>>) Defines how long to wait while trying to
+route the request to the current master node in the cluster. Defaults to `30s`.
+Applies to both `POST` and `DELETE` forms of this API.
 
 `wait_for_removal`::
 (Optional, Boolean) Specifies whether to wait for all excluded nodes to be
@@ -77,7 +84,7 @@ removed from the cluster before clearing the voting configuration exclusions
 list. Defaults to `true`, meaning that all excluded nodes must be removed from
 the cluster before this API takes any action. If set to `false` then the voting
 configuration exclusions list is cleared even if some excluded nodes are still
-in the cluster.
+in the cluster. Only applies to the `DELETE` form of this API.
   
 [[voting-config-exclusions-api-example]]
 ==== {api-examples-title}

rest-api-spec/src/main/resources/rest-api-spec/api/cluster.delete_voting_config_exclusions.json

@@ -24,6 +24,11 @@
         "type":"boolean",
         "description":"Specifies whether to wait for all excluded nodes to be removed from the cluster before clearing the voting configuration exclusions list.",
         "default":true
+      },
+      "master_timeout":{
+        "type":"time",
+        "description":"Timeout for submitting request to master",
+        "default":"30s"
       }
     }
   }

rest-api-spec/src/main/resources/rest-api-spec/api/cluster.post_voting_config_exclusions.json

@@ -32,6 +32,11 @@
         "type":"time",
         "description":"Explicit operation timeout",
         "default":"30s"
+      },
+      "master_timeout":{
+        "type":"time",
+        "description":"Timeout for submitting request to master",
+        "default":"30s"
       }
     }
   }

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

@@ -55,7 +55,7 @@ public class RestAddVotingConfigExclusionAction extends BaseRestHandler {
         );
     }
 
-    AddVotingConfigExclusionsRequest resolveVotingConfigExclusionsRequest(final RestRequest request) {
+    static AddVotingConfigExclusionsRequest resolveVotingConfigExclusionsRequest(final RestRequest request) {
         String nodeIds = null;
         String nodeNames = null;
 
@@ -71,11 +71,13 @@ public class RestAddVotingConfigExclusionAction extends BaseRestHandler {
             nodeNames = request.param("node_names");
         }
 
-        return new AddVotingConfigExclusionsRequest(
+        final var resolvedRequest = new AddVotingConfigExclusionsRequest(
             Strings.splitStringByCommaToArray(nodeIds),
             Strings.splitStringByCommaToArray(nodeNames),
-            TimeValue.parseTimeValue(request.param("timeout"), DEFAULT_TIMEOUT, getClass().getSimpleName() + ".timeout")
+            request.paramAsTime("timeout", DEFAULT_TIMEOUT)
         );
+
+        return resolvedRequest.masterNodeTimeout(request.paramAsTime("master_timeout", resolvedRequest.masterNodeTimeout()));
     }
 
 }

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

@@ -34,10 +34,16 @@ public class RestClearVotingConfigExclusionsAction extends BaseRestHandler {
 
     @Override
     protected RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException {
-        ClearVotingConfigExclusionsRequest req = new ClearVotingConfigExclusionsRequest();
-        if (request.hasParam("wait_for_removal")) {
-            req.setWaitForRemoval(request.paramAsBoolean("wait_for_removal", true));
-        }
+        final var req = resolveVotingConfigExclusionsRequest(request);
         return channel -> client.execute(ClearVotingConfigExclusionsAction.INSTANCE, req, new RestToXContentListener<>(channel));
     }
+
+    static ClearVotingConfigExclusionsRequest resolveVotingConfigExclusionsRequest(final RestRequest request) {
+        final var resolvedRequest = new ClearVotingConfigExclusionsRequest();
+        resolvedRequest.masterNodeTimeout(request.paramAsTime("master_timeout", resolvedRequest.masterNodeTimeout()));
+        resolvedRequest.setTimeout(resolvedRequest.masterNodeTimeout());
+        resolvedRequest.setWaitForRemoval(request.paramAsBoolean("wait_for_removal", resolvedRequest.getWaitForRemoval()));
+        return resolvedRequest;
+    }
+
 }

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

@@ -10,50 +10,89 @@ package org.elasticsearch.rest.action.admin.cluster;
 
 import org.elasticsearch.action.admin.cluster.configuration.AddVotingConfigExclusionsRequest;
 import org.elasticsearch.common.Strings;
+import org.elasticsearch.core.TimeValue;
 import org.elasticsearch.rest.RestRequest;
+import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.test.rest.FakeRestRequest;
-import org.elasticsearch.test.rest.RestActionTestCase;
-import org.junit.Before;
 
 import java.util.HashMap;
 import java.util.Map;
 
-public class RestAddVotingConfigExclusionActionTests extends RestActionTestCase {
+import static org.elasticsearch.rest.action.admin.cluster.RestAddVotingConfigExclusionAction.resolveVotingConfigExclusionsRequest;
 
-    private RestAddVotingConfigExclusionAction action;
-
-    @Before
-    public void setupAction() {
-        action = new RestAddVotingConfigExclusionAction();
-        controller().registerHandler(action);
-    }
+public class RestAddVotingConfigExclusionActionTests extends ESTestCase {
 
     public void testResolveVotingConfigExclusionsRequestNodeIds() {
         Map<String, String> params = new HashMap<>();
         params.put("node_ids", "node-1,node-2,node-3");
-        RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withMethod(RestRequest.Method.PUT)
+        RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withMethod(RestRequest.Method.POST)
             .withPath("/_cluster/voting_config_exclusions")
             .withParams(params)
             .build();
 
-        AddVotingConfigExclusionsRequest addVotingConfigExclusionsRequest = action.resolveVotingConfigExclusionsRequest(request);
+        AddVotingConfigExclusionsRequest addVotingConfigExclusionsRequest = resolveVotingConfigExclusionsRequest(request);
         String[] expected = { "node-1", "node-2", "node-3" };
         assertArrayEquals(expected, addVotingConfigExclusionsRequest.getNodeIds());
         assertArrayEquals(Strings.EMPTY_ARRAY, addVotingConfigExclusionsRequest.getNodeNames());
+        assertEquals(TimeValue.timeValueSeconds(30), addVotingConfigExclusionsRequest.getTimeout());
+        assertEquals(TimeValue.timeValueSeconds(30), addVotingConfigExclusionsRequest.masterNodeTimeout());
     }
 
     public void testResolveVotingConfigExclusionsRequestNodeNames() {
         Map<String, String> params = new HashMap<>();
         params.put("node_names", "node-1,node-2,node-3");
-        RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withMethod(RestRequest.Method.PUT)
+        RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withMethod(RestRequest.Method.POST)
             .withPath("/_cluster/voting_config_exclusions")
             .withParams(params)
             .build();
 
-        AddVotingConfigExclusionsRequest addVotingConfigExclusionsRequest = action.resolveVotingConfigExclusionsRequest(request);
+        AddVotingConfigExclusionsRequest addVotingConfigExclusionsRequest = resolveVotingConfigExclusionsRequest(request);
         String[] expected = { "node-1", "node-2", "node-3" };
         assertArrayEquals(Strings.EMPTY_ARRAY, addVotingConfigExclusionsRequest.getNodeIds());
         assertArrayEquals(expected, addVotingConfigExclusionsRequest.getNodeNames());
     }
 
+    public void testResolveVotingConfigExclusionsRequestTimeout() {
+        Map<String, String> params = new HashMap<>();
+        params.put("node_names", "node-1,node-2,node-3");
+        params.put("timeout", "60s");
+        RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withMethod(RestRequest.Method.POST)
+            .withPath("/_cluster/voting_config_exclusions")
+            .withParams(params)
+            .build();
+
+        AddVotingConfigExclusionsRequest addVotingConfigExclusionsRequest = resolveVotingConfigExclusionsRequest(request);
+        assertEquals(TimeValue.timeValueMinutes(1), addVotingConfigExclusionsRequest.getTimeout());
+        assertEquals(TimeValue.timeValueSeconds(30), addVotingConfigExclusionsRequest.masterNodeTimeout());
+    }
+
+    public void testResolveVotingConfigExclusionsRequestMasterTimeout() {
+        Map<String, String> params = new HashMap<>();
+        params.put("node_names", "node-1,node-2,node-3");
+        params.put("master_timeout", "60s");
+        RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withMethod(RestRequest.Method.POST)
+            .withPath("/_cluster/voting_config_exclusions")
+            .withParams(params)
+            .build();
+
+        AddVotingConfigExclusionsRequest addVotingConfigExclusionsRequest = resolveVotingConfigExclusionsRequest(request);
+        assertEquals(TimeValue.timeValueSeconds(30), addVotingConfigExclusionsRequest.getTimeout());
+        assertEquals(TimeValue.timeValueMinutes(1), addVotingConfigExclusionsRequest.masterNodeTimeout());
+    }
+
+    public void testResolveVotingConfigExclusionsRequestTimeoutAndMasterTimeout() {
+        Map<String, String> params = new HashMap<>();
+        params.put("node_names", "node-1,node-2,node-3");
+        params.put("timeout", "60s");
+        params.put("master_timeout", "120s");
+        RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withMethod(RestRequest.Method.POST)
+            .withPath("/_cluster/voting_config_exclusions")
+            .withParams(params)
+            .build();
+
+        AddVotingConfigExclusionsRequest addVotingConfigExclusionsRequest = resolveVotingConfigExclusionsRequest(request);
+        assertEquals(TimeValue.timeValueMinutes(1), addVotingConfigExclusionsRequest.getTimeout());
+        assertEquals(TimeValue.timeValueMinutes(2), addVotingConfigExclusionsRequest.masterNodeTimeout());
+    }
+
 }

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

@@ -0,0 +1,46 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+package org.elasticsearch.rest.action.admin.cluster;
+
+import org.elasticsearch.core.TimeValue;
+import org.elasticsearch.rest.RestRequest;
+import org.elasticsearch.test.ESTestCase;
+import org.elasticsearch.test.rest.FakeRestRequest;
+
+import java.util.Map;
+
+import static org.elasticsearch.rest.action.admin.cluster.RestClearVotingConfigExclusionsAction.resolveVotingConfigExclusionsRequest;
+
+public class RestClearVotingConfigExclusionActionTests extends ESTestCase {
+
+    public void testDefaultRequest() {
+        final var request = resolveVotingConfigExclusionsRequest(
+            new FakeRestRequest.Builder(xContentRegistry()).withMethod(RestRequest.Method.DELETE)
+                .withPath("/_cluster/voting_config_exclusions")
+                .withParams(Map.of())
+                .build()
+        );
+        assertEquals(TimeValue.timeValueSeconds(30), request.masterNodeTimeout());
+        assertEquals(TimeValue.timeValueSeconds(30), request.getTimeout());
+        assertTrue(request.getWaitForRemoval());
+    }
+
+    public void testResolveRequestParameters() {
+        final var request = resolveVotingConfigExclusionsRequest(
+            new FakeRestRequest.Builder(xContentRegistry()).withMethod(RestRequest.Method.DELETE)
+                .withPath("/_cluster/voting_config_exclusions")
+                .withParams(Map.of("master_timeout", "60s", "wait_for_removal", "false"))
+                .build()
+        );
+        assertEquals(TimeValue.timeValueMinutes(1), request.masterNodeTimeout());
+        assertEquals(TimeValue.timeValueMinutes(1), request.getTimeout());
+        assertFalse(request.getWaitForRemoval());
+    }
+
+}