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.

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

@@ -92,8 +92,7 @@ public class DocsClientYamlTestSuiteIT extends ESClientYamlSuiteTestCase {
             final List<HttpHost> 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);
     }
 
     /**

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<HttpHost> hosts,
             final Version esVersion,
             final Version masterVersion,
-            final CheckedConsumer<RestClientBuilder, IOException> clientBuilderConsumer) {
-        super(restSpec, restClient, hosts, esVersion, masterVersion, clientBuilderConsumer);
+            final CheckedSupplier<RestClientBuilder, IOException> clientBuilderWithSniffedNodes) {
+        super(restSpec, restClient, hosts, esVersion, masterVersion, clientBuilderWithSniffedNodes);
     }
 
     @Override

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<NodeSelector, RestClient> restClients = new HashMap<>();
     private final Version esVersion;
     private final Version masterVersion;
-    private final CheckedConsumer<RestClientBuilder, IOException> clientBuilderConsumer;
+    private final CheckedSupplier<RestClientBuilder, IOException> clientBuilderWithSniffedNodes;
 
     ClientYamlTestClient(
             final ClientYamlSuiteRestSpec restSpec,
@@ -74,13 +73,13 @@ public class ClientYamlTestClient implements Closeable {
             final List<HttpHost> hosts,
             final Version esVersion,
             final Version masterVersion,
-            final CheckedConsumer<RestClientBuilder, IOException> clientBuilderConsumer) {
+            final CheckedSupplier<RestClientBuilder, IOException> 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);
             }

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.
- * <p>
- * <strong>IMPORTANT</strong>: 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 <code>tests.rest.cluster</code> 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<Node> 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<HttpHost> 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<Node> 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;
     }
 }

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<Node> 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 {

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<Node> 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<Node> 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" +

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

@@ -58,8 +58,7 @@ public class XDocsClientYamlTestSuiteIT extends XPackRestIT {
             final List<HttpHost> 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);
     }
 
     /**