Skip to content

Commit

Permalink
Avoid closing InternalTestCluster (#102493)
Browse files Browse the repository at this point in the history 
We have a few `ESIntegTestCase` tests which explicitly close the test
cluster at the end. Test code itself shouldn't close this cluster: these
cases all run with cluster scope `TEST` so the test framework closes the
cluster for us anyway. This commit removes the unwanted closing.
  • Loading branch information
@DaveCTurner
DaveCTurner authored Nov 23, 2023
1 parent 484bde9 commit dee5b6f
Show file tree
Hide file tree
Showing 7 changed files with 594 additions and 619 deletions.
363 changes: 180 additions & 183 deletions server/src/internalClusterTest/java/org/elasticsearch/health/HealthMetadataServiceIT.java
View file

Large diffs are not rendered by default.

57 changes: 28 additions & 29 deletions server/src/internalClusterTest/java/org/elasticsearch/health/HealthServiceIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,35 +52,34 @@ protected Settings nodeSettings(int ordinal, Settings otherSettings) {
}

public void testThatHealthNodeDataIsFetchedAndPassedToIndicators() throws Exception {
try (InternalTestCluster internalCluster = internalCluster()) {
ensureStableCluster(internalCluster.getNodeNames().length);
waitForAllNodesToReportHealth();
for (String node : internalCluster.getNodeNames()) {
HealthService healthService = internalCluster.getInstance(HealthService.class, node);
AtomicBoolean onResponseCalled = new AtomicBoolean(false);
ActionListener<List<HealthIndicatorResult>> listener = new ActionListener<>() {
@Override
public void onResponse(List<HealthIndicatorResult> resultList) {
/*
* The following is really just asserting that the TestHealthIndicatorService's calculate method was called. The
* assertions that it actually got the HealthInfo data are in the calculate method of TestHealthIndicatorService.
*/
assertNotNull(resultList);
assertThat(resultList.size(), equalTo(1));
HealthIndicatorResult testIndicatorResult = resultList.get(0);
assertThat(testIndicatorResult.status(), equalTo(HealthStatus.RED));
assertThat(testIndicatorResult.symptom(), equalTo(TestHealthIndicatorService.SYMPTOM));
onResponseCalled.set(true);
}

@Override
public void onFailure(Exception e) {
throw new RuntimeException(e);
}
};
healthService.getHealth(internalCluster.client(node), TestHealthIndicatorService.NAME, true, 1000, listener);
assertBusy(() -> assertThat(onResponseCalled.get(), equalTo(true)));
}
final InternalTestCluster internalCluster = internalCluster();
ensureStableCluster(internalCluster.getNodeNames().length);
waitForAllNodesToReportHealth();
for (String node : internalCluster.getNodeNames()) {
HealthService healthService = internalCluster.getInstance(HealthService.class, node);
AtomicBoolean onResponseCalled = new AtomicBoolean(false);
ActionListener<List<HealthIndicatorResult>> listener = new ActionListener<>() {
@Override
public void onResponse(List<HealthIndicatorResult> resultList) {
/*
* The following is really just asserting that the TestHealthIndicatorService's calculate method was called. The
* assertions that it actually got the HealthInfo data are in the calculate method of TestHealthIndicatorService.
*/
assertNotNull(resultList);
assertThat(resultList.size(), equalTo(1));
HealthIndicatorResult testIndicatorResult = resultList.get(0);
assertThat(testIndicatorResult.status(), equalTo(HealthStatus.RED));
assertThat(testIndicatorResult.symptom(), equalTo(TestHealthIndicatorService.SYMPTOM));
onResponseCalled.set(true);
}

@Override
public void onFailure(Exception e) {
throw new RuntimeException(e);
}
};
healthService.getHealth(internalCluster.client(node), TestHealthIndicatorService.NAME, true, 1000, listener);
assertBusy(() -> assertThat(onResponseCalled.get(), equalTo(true)));
}
}

Expand Down
111 changes: 49 additions & 62 deletions server/src/internalClusterTest/java/org/elasticsearch/health/UpdateHealthInfoCacheIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import org.elasticsearch.test.InternalTestCluster;
import org.elasticsearch.test.junit.annotations.TestLogging;

import java.io.IOException;
import java.util.Collection;
import java.util.List;
import java.util.Map;
Expand All @@ -35,78 +34,66 @@ public class UpdateHealthInfoCacheIT extends ESIntegTestCase {
private static final DiskHealthInfo GREEN = new DiskHealthInfo(HealthStatus.GREEN, null);

public void testNodesReportingHealth() throws Exception {
try (InternalTestCluster internalCluster = internalCluster()) {
decreasePollingInterval(internalCluster);
String[] nodeIds = getNodes(internalCluster).keySet().toArray(new String[0]);
DiscoveryNode healthNode = waitAndGetHealthNode(internalCluster);
assertThat(healthNode, notNullValue());
assertBusy(() -> assertResultsCanBeFetched(internalCluster, healthNode, List.of(nodeIds), null));
} catch (IOException e) {
throw new RuntimeException("Failed to close internal cluster: " + e.getMessage(), e);
}
final InternalTestCluster internalCluster = internalCluster();
decreasePollingInterval(internalCluster);
String[] nodeIds = getNodes(internalCluster).keySet().toArray(new String[0]);
DiscoveryNode healthNode = waitAndGetHealthNode(internalCluster);
assertThat(healthNode, notNullValue());
assertBusy(() -> assertResultsCanBeFetched(internalCluster, healthNode, List.of(nodeIds), null));
}

public void testNodeLeavingCluster() throws Exception {
try (InternalTestCluster internalCluster = internalCluster()) {
decreasePollingInterval(internalCluster);
Collection<DiscoveryNode> nodes = getNodes(internalCluster).values();
DiscoveryNode healthNode = waitAndGetHealthNode(internalCluster);
assertThat(healthNode, notNullValue());
DiscoveryNode nodeToLeave = nodes.stream().filter(node -> {
boolean isMaster = node.getName().equals(internalCluster.getMasterName());
boolean isHealthNode = node.getId().equals(healthNode.getId());
// We have dedicated tests for master and health node
return isMaster == false && isHealthNode == false;
}).findAny().orElseThrow();
internalCluster.stopNode(nodeToLeave.getName());
assertBusy(
() -> assertResultsCanBeFetched(
internalCluster,
healthNode,
nodes.stream().filter(node -> node.equals(nodeToLeave) == false).map(DiscoveryNode::getId).toList(),
nodeToLeave.getId()
)
);
} catch (IOException e) {
throw new RuntimeException("Failed to close internal cluster: " + e.getMessage(), e);
}
final InternalTestCluster internalCluster = internalCluster();
decreasePollingInterval(internalCluster);
Collection<DiscoveryNode> nodes = getNodes(internalCluster).values();
DiscoveryNode healthNode = waitAndGetHealthNode(internalCluster);
assertThat(healthNode, notNullValue());
DiscoveryNode nodeToLeave = nodes.stream().filter(node -> {
boolean isMaster = node.getName().equals(internalCluster.getMasterName());
boolean isHealthNode = node.getId().equals(healthNode.getId());
// We have dedicated tests for master and health node
return isMaster == false && isHealthNode == false;
}).findAny().orElseThrow();
internalCluster.stopNode(nodeToLeave.getName());
assertBusy(
() -> assertResultsCanBeFetched(
internalCluster,
healthNode,
nodes.stream().filter(node -> node.equals(nodeToLeave) == false).map(DiscoveryNode::getId).toList(),
nodeToLeave.getId()
)
);
}

@TestLogging(value = "org.elasticsearch.health.node:DEBUG", reason = "https://github.com/elastic/elasticsearch/issues/97213")
public void testHealthNodeFailOver() throws Exception {
try (InternalTestCluster internalCluster = internalCluster()) {
decreasePollingInterval(internalCluster);
String[] nodeIds = getNodes(internalCluster).keySet().toArray(new String[0]);
DiscoveryNode healthNodeToBeShutDown = waitAndGetHealthNode(internalCluster);
assertThat(healthNodeToBeShutDown, notNullValue());
internalCluster.restartNode(healthNodeToBeShutDown.getName());
ensureStableCluster(nodeIds.length);
DiscoveryNode newHealthNode = waitAndGetHealthNode(internalCluster);
assertThat(newHealthNode, notNullValue());
logger.info("Previous health node {}, new health node {}.", healthNodeToBeShutDown, newHealthNode);
assertBusy(() -> assertResultsCanBeFetched(internalCluster, newHealthNode, List.of(nodeIds), null));
} catch (IOException e) {
throw new RuntimeException("Failed to close internal cluster: " + e.getMessage(), e);
}
final InternalTestCluster internalCluster = internalCluster();
decreasePollingInterval(internalCluster);
String[] nodeIds = getNodes(internalCluster).keySet().toArray(new String[0]);
DiscoveryNode healthNodeToBeShutDown = waitAndGetHealthNode(internalCluster);
assertThat(healthNodeToBeShutDown, notNullValue());
internalCluster.restartNode(healthNodeToBeShutDown.getName());
ensureStableCluster(nodeIds.length);
DiscoveryNode newHealthNode = waitAndGetHealthNode(internalCluster);
assertThat(newHealthNode, notNullValue());
logger.info("Previous health node {}, new health node {}.", healthNodeToBeShutDown, newHealthNode);
assertBusy(() -> assertResultsCanBeFetched(internalCluster, newHealthNode, List.of(nodeIds), null));
}

@TestLogging(value = "org.elasticsearch.health.node:DEBUG", reason = "https://github.com/elastic/elasticsearch/issues/97213")
public void testMasterFailure() throws Exception {
try (InternalTestCluster internalCluster = internalCluster()) {
decreasePollingInterval(internalCluster);
String[] nodeIds = getNodes(internalCluster).keySet().toArray(new String[0]);
DiscoveryNode healthNodeBeforeIncident = waitAndGetHealthNode(internalCluster);
assertThat(healthNodeBeforeIncident, notNullValue());
String masterName = internalCluster.getMasterName();
logger.info("Restarting elected master node {}.", masterName);
internalCluster.restartNode(masterName);
ensureStableCluster(nodeIds.length);
DiscoveryNode newHealthNode = waitAndGetHealthNode(internalCluster);
assertThat(newHealthNode, notNullValue());
assertBusy(() -> assertResultsCanBeFetched(internalCluster, newHealthNode, List.of(nodeIds), null));
} catch (IOException e) {
throw new RuntimeException("Failed to close internal cluster: " + e.getMessage(), e);
}
final InternalTestCluster internalCluster = internalCluster();
decreasePollingInterval(internalCluster);
String[] nodeIds = getNodes(internalCluster).keySet().toArray(new String[0]);
DiscoveryNode healthNodeBeforeIncident = waitAndGetHealthNode(internalCluster);
assertThat(healthNodeBeforeIncident, notNullValue());
String masterName = internalCluster.getMasterName();
logger.info("Restarting elected master node {}.", masterName);
internalCluster.restartNode(masterName);
ensureStableCluster(nodeIds.length);
DiscoveryNode newHealthNode = waitAndGetHealthNode(internalCluster);
assertThat(newHealthNode, notNullValue());
assertBusy(() -> assertResultsCanBeFetched(internalCluster, newHealthNode, List.of(nodeIds), null));
}

/**
Expand Down
61 changes: 29 additions & 32 deletions .../internalClusterTest/java/org/elasticsearch/health/node/DiskHealthIndicatorServiceIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import org.elasticsearch.health.HealthStatus;
import org.elasticsearch.health.node.selection.HealthNode;
import org.elasticsearch.test.ESIntegTestCase;
import org.elasticsearch.test.InternalTestCluster;

import java.util.List;
import java.util.Map;
Expand All @@ -30,41 +29,39 @@
public class DiskHealthIndicatorServiceIT extends ESIntegTestCase {

public void testGreen() throws Exception {
try (InternalTestCluster internalCluster = internalCluster()) {
internalCluster.startMasterOnlyNode();
internalCluster.startDataOnlyNode();
ensureStableCluster(internalCluster.getNodeNames().length);
waitForAllNodesToReportHealth();
for (String node : internalCluster.getNodeNames()) {
HealthService healthService = internalCluster.getInstance(HealthService.class, node);
List<HealthIndicatorResult> resultList = getHealthServiceResults(healthService, node);
assertNotNull(resultList);
assertThat(resultList.size(), equalTo(1));
HealthIndicatorResult testIndicatorResult = resultList.get(0);
assertThat(testIndicatorResult.status(), equalTo(HealthStatus.GREEN));
assertThat(testIndicatorResult.symptom(), equalTo("The cluster has enough available disk space."));
}
final var internalCluster = internalCluster();
internalCluster.startMasterOnlyNode();
internalCluster.startDataOnlyNode();
ensureStableCluster(internalCluster.getNodeNames().length);
waitForAllNodesToReportHealth();
for (String node : internalCluster.getNodeNames()) {
HealthService healthService = internalCluster.getInstance(HealthService.class, node);
List<HealthIndicatorResult> resultList = getHealthServiceResults(healthService, node);
assertNotNull(resultList);
assertThat(resultList.size(), equalTo(1));
HealthIndicatorResult testIndicatorResult = resultList.get(0);
assertThat(testIndicatorResult.status(), equalTo(HealthStatus.GREEN));
assertThat(testIndicatorResult.symptom(), equalTo("The cluster has enough available disk space."));
}
}

public void testRed() throws Exception {
try (InternalTestCluster internalCluster = internalCluster()) {
internalCluster.startMasterOnlyNode(getVeryLowWatermarksSettings());
internalCluster.startDataOnlyNode(getVeryLowWatermarksSettings());
ensureStableCluster(internalCluster.getNodeNames().length);
waitForAllNodesToReportHealth();
for (String node : internalCluster.getNodeNames()) {
HealthService healthService = internalCluster.getInstance(HealthService.class, node);
List<HealthIndicatorResult> resultList = getHealthServiceResults(healthService, node);
assertNotNull(resultList);
assertThat(resultList.size(), equalTo(1));
HealthIndicatorResult testIndicatorResult = resultList.get(0);
assertThat(testIndicatorResult.status(), equalTo(HealthStatus.RED));
assertThat(
testIndicatorResult.symptom(),
equalTo("2 nodes with roles: [data, master] are out of disk or running low on disk space.")
);
}
final var internalCluster = internalCluster();
internalCluster.startMasterOnlyNode(getVeryLowWatermarksSettings());
internalCluster.startDataOnlyNode(getVeryLowWatermarksSettings());
ensureStableCluster(internalCluster.getNodeNames().length);
waitForAllNodesToReportHealth();
for (String node : internalCluster.getNodeNames()) {
HealthService healthService = internalCluster.getInstance(HealthService.class, node);
List<HealthIndicatorResult> resultList = getHealthServiceResults(healthService, node);
assertNotNull(resultList);
assertThat(resultList.size(), equalTo(1));
HealthIndicatorResult testIndicatorResult = resultList.get(0);
assertThat(testIndicatorResult.status(), equalTo(HealthStatus.RED));
assertThat(
testIndicatorResult.symptom(),
equalTo("2 nodes with roles: [data, master] are out of disk or running low on disk space.")
);
}
}

Expand Down
Loading

0 comments on commit dee5b6f

Please sign in to comment.