Skip to content
New issue

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

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

Already on GitHub? # to your account

feat: support multi_az_with_standby_enabled for opensearch #196

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lukehsiao
Copy link

@lukehsiao lukehsiao commented Oct 10, 2024

feat: support multi_az_with_standby_enabled for opensearch

Note that this bumps the minimum hashicorp/aws provider version to
5.15.0, where this parameter was introduced [1].

The README diff was generated with make init and make readme, and
introduces some minor unrelated changes.

Closes: #195


what

This PR simply exposes a new variable (multi_az_with_standby_enabled) for OpenSearch clusters.

why

This is the recommended setting by AWS, so it makes sense to be able to do this via terraform.

references

Closes: #195

@lukehsiao lukehsiao requested review from a team as code owners October 10, 2024 23:35
@lukehsiao lukehsiao requested review from Gowiem and jamengual October 10, 2024 23:35
@mergify mergify bot added the triage Needs triage label Oct 10, 2024
Copy link
Member

@Gowiem Gowiem left a comment

Choose a reason for hiding this comment

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

One request to confirm before we move this forward 👍

dedicated_master_enabled = var.dedicated_master_enabled
dedicated_master_count = var.dedicated_master_count
dedicated_master_type = var.dedicated_master_type
multi_az_with_standby_enabled = var.multi_az_with_standby_enabled
Copy link
Member

Choose a reason for hiding this comment

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

What version of the AWS provider was multi_az_with_standby_enabled added in? Does our current pin of the AWS provider in versions.tf require that version or above? Please look into this, update if needed, and then rerun the README generation with make init && make readme. Thanks!

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the callout! It does need a minimum bump, the variable was introduced in https://github.com/hashicorp/terraform-provider-aws/blob/main/CHANGELOG.md#5150-august-31-2023. I've updated that value and run README generation. Note that that make readme fixes some things I missed when I did it manually, but also introduces some minor, seemingly unrelated changes (e.g., extra /).

@lukehsiao lukehsiao force-pushed the main branch 3 times, most recently from 0232f59 to a0c05dd Compare October 12, 2024 16:29
@lukehsiao lukehsiao requested a review from Gowiem October 12, 2024 16:29
@Gowiem Gowiem added enhancement New feature or request minor New features that do not break anything labels Oct 12, 2024
@Gowiem
Copy link
Member

Gowiem commented Oct 12, 2024

/terratest

@lukehsiao
Copy link
Author

I see some test failures, but looking at the logs, they don't look directly related to my change. If I'm responsible for getting these to passing, could anyone provide some guidance?

Note that this bumps the minimum `hashicorp/aws` provider version to
5.15.0, where this parameter was introduced [[1]].

The README diff was generated with `make init` and `make readme`, and
introduces some minor unrelated changes.

[1]: https://github.com/hashicorp/terraform-provider-aws/blob/main/CHANGELOG.md#5150-august-31-2023
Closes: cloudposse#195
@Gowiem
Copy link
Member

Gowiem commented Feb 6, 2025

/terratest

@Gowiem Gowiem enabled auto-merge (squash) February 6, 2025 16:12
Copy link
Member

@Gowiem Gowiem left a comment

Choose a reason for hiding this comment

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

:shipit:

@mergify mergify bot removed the triage Needs triage label Feb 6, 2025
@Gowiem Gowiem added the needs-cloudposse Needs Cloud Posse assistance label Feb 6, 2025
Copy link

mergify bot commented Feb 6, 2025

Important

Cloud Posse Engineering Team Review Required

This pull request modifies files that require Cloud Posse's review. Please be patient, and a core maintainer will review your changes.

To expedite this process, reach out to us on Slack in the #pr-reviews channel.

@Gowiem
Copy link
Member

Gowiem commented Feb 6, 2025

@lukehsiao the tests for this child module need some action from the @cloudposse team as there is an issue with the tests failing to find some R53 records in their AWS account. I'll bring this up internally to the contributors group and we'll work to get someone on it shortly. Thanks for the patience!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request minor New features that do not break anything needs-cloudposse Needs Cloud Posse assistance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support multi_az_with_standby_enabled
2 participants