Skip to content

Commit

Permalink
Migrate some tests that use nodeDescriptions to using nodeNames
Browse files Browse the repository at this point in the history
  • Loading branch information
@zacharymorn
zacharymorn committed Apr 8, 2020
1 parent b0d0a97 commit 5c7a226
Show file tree
Hide file tree
Showing 7 changed files with 45 additions and 58 deletions.
10 changes: 6 additions & 4 deletions ...rg/elasticsearch/action/admin/cluster/configuration/AddVotingConfigExclusionsRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,18 +54,20 @@ public class AddVotingConfigExclusionsRequest extends MasterNodeRequest<AddVotin
private final TimeValue timeout;

/**
* Construct a request to add voting config exclusions for master-eligible nodes matching the given descriptions, and wait for a
* Construct a request to add voting config exclusions for master-eligible nodes matching the given node names, and wait for a
* default 30 seconds for these exclusions to take effect, removing the nodes from the voting configuration.
* @param nodeDescriptions Descriptions of the nodes to add - see {@link DiscoveryNodes#resolveNodes(String...)}
* @param nodeNames Names of the nodes to add - see {@link AddVotingConfigExclusionsRequest#resolveVotingConfigExclusions(ClusterState)}
*/
public AddVotingConfigExclusionsRequest(String[] nodeDescriptions) {
this(nodeDescriptions, Strings.EMPTY_ARRAY, Strings.EMPTY_ARRAY, TimeValue.timeValueSeconds(30));
public AddVotingConfigExclusionsRequest(String... nodeNames) {
this(Strings.EMPTY_ARRAY, Strings.EMPTY_ARRAY, nodeNames, TimeValue.timeValueSeconds(30));
}

/**
* Construct a request to add voting config exclusions for master-eligible nodes matching the given descriptions, and wait for these
* nodes to be removed from the voting configuration.
* @param nodeDescriptions Descriptions of the nodes whose exclusions to add - see {@link DiscoveryNodes#resolveNodes(String...)}.
* @param nodeIds Ids of the nodes whose exclusions to add - see {@link AddVotingConfigExclusionsRequest#resolveVotingConfigExclusions(ClusterState)
* @param nodeNames Names of the nodes whose exclusions to add - see {@link AddVotingConfigExclusionsRequest#resolveVotingConfigExclusions(ClusterState)
* @param timeout How long to wait for the added exclusions to take effect and be removed from the voting configuration.
*/
public AddVotingConfigExclusionsRequest(String[] nodeDescriptions, String[] nodeIds, String[] nodeNames, TimeValue timeout) {
Expand Down
31 changes: 13 additions & 18 deletions ...asticsearch/action/admin/cluster/configuration/AddVotingConfigExclusionsRequestTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,8 @@ public void testSerializationForNodeIdOrNodeName() throws IOException {
assertThat(deserialized.getNodeNames(), equalTo(originalRequest.getNodeNames()));
assertThat(deserialized.getTimeout(), equalTo(originalRequest.getTimeout()));

originalRequest = new AddVotingConfigExclusionsRequest(Strings.EMPTY_ARRAY,
Strings.EMPTY_ARRAY, new String[]{"nodeName1", "nodeName2"}, TimeValue.ZERO);
deserialized = copyWriteable(originalRequest, writableRegistry(),
AddVotingConfigExclusionsRequest::new);
originalRequest = new AddVotingConfigExclusionsRequest("nodeName1", "nodeName2");
deserialized = copyWriteable(originalRequest, writableRegistry(), AddVotingConfigExclusionsRequest::new);

assertThat(deserialized.getNodeDescriptions(), equalTo(originalRequest.getNodeDescriptions()));
assertThat(deserialized.getNodeIds(), equalTo(originalRequest.getNodeIds()));
Expand Down Expand Up @@ -110,15 +108,15 @@ 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(makeRequest("_all").resolveVotingConfigExclusions(clusterState),
assertThat(makeRequestWithNodeDescriptions("_all").resolveVotingConfigExclusions(clusterState),
containsInAnyOrder(localNodeExclusion, otherNode1Exclusion, otherNode2Exclusion));
assertThat(makeRequest("_local").resolveVotingConfigExclusions(clusterState),
assertThat(makeRequestWithNodeDescriptions("_local").resolveVotingConfigExclusions(clusterState),
contains(localNodeExclusion));
assertThat(makeRequest("other*").resolveVotingConfigExclusions(clusterState),
assertThat(makeRequestWithNodeDescriptions("other*").resolveVotingConfigExclusions(clusterState),
containsInAnyOrder(otherNode1Exclusion, otherNode2Exclusion));

assertThat(expectThrows(IllegalArgumentException.class,
() -> makeRequest("not-a-node").resolveVotingConfigExclusions(clusterState)).getMessage(),
() -> 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);
}
Expand Down Expand Up @@ -230,14 +228,10 @@ public void testResolveByNodeNames() {
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, Strings.EMPTY_ARRAY,
new String[]{"nodeName1", "nodeName2"}, TimeValue.ZERO)
.resolveVotingConfigExclusions(clusterState),
assertThat(new AddVotingConfigExclusionsRequest("nodeName1", "nodeName2").resolveVotingConfigExclusions(clusterState),
containsInAnyOrder(node1Exclusion, node2Exclusion));

assertThat(new AddVotingConfigExclusionsRequest(Strings.EMPTY_ARRAY, Strings.EMPTY_ARRAY,
new String[]{"nodeName1", "unresolvableNodeName"}, TimeValue.ZERO)
.resolveVotingConfigExclusions(clusterState),
assertThat(new AddVotingConfigExclusionsRequest("nodeName1", "unresolvableNodeName").resolveVotingConfigExclusions(clusterState),
containsInAnyOrder(node1Exclusion, unresolvableVotingConfigExclusion));
}

Expand Down Expand Up @@ -316,16 +310,17 @@ public void testResolveAndCheckMaximum() {
.coordinationMetaData(CoordinationMetaData.builder().addVotingConfigExclusion(otherNode1Exclusion).build()));
final ClusterState clusterState = builder.build();

assertThat(makeRequest("_local").resolveVotingConfigExclusionsAndCheckMaximum(clusterState, 2, "setting.name"),
assertThat(makeRequestWithNodeDescriptions("_local").resolveVotingConfigExclusionsAndCheckMaximum(clusterState, 2, "setting.name"),
contains(localNodeExclusion));
assertThat(expectThrows(IllegalArgumentException.class,
() -> makeRequest("_local").resolveVotingConfigExclusionsAndCheckMaximum(clusterState, 1, "setting.name")).getMessage(),
() -> 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);
}

private static AddVotingConfigExclusionsRequest makeRequest(String... descriptions) {
return new AddVotingConfigExclusionsRequest(descriptions);
private static AddVotingConfigExclusionsRequest makeRequestWithNodeDescriptions(String... nodeDescriptions) {
return new AddVotingConfigExclusionsRequest(nodeDescriptions, Strings.EMPTY_ARRAY,
Strings.EMPTY_ARRAY, TimeValue.timeValueSeconds(30));
}
}
52 changes: 21 additions & 31 deletions ...rch/action/admin/cluster/configuration/TransportAddVotingConfigExclusionsActionTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,7 @@ public void testWithdrawsVoteFromANode() throws InterruptedException {
final CountDownLatch countDownLatch = new CountDownLatch(1);

clusterStateObserver.waitForNextChange(new AdjustConfigurationForExclusions());
transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME,
new AddVotingConfigExclusionsRequest(new String[]{"other1"}),
transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME, new AddVotingConfigExclusionsRequest("other1"),
expectSuccess(r -> {
assertNotNull(r);
countDownLatch.countDown();
Expand All @@ -162,15 +161,14 @@ public void testWithdrawsVoteFromANode() throws InterruptedException {

assertTrue(countDownLatch.await(30, TimeUnit.SECONDS));
assertThat(clusterService.getClusterApplierService().state().getVotingConfigExclusions(), contains(otherNode1Exclusion));
assertWarnings(AddVotingConfigExclusionsRequest.DEPRECATION_MESSAGE);
}

public void testWithdrawsVotesFromMultipleNodes() throws InterruptedException {
final CountDownLatch countDownLatch = new CountDownLatch(1);

clusterStateObserver.waitForNextChange(new AdjustConfigurationForExclusions());
transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME,
new AddVotingConfigExclusionsRequest(new String[]{"other1", "other2"}),
new AddVotingConfigExclusionsRequest("other1", "other2"),
expectSuccess(r -> {
assertNotNull(r);
countDownLatch.countDown();
Expand All @@ -180,15 +178,13 @@ public void testWithdrawsVotesFromMultipleNodes() throws InterruptedException {
assertTrue(countDownLatch.await(30, TimeUnit.SECONDS));
assertThat(clusterService.getClusterApplierService().state().getVotingConfigExclusions(),
containsInAnyOrder(otherNode1Exclusion, otherNode2Exclusion));
assertWarnings(AddVotingConfigExclusionsRequest.DEPRECATION_MESSAGE);
}

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*"}),
transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME, makeRequestWithNodeDescriptions("other*"),
expectSuccess(r -> {
assertNotNull(r);
countDownLatch.countDown();
Expand All @@ -205,8 +201,7 @@ public void testWithdrawsVotesFromAllMasterEligibleNodes() throws InterruptedExc
final CountDownLatch countDownLatch = new CountDownLatch(1);

clusterStateObserver.waitForNextChange(new AdjustConfigurationForExclusions());
transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME,
new AddVotingConfigExclusionsRequest(new String[]{"_all"}),
transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME, makeRequestWithNodeDescriptions("_all"),
expectSuccess(r -> {
assertNotNull(r);
countDownLatch.countDown();
Expand All @@ -223,8 +218,7 @@ 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"}),
transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME, makeRequestWithNodeDescriptions("_local"),
expectSuccess(r -> {
assertNotNull(r);
countDownLatch.countDown();
Expand All @@ -236,6 +230,7 @@ public void testWithdrawsVoteFromLocalNode() throws InterruptedException {
contains(localNodeExclusion));
assertWarnings(AddVotingConfigExclusionsRequest.DEPRECATION_MESSAGE);
}

public void testReturnsImmediatelyIfVoteAlreadyWithdrawn() throws InterruptedException {
final ClusterState state = clusterService.state();
setState(clusterService, builder(state)
Expand All @@ -248,8 +243,7 @@ public void testReturnsImmediatelyIfVoteAlreadyWithdrawn() throws InterruptedExc
final CountDownLatch countDownLatch = new CountDownLatch(1);

// no observer to reconfigure
transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME,
new AddVotingConfigExclusionsRequest(new String[]{"other1"}, Strings.EMPTY_ARRAY, Strings.EMPTY_ARRAY, TimeValue.ZERO),
transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME, new AddVotingConfigExclusionsRequest("other1"),
expectSuccess(r -> {
assertNotNull(r);
countDownLatch.countDown();
Expand All @@ -259,15 +253,13 @@ public void testReturnsImmediatelyIfVoteAlreadyWithdrawn() throws InterruptedExc
assertTrue(countDownLatch.await(30, TimeUnit.SECONDS));
assertThat(clusterService.getClusterApplierService().state().getVotingConfigExclusions(),
contains(otherNode1Exclusion));
assertWarnings(AddVotingConfigExclusionsRequest.DEPRECATION_MESSAGE);
}

public void testReturnsErrorIfNoMatchingNodes() throws InterruptedException {
public void testReturnsErrorIfNoMatchingNodeDescriptions() 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"}),
transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME, makeRequestWithNodeDescriptions("not-a-node"),
expectError(e -> {
exceptionHolder.set(e);
countDownLatch.countDown();
Expand All @@ -287,7 +279,7 @@ public void testOnlyMatchesMasterEligibleNodes() throws InterruptedException {
final SetOnce<TransportException> exceptionHolder = new SetOnce<>();

transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME,
new AddVotingConfigExclusionsRequest(new String[]{"_all", "master:false"}),
makeRequestWithNodeDescriptions("_all", "master:false"),
expectError(e -> {
exceptionHolder.set(e);
countDownLatch.countDown();
Expand Down Expand Up @@ -341,9 +333,7 @@ public void testExcludeAbsentNodesByNodeNames() throws InterruptedException {
final CountDownLatch countDownLatch = new CountDownLatch(1);

clusterStateObserver.waitForNextChange(new AdjustConfigurationForExclusions());
transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME,
new AddVotingConfigExclusionsRequest(Strings.EMPTY_ARRAY, Strings.EMPTY_ARRAY,
new String[]{"absent_node"}, TimeValue.timeValueSeconds(30)),
transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME, new AddVotingConfigExclusionsRequest("absent_node"),
expectSuccess(e -> {
countDownLatch.countDown();
})
Expand All @@ -359,8 +349,7 @@ public void testExcludeExistingNodesByNodeNames() throws InterruptedException {

clusterStateObserver.waitForNextChange(new AdjustConfigurationForExclusions());
transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME,
new AddVotingConfigExclusionsRequest(Strings.EMPTY_ARRAY, Strings.EMPTY_ARRAY,
new String[]{"other1", "other2"}, TimeValue.timeValueSeconds(30)),
new AddVotingConfigExclusionsRequest("other1", "other2"),
expectSuccess(r -> {
assertNotNull(r);
countDownLatch.countDown();
Expand All @@ -384,8 +373,7 @@ public void testSucceedsEvenIfAllExclusionsAlreadyAdded() throws InterruptedExce

final CountDownLatch countDownLatch = new CountDownLatch(1);

transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME,
new AddVotingConfigExclusionsRequest(new String[]{"other1"}),
transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME, new AddVotingConfigExclusionsRequest("other1"),
expectSuccess(r -> {
assertNotNull(r);
countDownLatch.countDown();
Expand All @@ -395,7 +383,6 @@ public void testSucceedsEvenIfAllExclusionsAlreadyAdded() throws InterruptedExce
assertTrue(countDownLatch.await(30, TimeUnit.SECONDS));
assertThat(clusterService.getClusterApplierService().state().getVotingConfigExclusions(),
contains(otherNode1Exclusion));
assertWarnings(AddVotingConfigExclusionsRequest.DEPRECATION_MESSAGE);
}

public void testExcludeByNodeIdSucceedsEvenIfAllExclusionsAlreadyAdded() throws InterruptedException {
Expand Down Expand Up @@ -436,9 +423,7 @@ public void testExcludeByNodeNameSucceedsEvenIfAllExclusionsAlreadyAdded() throw

final CountDownLatch countDownLatch = new CountDownLatch(1);

transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME,
new AddVotingConfigExclusionsRequest(Strings.EMPTY_ARRAY, Strings.EMPTY_ARRAY,
new String[]{"other1"}, TimeValue.timeValueSeconds(30)),
transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME, new AddVotingConfigExclusionsRequest("other1"),
expectSuccess(r -> {
assertNotNull(r);
countDownLatch.countDown();
Expand Down Expand Up @@ -488,8 +473,7 @@ public void testReturnsErrorIfMaximumExclusionCountExceeded() throws Interrupted
final CountDownLatch countDownLatch = new CountDownLatch(1);
final SetOnce<TransportException> exceptionHolder = new SetOnce<>();

transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME,
new AddVotingConfigExclusionsRequest(new String[]{"other*"}),
transportService.sendRequest(localNode, AddVotingConfigExclusionsAction.NAME, makeRequestWithNodeDescriptions("other*"),
expectError(e -> {
exceptionHolder.set(e);
countDownLatch.countDown();
Expand Down Expand Up @@ -601,4 +585,10 @@ public void onTimeout(TimeValue timeout) {
throw new AssertionError("unexpected timeout");
}
}

private AddVotingConfigExclusionsRequest makeRequestWithNodeDescriptions(String... nodeDescriptions) {
return new AddVotingConfigExclusionsRequest(nodeDescriptions, Strings.EMPTY_ARRAY,
Strings.EMPTY_ARRAY, TimeValue.timeValueSeconds(30));
}

}

0 comments on commit 5c7a226

Please sign in to comment.