Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test: Only sniff host metadata for node_selectors #32750

Merged
merged 6 commits into from
Aug 10, 2018
Merged

Conversation

nik9000
Copy link
Member

@nik9000 nik9000 commented Aug 9, 2018

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.

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.
@nik9000 nik9000 added review :Delivery/Build Build or test infrastructure v7.0.0 v6.5.0 labels Aug 9, 2018
@nik9000 nik9000 requested a review from javanna August 9, 2018 16:16
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@nik9000
Copy link
Member Author

nik9000 commented Aug 9, 2018

I've intentionally opened this agianst 6.x because 6.x is where we actually use the node_selector. I'll forward port to master once reviewed and merged.

Copy link
Member

@javanna javanna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left a couple of nits on naming, LGTM otherwise. thanks @nik9000 !

*/
private List<Node> sniffHostMetadata() throws IOException {
protected final RestClientBuilder sniffHostMetadata() throws IOException {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rename this method to something like getClientBuilderWithSniffedHosts

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll do it!


ClientYamlTestClient(
final ClientYamlSuiteRestSpec restSpec,
final RestClient restClient,
final List<HttpHost> hosts,
final Version esVersion,
final CheckedConsumer<RestClientBuilder, IOException> clientBuilderConsumer) {
final CheckedSupplier<RestClientBuilder, IOException> clientBuilderBuilder) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we rename the instance member and corresponding arguments to something like clientBuilderSupplier or clientBuilderWithSniffedNodes?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me!

@nik9000 nik9000 merged commit f72cf57 into elastic:6.x Aug 10, 2018
nik9000 added a commit that referenced this pull request Aug 10, 2018
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.
@nik9000 nik9000 added the v6.4.0 label Aug 10, 2018
nik9000 added a commit that referenced this pull request Aug 10, 2018
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.
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Aug 13, 2018
…listeners

* elastic/master: (58 commits)
  [ML] Partition-wise maximum scores (elastic#32748)
  [DOCS] XContentBuilder#bytes method removed, using BytesReference.bytes(docBuilder) (elastic#32771)
  HLRC: migration get assistance API (elastic#32744)
  Add a task to run forbiddenapis using cli (elastic#32076)
  [Kerberos] Add debug log statement for exceptions (elastic#32663)
  Make x-pack core pull transport-nio (elastic#32757)
  Painless: Clean Up Whitelist Names (elastic#32791)
  Cat apis: Fix index creation time to use strict date format (elastic#32510)
  Clear Job#finished_time when it is opened (elastic#32605) (elastic#32755)
  Test: Only sniff host metadata for node_selectors (elastic#32750)
  Update scripted metric docs to use `state` variable (elastic#32695)
  Painless: Clean up PainlessCast (elastic#32754)
  [TEST] Certificate NONE not allowed in FIPS JVM (elastic#32753)
  [ML] Refactor ProcessCtrl into Autodetect and Normalizer builders (elastic#32720)
  Access build tools resources (elastic#32201)
  Tests: Disable rolling upgrade tests with system key on fips JVM (elastic#32775)
  HLRC: Ban LoggingDeprecationHandler (elastic#32756)
  Fix test reproducability in AbstractBuilderTestCase setup (elastic#32403)
  Only require java<version>_home env var if needed
  Tests: Muted ScriptDocValuesDatesTests.testJodaTimeBwc
  ...
@jimczi jimczi added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019
@mark-vieira mark-vieira added the Team:Delivery Meta label for Delivery team label Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Delivery/Build Build or test infrastructure Team:Delivery Meta label for Delivery team v6.4.0 v6.5.0 v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants