Remove usage of max_local_storage_nodes in test infrastructure (#41652)

Moves the test infrastructure away from using node.max_local_storage_nodes, allowing us in a
follow-up PR to deprecate this setting in 7.x and to remove it in 8.0.

This also changes the behavior of InternalTestCluster so that starting up nodes will not automatically
reuse data folders of previously stopped nodes. If this behavior is desired, it needs to be explicitly
done by passing the data path from the stopped node to the new node that is started.

Author: ywelsch

buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterFormationTasks.groovy

@@ -382,7 +382,6 @@ class ClusterFormationTasks {
                 // Don't wait for state, just start up quickly. This will also allow new and old nodes in the BWC case to become the master
                 'discovery.initial_state_timeout' : '0s'
         ]
-        esConfig['node.max_local_storage_nodes'] = node.config.numNodes
         esConfig['http.port'] = node.config.httpPort
         if (node.nodeVersion.onOrAfter('6.7.0')) {
             esConfig['transport.port'] =  node.config.transportPort

modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/IngestRestartIT.java

@@ -91,9 +91,15 @@ public void testScriptDisabled() throws Exception {
         checkPipelineExists.accept(pipelineIdWithoutScript);
 
 
-        internalCluster().stopCurrentMasterNode();
-        internalCluster().startNode(Settings.builder().put("script.allowed_types", "none"));
+        internalCluster().restartNode(internalCluster().getMasterName(), new InternalTestCluster.RestartCallback() {
 
+            @Override
+            public Settings onNodeStopped(String nodeName) {
+                return Settings.builder().put("script.allowed_types", "none").build();
+            }
+
+        });
+        
         checkPipelineExists.accept(pipelineIdWithoutScript);
         checkPipelineExists.accept(pipelineIdWithScript);
 

server/src/test/java/org/elasticsearch/action/admin/cluster/allocation/ClusterAllocationExplainIT.java

@@ -277,6 +277,8 @@ public void testUnassignedReplicaWithPriorCopy() throws Exception {
         nodes.remove(primaryNodeName);
 
         logger.info("--> shutting down all nodes except the one that holds the primary");
+        Settings node0DataPathSettings = internalCluster().dataPathSettings(nodes.get(0));
+        Settings node1DataPathSettings = internalCluster().dataPathSettings(nodes.get(1));
         internalCluster().stopRandomNode(InternalTestCluster.nameFilter(nodes.get(0)));
         internalCluster().stopRandomNode(InternalTestCluster.nameFilter(nodes.get(1)));
         ensureStableCluster(1);
@@ -286,8 +288,8 @@ public void testUnassignedReplicaWithPriorCopy() throws Exception {
             Settings.builder().put("index.routing.allocation.include._name", primaryNodeName)).get();
 
         logger.info("--> restarting the stopped nodes");
-        internalCluster().startNode(Settings.builder().put("node.name", nodes.get(0)).build());
-        internalCluster().startNode(Settings.builder().put("node.name", nodes.get(1)).build());
+        internalCluster().startNode(Settings.builder().put("node.name", nodes.get(0)).put(node0DataPathSettings).build());
+        internalCluster().startNode(Settings.builder().put("node.name", nodes.get(1)).put(node1DataPathSettings).build());
         ensureStableCluster(3);
 
         boolean includeYesDecisions = randomBoolean();
@@ -1017,6 +1019,7 @@ public void testCannotAllocateStaleReplicaExplanation() throws Exception {
         // start replica node first, so it's path will be used first when we start a node after
         // stopping all of them at end of test.
         final String replicaNode = internalCluster().startNode();
+        Settings replicaDataPathSettings = internalCluster().dataPathSettings(replicaNode);
         final String primaryNode = internalCluster().startNode();
 
         prepareIndex(IndexMetaData.State.OPEN, 1, 1,
@@ -1057,7 +1060,7 @@ public void testCannotAllocateStaleReplicaExplanation() throws Exception {
         internalCluster().stopRandomNode(InternalTestCluster.nameFilter(primaryNode));
 
         logger.info("--> restart the node with the stale replica");
-        String restartedNode = internalCluster().startDataOnlyNode();
+        String restartedNode = internalCluster().startDataOnlyNode(replicaDataPathSettings);
         ensureClusterSizeConsistency(); // wait for the master to finish processing join.
 
         // wait until the system has fetched shard data and we know there is no valid shard copy

server/src/test/java/org/elasticsearch/cluster/ClusterInfoServiceIT.java

@@ -36,7 +36,6 @@
 import org.elasticsearch.common.collect.ImmutableOpenMap;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.unit.TimeValue;
-import org.elasticsearch.env.NodeEnvironment;
 import org.elasticsearch.index.IndexService;
 import org.elasticsearch.index.shard.IndexShard;
 import org.elasticsearch.index.store.Store;
@@ -105,15 +104,6 @@ public void blockActions(String... actions) {
         }
     }
 
-    @Override
-    protected Settings nodeSettings(int nodeOrdinal) {
-        return Settings.builder()
-            .put(super.nodeSettings(nodeOrdinal))
-            // manual collection or upon cluster forming.
-            .put(NodeEnvironment.MAX_LOCAL_STORAGE_NODES_SETTING.getKey(), 2)
-            .build();
-    }
-
     @Override
     protected Collection<Class<? extends Plugin>> nodePlugins() {
         return Arrays.asList(TestPlugin.class, MockTransportService.TestPlugin.class);

server/src/test/java/org/elasticsearch/cluster/MinimumMasterNodesIT.java

@@ -32,6 +32,7 @@
 import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.Priority;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.util.set.Sets;
 import org.elasticsearch.index.query.QueryBuilders;
 import org.elasticsearch.plugins.Plugin;
 import org.elasticsearch.test.ESIntegTestCase;
@@ -43,10 +44,12 @@
 import org.elasticsearch.test.junit.annotations.TestLogging;
 import org.elasticsearch.test.transport.MockTransportService;
 
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Set;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.atomic.AtomicReference;
@@ -124,6 +127,7 @@ public void testTwoNodesNoMasterBlock() throws Exception {
         logger.info("--> add voting config exclusion for non-master node, to be sure it's not elected");
         client().execute(AddVotingConfigExclusionsAction.INSTANCE, new AddVotingConfigExclusionsRequest(new String[]{otherNode})).get();
         logger.info("--> stop master node, no master block should appear");
+        Settings masterDataPathSettings = internalCluster().dataPathSettings(masterNode);
         internalCluster().stopRandomNode(InternalTestCluster.nameFilter(masterNode));
 
         awaitBusy(() -> {
@@ -137,7 +141,7 @@ public void testTwoNodesNoMasterBlock() throws Exception {
         assertThat(state.nodes().getMasterNode(), equalTo(null));
 
         logger.info("--> starting the previous master node again...");
-        node2Name = internalCluster().startNode(settings);
+        node2Name = internalCluster().startNode(Settings.builder().put(settings).put(masterDataPathSettings).build());
 
         clusterHealthResponse = client().admin().cluster().prepareHealth().setWaitForEvents(Priority.LANGUID)
             .setWaitForYellowStatus().setWaitForNodes("2").execute().actionGet();
@@ -169,6 +173,7 @@ public void testTwoNodesNoMasterBlock() throws Exception {
         logger.info("--> add voting config exclusion for master node, to be sure it's not elected");
         client().execute(AddVotingConfigExclusionsAction.INSTANCE, new AddVotingConfigExclusionsRequest(new String[]{masterNode})).get();
         logger.info("--> stop non-master node, no master block should appear");
+        Settings otherNodeDataPathSettings = internalCluster().dataPathSettings(otherNode);
         internalCluster().stopRandomNode(InternalTestCluster.nameFilter(otherNode));
 
         assertBusy(() -> {
@@ -177,7 +182,7 @@ public void testTwoNodesNoMasterBlock() throws Exception {
         });
 
         logger.info("--> starting the previous master node again...");
-        internalCluster().startNode(settings);
+        internalCluster().startNode(Settings.builder().put(settings).put(otherNodeDataPathSettings).build());
 
         ensureGreen();
         clusterHealthResponse = client().admin().cluster().prepareHealth().setWaitForEvents(Priority.LANGUID)
@@ -251,6 +256,10 @@ public void testThreeNodesNoMasterBlock() throws Exception {
             assertHitCount(client().prepareSearch().setSize(0).setQuery(QueryBuilders.matchAllQuery()).execute().actionGet(), 100);
         }
 
+        List<String> nonMasterNodes = new ArrayList<>(Sets.difference(Sets.newHashSet(internalCluster().getNodeNames()),
+            Collections.singleton(internalCluster().getMasterName())));
+        Settings nonMasterDataPathSettings1 = internalCluster().dataPathSettings(nonMasterNodes.get(0));
+        Settings nonMasterDataPathSettings2 = internalCluster().dataPathSettings(nonMasterNodes.get(1));
         internalCluster().stopRandomNonMasterNode();
         internalCluster().stopRandomNonMasterNode();
 
@@ -262,7 +271,7 @@ public void testThreeNodesNoMasterBlock() throws Exception {
         });
 
         logger.info("--> start back the 2 nodes ");
-        internalCluster().startNodes(2, settings);
+        internalCluster().startNodes(nonMasterDataPathSettings1, nonMasterDataPathSettings2);
 
         internalCluster().validateClusterFormed();
         ensureGreen();

server/src/test/java/org/elasticsearch/cluster/SpecificMasterNodesIT.java

@@ -65,6 +65,7 @@ public void testSimpleOnlyMasterNodeElection() throws IOException {
             .execute().actionGet().getState().nodes().getMasterNode().getName(), equalTo(masterNodeName));
 
         logger.info("--> stop master node");
+        Settings masterDataPathSettings = internalCluster().dataPathSettings(internalCluster().getMasterName());
         internalCluster().stopCurrentMasterNode();
 
         try {
@@ -75,9 +76,10 @@ public void testSimpleOnlyMasterNodeElection() throws IOException {
             // all is well, no master elected
         }
 
-        logger.info("--> start master node");
+        logger.info("--> start previous master node again");
         final String nextMasterEligibleNodeName = internalCluster()
-            .startNode(Settings.builder().put(Node.NODE_DATA_SETTING.getKey(), false).put(Node.NODE_MASTER_SETTING.getKey(), true));
+            .startNode(Settings.builder().put(Node.NODE_DATA_SETTING.getKey(), false).put(Node.NODE_MASTER_SETTING.getKey(), true)
+                .put(masterDataPathSettings));
         assertThat(internalCluster().nonMasterClient().admin().cluster().prepareState()
             .execute().actionGet().getState().nodes().getMasterNode().getName(), equalTo(nextMasterEligibleNodeName));
         assertThat(internalCluster().masterClient().admin().cluster().prepareState()