-
Notifications
You must be signed in to change notification settings - Fork 59
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
fix(opensearch-provider): fix working with opensearch 2.x.x #16
Conversation
732a233
to
4952486
Compare
can we see this PR merged soon ? |
Hey thanks for the PR @serge-r and apologies for the long wait, ready to roll. |
Thank you guys and sorry for delay, I will check and send a response ASAP |
For us, the resources
Happy to test when fixes are implemented. |
@phillbaker @jackson-theisen @prudhvigodithi or @peterzhuamazon can one of you please review this? I also need this feature. |
4952486
to
d09a94a
Compare
Hello guys! I have pushed fixed version. Please review. @andrejvanderzee I tested opensearch_dashboard_object and opensearch_index_template with self-hosted opensearch and dashboard version 2.3, everything working normally. Unfortunately I have no ability to test it with AWS opensearch. I think that need to use this version carefully, because Opensearch can use different API - at this time this provider using elastic7 library. Perhaps it will work better with native opensearch go lib. |
3d08113
to
3b8b81d
Compare
…astic6 lib, remove version check Signed-off-by: Sergei Razukov <[email protected]>
3b8b81d
to
0f50f68
Compare
@serge-r The index templates are correctly created with your fork on AWS Opensearch 2.5. But, it keeps giving me changes after multiple applies:
|
@serge-r The problem is with keeping appplying non-changes while it already has been applied, on AWS you soon run into errors such as the below (for
Would be nice if the above can be fixed. |
@phillbaker @prudhvigodithi @peterzhuamazon @jackson-theisen Can someone please review? Looks good to me at least until the Opensearch Go client is used by this provider. |
Hey @serge-r I see the tests are failing also, I assume the PR is not rebased can you please check? Also @phillbaker can you add your initial thoughts on this? |
Thank you for the contribution. Right now, I'd like to focus on getting a v1 of this provider out the door. I'm happy to fully review this once that happens. At a high level, this PR is pretty large and widely impacting. It should be broken up into separate PRs, one for each major change. E.g. a first PR for dropping elasticsearch 6 support, a second PR for deprecated API for index templates, etc. Those smaller PRs will also make merging individual fixes easier without having to wait for the entire PR to be ready. Overall, these changes will need to have appropriate updates to the tests. |
@serge-r Do you have an example of
It applies, but in AWS Opensearch 2.5 I see now saved object / index pattern. |
Hello! Sorry for delay, unfortunately I can't spend many times on it( Removing elastic6 it's already huge amount of changes, but I understand the problem and try to divide the PR to small PRs. @andrejvanderzee about consistent changing errors - I will check, thank you for the info about problem with opensearch_dashboard_object - check please which index in AWS using for store dashboard objects, perhaps it have different name. Default index in provider is ".dashboard". In AWS it can be ".kibana" or any other name. You can set dashboards index via index field in the resource like
|
Thanks @serge-r adding |
Any thoughts on when this might get merged and a new version made available? |
Hey @serge-r based on @phillbaker suggestion, can you please split the changes into different PR's and adding unit tests for each of them? I have created an issue to track the 2.x support #39 please check. Also if you are ok we can close this PR and move forward with smaller PRs that can track 2.x support. |
Hello guys, and sorry for waiting. I have made new PR with less amount of changes, please review. |
@prudhvigodithi @phillbaker Can we prioritize this PR? It would be beneficial for lots of users using 2.x version. |
+1 |
@prudhvigodithi @phillbaker Can someone please take a look at this PR please? |
What is the minimum change necessary to support Opensearch 2.x.x? Is it just this?:
Or do we need the Elastic 6/7 deprecations as well to support Opensearch 2.x.x? |
totally appreciate everyone's time in the open source project, any updates on this? |
Any news? |
This would be huge for those in the community running OpenSearch 2.x.x |
Closing this PR, please check the latest PR that has all the support for |
Hey All, we just released the Beta version of OpenSearch Terraform Provider with 2.x support. We aim to release the non-beta version based on the feedback. Additionally, we encourage and welcome contributions from the community. If you come across any issues or wish to expand the coverage of APIs, we invite you to contribute to the project. Thank you |
Signed-off-by: serge-r [email protected]
Description
Hello!
Some time ago, I tried to use this provider to support our Opensearch 2.4.0 infrastructure in Terraform. But I have found the next problems:
Perhaps there are more problems, but at this time I have tested only Indexes and IndexTemplates resources.
I know also about https://github.com/phillbaker/terraform-provider-elasticsearch, which this provider was forked from, but I think, that is more about Elasticsearch, not Opensearch. Unfortunately, this provider also has the same problems with version detection.
Please say what are you thinking about this PR. And also big thanks for your and Phillbaker's work!
Issues Resolved
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.