Updating validation blocking single-AZ ES clusters when Zone Awareness is disabled

cloudposse / terraform-aws-elasticsearch

Terraform module to provision an Elasticsearch cluster with built-in integrations with Kibana and Logstash.

https://cloudposse.com/accelerate

Apache License 2.0

218 stars 231 forks source link

Updating validation blocking single-AZ ES clusters when Zone Awareness is disabled #159

Closed jacob-hudson closed 1 year ago

jacob-hudson commented 1 year ago

what

Removing errant validation preventing the creation of Amazon OpenSearch Service ElasticSearch (ES) or OpenSearch (OS) clusters which live in a single Available Zone on Amazon Web Services (AWS)

Simple Process:

You merge this PR
You release a new version
We can create clusters which live in one AZ
Everyone wins

why

Development and testing clusters may not want or need two or more Availability Zones for cost (among other reasons, but FinOps - saving money - is important)

references

https://github.com/cloudposse/terraform-aws-elasticsearch/issues/158

Identical errors in internal company environment

milldr commented 1 year ago

valid values for zone_awareness_config are only 2 and 3 for the Terraform resource. Why remove that check?

terraform documentation: https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/elasticsearch_domain
relevant module resource attribute: https://github.com/cloudposse/terraform-aws-elasticsearch/blob/3125d5a76a4c3a7021aec48459086de6aac35b01/main.tf#L155-L160

jacob-hudson commented 1 year ago

valid values for zone_awareness_config are only 2 and 3 for the Terraform resource. Why remove that check?

terraform documentation: https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/elasticsearch_domain

relevant module resource attribute: https://github.com/cloudposse/terraform-aws-elasticsearch/blob/3125d5a76a4c3a7021aec48459086de6aac35b01/main.tf#L155-L160

You guys do not pin module versions to TF versions, meaning we could need the latest module (eg for EBS GP3) and not care about which aws provider version

Maybe remove the check and let AWS respond with an error a la GraphQL?

jacob-hudson commented 1 year ago

This seems unnecessary to me. I would need to see added testing before I would approve this.

Users should be able to set zone_awareness_enabled to false and leave availability_zone_count unset and defaulted to 2, or set to 2 if the configuration of the root module makes leaving it unset too difficult.

Working on this, give me a few

jacob-hudson commented 1 year ago

Interesting point:

Would there ever be a chance you would want Zone Awareness disabled with more than 1 AZ?

ManuelMueller1st commented 1 year ago

Can't we add 1 as a valid count? This seems unnecessarily complex to me.