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

MM-52337: Use aws_opensearch_domain #701

Merged
merged 7 commits into from
Mar 6, 2024
Merged

Conversation

agarciamontoro
Copy link
Member

Summary

We decided to use the latest OpenSearch version compatible with ElasticSearch (i.e., those until the latest OSS version of ElasticSearch, v7.10).

This PR addresses this decision, replacing the aws_elasticsearch_domain resource with aws_opensearch_domain. This forced me to upgrade the Terraform's aws provider itself, and some things had to be changed because of that.

This has been tested in a deployment using an r6g.large.search instance with Elasticsearch_7.10. It deployed successfully, and a manual index of the 20M posts DB finished in 5 hours.

image

Ticket Link

https://mattermost.atlassian.net/browse/MM-52337

The previous version, 3.0, did not support aws_opensearch_domain
resource, so we bumped it to the latest one.

In turn, the latest version, 5.39 does not support aws_subnet_ids, which
was replaced with aws_subnets.
This is done running terraform init -upgrade inside the
deployment/terraform/assets directory.
The version is now specified in the engine_version field, not in the
elasticsearch_version field. This field is now a string instead of a
number.
@agarciamontoro agarciamontoro added the 2: Dev Review Requires review by a core committer label Mar 5, 2024
@@ -2,21 +2,81 @@
# Manual edits may be lost in future updates.
Copy link
Member

Choose a reason for hiding this comment

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

Is this file meant to be checked in?

Copy link
Member Author

Choose a reason for hiding this comment

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

image

You asked this exact same question almost three years ago hahaha

Answering your question: yes. I had the same question you had three years ago (and today), double-checked the docs and yes, they still recommend to check it in.

Copy link
Member

Choose a reason for hiding this comment

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

🤦

Copy link
Member

@isacikgoz isacikgoz left a comment

Choose a reason for hiding this comment

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

LGTM, love to see defaults package getting more and more features :)

@agarciamontoro
Copy link
Member Author

love to see defaults package getting more and more features

I'm always impressed about the groundwork of the defaults.Validate function, it's so easy to add a new validation! A very nice piece of code indeed :)

@agarciamontoro agarciamontoro added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core committer labels Mar 6, 2024
@agarciamontoro
Copy link
Member Author

Merging, this has already been successfully tested.

@agarciamontoro agarciamontoro merged commit f0d71f9 into master Mar 6, 2024
1 check passed
@agarciamontoro agarciamontoro deleted the MM-52337.opensearch branch March 6, 2024 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4: Reviews Complete All reviewers have approved the pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants