diff --git a/docs/reference/migration/migrate_8_0.asciidoc b/docs/reference/migration/migrate_8_0.asciidoc index bf363ea30f858..07a076c34244c 100644 --- a/docs/reference/migration/migrate_8_0.asciidoc +++ b/docs/reference/migration/migrate_8_0.asciidoc @@ -16,6 +16,7 @@ coming[8.0.0] * <> * <> * <> +* <> * <> * <> * <> @@ -69,6 +70,7 @@ include::migrate_8_0/analysis.asciidoc[] include::migrate_8_0/allocation.asciidoc[] include::migrate_8_0/breaker.asciidoc[] include::migrate_8_0/discovery.asciidoc[] +include::migrate_8_0/cluster.asciidoc[] include::migrate_8_0/mappings.asciidoc[] include::migrate_8_0/packaging.asciidoc[] include::migrate_8_0/rollup.asciidoc[] diff --git a/docs/reference/migration/migrate_8_0/cluster.asciidoc b/docs/reference/migration/migrate_8_0/cluster.asciidoc new file mode 100644 index 0000000000000..67df114971e14 --- /dev/null +++ b/docs/reference/migration/migrate_8_0/cluster.asciidoc @@ -0,0 +1,18 @@ +[float] +[[breaking_80_cluster_changes]] +=== Cluster changes + +//NOTE: The notable-breaking-changes tagged regions are re-used in the +//Installation and Upgrade Guide + +//tag::notable-breaking-changes[] + +// end::notable-breaking-changes[] + +[float] +==== Change to API to add voting configuration exclusions + +The `POST /_cluster/voting_config_exclusions/{node_filter}` API has been +removed in favour of `POST /_cluster/voting_config_exclusions?node_names=...` +and `POST /_cluster/voting_config_exclusions?node_ids=...` which allow you to +specify the names or IDs of the nodes to exclude. diff --git a/docs/reference/migration/migrate_8_0/discovery.asciidoc b/docs/reference/migration/migrate_8_0/discovery.asciidoc index bc617e289ce28..8aa1062017374 100644 --- a/docs/reference/migration/migrate_8_0/discovery.asciidoc +++ b/docs/reference/migration/migrate_8_0/discovery.asciidoc @@ -13,7 +13,7 @@ ==== Removal of old discovery settings All settings under the `discovery.zen` namespace, which existed only for BWC reasons in 7.x, -will no longer be supported. In particular, this includes: +are no longer supported. In particular, this includes: - `discovery.zen.no_master_block` - `discovery.zen.hosts_provider` @@ -36,4 +36,4 @@ will no longer be supported. In particular, this includes: - `discovery.zen.send_leave_request` - `discovery.zen.master_election.wait_for_joins_timeout` - `discovery.zen.master_election.ignore_non_master_pings` -- `discovery.zen.publish.max_pending_cluster_states` \ No newline at end of file +- `discovery.zen.publish.max_pending_cluster_states` diff --git a/modules/transport-netty4/src/test/java/org/elasticsearch/rest/discovery/Zen2RestApiIT.java b/modules/transport-netty4/src/test/java/org/elasticsearch/rest/discovery/Zen2RestApiIT.java index 741153793948b..def985ee94da4 100644 --- a/modules/transport-netty4/src/test/java/org/elasticsearch/rest/discovery/Zen2RestApiIT.java +++ b/modules/transport-netty4/src/test/java/org/elasticsearch/rest/discovery/Zen2RestApiIT.java @@ -26,9 +26,9 @@ import org.elasticsearch.client.Node; import org.elasticsearch.client.Request; import org.elasticsearch.client.Response; -import org.elasticsearch.client.ResponseException; import org.elasticsearch.client.RestClient; import org.elasticsearch.cluster.metadata.IndexMetadata; +import org.elasticsearch.cluster.node.DiscoveryNodes; import org.elasticsearch.cluster.routing.UnassignedInfo; import org.elasticsearch.common.Priority; import org.elasticsearch.common.settings.Settings; @@ -36,11 +36,12 @@ import org.elasticsearch.http.HttpServerTransport; import org.elasticsearch.test.ESIntegTestCase; import org.elasticsearch.test.InternalTestCluster; -import org.hamcrest.Matchers; import java.io.IOException; import java.util.Collections; +import java.util.HashMap; import java.util.List; +import java.util.Map; import static org.hamcrest.core.Is.is; @@ -66,15 +67,27 @@ public void testRollingRestartOfTwoNodeCluster() throws Exception { .build()); ensureGreen("test"); + final DiscoveryNodes discoveryNodes = client().admin().cluster().prepareState().clear().setNodes(true).get().getState().nodes(); + final Map nodeIdsByName = new HashMap<>(discoveryNodes.getSize()); + discoveryNodes.forEach(n -> nodeIdsByName.put(n.getName(), n.getId())); + RestClient restClient = getRestClient(); internalCluster().rollingRestart(new InternalTestCluster.RestartCallback() { @Override public void doAfterNodes(int n, Client client) throws IOException { ensureGreen("test"); - Response response = - restClient.performRequest(new Request("POST", "/_cluster/voting_config_exclusions/" + - internalCluster().getNodeNames()[n])); + final Request request = new Request("POST", "/_cluster/voting_config_exclusions"); + final String nodeName = internalCluster().getNodeNames()[n]; + if (randomBoolean()) { + request.addParameter("node_names", nodeName); + } else { + final String nodeId = nodeIdsByName.get(nodeName); + assertNotNull(nodeName, nodeId); + request.addParameter("node_ids", nodeId); + } + + final Response response = restClient.performRequest(request); assertThat(response.getStatusLine().getStatusCode(), is(200)); } @@ -120,7 +133,9 @@ public void testClearVotingTombstonesNotWaitingForRemoval() throws Exception { List nodes = internalCluster().startNodes(3); ensureStableCluster(3); RestClient restClient = getRestClient(); - Response response = restClient.performRequest(new Request("POST", "/_cluster/voting_config_exclusions/" + nodes.get(2))); + final Request request = new Request("POST", "/_cluster/voting_config_exclusions"); + request.addParameter("node_names", nodes.get(2)); + final Response response = restClient.performRequest(request); assertThat(response.getStatusLine().getStatusCode(), is(200)); assertThat(response.getEntity().getContentLength(), is(0L)); Response deleteResponse = restClient.performRequest( @@ -135,7 +150,9 @@ public void testClearVotingTombstonesWaitingForRemoval() throws Exception { ensureStableCluster(3); RestClient restClient = getRestClient(); String nodeToWithdraw = nodes.get(randomIntBetween(0, 2)); - Response response = restClient.performRequest(new Request("POST", "/_cluster/voting_config_exclusions/" + nodeToWithdraw)); + final Request request = new Request("POST", "/_cluster/voting_config_exclusions"); + request.addParameter("node_names", nodeToWithdraw); + final Response response = restClient.performRequest(request); assertThat(response.getStatusLine().getStatusCode(), is(200)); assertThat(response.getEntity().getContentLength(), is(0L)); internalCluster().stopRandomNode(InternalTestCluster.nameFilter(nodeToWithdraw)); @@ -144,30 +161,14 @@ public void testClearVotingTombstonesWaitingForRemoval() throws Exception { assertThat(deleteResponse.getEntity().getContentLength(), is(0L)); } - public void testFailsOnUnknownNode() throws Exception { - internalCluster().setBootstrapMasterNodeIndex(2); - internalCluster().startNodes(3); - ensureStableCluster(3); - RestClient restClient = getRestClient(); - try { - restClient.performRequest(new Request("POST", "/_cluster/voting_config_exclusions/invalid")); - fail("Invalid node name should throw."); - } catch (ResponseException e) { - assertThat(e.getResponse().getStatusLine().getStatusCode(), is(400)); - assertThat( - e.getMessage(), - Matchers.containsString("add voting config exclusions request for [invalid] matched no master-eligible nodes") - ); - } - } - public void testRemoveTwoNodesAtOnce() throws Exception { internalCluster().setBootstrapMasterNodeIndex(2); List nodes = internalCluster().startNodes(3); ensureStableCluster(3); RestClient restClient = getRestClient(); - Response response = restClient.performRequest(new Request("POST", "/_cluster/voting_config_exclusions/" + - nodes.get(2) + "," + nodes.get(0))); + final Request request = new Request("POST", "/_cluster/voting_config_exclusions"); + request.addParameter("node_names", nodes.get(2) + "," + nodes.get(0)); + final Response response = restClient.performRequest(request); assertThat(response.getStatusLine().getStatusCode(), is(200)); assertThat(response.getEntity().getContentLength(), is(0L)); internalCluster().stopRandomNode(InternalTestCluster.nameFilter(nodes.get(0))); diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/configuration/AddVotingConfigExclusionsRequest.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/configuration/AddVotingConfigExclusionsRequest.java index 1f4e273b30803..02227c39d5fee 100644 --- a/server/src/main/java/org/elasticsearch/action/admin/cluster/configuration/AddVotingConfigExclusionsRequest.java +++ b/server/src/main/java/org/elasticsearch/action/admin/cluster/configuration/AddVotingConfigExclusionsRequest.java @@ -18,7 +18,7 @@ */ package org.elasticsearch.action.admin.cluster.configuration; -import org.apache.logging.log4j.LogManager; +import org.elasticsearch.Version; import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.support.master.MasterNodeRequest; import org.elasticsearch.cluster.ClusterState; @@ -28,7 +28,6 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; -import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.unit.TimeValue; import java.io.IOException; @@ -45,10 +44,6 @@ * configuration. */ public class AddVotingConfigExclusionsRequest extends MasterNodeRequest { - public static final String DEPRECATION_MESSAGE = "nodeDescription is deprecated and will be removed, use nodeIds or nodeNames instead"; - private static final DeprecationLogger deprecationLogger = new DeprecationLogger( - LogManager.getLogger(AddVotingConfigExclusionsRequest.class)); - private final String[] nodeDescriptions; private final String[] nodeIds; private final String[] nodeNames; private final TimeValue timeout; @@ -56,37 +51,32 @@ public class AddVotingConfigExclusionsRequest extends MasterNodeRequest 0) == (nodeNames.length > 0)) { + throw new IllegalArgumentException("You must set [node_names] or [node_ids] but not both"); } - if (nodeDescriptions.length > 0) { - deprecationLogger.deprecatedAndMaybeLog("voting_config_exclusion", DEPRECATION_MESSAGE); - } - - this.nodeDescriptions = nodeDescriptions; this.nodeIds = nodeIds; this.nodeNames = nodeNames; this.timeout = timeout; @@ -94,31 +84,23 @@ public AddVotingConfigExclusionsRequest(String[] nodeDescriptions, String[] node public AddVotingConfigExclusionsRequest(StreamInput in) throws IOException { super(in); - nodeDescriptions = in.readStringArray(); + if (in.getVersion().before(Version.V_8_0_0)) { + final String[] legacyNodeDescriptions = in.readStringArray(); + if (legacyNodeDescriptions.length > 0) { + throw new IllegalArgumentException("legacy [node_name] field was deprecated and must be empty"); + } + } nodeIds = in.readStringArray(); nodeNames = in.readStringArray(); timeout = in.readTimeValue(); - - if (nodeDescriptions.length > 0) { - deprecationLogger.deprecatedAndMaybeLog("voting_config_exclusion", - "nodeDescription is deprecated and will be removed, use nodeIds or nodeNames instead"); - } - + assert (nodeIds.length > 0) != (nodeNames.length > 0); } Set resolveVotingConfigExclusions(ClusterState currentState) { final DiscoveryNodes allNodes = currentState.nodes(); Set newVotingConfigExclusions = new HashSet<>(); - if (nodeDescriptions.length >= 1) { - newVotingConfigExclusions = Arrays.stream(allNodes.resolveNodes(nodeDescriptions)).map(allNodes::get) - .filter(DiscoveryNode::isMasterNode).map(VotingConfigExclusion::new).collect(Collectors.toSet()); - - if (newVotingConfigExclusions.isEmpty()) { - throw new IllegalArgumentException("add voting config exclusions request for " + Arrays.asList(nodeDescriptions) - + " matched no master-eligible nodes"); - } - } else if (nodeIds.length >= 1) { + if (nodeIds.length > 0) { for (String nodeId : nodeIds) { if (allNodes.nodeExists(nodeId)) { DiscoveryNode discoveryNode = allNodes.get(nodeId); @@ -130,12 +112,12 @@ Set resolveVotingConfigExclusions(ClusterState currentSta } } } else { - assert nodeNames.length >= 1; + assert nodeNames.length > 0; Map existingNodes = StreamSupport.stream(allNodes.spliterator(), false) - .collect(Collectors.toMap(DiscoveryNode::getName, Function.identity())); + .collect(Collectors.toMap(DiscoveryNode::getName, Function.identity())); for (String nodeName : nodeNames) { - if (existingNodes.containsKey(nodeName)){ + if (existingNodes.containsKey(nodeName)) { DiscoveryNode discoveryNode = existingNodes.get(nodeName); if (discoveryNode.isMasterNode()) { newVotingConfigExclusions.add(new VotingConfigExclusion(discoveryNode)); @@ -157,7 +139,10 @@ Set resolveVotingConfigExclusionsAndCheckMaximum(ClusterS final int oldExclusionsCount = currentState.getVotingConfigExclusions().size(); final int newExclusionsCount = resolvedExclusions.size(); if (oldExclusionsCount + newExclusionsCount > maxExclusionsCount) { - throw new IllegalArgumentException("add voting config exclusions request for " + Arrays.asList(nodeDescriptions) + throw new IllegalArgumentException("add voting config exclusions request for " + + (nodeNames.length > 0 + ? "nodes named " + Arrays.asList(nodeNames) + : "nodes with ids " + Arrays.asList(nodeIds)) + " would add [" + newExclusionsCount + "] exclusions to the existing [" + oldExclusionsCount + "] which would exceed the maximum of [" + maxExclusionsCount + "] set by [" + maximumSettingKey + "]"); @@ -165,23 +150,6 @@ Set resolveVotingConfigExclusionsAndCheckMaximum(ClusterS return resolvedExclusions; } - private boolean noneOrMoreThanOneIsSet(String[] deprecatedNodeDescription, String[] nodeIds, String[] nodeNames) { - if (deprecatedNodeDescription.length > 0) { - return nodeIds.length > 0 || nodeNames.length > 0; - } else if (nodeIds.length > 0) { - return nodeNames.length > 0; - } else { - return nodeNames.length > 0 == false; - } - } - - /** - * @return descriptions of the nodes for whom to add voting config exclusions. - */ - public String[] getNodeDescriptions() { - return nodeDescriptions; - } - /** * @return ids of the nodes for whom to add voting config exclusions. */ @@ -211,7 +179,9 @@ public ActionRequestValidationException validate() { @Override public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); - out.writeStringArray(nodeDescriptions); + if (out.getVersion().before(Version.V_8_0_0)) { + out.writeStringArray(Strings.EMPTY_ARRAY); + } out.writeStringArray(nodeIds); out.writeStringArray(nodeNames); out.writeTimeValue(timeout); @@ -220,7 +190,6 @@ public void writeTo(StreamOutput out) throws IOException { @Override public String toString() { return "AddVotingConfigExclusionsRequest{" + - "nodeDescriptions=" + Arrays.asList(nodeDescriptions) + ", " + "nodeIds=" + Arrays.asList(nodeIds) + ", " + "nodeNames=" + Arrays.asList(nodeNames) + ", " + "timeout=" + timeout + diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestAddVotingConfigExclusionAction.java b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestAddVotingConfigExclusionAction.java index 01355c5e50e71..d3f2bfa7785a6 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestAddVotingConfigExclusionAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestAddVotingConfigExclusionAction.java @@ -28,8 +28,6 @@ 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; import java.util.List; @@ -37,12 +35,6 @@ 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 String DEPRECATION_MESSAGE = "POST /_cluster/voting_config_exclusions/{node_name} " + - "will be removed in a future version. " + - "Please use POST /_cluster/voting_config_exclusions?node_ids=... " + - "or POST /_cluster/voting_config_exclusions?node_names=... instead."; @Override public String getName() { @@ -51,8 +43,7 @@ public String getName() { @Override public List routes() { - return List.of(new DeprecatedRoute(POST, "/_cluster/voting_config_exclusions/{node_name}", DEPRECATION_MESSAGE), - new Route(POST, "/_cluster/voting_config_exclusions")); + return List.of(new Route(POST, "/_cluster/voting_config_exclusions")); } @Override @@ -66,29 +57,22 @@ protected RestChannelConsumer prepareRequest(final RestRequest request, final No } AddVotingConfigExclusionsRequest resolveVotingConfigExclusionsRequest(final RestRequest request) { - String deprecatedNodeDescription = null; String nodeIds = null; String nodeNames = null; - if (request.hasParam("node_name")) { - deprecatedNodeDescription = request.param("node_name"); - } - - if (request.hasParam("node_ids")){ + if (request.hasParam("node_ids")) { nodeIds = request.param("node_ids"); } - if (request.hasParam("node_names")){ - nodeNames = request.param("node_names"); + if (request.hasParam("node_names")) { + nodeNames = request.param("node_names"); } return new AddVotingConfigExclusionsRequest( - Strings.splitStringByCommaToArray(deprecatedNodeDescription), Strings.splitStringByCommaToArray(nodeIds), Strings.splitStringByCommaToArray(nodeNames), TimeValue.parseTimeValue(request.param("timeout"), DEFAULT_TIMEOUT, getClass().getSimpleName() + ".timeout") ); } - } diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/configuration/AddVotingConfigExclusionsRequestTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/configuration/AddVotingConfigExclusionsRequestTests.java index ead56d7de7fcd..b5f54b331849e 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/configuration/AddVotingConfigExclusionsRequestTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/configuration/AddVotingConfigExclusionsRequestTests.java @@ -41,32 +41,14 @@ import static org.hamcrest.Matchers.equalTo; public class AddVotingConfigExclusionsRequestTests extends ESTestCase { - private static final String NODE_IDENTIFIERS_INCORRECTLY_SET_MSG = "Please set node identifiers correctly. " + - "One and only one of [node_name], [node_names] and [node_ids] has to be set"; - - public void testSerialization() throws IOException { - int descriptionCount = between(1, 5); - String[] descriptions = new String[descriptionCount]; - for (int i = 0; i < descriptionCount; i++) { - descriptions[i] = randomAlphaOfLength(10); - } - TimeValue timeout = TimeValue.timeValueMillis(between(0, 30000)); - final AddVotingConfigExclusionsRequest originalRequest = new AddVotingConfigExclusionsRequest(descriptions, Strings.EMPTY_ARRAY, - Strings.EMPTY_ARRAY, timeout); - final AddVotingConfigExclusionsRequest deserialized = copyWriteable(originalRequest, writableRegistry(), - AddVotingConfigExclusionsRequest::new); - assertThat(deserialized.getNodeDescriptions(), equalTo(originalRequest.getNodeDescriptions())); - assertThat(deserialized.getTimeout(), equalTo(originalRequest.getTimeout())); - assertWarnings(AddVotingConfigExclusionsRequest.DEPRECATION_MESSAGE); - } + private static final String NODE_IDENTIFIERS_INCORRECTLY_SET_MSG = "You must set [node_names] or [node_ids] but not both"; public void testSerializationForNodeIdOrNodeName() throws IOException { - AddVotingConfigExclusionsRequest originalRequest = new AddVotingConfigExclusionsRequest(Strings.EMPTY_ARRAY, - new String[]{"nodeId1", "nodeId2"}, Strings.EMPTY_ARRAY, TimeValue.ZERO); + AddVotingConfigExclusionsRequest originalRequest = new AddVotingConfigExclusionsRequest( + new String[]{"nodeId1", "nodeId2"}, Strings.EMPTY_ARRAY, TimeValue.ZERO); AddVotingConfigExclusionsRequest deserialized = copyWriteable(originalRequest, writableRegistry(), AddVotingConfigExclusionsRequest::new); - assertThat(deserialized.getNodeDescriptions(), equalTo(originalRequest.getNodeDescriptions())); assertThat(deserialized.getNodeIds(), equalTo(originalRequest.getNodeIds())); assertThat(deserialized.getNodeNames(), equalTo(originalRequest.getNodeNames())); assertThat(deserialized.getTimeout(), equalTo(originalRequest.getTimeout())); @@ -74,7 +56,6 @@ public void testSerializationForNodeIdOrNodeName() throws IOException { originalRequest = new AddVotingConfigExclusionsRequest("nodeName1", "nodeName2"); deserialized = copyWriteable(originalRequest, writableRegistry(), AddVotingConfigExclusionsRequest::new); - assertThat(deserialized.getNodeDescriptions(), equalTo(originalRequest.getNodeDescriptions())); assertThat(deserialized.getNodeIds(), equalTo(originalRequest.getNodeIds())); assertThat(deserialized.getNodeNames(), equalTo(originalRequest.getNodeNames())); assertThat(deserialized.getTimeout(), equalTo(originalRequest.getTimeout())); @@ -111,45 +92,26 @@ public void testResolve() { final ClusterState clusterState = ClusterState.builder(new ClusterName("cluster")).nodes(new Builder() .add(localNode).add(otherNode1).add(otherNode2).add(otherDataNode).localNodeId(localNode.getId())).build(); - assertThat(makeRequestWithNodeDescriptions("_all").resolveVotingConfigExclusions(clusterState), + assertThat(new AddVotingConfigExclusionsRequest("local", "other1", "other2").resolveVotingConfigExclusions(clusterState), containsInAnyOrder(localNodeExclusion, otherNode1Exclusion, otherNode2Exclusion)); - assertThat(makeRequestWithNodeDescriptions("_local").resolveVotingConfigExclusions(clusterState), + assertThat(new AddVotingConfigExclusionsRequest("local").resolveVotingConfigExclusions(clusterState), contains(localNodeExclusion)); - assertThat(makeRequestWithNodeDescriptions("other*").resolveVotingConfigExclusions(clusterState), + assertThat(new AddVotingConfigExclusionsRequest("other1", "other2").resolveVotingConfigExclusions(clusterState), + containsInAnyOrder(otherNode1Exclusion, otherNode2Exclusion)); + assertThat(new AddVotingConfigExclusionsRequest(Strings.EMPTY_ARRAY, new String[]{"other1", "other2"}, TimeValue.ZERO) + .resolveVotingConfigExclusions(clusterState), containsInAnyOrder(otherNode1Exclusion, otherNode2Exclusion)); - - assertThat(expectThrows(IllegalArgumentException.class, - () -> makeRequestWithNodeDescriptions("not-a-node").resolveVotingConfigExclusions(clusterState)).getMessage(), - equalTo("add voting config exclusions request for [not-a-node] matched no master-eligible nodes")); - assertWarnings(AddVotingConfigExclusionsRequest.DEPRECATION_MESSAGE); } public void testResolveAllNodeIdentifiersNullOrEmpty() { assertThat(expectThrows(IllegalArgumentException.class, - () -> new AddVotingConfigExclusionsRequest(Strings.EMPTY_ARRAY, Strings.EMPTY_ARRAY, - Strings.EMPTY_ARRAY, TimeValue.ZERO)).getMessage(), + () -> new AddVotingConfigExclusionsRequest(Strings.EMPTY_ARRAY, Strings.EMPTY_ARRAY, TimeValue.ZERO)).getMessage(), equalTo(NODE_IDENTIFIERS_INCORRECTLY_SET_MSG)); } public void testResolveMoreThanOneNodeIdentifiersSet() { assertThat(expectThrows(IllegalArgumentException.class, - () -> new AddVotingConfigExclusionsRequest(new String[]{"local"}, new String[]{"nodeId"}, - Strings.EMPTY_ARRAY, TimeValue.ZERO)).getMessage(), - equalTo(NODE_IDENTIFIERS_INCORRECTLY_SET_MSG)); - - assertThat(expectThrows(IllegalArgumentException.class, - () -> new AddVotingConfigExclusionsRequest(new String[]{"local"}, Strings.EMPTY_ARRAY, - new String[]{"nodeName"}, TimeValue.ZERO)).getMessage(), - equalTo(NODE_IDENTIFIERS_INCORRECTLY_SET_MSG)); - - assertThat(expectThrows(IllegalArgumentException.class, - () -> new AddVotingConfigExclusionsRequest(Strings.EMPTY_ARRAY, new String[]{"nodeId"}, - new String[]{"nodeName"}, TimeValue.ZERO)).getMessage(), - equalTo(NODE_IDENTIFIERS_INCORRECTLY_SET_MSG)); - - assertThat(expectThrows(IllegalArgumentException.class, - () -> new AddVotingConfigExclusionsRequest(new String[]{"local"}, new String[]{"nodeId"}, - new String[]{"nodeName"}, TimeValue.ZERO)).getMessage(), + () -> new AddVotingConfigExclusionsRequest(new String[]{"nodeId"}, new String[]{"nodeName"}, TimeValue.ZERO)).getMessage(), equalTo(NODE_IDENTIFIERS_INCORRECTLY_SET_MSG)); } @@ -186,11 +148,11 @@ public void testResolveByNodeIds() { final ClusterState clusterState = ClusterState.builder(new ClusterName("cluster")) .nodes(new Builder().add(node1).add(node2).add(node3).localNodeId(node1.getId())).build(); - assertThat(new AddVotingConfigExclusionsRequest(Strings.EMPTY_ARRAY, new String[]{"nodeId1", "nodeId2"}, + assertThat(new AddVotingConfigExclusionsRequest(new String[]{"nodeId1", "nodeId2"}, Strings.EMPTY_ARRAY, TimeValue.ZERO).resolveVotingConfigExclusions(clusterState), containsInAnyOrder(node1Exclusion, node2Exclusion)); - assertThat(new AddVotingConfigExclusionsRequest(Strings.EMPTY_ARRAY, new String[]{"nodeId1", "unresolvableNodeId"}, + assertThat(new AddVotingConfigExclusionsRequest(new String[]{"nodeId1", "unresolvableNodeId"}, Strings.EMPTY_ARRAY, TimeValue.ZERO).resolveVotingConfigExclusions(clusterState), containsInAnyOrder(node1Exclusion, unresolvableVotingConfigExclusion)); } @@ -265,7 +227,7 @@ public void testResolveRemoveExistingVotingConfigExclusions() { final ClusterState clusterState = ClusterState.builder(new ClusterName("cluster")).metadata(metadata) .nodes(new Builder().add(node1).add(node2).add(node3).localNodeId(node1.getId())).build(); - assertThat(new AddVotingConfigExclusionsRequest(Strings.EMPTY_ARRAY, new String[]{"nodeId1", "nodeId2"}, + assertThat(new AddVotingConfigExclusionsRequest(new String[]{"nodeId1", "nodeId2"}, Strings.EMPTY_ARRAY, TimeValue.ZERO).resolveVotingConfigExclusions(clusterState), contains(node2Exclusion)); } @@ -294,7 +256,6 @@ public void testResolveAndCheckMaximum() { emptyMap(), Set.of(DiscoveryNodeRole.MASTER_ROLE), Version.CURRENT); - final VotingConfigExclusion otherNode2Exclusion = new VotingConfigExclusion(otherNode2); final ClusterState.Builder builder = ClusterState.builder(new ClusterName("cluster")).nodes(new Builder() .add(localNode).add(otherNode1).add(otherNode2).localNodeId(localNode.getId())); @@ -302,18 +263,14 @@ public void testResolveAndCheckMaximum() { .coordinationMetadata(CoordinationMetadata.builder().addVotingConfigExclusion(otherNode1Exclusion).build())); final ClusterState clusterState = builder.build(); - assertThat(makeRequestWithNodeDescriptions("_local").resolveVotingConfigExclusionsAndCheckMaximum(clusterState, 2, "setting.name"), + assertThat(new AddVotingConfigExclusionsRequest("local") + .resolveVotingConfigExclusionsAndCheckMaximum(clusterState, 2, "setting.name"), contains(localNodeExclusion)); assertThat(expectThrows(IllegalArgumentException.class, - () -> makeRequestWithNodeDescriptions("_local").resolveVotingConfigExclusionsAndCheckMaximum(clusterState, 1, "setting.name")) - .getMessage(), - equalTo("add voting config exclusions request for [_local] would add [1] exclusions to the existing [1] which would " + - "exceed the maximum of [1] set by [setting.name]")); - assertWarnings(AddVotingConfigExclusionsRequest.DEPRECATION_MESSAGE); + () -> new AddVotingConfigExclusionsRequest("local") + .resolveVotingConfigExclusionsAndCheckMaximum(clusterState, 1, "setting.name")).getMessage(), + equalTo("add voting config exclusions request for nodes named [local] would add [1] exclusions to the existing [1] which " + + "would exceed the maximum of [1] set by [setting.name]")); } - private static AddVotingConfigExclusionsRequest makeRequestWithNodeDescriptions(String... nodeDescriptions) { - return new AddVotingConfigExclusionsRequest(nodeDescriptions, Strings.EMPTY_ARRAY, - Strings.EMPTY_ARRAY, TimeValue.timeValueSeconds(30)); - } } diff --git a/server/src/test/java/org/elasticsearch/action/admin/cluster/configuration/TransportAddVotingConfigExclusionsActionTests.java b/server/src/test/java/org/elasticsearch/action/admin/cluster/configuration/TransportAddVotingConfigExclusionsActionTests.java index 462940f5c90d0..221a9a1114c65 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/cluster/configuration/TransportAddVotingConfigExclusionsActionTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/cluster/configuration/TransportAddVotingConfigExclusionsActionTests.java @@ -180,57 +180,6 @@ public void testWithdrawsVotesFromMultipleNodes() throws InterruptedException { containsInAnyOrder(otherNode1Exclusion, otherNode2Exclusion)); } - public void testWithdrawsVotesFromNodesMatchingWildcard() throws InterruptedException { - final CountDownLatch countDownLatch = new CountDownLatch(2); - - clusterStateObserver.waitForNextChange(new AdjustConfigurationForExclusions(countDownLatch)); - transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME, makeRequestWithNodeDescriptions("other*"), - expectSuccess(r -> { - assertNotNull(r); - countDownLatch.countDown(); - }) - ); - - assertTrue(countDownLatch.await(30, TimeUnit.SECONDS)); - assertThat(clusterService.getClusterApplierService().state().getVotingConfigExclusions(), - containsInAnyOrder(otherNode1Exclusion, otherNode2Exclusion)); - assertWarnings(AddVotingConfigExclusionsRequest.DEPRECATION_MESSAGE); - } - - public void testWithdrawsVotesFromAllMasterEligibleNodes() throws InterruptedException { - final CountDownLatch countDownLatch = new CountDownLatch(2); - - clusterStateObserver.waitForNextChange(new AdjustConfigurationForExclusions(countDownLatch)); - transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME, makeRequestWithNodeDescriptions("_all"), - expectSuccess(r -> { - assertNotNull(r); - countDownLatch.countDown(); - }) - ); - - assertTrue(countDownLatch.await(30, TimeUnit.SECONDS)); - assertThat(clusterService.getClusterApplierService().state().getVotingConfigExclusions(), - containsInAnyOrder(localNodeExclusion, otherNode1Exclusion, otherNode2Exclusion)); - assertWarnings(AddVotingConfigExclusionsRequest.DEPRECATION_MESSAGE); - } - - public void testWithdrawsVoteFromLocalNode() throws InterruptedException { - final CountDownLatch countDownLatch = new CountDownLatch(2); - - clusterStateObserver.waitForNextChange(new AdjustConfigurationForExclusions(countDownLatch)); - transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME, makeRequestWithNodeDescriptions("_local"), - expectSuccess(r -> { - assertNotNull(r); - countDownLatch.countDown(); - }) - ); - - assertTrue(countDownLatch.await(30, TimeUnit.SECONDS)); - assertThat(clusterService.getClusterApplierService().state().getVotingConfigExclusions(), - contains(localNodeExclusion)); - assertWarnings(AddVotingConfigExclusionsRequest.DEPRECATION_MESSAGE); - } - public void testReturnsImmediatelyIfVoteAlreadyWithdrawn() throws InterruptedException { final ClusterState state = clusterService.state(); setState(clusterService, builder(state) @@ -255,55 +204,14 @@ public void testReturnsImmediatelyIfVoteAlreadyWithdrawn() throws InterruptedExc contains(otherNode1Exclusion)); } - public void testReturnsErrorIfNoMatchingNodeDescriptions() throws InterruptedException { - final CountDownLatch countDownLatch = new CountDownLatch(1); - final SetOnce exceptionHolder = new SetOnce<>(); - - transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME, makeRequestWithNodeDescriptions("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")); - assertWarnings(AddVotingConfigExclusionsRequest.DEPRECATION_MESSAGE); - } - - public void testOnlyMatchesMasterEligibleNodes() throws InterruptedException { - final CountDownLatch countDownLatch = new CountDownLatch(1); - final SetOnce exceptionHolder = new SetOnce<>(); - - transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME, - makeRequestWithNodeDescriptions("_all", "master:false"), - 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 [_all, master:false] matched no master-eligible nodes")); - assertWarnings(AddVotingConfigExclusionsRequest.DEPRECATION_MESSAGE); - } - public void testExcludeAbsentNodesByNodeIds() throws InterruptedException { final CountDownLatch countDownLatch = new CountDownLatch(2); clusterStateObserver.waitForNextChange(new AdjustConfigurationForExclusions(countDownLatch)); transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME, - new AddVotingConfigExclusionsRequest(Strings.EMPTY_ARRAY, new String[]{"absent_id"}, - Strings.EMPTY_ARRAY, TimeValue.timeValueSeconds(30)), - expectSuccess(e -> { - countDownLatch.countDown(); - }) + new AddVotingConfigExclusionsRequest(new String[]{"absent_id"}, + Strings.EMPTY_ARRAY, TimeValue.timeValueSeconds(30)), + expectSuccess(e -> countDownLatch.countDown()) ); assertTrue(countDownLatch.await(30, TimeUnit.SECONDS)); @@ -316,8 +224,8 @@ public void testExcludeExistingNodesByNodeIds() throws InterruptedException { clusterStateObserver.waitForNextChange(new AdjustConfigurationForExclusions(countDownLatch)); transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME, - new AddVotingConfigExclusionsRequest(Strings.EMPTY_ARRAY, new String[]{"other1", "other2"}, - Strings.EMPTY_ARRAY, TimeValue.timeValueSeconds(30)), + new AddVotingConfigExclusionsRequest(new String[]{"other1", "other2"}, + Strings.EMPTY_ARRAY, TimeValue.timeValueSeconds(30)), expectSuccess(r -> { assertNotNull(r); countDownLatch.countDown(); @@ -334,14 +242,12 @@ public void testExcludeAbsentNodesByNodeNames() throws InterruptedException { clusterStateObserver.waitForNextChange(new AdjustConfigurationForExclusions(countDownLatch)); transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME, new AddVotingConfigExclusionsRequest("absent_node"), - expectSuccess(e -> { - countDownLatch.countDown(); - }) + expectSuccess(e -> countDownLatch.countDown()) ); assertTrue(countDownLatch.await(30, TimeUnit.SECONDS)); assertEquals(Set.of(new VotingConfigExclusion(VotingConfigExclusion.MISSING_VALUE_MARKER, "absent_node")), - clusterService.getClusterApplierService().state().getVotingConfigExclusions()); + clusterService.getClusterApplierService().state().getVotingConfigExclusions()); } public void testExcludeExistingNodesByNodeNames() throws InterruptedException { @@ -382,7 +288,7 @@ public void testSucceedsEvenIfAllExclusionsAlreadyAdded() throws InterruptedExce assertTrue(countDownLatch.await(30, TimeUnit.SECONDS)); assertThat(clusterService.getClusterApplierService().state().getVotingConfigExclusions(), - contains(otherNode1Exclusion)); + contains(otherNode1Exclusion)); } public void testExcludeByNodeIdSucceedsEvenIfAllExclusionsAlreadyAdded() throws InterruptedException { @@ -398,8 +304,8 @@ public void testExcludeByNodeIdSucceedsEvenIfAllExclusionsAlreadyAdded() throws final CountDownLatch countDownLatch = new CountDownLatch(1); transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME, - new AddVotingConfigExclusionsRequest(Strings.EMPTY_ARRAY, new String[]{"other1"}, - Strings.EMPTY_ARRAY, TimeValue.timeValueSeconds(30)), + new AddVotingConfigExclusionsRequest(new String[]{"other1"}, + Strings.EMPTY_ARRAY, TimeValue.timeValueSeconds(30)), expectSuccess(r -> { assertNotNull(r); countDownLatch.countDown(); @@ -473,7 +379,8 @@ public void testReturnsErrorIfMaximumExclusionCountExceeded() throws Interrupted final CountDownLatch countDownLatch = new CountDownLatch(1); final SetOnce exceptionHolder = new SetOnce<>(); - transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME, makeRequestWithNodeDescriptions("other*"), + transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME, + new AddVotingConfigExclusionsRequest("other1", "other2"), expectError(e -> { exceptionHolder.set(e); countDownLatch.countDown(); @@ -483,10 +390,9 @@ public void testReturnsErrorIfMaximumExclusionCountExceeded() throws Interrupted 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 [other*] would add [" + newCount + - "] exclusions to the existing [" + existingCount + + assertThat(rootCause.getMessage(), equalTo("add voting config exclusions request for nodes named [other1, other2] would add [" + + newCount + "] exclusions to the existing [" + existingCount + "] which would exceed the maximum of [" + actualMaximum + "] set by [cluster.max_voting_config_exclusions]")); - assertWarnings(AddVotingConfigExclusionsRequest.DEPRECATION_MESSAGE); } public void testTimesOut() throws InterruptedException { @@ -494,8 +400,7 @@ public void testTimesOut() throws InterruptedException { final SetOnce exceptionHolder = new SetOnce<>(); transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME, - new AddVotingConfigExclusionsRequest(new String[]{"other1"}, Strings.EMPTY_ARRAY, Strings.EMPTY_ARRAY, - TimeValue.timeValueMillis(100)), + new AddVotingConfigExclusionsRequest(Strings.EMPTY_ARRAY, new String[]{"other1"}, TimeValue.timeValueMillis(100)), expectError(e -> { exceptionHolder.set(e); countDownLatch.countDown(); @@ -504,10 +409,8 @@ public void testTimesOut() throws InterruptedException { assertTrue(countDownLatch.await(30, TimeUnit.SECONDS)); final Throwable rootCause = exceptionHolder.get().getRootCause(); - assertThat(rootCause,instanceOf(ElasticsearchTimeoutException.class)); + assertThat(rootCause, instanceOf(ElasticsearchTimeoutException.class)); assertThat(rootCause.getMessage(), startsWith("timed out waiting for voting config exclusions [{other1}")); - assertWarnings(AddVotingConfigExclusionsRequest.DEPRECATION_MESSAGE); - } private TransportResponseHandler expectSuccess( @@ -598,9 +501,4 @@ public void onTimeout(TimeValue timeout) { } } - private AddVotingConfigExclusionsRequest makeRequestWithNodeDescriptions(String... nodeDescriptions) { - return new AddVotingConfigExclusionsRequest(nodeDescriptions, Strings.EMPTY_ARRAY, - Strings.EMPTY_ARRAY, TimeValue.timeValueSeconds(30)); - } - } diff --git a/server/src/test/java/org/elasticsearch/rest/action/admin/cluster/RestAddVotingConfigExclusionActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/admin/cluster/RestAddVotingConfigExclusionActionTests.java index 2713f062b4573..b91fb021ceeca 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/admin/cluster/RestAddVotingConfigExclusionActionTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/admin/cluster/RestAddVotingConfigExclusionActionTests.java @@ -25,6 +25,7 @@ import org.elasticsearch.test.rest.FakeRestRequest; import org.elasticsearch.test.rest.RestActionTestCase; import org.junit.Before; + import java.util.HashMap; import java.util.Map; @@ -39,23 +40,6 @@ public void setupAction() { controller().registerHandler(action); } - public void testResolveVotingConfigExclusionsRequest() { - Map params = new HashMap<>(); - params.put("node_name", "node-1,node-2,node-3"); - RestRequest deprecatedRequest = new FakeRestRequest.Builder(xContentRegistry()) - .withMethod(RestRequest.Method.PUT) - .withPath("/_cluster/voting_config_exclusions") - .withParams(params) - .build(); - - AddVotingConfigExclusionsRequest addVotingConfigExclusionsRequest = action.resolveVotingConfigExclusionsRequest(deprecatedRequest); - String[] expected = {"node-1","node-2", "node-3"}; - assertArrayEquals(expected, addVotingConfigExclusionsRequest.getNodeDescriptions()); - assertArrayEquals(Strings.EMPTY_ARRAY, addVotingConfigExclusionsRequest.getNodeIds()); - assertArrayEquals(Strings.EMPTY_ARRAY, addVotingConfigExclusionsRequest.getNodeNames()); - assertWarnings("nodeDescription is deprecated and will be removed, use nodeIds or nodeNames instead"); - } - public void testResolveVotingConfigExclusionsRequestNodeIds() { Map params = new HashMap<>(); params.put("node_ids", "node-1,node-2,node-3"); @@ -66,8 +50,7 @@ public void testResolveVotingConfigExclusionsRequestNodeIds() { .build(); AddVotingConfigExclusionsRequest addVotingConfigExclusionsRequest = action.resolveVotingConfigExclusionsRequest(request); - String[] expected = {"node-1","node-2", "node-3"}; - assertArrayEquals(Strings.EMPTY_ARRAY, addVotingConfigExclusionsRequest.getNodeDescriptions()); + String[] expected = {"node-1", "node-2", "node-3"}; assertArrayEquals(expected, addVotingConfigExclusionsRequest.getNodeIds()); assertArrayEquals(Strings.EMPTY_ARRAY, addVotingConfigExclusionsRequest.getNodeNames()); } @@ -82,8 +65,7 @@ public void testResolveVotingConfigExclusionsRequestNodeNames() { .build(); AddVotingConfigExclusionsRequest addVotingConfigExclusionsRequest = action.resolveVotingConfigExclusionsRequest(request); - String[] expected = {"node-1","node-2", "node-3"}; - assertArrayEquals(Strings.EMPTY_ARRAY, addVotingConfigExclusionsRequest.getNodeDescriptions()); + String[] expected = {"node-1", "node-2", "node-3"}; assertArrayEquals(Strings.EMPTY_ARRAY, addVotingConfigExclusionsRequest.getNodeIds()); assertArrayEquals(expected, addVotingConfigExclusionsRequest.getNodeNames()); }