Skip to content

Commit 295193c

Browse files
nik9000nik9000
committed
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.
1 parent 59caf07 commit 295193c

File tree

7 files changed

+62
-33
lines changed

7 files changed

+62
-33
lines changed

‎docs/src/test/java/org/elasticsearch/smoketest/DocsClientYamlTestSuiteIT.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,7 @@ protected ClientYamlTestClient initClientYamlTestClient(
9292
final RestClient restClient,
9393
final List<HttpHost> hosts,
9494
final Version esVersion) {
95-
return new ClientYamlDocsTestClient(restSpec, restClient, hosts, esVersion,
96-
restClientBuilder -> configureClient(restClientBuilder, restClientSettings()));
95+
return new ClientYamlDocsTestClient(restSpec, restClient, hosts, esVersion, this::getClientBuilderWithSniffedHosts);
9796
}
9897

9998
/**

‎test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlDocsTestClient.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
import org.apache.http.HttpEntity;
2929
import org.apache.http.HttpHost;
3030
import org.elasticsearch.client.RestClientBuilder;
31-
import org.elasticsearch.common.CheckedConsumer;
31+
import org.elasticsearch.common.CheckedSupplier;
3232
import org.elasticsearch.test.rest.yaml.restspec.ClientYamlSuiteRestSpec;
3333

3434
import java.io.IOException;
@@ -49,8 +49,8 @@ public ClientYamlDocsTestClient(
4949
final RestClient restClient,
5050
final List<HttpHost> hosts,
5151
final Version esVersion,
52-
final CheckedConsumer<RestClientBuilder, IOException> clientBuilderConsumer) {
53-
super(restSpec, restClient, hosts, esVersion, clientBuilderConsumer);
52+
final CheckedSupplier<RestClientBuilder, IOException> clientBuilderWithSniffedNodes) {
53+
super(restSpec, restClient, hosts, esVersion, clientBuilderWithSniffedNodes);
5454
}
5555

5656
@Override

‎test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ClientYamlTestClient.java‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,15 +26,14 @@
2626
import org.apache.http.util.EntityUtils;
2727
import org.apache.logging.log4j.Logger;
2828
import org.elasticsearch.Version;
29-
import org.elasticsearch.client.Node;
3029
import org.elasticsearch.client.NodeSelector;
3130
import org.elasticsearch.client.Request;
3231
import org.elasticsearch.client.RequestOptions;
3332
import org.elasticsearch.client.Response;
3433
import org.elasticsearch.client.ResponseException;
3534
import org.elasticsearch.client.RestClient;
3635
import org.elasticsearch.client.RestClientBuilder;
37-
import org.elasticsearch.common.CheckedConsumer;
36+
import org.elasticsearch.common.CheckedSupplier;
3837
import org.elasticsearch.common.logging.Loggers;
3938
import org.elasticsearch.test.rest.yaml.restspec.ClientYamlSuiteRestApi;
4039
import org.elasticsearch.test.rest.yaml.restspec.ClientYamlSuiteRestPath;
@@ -65,19 +64,19 @@ public class ClientYamlTestClient implements Closeable {
6564
private final ClientYamlSuiteRestSpec restSpec;
6665
private final Map<NodeSelector, RestClient> restClients = new HashMap<>();
6766
private final Version esVersion;
68-
private final CheckedConsumer<RestClientBuilder, IOException> clientBuilderConsumer;
67+
private final CheckedSupplier<RestClientBuilder, IOException> clientBuilderWithSniffedNodes;
6968

7069
ClientYamlTestClient(
7170
final ClientYamlSuiteRestSpec restSpec,
7271
final RestClient restClient,
7372
final List<HttpHost> hosts,
7473
final Version esVersion,
75-
final CheckedConsumer<RestClientBuilder, IOException> clientBuilderConsumer) {
74+
final CheckedSupplier<RestClientBuilder, IOException> clientBuilderWithSniffedNodes) {
7675
assert hosts.size() > 0;
7776
this.restSpec = restSpec;
7877
this.restClients.put(NodeSelector.ANY, restClient);
7978
this.esVersion = esVersion;
80-
this.clientBuilderConsumer = clientBuilderConsumer;
79+
this.clientBuilderWithSniffedNodes = clientBuilderWithSniffedNodes;
8180
}
8281

8382
public Version getEsVersion() {
@@ -192,10 +191,9 @@ public ClientYamlTestResponse callApi(String apiName, Map<String, String> params
192191
protected RestClient getRestClient(NodeSelector nodeSelector) {
193192
//lazily build a new client in case we need to point to some specific node
194193
return restClients.computeIfAbsent(nodeSelector, selector -> {
195-
RestClient anyClient = restClients.get(NodeSelector.ANY);
196-
RestClientBuilder builder = RestClient.builder(anyClient.getNodes().toArray(new Node[0]));
194+
RestClientBuilder builder;
197195
try {
198-
clientBuilderConsumer.accept(builder);
196+
builder = clientBuilderWithSniffedNodes.get();
199197
} catch (IOException e) {
200198
throw new UncheckedIOException(e);
201199
}

‎test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ESClientYamlSuiteTestCase.java‎

Lines changed: 8 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import org.elasticsearch.client.Response;
2828
import org.elasticsearch.client.ResponseException;
2929
import org.elasticsearch.client.RestClient;
30+
import org.elasticsearch.client.RestClientBuilder;
3031
import org.elasticsearch.client.sniff.ElasticsearchNodesSniffer;
3132
import org.elasticsearch.common.Strings;
3233
import org.elasticsearch.common.collect.Tuple;
@@ -58,13 +59,6 @@
5859
/**
5960
* Runs a suite of yaml tests shared with all the official Elasticsearch
6061
* clients against against an elasticsearch cluster.
61-
* <p>
62-
* <strong>IMPORTANT</strong>: These tests sniff the cluster for metadata
63-
* and hosts on startup and replace the list of hosts that they are
64-
* configured to use with the list sniffed from the cluster. So you can't
65-
* control which nodes receive the request by providing the right list of
66-
* nodes in the <code>tests.rest.cluster</code> system property. Instead
67-
* the tests must explictly use `node_selector`s.
6862
*/
6963
public abstract class ESClientYamlSuiteTestCase extends ESRestTestCase {
7064

@@ -116,11 +110,6 @@ protected ESClientYamlSuiteTestCase(ClientYamlTestCandidate testCandidate) {
116110
@Before
117111
public void initAndResetContext() throws Exception {
118112
if (restTestExecutionContext == null) {
119-
// Sniff host metadata in case we need it in the yaml tests
120-
List<Node> nodesWithMetadata = sniffHostMetadata();
121-
client().setNodes(nodesWithMetadata);
122-
adminClient().setNodes(nodesWithMetadata);
123-
124113
assert adminExecutionContext == null;
125114
assert blacklistPathMatchers == null;
126115
ClientYamlSuiteRestSpec restSpec = ClientYamlSuiteRestSpec.load(SPEC_PATH);
@@ -172,8 +161,7 @@ protected ClientYamlTestClient initClientYamlTestClient(
172161
final RestClient restClient,
173162
final List<HttpHost> hosts,
174163
final Version esVersion) {
175-
return new ClientYamlTestClient(restSpec, restClient, hosts, esVersion,
176-
restClientBuilder -> configureClient(restClientBuilder, restClientSettings()));
164+
return new ClientYamlTestClient(restSpec, restClient, hosts, esVersion, this::getClientBuilderWithSniffedHosts);
177165
}
178166

179167
@AfterClass
@@ -407,13 +395,16 @@ protected boolean randomizeContentType() {
407395
}
408396

409397
/**
410-
* Sniff the cluster for host metadata.
398+
* Sniff the cluster for host metadata and return a
399+
* {@link RestClientBuilder} for a client with that metadata.
411400
*/
412-
private List<Node> sniffHostMetadata() throws IOException {
401+
protected final RestClientBuilder getClientBuilderWithSniffedHosts() throws IOException {
413402
ElasticsearchNodesSniffer.Scheme scheme =
414403
ElasticsearchNodesSniffer.Scheme.valueOf(getProtocol().toUpperCase(Locale.ROOT));
415404
ElasticsearchNodesSniffer sniffer = new ElasticsearchNodesSniffer(
416405
adminClient(), ElasticsearchNodesSniffer.DEFAULT_SNIFF_REQUEST_TIMEOUT, scheme);
417-
return sniffer.sniff();
406+
RestClientBuilder builder = RestClient.builder(sniffer.sniff().toArray(new Node[0]));
407+
configureClient(builder, restClientSettings());
408+
return builder;
418409
}
419410
}

‎test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -389,7 +389,32 @@ private static NodeSelector parseAttributeValuesSelector(XContentParser parser)
389389
if (token == XContentParser.Token.FIELD_NAME) {
390390
key = parser.currentName();
391391
} else if (token.isValue()) {
392-
NodeSelector newSelector = new HasAttributeNodeSelector(key, parser.text());
392+
/*
393+
* HasAttributeNodeSelector selects nodes that do not have
394+
* attribute metadata set so it can be used against nodes that
395+
* have not yet been sniffed. In these tests we expect the node
396+
* metadata to be explicitly sniffed if we need it and we'd
397+
* like to hard fail if it is not so we wrap the selector so we
398+
* can assert that the data is sniffed.
399+
*/
400+
NodeSelector delegate = new HasAttributeNodeSelector(key, parser.text());
401+
NodeSelector newSelector = new NodeSelector() {
402+
@Override
403+
public void select(Iterable<Node> nodes) {
404+
for (Node node : nodes) {
405+
if (node.getAttributes() == null) {
406+
throw new IllegalStateException("expected [attributes] metadata to be set but got "
407+
+ node);
408+
}
409+
}
410+
delegate.select(nodes);
411+
}
412+
413+
@Override
414+
public String toString() {
415+
return delegate.toString();
416+
}
417+
};
393418
result = result == NodeSelector.ANY ?
394419
newSelector : new ComposeNodeSelector(result, newSelector);
395420
} else {

‎test/framework/src/test/java/org/elasticsearch/test/rest/yaml/section/DoSectionTests.java‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -539,6 +539,15 @@ public void testNodeSelectorByVersion() throws IOException {
539539
doSection.execute(context);
540540
verify(context).callApi("indices.get_field_mapping", singletonMap("index", "test_index"),
541541
emptyList(), emptyMap(), doSection.getApiCallSection().getNodeSelector());
542+
543+
{
544+
List<Node> badNodes = new ArrayList<>();
545+
badNodes.add(new Node(new HttpHost("dummy")));
546+
Exception e = expectThrows(IllegalStateException.class, () ->
547+
doSection.getApiCallSection().getNodeSelector().select(badNodes));
548+
assertEquals("expected [version] metadata to be set but got [host=http://dummy]",
549+
e.getMessage());
550+
}
542551
}
543552

544553
private static Node nodeWithVersion(String version) {
@@ -567,6 +576,14 @@ public void testNodeSelectorByAttribute() throws IOException {
567576
doSection.getApiCallSection().getNodeSelector().select(nodes);
568577
assertEquals(Arrays.asList(hasAttr), nodes);
569578
}
579+
{
580+
List<Node> badNodes = new ArrayList<>();
581+
badNodes.add(new Node(new HttpHost("dummy")));
582+
Exception e = expectThrows(IllegalStateException.class, () ->
583+
doSection.getApiCallSection().getNodeSelector().select(badNodes));
584+
assertEquals("expected [attributes] metadata to be set but got [host=http://dummy]",
585+
e.getMessage());
586+
}
570587

571588
parser = createParser(YamlXContent.yamlXContent,
572589
"node_selector:\n" +

‎x-pack/docs/src/test/java/org/elasticsearch/smoketest/XDocsClientYamlTestSuiteIT.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,7 @@ protected ClientYamlTestClient initClientYamlTestClient(
5858
final RestClient restClient,
5959
final List<HttpHost> hosts,
6060
final Version esVersion) {
61-
return new ClientYamlDocsTestClient(restSpec, restClient, hosts, esVersion,
62-
restClientBuilder -> configureClient(restClientBuilder, restClientSettings()));
61+
return new ClientYamlDocsTestClient(restSpec, restClient, hosts, esVersion, this::getClientBuilderWithSniffedHosts);
6362
}
6463

6564
/**

0 commit comments

Comments
 (0)