Skip to content

Cleanup Transport*VotingConfigExclusionsActionTests #98146

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 6, 2023
Merged

Cleanup Transport*VotingConfigExclusionsActionTests #98146

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
167 changes: 75 additions & 92 deletions ...rch/action/admin/cluster/configuration/TransportAddVotingConfigExclusionsActionTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
*/
package org.elasticsearch.action.admin.cluster.configuration;

import org.apache.lucene.util.SetOnce;
import org.elasticsearch.ElasticsearchTimeoutException;
import org.elasticsearch.action.ActionResponse;
import org.elasticsearch.action.support.ActionFilters;
Expand Down Expand Up @@ -47,7 +46,6 @@
import java.util.Set;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.Executor;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Consumer;

Expand Down Expand Up @@ -162,7 +160,7 @@ public void setupForTest() {
clusterStateObserver = new ClusterStateObserver(clusterService, null, logger, threadPool.getThreadContext());
}

public void testWithdrawsVoteFromANode() throws InterruptedException {
public void testWithdrawsVoteFromANode() {
final CountDownLatch countDownLatch = new CountDownLatch(2);

clusterStateObserver.waitForNextChange(new AdjustConfigurationForExclusions(countDownLatch));
Expand All @@ -172,15 +170,14 @@ public void testWithdrawsVoteFromANode() throws InterruptedException {
new AddVotingConfigExclusionsRequest("other1"),
expectSuccess(r -> {
assertNotNull(r);
assertThat(clusterService.getClusterApplierService().state().getVotingConfigExclusions(), contains(otherNode1Exclusion));
countDownLatch.countDown();
})
);

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

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

clusterStateObserver.waitForNextChange(new AdjustConfigurationForExclusions(countDownLatch));
Expand All @@ -190,18 +187,17 @@ public void testWithdrawsVotesFromMultipleNodes() throws InterruptedException {
new AddVotingConfigExclusionsRequest("other1", "other2"),
expectSuccess(r -> {
assertNotNull(r);
assertThat(
clusterService.getClusterApplierService().state().getVotingConfigExclusions(),
containsInAnyOrder(otherNode1Exclusion, otherNode2Exclusion)
);
countDownLatch.countDown();
})
);

assertTrue(countDownLatch.await(30, TimeUnit.SECONDS));
assertThat(
clusterService.getClusterApplierService().state().getVotingConfigExclusions(),
containsInAnyOrder(otherNode1Exclusion, otherNode2Exclusion)
);
safeAwait(countDownLatch);
}

public void testReturnsImmediatelyIfVoteAlreadyWithdrawn() throws InterruptedException {
public void testReturnsImmediatelyIfVoteAlreadyWithdrawn() {
final ClusterState state = clusterService.state();
setState(
clusterService,
Expand All @@ -225,33 +221,33 @@ public void testReturnsImmediatelyIfVoteAlreadyWithdrawn() throws InterruptedExc
new AddVotingConfigExclusionsRequest("other1"),
expectSuccess(r -> {
assertNotNull(r);
assertThat(clusterService.getClusterApplierService().state().getVotingConfigExclusions(), contains(otherNode1Exclusion));
countDownLatch.countDown();
})
);

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

public void testExcludeAbsentNodesByNodeIds() throws InterruptedException {
public void testExcludeAbsentNodesByNodeIds() {
final CountDownLatch countDownLatch = new CountDownLatch(2);

clusterStateObserver.waitForNextChange(new AdjustConfigurationForExclusions(countDownLatch));
transportService.sendRequest(
localNode,
AddVotingConfigExclusionsAction.NAME,
new AddVotingConfigExclusionsRequest(new String[] { "absent_id" }, Strings.EMPTY_ARRAY, TimeValue.timeValueSeconds(30)),
expectSuccess(e -> countDownLatch.countDown())
);

assertTrue(countDownLatch.await(30, TimeUnit.SECONDS));
assertEquals(
Set.of(new VotingConfigExclusion("absent_id", VotingConfigExclusion.MISSING_VALUE_MARKER)),
clusterService.getClusterApplierService().state().getVotingConfigExclusions()
expectSuccess(e -> {
assertEquals(
Set.of(new VotingConfigExclusion("absent_id", VotingConfigExclusion.MISSING_VALUE_MARKER)),
clusterService.getClusterApplierService().state().getVotingConfigExclusions()
);
countDownLatch.countDown();
})
);
safeAwait(countDownLatch);
}

public void testExcludeExistingNodesByNodeIds() throws InterruptedException {
public void testExcludeExistingNodesByNodeIds() {
final CountDownLatch countDownLatch = new CountDownLatch(2);

clusterStateObserver.waitForNextChange(new AdjustConfigurationForExclusions(countDownLatch));
Expand All @@ -261,36 +257,36 @@ public void testExcludeExistingNodesByNodeIds() throws InterruptedException {
new AddVotingConfigExclusionsRequest(new String[] { "other1", "other2" }, Strings.EMPTY_ARRAY, TimeValue.timeValueSeconds(30)),
expectSuccess(r -> {
assertNotNull(r);
assertThat(
clusterService.getClusterApplierService().state().getVotingConfigExclusions(),
containsInAnyOrder(otherNode1Exclusion, otherNode2Exclusion)
);
countDownLatch.countDown();
})
);

assertTrue(countDownLatch.await(30, TimeUnit.SECONDS));
assertThat(
clusterService.getClusterApplierService().state().getVotingConfigExclusions(),
containsInAnyOrder(otherNode1Exclusion, otherNode2Exclusion)
);
safeAwait(countDownLatch);
}

public void testExcludeAbsentNodesByNodeNames() throws InterruptedException {
public void testExcludeAbsentNodesByNodeNames() {
final CountDownLatch countDownLatch = new CountDownLatch(2);

clusterStateObserver.waitForNextChange(new AdjustConfigurationForExclusions(countDownLatch));
transportService.sendRequest(
localNode,
AddVotingConfigExclusionsAction.NAME,
new AddVotingConfigExclusionsRequest("absent_node"),
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()
expectSuccess(e -> {
assertEquals(
Set.of(new VotingConfigExclusion(VotingConfigExclusion.MISSING_VALUE_MARKER, "absent_node")),
clusterService.getClusterApplierService().state().getVotingConfigExclusions()
);
countDownLatch.countDown();
})
);
safeAwait(countDownLatch);
}

public void testExcludeExistingNodesByNodeNames() throws InterruptedException {
public void testExcludeExistingNodesByNodeNames() {
final CountDownLatch countDownLatch = new CountDownLatch(2);

clusterStateObserver.waitForNextChange(new AdjustConfigurationForExclusions(countDownLatch));
Expand All @@ -300,18 +296,17 @@ public void testExcludeExistingNodesByNodeNames() throws InterruptedException {
new AddVotingConfigExclusionsRequest("other1", "other2"),
expectSuccess(r -> {
assertNotNull(r);
assertThat(
clusterService.getClusterApplierService().state().getVotingConfigExclusions(),
containsInAnyOrder(otherNode1Exclusion, otherNode2Exclusion)
);
countDownLatch.countDown();
})
);

assertTrue(countDownLatch.await(30, TimeUnit.SECONDS));
assertThat(
clusterService.getClusterApplierService().state().getVotingConfigExclusions(),
containsInAnyOrder(otherNode1Exclusion, otherNode2Exclusion)
);
safeAwait(countDownLatch);
}

public void testSucceedsEvenIfAllExclusionsAlreadyAdded() throws InterruptedException {
public void testSucceedsEvenIfAllExclusionsAlreadyAdded() {
final ClusterState state = clusterService.state();
final ClusterState.Builder builder = builder(state);
builder.metadata(
Expand All @@ -330,15 +325,14 @@ public void testSucceedsEvenIfAllExclusionsAlreadyAdded() throws InterruptedExce
new AddVotingConfigExclusionsRequest("other1"),
expectSuccess(r -> {
assertNotNull(r);
assertThat(clusterService.getClusterApplierService().state().getVotingConfigExclusions(), contains(otherNode1Exclusion));
countDownLatch.countDown();
})
);

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

public void testExcludeByNodeIdSucceedsEvenIfAllExclusionsAlreadyAdded() throws InterruptedException {
public void testExcludeByNodeIdSucceedsEvenIfAllExclusionsAlreadyAdded() {
final ClusterState state = clusterService.state();
final ClusterState.Builder builder = builder(state);
builder.metadata(
Expand All @@ -357,15 +351,14 @@ public void testExcludeByNodeIdSucceedsEvenIfAllExclusionsAlreadyAdded() throws
new AddVotingConfigExclusionsRequest(new String[] { "other1" }, Strings.EMPTY_ARRAY, TimeValue.timeValueSeconds(30)),
expectSuccess(r -> {
assertNotNull(r);
assertThat(clusterService.getClusterApplierService().state().getVotingConfigExclusions(), contains(otherNode1Exclusion));
countDownLatch.countDown();
})
);

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

public void testExcludeByNodeNameSucceedsEvenIfAllExclusionsAlreadyAdded() throws InterruptedException {
public void testExcludeByNodeNameSucceedsEvenIfAllExclusionsAlreadyAdded() {
final ClusterState state = clusterService.state();
final ClusterState.Builder builder = builder(state);
builder.metadata(
Expand All @@ -384,15 +377,14 @@ public void testExcludeByNodeNameSucceedsEvenIfAllExclusionsAlreadyAdded() throw
new AddVotingConfigExclusionsRequest("other1"),
expectSuccess(r -> {
assertNotNull(r);
assertThat(clusterService.getClusterApplierService().state().getVotingConfigExclusions(), contains(otherNode1Exclusion));
countDownLatch.countDown();
})
);

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

public void testReturnsErrorIfMaximumExclusionCountExceeded() throws InterruptedException {
public void testReturnsErrorIfMaximumExclusionCountExceeded() {
final Metadata.Builder metadataBuilder = Metadata.builder(clusterService.state().metadata());
CoordinationMetadata.Builder coordinationMetadataBuilder = CoordinationMetadata.builder(
clusterService.state().coordinationMetadata()
Expand Down Expand Up @@ -433,58 +425,51 @@ public void testReturnsErrorIfMaximumExclusionCountExceeded() throws Interrupted
setState(clusterService, builder);

final CountDownLatch countDownLatch = new CountDownLatch(1);
final SetOnce<TransportException> exceptionHolder = new SetOnce<>();

transportService.sendRequest(
localNode,
AddVotingConfigExclusionsAction.NAME,
new AddVotingConfigExclusionsRequest("other1", "other2"),
expectError(e -> {
exceptionHolder.set(e);
final Throwable rootCause = e.getRootCause();
assertThat(rootCause, instanceOf(IllegalArgumentException.class));
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]"
)
);
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 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]"
)
);
safeAwait(countDownLatch);
}

public void testTimesOut() throws InterruptedException {
public void testTimesOut() {
final CountDownLatch countDownLatch = new CountDownLatch(1);
final SetOnce<TransportException> exceptionHolder = new SetOnce<>();

transportService.sendRequest(
localNode,
AddVotingConfigExclusionsAction.NAME,
new AddVotingConfigExclusionsRequest(Strings.EMPTY_ARRAY, new String[] { "other1" }, TimeValue.timeValueMillis(100)),
expectError(e -> {
exceptionHolder.set(e);
final Throwable rootCause = e.getRootCause();
assertThat(rootCause, instanceOf(ElasticsearchTimeoutException.class));
assertThat(rootCause.getMessage(), startsWith("timed out waiting for voting config exclusions [{other1}"));
countDownLatch.countDown();
})
);

assertTrue(countDownLatch.await(30, TimeUnit.SECONDS));
final Throwable rootCause = exceptionHolder.get().getRootCause();
assertThat(rootCause, instanceOf(ElasticsearchTimeoutException.class));
assertThat(rootCause.getMessage(), startsWith("timed out waiting for voting config exclusions [{other1}"));
safeAwait(countDownLatch);
}

public void testCannotAddVotingConfigExclusionsWhenItIsDisabled() {
final CountDownLatch countDownLatch = new CountDownLatch(1);
final SetOnce<TransportException> exceptionHolder = new SetOnce<>();

reconfigurator.disableUserVotingConfigModifications();

Expand All @@ -493,15 +478,13 @@ public void testCannotAddVotingConfigExclusionsWhenItIsDisabled() {
AddVotingConfigExclusionsAction.NAME,
new AddVotingConfigExclusionsRequest(Strings.EMPTY_ARRAY, new String[] { "other1" }, TimeValue.timeValueMillis(100)),
expectError(e -> {
exceptionHolder.set(e);
final Throwable rootCause = e.getRootCause();
assertThat(rootCause, instanceOf(IllegalStateException.class));
assertThat(rootCause.getMessage(), startsWith("Unable to modify the voting configuration"));
countDownLatch.countDown();
})
);

safeAwait(countDownLatch);
final Throwable rootCause = exceptionHolder.get().getRootCause();
assertThat(rootCause, instanceOf(IllegalStateException.class));
assertThat(rootCause.getMessage(), startsWith("Unable to modify the voting configuration"));
}

private TransportResponseHandler<ActionResponse.Empty> expectSuccess(Consumer<ActionResponse.Empty> onResponse) {
Expand Down
Loading