From 6fc5e13483f4e628de046b305793c62d13632753 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Thu, 9 Aug 2018 11:06:48 -0400 Subject: [PATCH 1/3] Test: Only sniff host metadata for node_selectors Our rest testing framework has support for sniffing the host metadata on startup and, before this change, it'd sniff that metadata before running the first test. This prevents running these tests against elasticsearch installations that won't support sniffing like Elastic Cloud. This change allows tests to only sniff for metadata when they encounter a test with a `node_selector`. These selectors are the things that need the metadata anyway and they are super rare. Tests that use these won't be able to run against installations that don't support sniffing but we can just skip them. In the case of Elastic Cloud, these tests were never going to work against Elastic Cloud anyway. --- .../smoketest/DocsClientYamlTestSuiteIT.java | 3 +-- .../rest/yaml/ClientYamlDocsTestClient.java | 6 ++--- .../test/rest/yaml/ClientYamlTestClient.java | 13 +++++---- .../rest/yaml/ESClientYamlSuiteTestCase.java | 25 ++++++----------- .../test/rest/yaml/section/DoSection.java | 27 ++++++++++++++++++- .../rest/yaml/section/DoSectionTests.java | 17 ++++++++++++ .../smoketest/XDocsClientYamlTestSuiteIT.java | 3 +-- 7 files changed, 62 insertions(+), 32 deletions(-) diff --git a/docs/src/test/java/org/elasticsearch/smoketest/DocsClientYamlTestSuiteIT.java b/docs/src/test/java/org/elasticsearch/smoketest/DocsClientYamlTestSuiteIT.java index ec350e3ed05b3..85e9327409e02 100644 --- a/docs/src/test/java/org/elasticsearch/smoketest/DocsClientYamlTestSuiteIT.java +++ b/docs/src/test/java/org/elasticsearch/smoketest/DocsClientYamlTestSuiteIT.java @@ -92,8 +92,7 @@ protected ClientYamlTestClient initClientYamlTestClient( final RestClient restClient, final List hosts, final Version esVersion) { - return new ClientYamlDocsTestClient(restSpec, restClient, hosts, esVersion, - restClientBuilder -> configureClient(restClientBuilder, restClientSettings())); + return new ClientYamlDocsTestClient(restSpec, restClient, hosts, esVersion, this::sniffHostMetadata); } /** diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlDocsTestClient.java b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlDocsTestClient.java index 403c738b84410..ca735cc3c04a5 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlDocsTestClient.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlDocsTestClient.java @@ -28,7 +28,7 @@ import org.apache.http.HttpEntity; import org.apache.http.HttpHost; import org.elasticsearch.client.RestClientBuilder; -import org.elasticsearch.common.CheckedConsumer; +import org.elasticsearch.common.CheckedSupplier; import org.elasticsearch.test.rest.yaml.restspec.ClientYamlSuiteRestSpec; import java.io.IOException; @@ -49,8 +49,8 @@ public ClientYamlDocsTestClient( final RestClient restClient, final List hosts, final Version esVersion, - final CheckedConsumer clientBuilderConsumer) { - super(restSpec, restClient, hosts, esVersion, clientBuilderConsumer); + final CheckedSupplier clientBuilderBuilder) { + super(restSpec, restClient, hosts, esVersion, clientBuilderBuilder); } @Override diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestClient.java b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestClient.java index 8a4f77dbd66f7..4988f47f8632f 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestClient.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestClient.java @@ -34,7 +34,7 @@ import org.elasticsearch.client.ResponseException; import org.elasticsearch.client.RestClient; import org.elasticsearch.client.RestClientBuilder; -import org.elasticsearch.common.CheckedConsumer; +import org.elasticsearch.common.CheckedSupplier; import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.test.rest.yaml.restspec.ClientYamlSuiteRestApi; import org.elasticsearch.test.rest.yaml.restspec.ClientYamlSuiteRestPath; @@ -65,19 +65,19 @@ public class ClientYamlTestClient implements Closeable { private final ClientYamlSuiteRestSpec restSpec; private final Map restClients = new HashMap<>(); private final Version esVersion; - private final CheckedConsumer clientBuilderConsumer; + private final CheckedSupplier clientBuilderBuilder; ClientYamlTestClient( final ClientYamlSuiteRestSpec restSpec, final RestClient restClient, final List hosts, final Version esVersion, - final CheckedConsumer clientBuilderConsumer) { + final CheckedSupplier clientBuilderBuilder) { assert hosts.size() > 0; this.restSpec = restSpec; this.restClients.put(NodeSelector.ANY, restClient); this.esVersion = esVersion; - this.clientBuilderConsumer = clientBuilderConsumer; + this.clientBuilderBuilder = clientBuilderBuilder; } public Version getEsVersion() { @@ -192,10 +192,9 @@ public ClientYamlTestResponse callApi(String apiName, Map params protected RestClient getRestClient(NodeSelector nodeSelector) { //lazily build a new client in case we need to point to some specific node return restClients.computeIfAbsent(nodeSelector, selector -> { - RestClient anyClient = restClients.get(NodeSelector.ANY); - RestClientBuilder builder = RestClient.builder(anyClient.getNodes().toArray(new Node[0])); + RestClientBuilder builder; try { - clientBuilderConsumer.accept(builder); + builder = clientBuilderBuilder.get(); } catch (IOException e) { throw new UncheckedIOException(e); } diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ESClientYamlSuiteTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ESClientYamlSuiteTestCase.java index 8218ce5dddaca..177b16c0689a3 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ESClientYamlSuiteTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ESClientYamlSuiteTestCase.java @@ -27,6 +27,7 @@ import org.elasticsearch.client.Response; import org.elasticsearch.client.ResponseException; import org.elasticsearch.client.RestClient; +import org.elasticsearch.client.RestClientBuilder; import org.elasticsearch.client.sniff.ElasticsearchNodesSniffer; import org.elasticsearch.common.Strings; import org.elasticsearch.common.collect.Tuple; @@ -58,13 +59,6 @@ /** * Runs a suite of yaml tests shared with all the official Elasticsearch * clients against against an elasticsearch cluster. - *

- * IMPORTANT: These tests sniff the cluster for metadata - * and hosts on startup and replace the list of hosts that they are - * configured to use with the list sniffed from the cluster. So you can't - * control which nodes receive the request by providing the right list of - * nodes in the tests.rest.cluster system property. Instead - * the tests must explictly use `node_selector`s. */ public abstract class ESClientYamlSuiteTestCase extends ESRestTestCase { @@ -116,11 +110,6 @@ protected ESClientYamlSuiteTestCase(ClientYamlTestCandidate testCandidate) { @Before public void initAndResetContext() throws Exception { if (restTestExecutionContext == null) { - // Sniff host metadata in case we need it in the yaml tests - List nodesWithMetadata = sniffHostMetadata(); - client().setNodes(nodesWithMetadata); - adminClient().setNodes(nodesWithMetadata); - assert adminExecutionContext == null; assert blacklistPathMatchers == null; ClientYamlSuiteRestSpec restSpec = ClientYamlSuiteRestSpec.load(SPEC_PATH); @@ -172,8 +161,7 @@ protected ClientYamlTestClient initClientYamlTestClient( final RestClient restClient, final List hosts, final Version esVersion) { - return new ClientYamlTestClient(restSpec, restClient, hosts, esVersion, - restClientBuilder -> configureClient(restClientBuilder, restClientSettings())); + return new ClientYamlTestClient(restSpec, restClient, hosts, esVersion, this::sniffHostMetadata); } @AfterClass @@ -407,13 +395,16 @@ protected boolean randomizeContentType() { } /** - * Sniff the cluster for host metadata. + * Sniff the cluster for host metadata and return a + * {@link RestClientBuilder} for a client with that metadata. */ - private List sniffHostMetadata() throws IOException { + protected final RestClientBuilder sniffHostMetadata() throws IOException { ElasticsearchNodesSniffer.Scheme scheme = ElasticsearchNodesSniffer.Scheme.valueOf(getProtocol().toUpperCase(Locale.ROOT)); ElasticsearchNodesSniffer sniffer = new ElasticsearchNodesSniffer( adminClient(), ElasticsearchNodesSniffer.DEFAULT_SNIFF_REQUEST_TIMEOUT, scheme); - return sniffer.sniff(); + RestClientBuilder builder = RestClient.builder(sniffer.sniff().toArray(new Node[0])); + configureClient(builder, restClientSettings()); + return builder; } } diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java index 33f776bc1f185..f941cc55b2743 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java @@ -389,7 +389,32 @@ private static NodeSelector parseAttributeValuesSelector(XContentParser parser) if (token == XContentParser.Token.FIELD_NAME) { key = parser.currentName(); } else if (token.isValue()) { - NodeSelector newSelector = new HasAttributeNodeSelector(key, parser.text()); + /* + * HasAttributeNodeSelector selects nodes that do not have + * attribute metadata set so it can be used against nodes that + * have not yet been sniffed. In these tests we expect the node + * metadata to be explicitly sniffed if we need it and we'd + * like to hard fail if it is not so we wrap the selector so we + * can assert that the data is sniffed. + */ + NodeSelector delegate = new HasAttributeNodeSelector(key, parser.text()); + NodeSelector newSelector = new NodeSelector() { + @Override + public void select(Iterable nodes) { + for (Node node : nodes) { + if (node.getAttributes() == null) { + throw new IllegalStateException("expected [attributes] metadata to be set but got " + + node); + } + } + delegate.select(nodes); + } + + @Override + public String toString() { + return delegate.toString(); + } + }; result = result == NodeSelector.ANY ? newSelector : new ComposeNodeSelector(result, newSelector); } else { diff --git a/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/section/DoSectionTests.java b/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/section/DoSectionTests.java index e36ddc5f1c2df..30f7918b14788 100644 --- a/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/section/DoSectionTests.java +++ b/test/framework/src/test/java/org/elasticsearch/test/rest/yaml/section/DoSectionTests.java @@ -539,6 +539,15 @@ public void testNodeSelectorByVersion() throws IOException { doSection.execute(context); verify(context).callApi("indices.get_field_mapping", singletonMap("index", "test_index"), emptyList(), emptyMap(), doSection.getApiCallSection().getNodeSelector()); + + { + List badNodes = new ArrayList<>(); + badNodes.add(new Node(new HttpHost("dummy"))); + Exception e = expectThrows(IllegalStateException.class, () -> + doSection.getApiCallSection().getNodeSelector().select(badNodes)); + assertEquals("expected [version] metadata to be set but got [host=http://dummy]", + e.getMessage()); + } } private static Node nodeWithVersion(String version) { @@ -567,6 +576,14 @@ public void testNodeSelectorByAttribute() throws IOException { doSection.getApiCallSection().getNodeSelector().select(nodes); assertEquals(Arrays.asList(hasAttr), nodes); } + { + List badNodes = new ArrayList<>(); + badNodes.add(new Node(new HttpHost("dummy"))); + Exception e = expectThrows(IllegalStateException.class, () -> + doSection.getApiCallSection().getNodeSelector().select(badNodes)); + assertEquals("expected [attributes] metadata to be set but got [host=http://dummy]", + e.getMessage()); + } parser = createParser(YamlXContent.yamlXContent, "node_selector:\n" + diff --git a/x-pack/docs/src/test/java/org/elasticsearch/smoketest/XDocsClientYamlTestSuiteIT.java b/x-pack/docs/src/test/java/org/elasticsearch/smoketest/XDocsClientYamlTestSuiteIT.java index 3d01594e6d730..269fdee7b72c7 100644 --- a/x-pack/docs/src/test/java/org/elasticsearch/smoketest/XDocsClientYamlTestSuiteIT.java +++ b/x-pack/docs/src/test/java/org/elasticsearch/smoketest/XDocsClientYamlTestSuiteIT.java @@ -58,8 +58,7 @@ protected ClientYamlTestClient initClientYamlTestClient( final RestClient restClient, final List hosts, final Version esVersion) { - return new ClientYamlDocsTestClient(restSpec, restClient, hosts, esVersion, - restClientBuilder -> configureClient(restClientBuilder, restClientSettings())); + return new ClientYamlDocsTestClient(restSpec, restClient, hosts, esVersion, this::sniffHostMetadata); } /** From c43f8009c3794b5eb68f35a411f901c7aa0df5c3 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Thu, 9 Aug 2018 14:00:06 -0400 Subject: [PATCH 2/3] Checkstyle --- .../org/elasticsearch/test/rest/yaml/ClientYamlTestClient.java | 1 - 1 file changed, 1 deletion(-) diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestClient.java b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestClient.java index 4988f47f8632f..5f742d7cd4e0b 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestClient.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestClient.java @@ -26,7 +26,6 @@ import org.apache.http.util.EntityUtils; import org.apache.logging.log4j.Logger; import org.elasticsearch.Version; -import org.elasticsearch.client.Node; import org.elasticsearch.client.NodeSelector; import org.elasticsearch.client.Request; import org.elasticsearch.client.RequestOptions; From 475a74c40e5ad25f99e7a643d2538bc5dbbe275a Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Fri, 10 Aug 2018 11:19:04 -0400 Subject: [PATCH 3/3] Renames --- .../smoketest/DocsClientYamlTestSuiteIT.java | 2 +- .../test/rest/yaml/ClientYamlDocsTestClient.java | 4 ++-- .../test/rest/yaml/ClientYamlTestClient.java | 8 ++++---- .../test/rest/yaml/ESClientYamlSuiteTestCase.java | 4 ++-- .../smoketest/XDocsClientYamlTestSuiteIT.java | 2 +- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/docs/src/test/java/org/elasticsearch/smoketest/DocsClientYamlTestSuiteIT.java b/docs/src/test/java/org/elasticsearch/smoketest/DocsClientYamlTestSuiteIT.java index 85e9327409e02..4b5d808550225 100644 --- a/docs/src/test/java/org/elasticsearch/smoketest/DocsClientYamlTestSuiteIT.java +++ b/docs/src/test/java/org/elasticsearch/smoketest/DocsClientYamlTestSuiteIT.java @@ -92,7 +92,7 @@ protected ClientYamlTestClient initClientYamlTestClient( final RestClient restClient, final List hosts, final Version esVersion) { - return new ClientYamlDocsTestClient(restSpec, restClient, hosts, esVersion, this::sniffHostMetadata); + return new ClientYamlDocsTestClient(restSpec, restClient, hosts, esVersion, this::getClientBuilderWithSniffedHosts); } /** diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlDocsTestClient.java b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlDocsTestClient.java index ca735cc3c04a5..82c177f0904ab 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlDocsTestClient.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlDocsTestClient.java @@ -49,8 +49,8 @@ public ClientYamlDocsTestClient( final RestClient restClient, final List hosts, final Version esVersion, - final CheckedSupplier clientBuilderBuilder) { - super(restSpec, restClient, hosts, esVersion, clientBuilderBuilder); + final CheckedSupplier clientBuilderWithSniffedNodes) { + super(restSpec, restClient, hosts, esVersion, clientBuilderWithSniffedNodes); } @Override diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestClient.java b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestClient.java index 5f742d7cd4e0b..2bacd20b35fb9 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestClient.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestClient.java @@ -64,19 +64,19 @@ public class ClientYamlTestClient implements Closeable { private final ClientYamlSuiteRestSpec restSpec; private final Map restClients = new HashMap<>(); private final Version esVersion; - private final CheckedSupplier clientBuilderBuilder; + private final CheckedSupplier clientBuilderWithSniffedNodes; ClientYamlTestClient( final ClientYamlSuiteRestSpec restSpec, final RestClient restClient, final List hosts, final Version esVersion, - final CheckedSupplier clientBuilderBuilder) { + final CheckedSupplier clientBuilderWithSniffedNodes) { assert hosts.size() > 0; this.restSpec = restSpec; this.restClients.put(NodeSelector.ANY, restClient); this.esVersion = esVersion; - this.clientBuilderBuilder = clientBuilderBuilder; + this.clientBuilderWithSniffedNodes = clientBuilderWithSniffedNodes; } public Version getEsVersion() { @@ -193,7 +193,7 @@ protected RestClient getRestClient(NodeSelector nodeSelector) { return restClients.computeIfAbsent(nodeSelector, selector -> { RestClientBuilder builder; try { - builder = clientBuilderBuilder.get(); + builder = clientBuilderWithSniffedNodes.get(); } catch (IOException e) { throw new UncheckedIOException(e); } diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ESClientYamlSuiteTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ESClientYamlSuiteTestCase.java index 177b16c0689a3..a86ff5d805ecb 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ESClientYamlSuiteTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ESClientYamlSuiteTestCase.java @@ -161,7 +161,7 @@ protected ClientYamlTestClient initClientYamlTestClient( final RestClient restClient, final List hosts, final Version esVersion) { - return new ClientYamlTestClient(restSpec, restClient, hosts, esVersion, this::sniffHostMetadata); + return new ClientYamlTestClient(restSpec, restClient, hosts, esVersion, this::getClientBuilderWithSniffedHosts); } @AfterClass @@ -398,7 +398,7 @@ protected boolean randomizeContentType() { * Sniff the cluster for host metadata and return a * {@link RestClientBuilder} for a client with that metadata. */ - protected final RestClientBuilder sniffHostMetadata() throws IOException { + protected final RestClientBuilder getClientBuilderWithSniffedHosts() throws IOException { ElasticsearchNodesSniffer.Scheme scheme = ElasticsearchNodesSniffer.Scheme.valueOf(getProtocol().toUpperCase(Locale.ROOT)); ElasticsearchNodesSniffer sniffer = new ElasticsearchNodesSniffer( diff --git a/x-pack/docs/src/test/java/org/elasticsearch/smoketest/XDocsClientYamlTestSuiteIT.java b/x-pack/docs/src/test/java/org/elasticsearch/smoketest/XDocsClientYamlTestSuiteIT.java index 269fdee7b72c7..f15944181922c 100644 --- a/x-pack/docs/src/test/java/org/elasticsearch/smoketest/XDocsClientYamlTestSuiteIT.java +++ b/x-pack/docs/src/test/java/org/elasticsearch/smoketest/XDocsClientYamlTestSuiteIT.java @@ -58,7 +58,7 @@ protected ClientYamlTestClient initClientYamlTestClient( final RestClient restClient, final List hosts, final Version esVersion) { - return new ClientYamlDocsTestClient(restSpec, restClient, hosts, esVersion, this::sniffHostMetadata); + return new ClientYamlDocsTestClient(restSpec, restClient, hosts, esVersion, this::getClientBuilderWithSniffedHosts); } /**