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
216 stars 231 forks source link

Added AutoTune support #149

Closed dm3ch closed 1 year ago

dm3ch commented 1 year ago

what

why

references

goruha commented 1 year ago

/test all

goruha commented 1 year ago

/test all

dm3ch commented 1 year ago

@goruha Reworked input variable for auto tune to an object to be able to validate values if auto tune is enabled to not add default values (cause provider doesn't have default values for that).

Could you please re-review?

goruha commented 1 year ago

/test all

goruha commented 1 year ago

@dm3ch

tests still failed

First reason - terraform lint

not ok 8 check if terraform code is valid
# (from function `setup' in test file test/.test-harness/test/terraform/validate.bats, line 7)
#   `terraform init >/dev/null' failed
# There are some problems with the configuration, described below.
#
# The Terraform configuration must be valid before initialization so that
# Terraform can determine which modules and providers need to be installed.
# 
# Error: Invalid reference in variable validation
#
#   on variables.tf line 394, in variable "auto_tune":
#  394:     condition     = var.auto_tune.enabled == true && var.cron_schedule != null
# 
# The condition for variable "auto_tune" can only refer to the variable itself,
# using var.auto_tune.
# 
# 
# Error: Invalid validation error message
#
#   on variables.tf line [39](https://github.com/cloudposse/actions/actions/runs/4203523341/jobs/7293033803#step:8:40)5, in variable "auto_tune":
#  395:     error_message = "var.auto_tune.cron_schedule should be set if var.auto_tune.enabled == true"
# 
# Validation error message must be at least one full English sentence starting
# with an uppercase letter and ending with a period or question mark.
# 
# 
# Error: Invalid reference in variable validation
#
#   on variables.tf line 399, in variable "auto_tune":
#  399:     condition     = var.auto_tune.enabled == true && var.duration != null
# 
# The condition for variable "auto_tune" can only refer to the variable itself,
# using var.auto_tune.
# 
# 
# Error: Invalid validation error message
#
#   on variables.tf line [40](https://github.com/cloudposse/actions/actions/runs/4203523341/jobs/7293033803#step:8:41)0, in variable "auto_tune":
#  400:     error_message = "var.auto_tune.duration should be set if var.auto_tune.enabled == true"
# 
# Validation error message must be at least one full English sentence starting
# with an uppercase letter and ending with a period or question mark.
# 
# 
# Error: Invalid validation error message
#
#   on variables.tf line 405, in variable "auto_tune":
#  405:     error_message = "var.auto_tune.rollback_on_disable valid values: DEFAULT_ROLLBACK or NO_ROLLBACK."
# 
# Validation error message must be at least one full English sentence starting
# with an uppercase letter and ending with a period or question mark.
#

The second reason - is, please, rebase the latest changes from the master branch - it looks like you lost it when did there rewrite

dm3ch commented 1 year ago

Seems to be done. But haven't try to run terraform lint locally

goruha commented 1 year ago

/test all

goruha commented 1 year ago

@dm3ch

Tests failed

Test: check if terraform code is valid
File: validate.bats
---------------------------------

Error: Invalid value for variable

  on ../../variables.tf line 367:
 367: variable "auto_tune" {

Variable auto_tune.cron_schedule should be set if var.auto_tune.enabled ==
true.

This was checked by the validation rule at ../../variables.tf:393,3-13.

Error: Invalid value for variable

  on ../../variables.tf line 367:
 367: variable "auto_tune" {

Variable auto_tune.duration should be set if var.auto_tune.enabled == true.

This was checked by the validation rule at ../../variables.tf:398,3-13.

Gime 5 minutes I will suggest fixes

goruha commented 1 year ago

@dm3ch check my suggestions

dm3ch commented 1 year ago

@goruha Yep, TY, missed it.

dm3ch commented 1 year ago

One more tests run?

Let's hope it would be final.

goruha commented 1 year ago

/test all

goruha commented 1 year ago

@dm3ch LGTM