From f5ba801c6b935097d96f921693d4914fff2049ba Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Fri, 10 Aug 2018 11:22:23 -0400 Subject: [PATCH] Test: Only sniff host metadata for node_selectors (#32750) 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 | 14 +++++----- .../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(+), 33 deletions(-) diff --git a/docs/src/test/java/org/elasticsearch/smoketest/DocsClientYamlTestSuiteIT.java b/docs/src/test/java/org/elasticsearch/smoketest/DocsClientYamlTestSuiteIT.java index a8dd91e8b6d..dbda453e5f9 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 @@ public class DocsClientYamlTestSuiteIT extends ESClientYamlSuiteTestCase { final List hosts, final Version esVersion, final Version masterVersion) { - return new ClientYamlDocsTestClient(restSpec, restClient, hosts, esVersion, masterVersion, - restClientBuilder -> configureClient(restClientBuilder, restClientSettings())); + return new ClientYamlDocsTestClient(restSpec, restClient, hosts, esVersion, masterVersion, 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 ddd58376635..b0b84cd2c21 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.elasticsearch.client.Response; 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.test.rest.yaml.restspec.ClientYamlSuiteRestSpec; import java.io.IOException; @@ -50,8 +50,8 @@ public final class ClientYamlDocsTestClient extends ClientYamlTestClient { final List hosts, final Version esVersion, final Version masterVersion, - final CheckedConsumer clientBuilderConsumer) { - super(restSpec, restClient, hosts, esVersion, masterVersion, clientBuilderConsumer); + final CheckedSupplier clientBuilderWithSniffedNodes) { + super(restSpec, restClient, hosts, esVersion, masterVersion, 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 2d6bcc8cf56..856fd2a32de 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.entity.ContentType; 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; @@ -34,7 +33,7 @@ import org.elasticsearch.client.Response; 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; @@ -66,7 +65,7 @@ public class ClientYamlTestClient implements Closeable { private final Map restClients = new HashMap<>(); private final Version esVersion; private final Version masterVersion; - private final CheckedConsumer clientBuilderConsumer; + private final CheckedSupplier clientBuilderWithSniffedNodes; ClientYamlTestClient( final ClientYamlSuiteRestSpec restSpec, @@ -74,13 +73,13 @@ public class ClientYamlTestClient implements Closeable { final List hosts, final Version esVersion, final Version masterVersion, - final CheckedConsumer clientBuilderConsumer) { + final CheckedSupplier clientBuilderWithSniffedNodes) { assert hosts.size() > 0; this.restSpec = restSpec; this.restClients.put(NodeSelector.ANY, restClient); this.esVersion = esVersion; this.masterVersion = masterVersion; - this.clientBuilderConsumer = clientBuilderConsumer; + this.clientBuilderWithSniffedNodes = clientBuilderWithSniffedNodes; } public Version getEsVersion() { @@ -199,10 +198,9 @@ public class ClientYamlTestClient implements Closeable { 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 = 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 b97b4e8f6da..91456d395d6 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 @@ -26,6 +26,7 @@ import org.elasticsearch.client.Node; import org.elasticsearch.client.Request; import org.elasticsearch.client.Response; 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 @@ import java.util.Set; /** * 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 { @@ -123,11 +117,6 @@ public abstract class ESClientYamlSuiteTestCase extends ESRestTestCase { @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; final ClientYamlSuiteRestSpec restSpec = ClientYamlSuiteRestSpec.load(SPEC_PATH); @@ -166,8 +155,7 @@ public abstract class ESClientYamlSuiteTestCase extends ESRestTestCase { final List hosts, final Version esVersion, final Version masterVersion) { - return new ClientYamlTestClient(restSpec, restClient, hosts, esVersion, masterVersion, - restClientBuilder -> configureClient(restClientBuilder, restClientSettings())); + return new ClientYamlTestClient(restSpec, restClient, hosts, esVersion, masterVersion, this::getClientBuilderWithSniffedHosts); } @AfterClass @@ -408,13 +396,16 @@ public abstract class ESClientYamlSuiteTestCase extends ESRestTestCase { } /** - * 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 getClientBuilderWithSniffedHosts() 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 4e46a9ec89f..e1346d3f696 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 @@ -391,7 +391,32 @@ public class DoSection implements ExecutableSection { 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 c4c96d9fe2b..4c714c7e9aa 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 @@ -540,6 +540,15 @@ public class DoSectionTests extends AbstractClientYamlTestFragmentParserTestCase 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) { @@ -568,6 +577,14 @@ public class DoSectionTests extends AbstractClientYamlTestFragmentParserTestCase 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 e534e4f41bb..366639d2482 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 @@ public class XDocsClientYamlTestSuiteIT extends XPackRestIT { final List hosts, final Version esVersion, final Version masterVersion) { - return new ClientYamlDocsTestClient(restSpec, restClient, hosts, esVersion, masterVersion, - restClientBuilder -> configureClient(restClientBuilder, restClientSettings())); + return new ClientYamlDocsTestClient(restSpec, restClient, hosts, esVersion, masterVersion, this::getClientBuilderWithSniffedHosts); } /**