Invoke TestCluster#assertAfterTest before closing the cluster (#122639) (#122710)

In test-scoped internal ITs the `cluster().assertAfterTest()` method was invoked
*after* the cluster nodes were closed. Consequently, the assertions that iterated
over the internal nodes (and asserted some state on nodes after the test) were
all effectively noops.
This PR reverses that order, so that after-test assertions are effective again.

Author: albertzaharovits

server/src/internalClusterTest/java/org/elasticsearch/cluster/PrevalidateNodeRemovalIT.java

@@ -179,28 +179,35 @@ public void testNodeRemovalFromRedClusterWithTimeout() throws Exception {
         // make it red!
         internalCluster().stopNode(node1);
         ensureRed(indexName);
+        CountDownLatch stallPrevalidateShardPathActionLatch = new CountDownLatch(1);
         MockTransportService.getInstance(node2)
             .addRequestHandlingBehavior(TransportPrevalidateShardPathAction.ACTION_NAME + "[n]", (handler, request, channel, task) -> {
                 logger.info("drop the check shards request");
+                safeAwait(stallPrevalidateShardPathActionLatch);
+                handler.messageReceived(request, channel, task);
             });
-        PrevalidateNodeRemovalRequest req = PrevalidateNodeRemovalRequest.builder()
-            .setNames(node2)
-            .build(TEST_REQUEST_TIMEOUT)
-            .masterNodeTimeout(TimeValue.timeValueSeconds(1))
-            .timeout(TimeValue.timeValueSeconds(1));
-        PrevalidateNodeRemovalResponse resp = client().execute(PrevalidateNodeRemovalAction.INSTANCE, req).get();
-        assertFalse("prevalidation result should return false", resp.getPrevalidation().isSafe());
-        String node2Id = getNodeId(node2);
-        assertThat(
-            resp.getPrevalidation().message(),
-            equalTo("cannot prevalidate removal of nodes with the following IDs: [" + node2Id + "]")
-        );
-        assertThat(resp.getPrevalidation().nodes().size(), equalTo(1));
-        NodesRemovalPrevalidation.NodeResult nodeResult = resp.getPrevalidation().nodes().get(0);
-        assertThat(nodeResult.name(), equalTo(node2));
-        assertFalse(nodeResult.result().isSafe());
-        assertThat(nodeResult.result().message(), startsWith("failed contacting the node"));
-        assertThat(nodeResult.result().reason(), equalTo(NodesRemovalPrevalidation.Reason.UNABLE_TO_VERIFY));
+        try {
+            PrevalidateNodeRemovalRequest req = PrevalidateNodeRemovalRequest.builder()
+                .setNames(node2)
+                .build(TEST_REQUEST_TIMEOUT)
+                .masterNodeTimeout(TimeValue.timeValueSeconds(1))
+                .timeout(TimeValue.timeValueSeconds(1));
+            PrevalidateNodeRemovalResponse resp = client().execute(PrevalidateNodeRemovalAction.INSTANCE, req).get();
+            assertFalse("prevalidation result should return false", resp.getPrevalidation().isSafe());
+            String node2Id = getNodeId(node2);
+            assertThat(
+                resp.getPrevalidation().message(),
+                equalTo("cannot prevalidate removal of nodes with the following IDs: [" + node2Id + "]")
+            );
+            assertThat(resp.getPrevalidation().nodes().size(), equalTo(1));
+            NodesRemovalPrevalidation.NodeResult nodeResult = resp.getPrevalidation().nodes().get(0);
+            assertThat(nodeResult.name(), equalTo(node2));
+            assertFalse(nodeResult.result().isSafe());
+            assertThat(nodeResult.result().message(), startsWith("failed contacting the node"));
+            assertThat(nodeResult.result().reason(), equalTo(NodesRemovalPrevalidation.Reason.UNABLE_TO_VERIFY));
+        } finally {
+            stallPrevalidateShardPathActionLatch.countDown();
+        }
     }
 
     private void ensureRed(String indexName) throws Exception {

test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java

@@ -576,10 +576,10 @@ private void afterInternal(boolean afterClass) throws Exception {
                     ensureClusterInfoServiceRunning();
                     beforeIndexDeletion();
                     cluster().wipe(excludeTemplates()); // wipe after to make sure we fail in the test that didn't ack the delete
+                    cluster().assertAfterTest();
                     if (afterClass || currentClusterScope == Scope.TEST) {
                         cluster().close();
                     }
-                    cluster().assertAfterTest();
                 }
             } finally {
                 if (currentClusterScope == Scope.TEST) {

x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/xpack/security/UnregisteredSettingsIntegTests.java

@@ -11,22 +11,28 @@
 import org.elasticsearch.test.ESIntegTestCase;
 import org.elasticsearch.test.SecurityIntegTestCase;
 
+import java.io.IOException;
+
 import static org.elasticsearch.test.NodeRoles.dataOnlyNode;
 import static org.elasticsearch.test.NodeRoles.masterNode;
 import static org.hamcrest.Matchers.containsString;
 
 @ESIntegTestCase.ClusterScope(scope = ESIntegTestCase.Scope.TEST, numDataNodes = 0, autoManageMasterNodes = false)
 public class UnregisteredSettingsIntegTests extends SecurityIntegTestCase {
 
-    public void testIncludeReservedRolesSettingNotRegistered() {
+    public void testIncludeReservedRolesSettingNotRegistered() throws IOException {
         internalCluster().setBootstrapMasterNodeIndex(0);
 
         final Settings.Builder builder = Settings.builder()
             .put(randomBoolean() ? masterNode() : dataOnlyNode())
             .putList("xpack.security.reserved_roles.include", "superuser");
 
-        final IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> internalCluster().startNode(builder));
-        assertThat(e.getMessage(), containsString("unknown setting [xpack.security.reserved_roles.include]"));
+        try {
+            final IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> internalCluster().startNode(builder));
+            assertThat(e.getMessage(), containsString("unknown setting [xpack.security.reserved_roles.include]"));
+        } finally {
+            internalCluster().close();
+        }
     }
 
     public void testSamlExcludeRolesSettingNotRegistered() throws Exception {
@@ -36,7 +42,11 @@ public void testSamlExcludeRolesSettingNotRegistered() throws Exception {
             .put(randomBoolean() ? masterNode() : dataOnlyNode())
             .putList("xpack.security.authc.realms.saml.saml1.exclude_roles", "superuser");
 
-        final IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> internalCluster().startNode(builder));
-        assertThat(e.getMessage(), containsString("unknown setting [xpack.security.authc.realms.saml.saml1.exclude_roles]"));
+        try {
+            final IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> internalCluster().startNode(builder));
+            assertThat(e.getMessage(), containsString("unknown setting [xpack.security.authc.realms.saml.saml1.exclude_roles]"));
+        } finally {
+            internalCluster().close();
+        }
     }
 }