Skip to content

Conversation

@lukehsiao
Copy link
Contributor

@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 👍

@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
Contributor 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?

@Gowiem
Copy link
Member

Gowiem commented Feb 6, 2025

/terratest

@Gowiem Gowiem enabled auto-merge (squash) February 6, 2025 16:12
Gowiem
Gowiem previously approved these changes Feb 6, 2025
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
@mergify
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!

@goruha
Copy link
Member

goruha commented Mar 20, 2025

@lukehsiao could you pls fix the code style formatting?
it seem there are some nonprintable chars (spaces or tabs) that need to be removed

diff --git a/variables.tf b/variables.tf
index 7b0d0b[5](https://github.com/cloudposse/terraform-aws-elasticsearch/actions/runs/13168882144/job/36798395645?pr=196#step:6:6)..63f2d31 100644
--- a/variables.tf
+++ b/variables.tf
@@ -143,[7](https://github.com/cloudposse/terraform-aws-elasticsearch/actions/runs/13168882144/job/36798395645?pr=196#step:6:8) +143,7 @@ variable "availability_zone_count" {
 
 variable "multi_az_with_standby_enabled" {
   type        = bool
-  default     = false  
+  default     = false
   description = "Enable domain with standby for OpenSearch cluster"
 }

@goruha goruha self-assigned this Mar 20, 2025
@lukehsiao
Copy link
Contributor Author

lukehsiao commented Mar 20, 2025

@lukehsiao could you pls fix the code style formatting? it seem there are some nonprintable chars (spaces or tabs) that need to be removed

diff --git a/variables.tf b/variables.tf
index 7b0d0b[5](https://github.com/cloudposse/terraform-aws-elasticsearch/actions/runs/13168882144/job/36798395645?pr=196#step:6:6)..63f2d31 100644
--- a/variables.tf
+++ b/variables.tf
@@ -143,[7](https://github.com/cloudposse/terraform-aws-elasticsearch/actions/runs/13168882144/job/36798395645?pr=196#step:6:8) +143,7 @@ variable "availability_zone_count" {
 
 variable "multi_az_with_standby_enabled" {
   type        = bool
-  default     = false  
+  default     = false
   description = "Enable domain with standby for OpenSearch cluster"
 }

Apologies. Making the change now.

update: this is done.

auto-merge was automatically disabled March 20, 2025 18:09

Head branch was pushed to by a user without write access

@Gowiem
Copy link
Member

Gowiem commented Mar 24, 2025

/terratest

@Gowiem Gowiem added major Breaking changes (or first stable release) and removed minor New features that do not break anything labels Mar 24, 2025
@Gowiem
Copy link
Member

Gowiem commented Mar 24, 2025

@goruha looks like we're still failing with the following, can you investigate?

TestExamplesComplete 2025-03-24T21:28:55Z retry.go:99: Returning due to fatal error: FatalError{Underlying: error while running command: exit status 1; ╷
│ Error: no matching Route 53 Hosted Zone found
│ 
│   with module.elasticsearch.module.domain_hostname.data.aws_route53_zone.default[0],
│   on .terraform/modules/elasticsearch.domain_hostname/main.tf line 5, in data "aws_route53_zone" "default":
│    5: data "aws_route53_zone" "default" {
│ 
╵
╷
│ Error: no matching Route 53 Hosted Zone found
│ 
│   with module.elasticsearch.module.kibana_hostname.data.aws_route53_zone.default[0],
│   on .terraform/modules/elasticsearch.kibana_hostname/main.tf line 5, in data "aws_route53_zone" "default":
│    5: data "aws_route53_zone" "default" {
│ 
╵}

Thank you!

@mergify
Copy link

mergify bot commented Jun 23, 2025

💥 This pull request now has conflicts. Could you fix it @lukehsiao? 🙏

@mergify mergify bot added conflict This PR has conflicts and removed conflict This PR has conflicts labels Jun 23, 2025
@goruha
Copy link
Member

goruha commented Jul 30, 2025

@lukehsiao Hello.
Sorry for the long response. I'm ready to merge the PR if you can resolve the conflicts.
CleanShot 2025-07-30 at 12 46 25@2x

@lukehsiao
Copy link
Contributor Author

lukehsiao commented Jul 30, 2025

@lukehsiao Hello. Sorry for the long response. I'm ready to merge the PR if you can resolve the conflicts.

https://github.com/cloudposse/.github/blob/main/CONTRIBUTING.md

It seems someone removed the Makefile, making the CONTRIBUTING guidelines for generating the README incorrect. What's the right way to re-generate the README now?

commit a03aefdb0e48161ac0fd78f8488b88985632bd85
Author:     Erik Osterman (CEO @ Cloud Posse) <[email protected]>
AuthorDate: Thu May 29 12:44
Commit:     GitHub <[email protected]>
CommitDate: Thu May 29 12:44

    chore: Replace Makefile with atmos.yaml (#202)

@lukehsiao
Copy link
Contributor Author

I regenerated the readme with

atmos docs generate readme

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: #195
@goruha
Copy link
Member

goruha commented Jul 30, 2025

/terratest

1 similar comment
@goruha
Copy link
Member

goruha commented Jul 30, 2025

/terratest

@goruha goruha merged commit 9bca2cd into cloudposse:main Jul 30, 2025
19 checks passed
@mergify mergify bot removed the needs-cloudposse Needs Cloud Posse assistance label Jul 30, 2025
@github-actions
Copy link
Contributor

These changes were released in v1.0.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request major Breaking changes (or first stable release)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support multi_az_with_standby_enabled

3 participants