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.
elastic · May 22, 2019 · 770d8e9 · 770d8e9
1 parent b1c413e
commit 770d8e9
Show file tree

Hide file tree

Showing 24 changed files with 266 additions and 207 deletions.
diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterFormationTasks.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterFormationTasks.groovy
@@ -410,7 +410,6 @@ class ClusterFormationTasks {
             // so we need to bail quicker than the default 30s for the cluster to form in time.
             esConfig['discovery.zen.master_election.wait_for_joins_timeout'] = '5s'
         }
-        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

diff --git a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/IngestRestartIT.java b/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);
 

diff --git a/...st/java/org/elasticsearch/action/admin/cluster/allocation/ClusterAllocationExplainIT.java b/...st/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

diff --git a/server/src/test/java/org/elasticsearch/cluster/ClusterInfoServiceIT.java b/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);

diff --git a/server/src/test/java/org/elasticsearch/cluster/MinimumMasterNodesIT.java b/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)
@@ -252,6 +257,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();
 
@@ -263,7 +272,7 @@ public void testThreeNodesNoMasterBlock() throws Exception {
         });
 
         logger.info("--> start back the 2 nodes ");
-        internalCluster().startNodes(2, settings);
+        internalCluster().startNodes(nonMasterDataPathSettings1, nonMasterDataPathSettings2);
 
         internalCluster().validateClusterFormed();
         ensureGreen();

diff --git a/server/src/test/java/org/elasticsearch/cluster/SpecificMasterNodesIT.java b/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()