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

[BUG] OpenSearch and Extension host and port are required before initializing either #729

Closed
dbwiddis opened this issue May 5, 2023 · 3 comments · Fixed by #730
Closed
Labels
bug Something isn't working discuss

Comments

@dbwiddis
Copy link
Member

dbwiddis commented May 5, 2023

What is the bug?

As currently designed:

  • 🐔 we must know the Extension host and port (extensions.yml file) and the extension must be running prior to starting OpenSearch.
  • 🥚 when initializing an Extension, we initialize and inject the SDKRestClient which requires the host and port of OpenSearch.

This works fine when we're testing everything on localhost but fails in the real world use case where you are starting up a cluster using automation and don't know in advance its eventual ELB hostname when starting the extension.

How can one reproduce the bug?

Attempt to use opensearch-cluster-cdk to spin up an OpenSearch cluster attached to an Extension on an EC2 instance in the same account.

What is the expected behavior?

  1. Extension does not need OpenSearch host/port on initialization
  2. When OpenSearch connects to extension, it passes its connection information
  3. Extension then knows how to connect to OpenSearch

Do you have any additional context?

I commented about this here.

This will absolutely be required to be handled in a future case involving stateless (AWS Lambda/Azure function/OCI function) connections.

One possible solution is to not inject the RestClient; just inject the SDKClient and initialize the RestClient internally after the extension has been initialized (similar pattern we use for NamedXContentRegistry which, I might note, is tightly integrated with RESTClient).

Another solution looks like there are potentially some workarounds with the low level RestClient which looks designed for this use case:

  • The constructor using HttpHost varargs has javadoc "You can use this if you do not have metadata up front about the nodes."
  • There is a setNodes(Collection<Node> nodes) method
@dbwiddis dbwiddis added bug Something isn't working untriaged labels May 5, 2023
@dbwiddis
Copy link
Member Author

dbwiddis commented May 5, 2023

Proceeding down the second solution (setNodes()) path right now to unblock myself but I think we need a whole re-think/redesign of Extension initialization.

@dbwiddis dbwiddis added discuss and removed untriaged labels May 5, 2023
@dblock
Copy link
Member

dblock commented May 5, 2023

@dbwiddis Shouldn't this information be exchanged at the (run)time an extension is registered?

@dbwiddis
Copy link
Member Author

dbwiddis commented May 5, 2023

@dbwiddis Shouldn't this information be exchanged at the (run)time an extension is registered?

Yes, and it is in one place. Our problem was/is in eagerly initializing the RestHighLevelClient and injecting it to ease migration efforts. That shortcut cost us some things.

Will open a new issue discussing better initialization sequence.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working discuss
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants