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

Add QA project and fixture based test for discovery-ec2 plugin #31107

Merged
merged 2 commits into from
Jun 15, 2018

Conversation

tlrx
Copy link
Member

@tlrx tlrx commented Jun 5, 2018

This commit adds a new QA sub project to the discovery-ec2 plugin. This project uses a fixture to test the plugin using a multi-node cluster. Once all nodes are started, the nodes transport addresses are written in a file that is later read by the fixture. This will need more work and thinking to be executable on real EC2 instances, but the test is already useful.

@tlrx tlrx added >test Issues or PRs that are addressing/adding tests review :Distributed Coordination/Discovery-Plugins Anything related to our integration plugins with EC2, GCP and Azure v7.0.0 v6.4.0 labels Jun 5, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

@tlrx tlrx force-pushed the discovery-ec2-integration-tests branch from f84d11a to d508b1a Compare June 6, 2018 07:28
@tlrx tlrx requested review from ywelsch and rjernst June 6, 2018 07:29
@tlrx
Copy link
Member Author

tlrx commented Jun 6, 2018

I'll open a follow up PR to mutualize the code of the multiple fixtures we now have.

Copy link
Contributor

@ywelsch ywelsch left a comment

Choose a reason for hiding this comment

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

I've left two minor comments / questions, looks good o.w.


@Override
public void handle(HttpExchange exchange) throws IOException {
RestStatus responseStatus = RestStatus.INTERNAL_SERVER_ERROR;
Copy link
Contributor

Choose a reason for hiding this comment

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

should this be BAD_REQUEST instead? If we cannot handle the request, it's not that there's an internal server error, but that the request is unexpected.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, BAD_REQUEST is better. It has been implemented in #31210 on top of which this PR has been rebased.

ec2DiscoveryFile.setText(integTest.nodes.collect { n -> "${n.transportUri()}" }.join('\n'), 'UTF-8')

File tmpFile = new File(node.cwd, 'wait.success')
ant.get(src: "http://${node.httpUri()}/_cluster/health?wait_for_nodes=>=${ec2NumberOfNodes}&wait_for_status=yellow",
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of waiting for the correct number of nodes here, we could do that in the yml with a cluster health request. Also, why do we wait for yellow status?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, I moved this to the yml test. The yellow status was a leftover.

tlrx added 2 commits June 14, 2018 14:23
This commit adds a new QA sub project to the discovery-ec2 plugin. This
project uses a fixture to test the plugin using a multi-node cluster.

Once all nodes are started, the nodes transport addresses are written in
a file that is later read by the fixture.
@tlrx tlrx force-pushed the discovery-ec2-integration-tests branch from d508b1a to a8c976b Compare June 14, 2018 13:00
@tlrx
Copy link
Member Author

tlrx commented Jun 14, 2018

Thanks @ywelsch for the review and sorry for the time it took me to apply your feedback.

I rebase this PR on top of master so that it beneficiates from #31210 and #31272 and has a chance to get a green build. Can you have another look please? Thanks

Copy link
Contributor

@ywelsch ywelsch left a comment

Choose a reason for hiding this comment

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

LGTM

@tlrx tlrx merged commit afc91e9 into elastic:master Jun 15, 2018
@tlrx
Copy link
Member Author

tlrx commented Jun 15, 2018

Thanks @ywelsch

@tlrx tlrx deleted the discovery-ec2-integration-tests branch June 15, 2018 13:49
dnhatn added a commit that referenced this pull request Jun 15, 2018
* master:
  Upgrade to Lucene-7.4.0-snapshot-518d303506 (#31360)
  Rankeval: Fold template test project into main module (#31203)
  Add QA project and fixture based test for discovery-ec2 plugin (#31107)
  [Docs] Remove reference to repository-s3 plugin creating an S3 bucket (#31359)
  REST Client: NodeSelector for node attributes (#31296)
  LLClient: Fix assertion on windows
  Add details section for dcg ranking metric (#31177)
  [ML] Re-enable tests muted in #30982
tlrx added a commit that referenced this pull request Jun 15, 2018
This commit adds a new QA sub project to the discovery-ec2 plugin.
This project uses a fixture to test the plugin using a multi-node cluster.
Once all nodes are started, the nodes transport addresses are written
in a file that is later read by the fixture.
dnhatn added a commit that referenced this pull request Jun 19, 2018
* 6.x:
  Add get stored script and delete stored script to high level REST API
  Increasing skip version for failing test on 6.x
  Skip get_alias tests for 5.x (#31397)
  Fix defaults in GeoShapeFieldMapper output (#31302)
  Test: better error message on failure
  Mute DefaultShardsIT#testDefaultShards test
  Fix reference to XContentBuilder.string() (#31337)
  [DOCS] Adds monitoring breaking change (#31369)
  [DOCS] Adds security breaking change (#31375)
  [DOCS] Backports breaking change (#31373)
  RestAPI: Reject forcemerge requests with a body (#30792)
  Docs: Use the default distribution to test docs (#31251)
  Use system context for cluster state update tasks (#31241)
  [DOCS] Adds testing for security APIs (#31345)
  [DOCS] Removes ML item from release highlights
  [DOCS] Removes breaking change (#31376)
  REST high-level client: add validate query API (#31077)
  Move language analyzers from server to analysis-common module. (#31300)
  Expose lucene's RemoveDuplicatesTokenFilter (#31275)
  [Test] Fix :example-plugins:rest-handler on Windows
  Delete typos in SAML docs (#31199)
  Ensure we don't use a remote profile if cluster name matches (#31331)
  Test: Skip alias tests that failed all weekend
  [DOCS] Fix version in SQL JDBC Maven template
  [DOCS] Improve install and setup section for SQL JDBC
  Add ingest-attachment support for per document `indexed_chars` limit (#31352)
  SQL: Fix rest endpoint names in node stats (#31371)
  [DOCS] Fixes small issue in release notes
  Support for remote path in reindex api Closes #22913
  [ML] Put ML filter API response should contain the filter (#31362)
  Remove trial status info from start trial doc (#31365)
  [DOCS] Added links in breaking changes pages
  [DOCS] Adds links to release notes and highlights
  Docs: Document changes in rest client
  QA: Fix tribe tests to use node selector
  REST Client: NodeSelector for node attributes (#31296)
  LLClient: Fix assertion on windows
  LLClient: Support host selection (#30523)
  Add QA project and fixture based test for discovery-ec2 plugin (#31107)
  [ML] Hold ML filter items in sorted set (#31338)
  [ML] Add description to ML filters (#31330)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed Coordination/Discovery-Plugins Anything related to our integration plugins with EC2, GCP and Azure >test Issues or PRs that are addressing/adding tests v6.4.0 v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants