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

New variables: create_parameter_group and parameter_group_name #208

Merged
merged 7 commits into from Dec 1, 2023

Conversation

y3ti
Copy link
Contributor

@y3ti y3ti commented Nov 27, 2023

what

  • Add redis family suffix to parameter group name
  • Add new variables: create_parameter_group and parameter_group_name

why

This module doesn't currently support major version upgrades of Redis (eg, 6.x to. 7.x) because:

  • Parameter groups are major-version specific, so when a user changes var.family from redis6 to redis7, Terraform needs to create a new parameter group. Without create_before_destroy, Terraform tries to first destroy the old Param group which fails because it's currently in use
  • Parameter groups must have unique names across families. When Terraform tries to create a new param group for redis7, it fails because it tries to do so using the same name as the old param group.

I have decided to add "redis cluster family" as a suffix. AWS follows a similar convention for default parameter groups, using names such as:

  • default.redis6.x
  • default.redis7

Since using . is not possible, I have opted to use - instead.

To prevent any breaking changes, I have introduced a new variable called parameter_group_name. By setting this variable to the current parameter group name, you can prevent any terraform configuration drift.

We can also reuse existing parameter groups.

If we want to use the default parameter group created by AWS (default.redis7)

create_parameter_group = false
engine                 = "redis7"

If we want to use any other existing parameter group:

create_parameter_group = false
parameter_group_name   = "existing-parameter-group-name"

references

Resolves #178

I see that other people tried to solve this problem before (see references), but the pull requests were not merged yet:

@y3ti y3ti requested review from a team as code owners November 27, 2023 11:59
@y3ti y3ti requested review from kevcube and woz5999 November 27, 2023 11:59
@y3ti y3ti changed the title new variable: parameter_group_name parametrize parameter_group_name Nov 27, 2023
main.tf Outdated Show resolved Hide resolved
Copy link
Member

@aknysh aknysh left a comment

Choose a reason for hiding this comment

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

thanks, please see comments

@y3ti
Copy link
Contributor Author

y3ti commented Nov 30, 2023

I added new variable create_parameter_group, the module can also use default parameter group.

@y3ti y3ti changed the title parametrize parameter_group_name New variables: create_parameter_group and parameter_group_name Nov 30, 2023
@y3ti y3ti requested a review from aknysh November 30, 2023 22:06
main.tf Outdated Show resolved Hide resolved
main.tf Outdated Show resolved Hide resolved
main.tf Outdated Show resolved Hide resolved
y3ti and others added 3 commits December 1, 2023 16:19
@y3ti y3ti requested a review from aknysh December 1, 2023 15:42
@aknysh
Copy link
Member

aknysh commented Dec 1, 2023

/terratest

@aknysh aknysh added the major Breaking changes (or first stable release) label Dec 1, 2023
main.tf Outdated Show resolved Hide resolved
@aknysh
Copy link
Member

aknysh commented Dec 1, 2023

/terratest

@aknysh aknysh merged commit 4a58a80 into cloudposse:main Dec 1, 2023
9 checks passed
@y3ti y3ti deleted the add-family-to-parameter-group-name branch December 1, 2023 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
major Breaking changes (or first stable release)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

aws_elasticache_parameter_group should have create_before_destroy set to true
2 participants