From d653167bc586b1033b34f4af57457d67476c93bf Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Tue, 15 Aug 2017 10:16:44 +0200 Subject: [PATCH 1/3] Several internal improvements to internal test cluster infra This chance adds several random test infrastructure improvments that caused issues in on-going developments but are generally useful. For instance is it impossible to restart a node with a secure setting source since we close it after the node is started. This change makes it cloneable such that we can reuse it for a restart. --- .../client/transport/TransportClient.java | 2 +- .../common/settings/MockSecureSettings.java | 14 ++++++++++++++ .../org/elasticsearch/test/ESIntegTestCase.java | 17 ++++++++--------- .../elasticsearch/test/ExternalTestCluster.java | 12 +++++++++--- .../elasticsearch/test/InternalTestCluster.java | 10 ++++++++++ .../org/elasticsearch/test/TestCluster.java | 3 +++ .../transport/MockTransportClient.java | 5 +++++ 7 files changed, 50 insertions(+), 13 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/client/transport/TransportClient.java b/core/src/main/java/org/elasticsearch/client/transport/TransportClient.java index 83cdd95119c3f..697e0373fb1be 100644 --- a/core/src/main/java/org/elasticsearch/client/transport/TransportClient.java +++ b/core/src/main/java/org/elasticsearch/client/transport/TransportClient.java @@ -235,7 +235,7 @@ ThreadPool getThreadPool() { public static final String CLIENT_TYPE = "transport"; final Injector injector; - final NamedWriteableRegistry namedWriteableRegistry; + protected final NamedWriteableRegistry namedWriteableRegistry; private final List pluginLifecycleComponents; private final TransportClientNodesService nodesService; diff --git a/test/framework/src/main/java/org/elasticsearch/common/settings/MockSecureSettings.java b/test/framework/src/main/java/org/elasticsearch/common/settings/MockSecureSettings.java index 22496642cb9b7..3a6161a9f7fa0 100644 --- a/test/framework/src/main/java/org/elasticsearch/common/settings/MockSecureSettings.java +++ b/test/framework/src/main/java/org/elasticsearch/common/settings/MockSecureSettings.java @@ -38,6 +38,15 @@ public class MockSecureSettings implements SecureSettings { private Set settingNames = new HashSet<>(); private final AtomicBoolean closed = new AtomicBoolean(false); + public MockSecureSettings() { + } + + private MockSecureSettings(MockSecureSettings source) { + secureStrings.putAll(source.secureStrings); + files.putAll(source.files); + settingNames.addAll(source.settingNames); + } + @Override public boolean isLoaded() { return true; @@ -94,4 +103,9 @@ private void ensureOpen() { throw new IllegalStateException("secure settings are already closed"); } } + + public SecureSettings clone() { + ensureOpen(); + return new MockSecureSettings(this); + } } diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java index 6815e4568f3d8..d6c9ed05d69ec 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java @@ -1079,19 +1079,13 @@ protected void ensureClusterSizeConsistency() { } } + /** * Verifies that all nodes that have the same version of the cluster state as master have same cluster state */ protected void ensureClusterStateConsistency() throws IOException { if (cluster() != null && cluster().size() > 0) { - final NamedWriteableRegistry namedWriteableRegistry; - if (isInternalCluster()) { - // If it's internal cluster - using existing registry in case plugin registered custom data - namedWriteableRegistry = internalCluster().getInstance(NamedWriteableRegistry.class); - } else { - // If it's external cluster - fall back to the standard set - namedWriteableRegistry = new NamedWriteableRegistry(ClusterModule.getNamedWriteables()); - } + final NamedWriteableRegistry namedWriteableRegistry = cluster().getNamedWriteableRegistry(); ClusterState masterClusterState = client().admin().cluster().prepareState().all().get().getState(); byte[] masterClusterStateBytes = ClusterState.Builder.toBytes(masterClusterState); // remove local node reference @@ -2192,7 +2186,12 @@ protected static RestClient createRestClient(RestClientBuilder.HttpClientConfigC } protected static RestClient createRestClient(RestClientBuilder.HttpClientConfigCallback httpClientConfigCallback, String protocol) { - final NodesInfoResponse nodeInfos = client().admin().cluster().prepareNodesInfo().get(); + return createRestClient(client(), httpClientConfigCallback, protocol); + } + + protected static RestClient createRestClient(Client client, RestClientBuilder.HttpClientConfigCallback httpClientConfigCallback, String + protocol) { + final NodesInfoResponse nodeInfos = client.admin().cluster().prepareNodesInfo().get(); final List nodes = nodeInfos.getNodes(); assertFalse(nodeInfos.hasFailures()); List hosts = new ArrayList<>(); diff --git a/test/framework/src/main/java/org/elasticsearch/test/ExternalTestCluster.java b/test/framework/src/main/java/org/elasticsearch/test/ExternalTestCluster.java index 8d8a3b3161676..f8d9c27fd4de9 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ExternalTestCluster.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ExternalTestCluster.java @@ -28,6 +28,7 @@ import org.elasticsearch.client.transport.TransportClient; import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.common.breaker.CircuitBreaker; +import org.elasticsearch.common.io.stream.NamedWriteableRegistry; import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.common.network.NetworkModule; import org.elasticsearch.common.settings.Settings; @@ -62,7 +63,7 @@ public final class ExternalTestCluster extends TestCluster { private static final AtomicInteger counter = new AtomicInteger(); public static final String EXTERNAL_CLUSTER_PREFIX = "external_"; - private final Client client; + private final MockTransportClient client; private final InetSocketAddress[] httpAddresses; @@ -95,8 +96,7 @@ public ExternalTestCluster(Path tempDir, Settings additionalSettings, Collection } } Settings clientSettings = clientSettingsBuilder.build(); - TransportClient client = new MockTransportClient(clientSettings, pluginClasses); - + MockTransportClient client = new MockTransportClient(clientSettings, pluginClasses); try { client.addTransportAddresses(transportAddresses); NodesInfoResponse nodeInfos = client.admin().cluster().prepareNodesInfo().clear().setSettings(true).setHttp(true).get(); @@ -117,6 +117,7 @@ public ExternalTestCluster(Path tempDir, Settings additionalSettings, Collection this.numDataNodes = dataNodes; this.numMasterAndDataNodes = masterAndDataNodes; this.client = client; + logger.info("Setup ExternalTestCluster [{}] made of [{}] nodes", nodeInfos.getClusterName().value(), size()); } catch (Exception e) { client.close(); @@ -186,6 +187,11 @@ public Iterable getClients() { return Collections.singleton(client); } + @Override + public NamedWriteableRegistry getNamedWriteableRegistry() { + return client.getNamedWriteableRegistry(); + } + @Override public String getClusterName() { return clusterName; diff --git a/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java b/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java index 458f77e3ed324..adedc25e40f1d 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java +++ b/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java @@ -51,9 +51,11 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.common.breaker.CircuitBreaker; import org.elasticsearch.common.io.FileSystemUtils; +import org.elasticsearch.common.io.stream.NamedWriteableRegistry; import org.elasticsearch.common.lease.Releasables; import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.common.network.NetworkModule; +import org.elasticsearch.common.settings.MockSecureSettings; import org.elasticsearch.common.settings.SecureSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.Settings.Builder; @@ -610,6 +612,9 @@ private NodeAndClient buildNode(int nodeId, long seed, Settings settings, throw new IllegalArgumentException(DISCOVERY_ZEN_MINIMUM_MASTER_NODES_SETTING.getKey() + " must be configured"); } SecureSettings secureSettings = finalSettings.getSecureSettings(); + if (secureSettings instanceof MockSecureSettings) { + secureSettings = ((MockSecureSettings) secureSettings).clone(); + } MockNode node = new MockNode(finalSettings.build(), plugins, nodeConfigurationSource.nodeConfigPath(nodeId)); try { IOUtils.close(secureSettings); @@ -1964,6 +1969,11 @@ public void remove() { }; } + @Override + public NamedWriteableRegistry getNamedWriteableRegistry() { + return getInstance(NamedWriteableRegistry.class); + } + /** * Returns a predicate that only accepts settings of nodes with one of the given names. */ diff --git a/test/framework/src/main/java/org/elasticsearch/test/TestCluster.java b/test/framework/src/main/java/org/elasticsearch/test/TestCluster.java index c2ac65d9980e1..29a41bd693545 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/TestCluster.java +++ b/test/framework/src/main/java/org/elasticsearch/test/TestCluster.java @@ -26,6 +26,7 @@ import org.elasticsearch.client.Client; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.cluster.metadata.IndexTemplateMetaData; +import org.elasticsearch.common.io.stream.NamedWriteableRegistry; import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.index.IndexNotFoundException; import org.elasticsearch.indices.IndexTemplateMissingException; @@ -34,6 +35,7 @@ import java.io.Closeable; import java.io.IOException; import java.net.InetSocketAddress; +import java.util.List; import java.util.Random; import java.util.Set; @@ -234,4 +236,5 @@ public void wipeRepositories(String... repositories) { public abstract Iterable getClients(); + public abstract NamedWriteableRegistry getNamedWriteableRegistry(); } diff --git a/test/framework/src/main/java/org/elasticsearch/transport/MockTransportClient.java b/test/framework/src/main/java/org/elasticsearch/transport/MockTransportClient.java index 1f712adb16105..027b26e32d736 100644 --- a/test/framework/src/main/java/org/elasticsearch/transport/MockTransportClient.java +++ b/test/framework/src/main/java/org/elasticsearch/transport/MockTransportClient.java @@ -19,6 +19,7 @@ package org.elasticsearch.transport; import org.elasticsearch.client.transport.TransportClient; +import org.elasticsearch.common.io.stream.NamedWriteableRegistry; import org.elasticsearch.common.network.NetworkModule; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.plugins.Plugin; @@ -69,4 +70,8 @@ private static Collection> addMockTransportIfMissing(Set } return plugins; } + + public NamedWriteableRegistry getNamedWriteableRegistry() { + return namedWriteableRegistry; + } } From 31fc06cacf9dff986dc64f3481d9d8c3bb0e8b02 Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Tue, 15 Aug 2017 17:17:38 +0200 Subject: [PATCH 2/3] cleanups and javadocs --- .../java/org/elasticsearch/test/ESIntegTestCase.java | 11 +++++------ .../org/elasticsearch/test/InternalTestCluster.java | 1 + .../main/java/org/elasticsearch/test/TestCluster.java | 5 ++++- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java index d6c9ed05d69ec..ca57aaf4e0903 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java @@ -2186,14 +2186,13 @@ protected static RestClient createRestClient(RestClientBuilder.HttpClientConfigC } protected static RestClient createRestClient(RestClientBuilder.HttpClientConfigCallback httpClientConfigCallback, String protocol) { - return createRestClient(client(), httpClientConfigCallback, protocol); + NodesInfoResponse nodesInfoResponse = client().admin().cluster().prepareNodesInfo().get(); + assertFalse(nodesInfoResponse.hasFailures()); + return createRestClient(nodesInfoResponse.getNodes(), httpClientConfigCallback, protocol); } - protected static RestClient createRestClient(Client client, RestClientBuilder.HttpClientConfigCallback httpClientConfigCallback, String - protocol) { - final NodesInfoResponse nodeInfos = client.admin().cluster().prepareNodesInfo().get(); - final List nodes = nodeInfos.getNodes(); - assertFalse(nodeInfos.hasFailures()); + protected static RestClient createRestClient(final List nodes, + RestClientBuilder.HttpClientConfigCallback httpClientConfigCallback, String protocol) { List hosts = new ArrayList<>(); for (NodeInfo node : nodes) { if (node.getHttp() != null) { diff --git a/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java b/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java index adedc25e40f1d..29bfbff29b20b 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java +++ b/test/framework/src/main/java/org/elasticsearch/test/InternalTestCluster.java @@ -613,6 +613,7 @@ private NodeAndClient buildNode(int nodeId, long seed, Settings settings, } SecureSettings secureSettings = finalSettings.getSecureSettings(); if (secureSettings instanceof MockSecureSettings) { + // we clone this here since in the case of a node restart we might need it again secureSettings = ((MockSecureSettings) secureSettings).clone(); } MockNode node = new MockNode(finalSettings.build(), plugins, nodeConfigurationSource.nodeConfigPath(nodeId)); diff --git a/test/framework/src/main/java/org/elasticsearch/test/TestCluster.java b/test/framework/src/main/java/org/elasticsearch/test/TestCluster.java index 29a41bd693545..470847e65f25f 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/TestCluster.java +++ b/test/framework/src/main/java/org/elasticsearch/test/TestCluster.java @@ -235,6 +235,9 @@ public void wipeRepositories(String... repositories) { */ public abstract Iterable getClients(); - + /** + * Returns this clusters {@link NamedWriteableRegistry} this is needed to + * deserialize binary content from this cluster that might include custom named writeables + */ public abstract NamedWriteableRegistry getNamedWriteableRegistry(); } From f59d0be51e4213435a1adc3f854075da43d51def Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Tue, 15 Aug 2017 17:40:56 +0200 Subject: [PATCH 3/3] fix newline --- .../src/main/java/org/elasticsearch/test/ESIntegTestCase.java | 1 - 1 file changed, 1 deletion(-) diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java index ca57aaf4e0903..80753687a4db4 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java @@ -1079,7 +1079,6 @@ protected void ensureClusterSizeConsistency() { } } - /** * Verifies that all nodes that have the same version of the cluster state as master have same cluster state */