Skip to content
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.

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

Test: Prevent usage of System Properties in the InternalTestCluster #6663

Merged
merged 1 commit into from
Jul 1, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions TESTING.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -184,14 +184,14 @@ To run backwards compatibiilty tests untar or unzip a release and run the tests
with the following command:

---------------------------------------------------------------------------
mvn test -Dtests.bwc=true -Dtests.bwc.version=x.y.z -Dtests.bwc.path=/path/to/elasticsearch -Des.node.mode=network
mvn test -Dtests.bwc=true -Dtests.bwc.version=x.y.z -Dtests.bwc.path=/path/to/elasticsearch
---------------------------------------------------------------------------

If the elasticsearch release is placed under `./backwards/elasticsearch-x.y.z` the path
can be omitted:

---------------------------------------------------------------------------
mvn test -Dtests.bwc=true -Dtests.bwc.version=x.y.z -Des.node.mode=network
mvn test -Dtests.bwc=true -Dtests.bwc.version=x.y.z
---------------------------------------------------------------------------

To setup the bwc test environment execute the following steps (provided you are
Expand Down
10 changes: 0 additions & 10 deletions src/test/java/org/apache/lucene/util/AbstractRandomizedTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -403,14 +403,4 @@ public static Class<?> getTestClass() {
public String getTestName() {
return threadAndTestNameRule.testMethodName;
}

static {
String nodeLocal = System.getProperty("es.node.mode", System.getProperty("es.node.local", ""));
if (Strings.isEmpty(nodeLocal)) {
// we default to local mode to speed up tests running in IDEs etc.
// compared to a mvn default value this will also work if executed from an IDE.
System.setProperty("es.node.mode", "local");
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ protected Settings nodeSettings(int nodeOrdinal) {
return ImmutableSettings.builder()
.put(TransportModule.TRANSPORT_TYPE_KEY, NettyTransportModule.class) // run same transport / disco as external
.put(DiscoveryModule.DISCOVERY_TYPE_KEY, ZenDiscoveryModule.class)
.put("node.mode", "network") // we need network mode for this
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it might make sense to do the same in ExternalTestCluster, so that we state that network mode is required there as well.

.put("gateway.type", "local") // we require local gateway to mimic upgrades of nodes
.put("discovery.type", "zen") // zen is needed since we start external nodes
.put(TransportModule.TRANSPORT_SERVICE_TYPE_KEY, TransportService.class.getName())
Expand Down
1 change: 1 addition & 0 deletions src/test/java/org/elasticsearch/test/ExternalNode.java
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ synchronized void startInternal(Client client, Settings settings, String nodeNam
case "path.home":
case "node.mode":
case "gateway.type":
case "config.ignore_system_properties":
continue;
default:
params.add("-Des." + entry.getKey() + "=" + entry.getValue());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,9 @@ public final class ExternalTestCluster extends TestCluster {
private final int numBenchNodes;

public ExternalTestCluster(TransportAddress... transportAddresses) {
this.client = new TransportClient(ImmutableSettings.settingsBuilder().put("client.transport.ignore_cluster_name", true))
this.client = new TransportClient(ImmutableSettings.settingsBuilder()
.put("client.transport.ignore_cluster_name", true)
.put("node.mode", "network")) // we require network here!
.addTransportAddresses(transportAddresses);

NodesInfoResponse nodeInfos = this.client.admin().cluster().prepareNodesInfo().clear().setSettings(true).setHttp(true).get();
Expand Down
42 changes: 40 additions & 2 deletions src/test/java/org/elasticsearch/test/InternalTestCluster.java
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import org.elasticsearch.cluster.node.DiscoveryNode;
import org.elasticsearch.cluster.node.DiscoveryNodes;
import org.elasticsearch.cluster.routing.ShardRouting;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.io.FileSystemUtils;
import org.elasticsearch.common.lease.Releasables;
import org.elasticsearch.common.logging.ESLogger;
Expand Down Expand Up @@ -141,6 +142,8 @@ public final class InternalTestCluster extends TestCluster {

static final boolean DEFAULT_ENABLE_RANDOM_BENCH_NODES = true;

private static final String NODE_MODE = nodeMode();

/* sorted map to make traverse order reproducible, concurrent since we do checks on it not within a sync block */
private final NavigableMap<String, NodeAndClient> nodes = new TreeMap<>();

Expand Down Expand Up @@ -235,13 +238,40 @@ public InternalTestCluster(long clusterSeed, int minNumDataNodes, int maxNumData
builder.put("path.data", dataPath.toString());
}
}
builder.put("config.ignore_system_properties", true);
builder.put("node.mode", NODE_MODE);
builder.put("script.disable_dynamic", false);
builder.put("plugins." + PluginsService.LOAD_PLUGIN_FROM_CLASSPATH, false);
if (Strings.hasLength(System.getProperty("es.logger.level"))) {
builder.put("logger.level", System.getProperty("es.logger.level"));
}
if (Strings.hasLength(System.getProperty("es.logger.prefix"))) {
builder.put("logger.prefix", System.getProperty("es.logger.level"));
}
defaultSettings = builder.build();
executor = EsExecutors.newCached(1, TimeUnit.MINUTES, EsExecutors.daemonThreadFactory("test_" + clusterName));
this.hasFilterCache = random.nextBoolean();
}


private static String nodeMode() {
Builder builder = ImmutableSettings.builder();
if (Strings.isEmpty(System.getProperty("es.node.mode"))&& Strings.isEmpty(System.getProperty("es.node.local"))) {
return "local"; // default if nothing is specified
}
if (Strings.hasLength(System.getProperty("es.node.mode"))) {
builder.put("node.mode", System.getProperty("es.node.mode"));
}
if (Strings.hasLength(System.getProperty("es.node.local"))) {
builder.put("node.local", System.getProperty("es.node.local"));
}
if (DiscoveryNode.localNode(builder.build())) {
return "local";
} else {
return "network";
}
}

public String getClusterName() {
return clusterName;
}
Expand Down Expand Up @@ -734,10 +764,18 @@ private TransportClientFactory(boolean sniff) {
@Override
public Client client(Node node, String clusterName, Random random) {
TransportAddress addr = ((InternalNode) node).injector().getInstance(TransportService.class).boundAddress().publishAddress();
TransportClient client = new TransportClient(settingsBuilder().put("client.transport.nodes_sampler_interval", "1s")
Settings nodeSettings = node.settings();
Builder builder = settingsBuilder().put("client.transport.nodes_sampler_interval", "1s")
.put("name", TRANSPORT_CLIENT_PREFIX + node.settings().get("name"))
.put("plugins." + PluginsService.LOAD_PLUGIN_FROM_CLASSPATH, false)
.put(ClusterName.SETTING, clusterName).put("client.transport.sniff", sniff).build());
.put(ClusterName.SETTING, clusterName).put("client.transport.sniff", sniff);
builder.put("node.mode", nodeSettings.get("node.mode", NODE_MODE));
builder.put("node.local", nodeSettings.get("node.local", ""));
builder.put("logger.prefix", nodeSettings.get("logger.prefix", ""));
builder.put("logger.level", nodeSettings.get("logger.level", "INFO"));
builder.put("config.ignore_system_properties", true);

TransportClient client = new TransportClient(builder.build());
client.addTransportAddress(addr);
return client;
}
Expand Down
12 changes: 12 additions & 0 deletions src/test/java/org/elasticsearch/tribe/TribeTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
package org.elasticsearch.tribe;

import com.google.common.base.Predicate;
import com.google.common.collect.ImmutableMap;
import org.elasticsearch.action.admin.cluster.health.ClusterHealthStatus;
import org.elasticsearch.client.Client;
import org.elasticsearch.cluster.ClusterState;
Expand All @@ -40,6 +41,7 @@
import org.junit.Test;

import java.io.IOException;
import java.util.Map;

import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount;
Expand Down Expand Up @@ -92,12 +94,22 @@ public void tearDownTribeNode() throws IOException {
}

private void setupTribeNode(Settings settings) {
ImmutableMap<String,String> asMap = internalCluster().getDefaultSettings().getAsMap();
ImmutableSettings.Builder tribe1Defaults = ImmutableSettings.builder();
ImmutableSettings.Builder tribe2Defaults = ImmutableSettings.builder();
for (Map.Entry<String, String> entry : asMap.entrySet()) {
tribe1Defaults.put("tribe.t1." + entry.getKey(), entry.getValue());
tribe2Defaults.put("tribe.t2." + entry.getKey(), entry.getValue());
}
Settings merged = ImmutableSettings.builder()
.put("tribe.t1.cluster.name", internalCluster().getClusterName())
.put("tribe.t2.cluster.name", cluster2.getClusterName())
.put("tribe.blocks.write", false)
.put("tribe.blocks.read", false)
.put(settings)
.put(tribe1Defaults.build())
.put(tribe2Defaults.build())
.put(internalCluster().getDefaultSettings())
.build();

tribeNode = NodeBuilder.nodeBuilder()
Expand Down