fix: Ignore instance engine version updates

[komljen]

Description

Updating engine version forces replacement of instances, and they shouldn't be replaced, because cluster will update them if the engine version is changed.

Motivation and Context

Fixes #118

Breaking Changes

none

How Has This Been Tested?

I tested this on multiple aurora clusters to upgrade MySQL engine version 2.07.1 -> 2.07.2.

[antonbabenko]

Hi Alen,

Unfortunately, this is not the fix we can accept in this module because it breaks the functionality for people who want to do upgrades with Terraform.

I recommend you change the value of an argument of this module and run terraform refresh to fetch correct one from AWS. In this case, Terraform won't tell you to replace anything because version is as in configs.

[komljen]

Unfortunately, this is not the fix we can accept in this module because it breaks the functionality for people who want to do upgrades with Terraform.

But, this makes it possible to upgrade with Terraform. Currently, it is not possible, because the module will recreate the instances. Recreating RDS instances means losing the data and huge downtime even if you want to restore from a snapshot.

With this PR, the module will update the cluster resource, and the cluster resource updates the instances. After the run is finished, both cluster and instance versions are aligned.

[antonbabenko]

Have you tried the solution I've just described?

[komljen]

I did not. That is not a solution, it is a workaround.

Let me explain the issue in more detail. For example, I want to update to the new engine version. I go and edit engine_version, and run terraform apply expecting that I will get my cluster upgraded the same way the RDS console does (by not replacing the instances).

What I get instead is:

module.app_rds_cluster.aws_rds_cluster.this will be updated in-place

and 

module.app_rds_cluster.aws_rds_cluster_instance.this[0] must be replaced

Why does the instance need to be replaced when that is not required for the engine upgrade?

With this PR, I would only see module.app_rds_cluster.aws_rds_cluster.this will be updated in-place, and both my cluster and the instance will have the desired versions.

As I workaround I targeted only cluster terraform apply -target=module.app_rds_cluster.aws_rds_cluster.this and after the cluster is upgraded I can run terraform apply and it would not make any changes or replace the instance. This is not how it should work in my opinion, hence this PR.

[dj5k]

Came here to see this issue and agree fully with @komljen about both the solution and the workaround not being useful.

@antonbabenko maybe you could lay out in more detail how you would expect a solution to this problem to be implemented if you don't want to accept this PR, that would help to see how we might arrive at a solution that suits both needs.

[bcwilsondotcom]

Agreed with @komljen and @dj5k. Instances shouldn't have to be replaced to perform an upgrade. I'd like to see this PR re-opened and approved, or an alternate solution presented if this PR is unacceptable for some reason.

[antonbabenko]

I was asked to give some input on this issue.

This module is designed to support the majority of use-cases out-of-the-box without a need to manage a separate fork. It does this part as best as it is possible with the current version of Terraform.

Introducing ignore_changes = [engine_version] lifecycle will simply turn off the possibility to manage engine_version argument for users who want to have everything managed with Terraform.

To make this work for both types of users we can't have this PR merged, but we need to use the workaround specified by @komljen in the comment above.

There is no better workaround which I am aware of, which can be less hacky because Terraform 0.13 still does not allow us to use variables inside of the lifecycle block.

[komljen]

Introducing ignore_changes = [engine_version] lifecycle will simply turn off the possibility to manage engine_version argument for users who want to have everything managed with Terraform.

The above claim is simply not true.

@antonbabenko even Terraform docs for aws_rds_cluster_instance resource says that this lifecycle is recommended.

engine_version - (Optional) The database engine version. When managing the engine version in the cluster, it is recommended to add the lifecycle ignore_changes configuration for this argument to prevent Terraform from proposing changes to the instance engine version directly.

Reference https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/rds_cluster_instance

Which makes perfect sense. If you put instance under the cluster, like this module does, from AWS perspective versions are one thing. You simply cannot update one version without another. However, to update the instance version, you just need to update the cluster version. That is how AWS API works and has not anything to do with Terraform features.

[antonbabenko]

Nice. Now we are talking, I didn't see this in the docs before. 👍

But I still don't understand what if users want to be able to control engine_version in the code? If we have ignore_changes, they will not be able (right?). This is how such a change is related to Terraform behavior.

Let's merge this PR and make a note in README about this so that everyone is aware of it.

[komljen]

But I still don't understand what if users want to be able to control engine_version in the code? If we have ignore_changes, they will not be able (right?). This is how such a change is related to Terraform behavior.

They still do, engine_version is controled here as well https://github.com/terraform-aws-modules/terraform-aws-rds-aurora/blob/master/main.tf#L40.

[antonbabenko]

Thanks for the explanation and for the pointer to docs, @komljen !

v2.27.0 has been just released.

[ArnisM]

Ok

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.