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

fix: set snapshot identifier and backup retention period to null to default to AWS provider settings #314

Conversation

bryantbiggs
Copy link
Member

Description

  • update snapshot identifier based on what is used in terraform-aws-rds-aurora module using the random provider. if skip_final_snapshot = false (default - matches AWS provider default) then users can either continue to use the existing final_snapshot_identifier variable if they chose, or remove/leave null (default) and a unique final snapshot identifier will be created for them
  • set backup retention period to null to default to AWS provider settings which should result in a retention period of 7 days
  • add missing parameters from MSSQL RDS instance of db_instance sub-module (parameters were set on default instance, but not on mssql instance:
    • ca_cert_identifier
    • delete_automated_backups
    • performances_insights_kms_key_id

Motivation and Context

  • align default settings with those provided by AWS provider
  • align settings/configurations with those used in the terraform-aws-rds-aurora; avoid name conflicts on final snapshot identifiers
  • ensure all parameters are available for both the default and the mssql db instance configurations (where appropriate)

Closes #90
Closes #91
Closes #249

Breaking Changes

  • Yes; see diffs below. both examples below were spun up using what is configured on master and the diffs shown are against the changes in this PR. these are in-place updates and safe to perform (most users will see the final snapshot identifier being removed if they are skipping the final snapshot since the identifier is not required in that circumstance).

Diff from master to this PR for examples/complete-postgresql:

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  + create
  ~ update in-place

Terraform will perform the following actions:

  # module.db.module.db_instance.aws_db_instance.this[0] will be updated in-place
  ~ resource "aws_db_instance" "this" {
      - final_snapshot_identifier             = "complete-postgresql" -> null
        id                                    = "complete-postgresql"
        name                                  = "completePostgresql"
        tags                                  = {
            "Environment" = "dev"
            "Name"        = "complete-postgresql"
            "Owner"       = "user"
        }
        # (46 unchanged attributes hidden)

        # (1 unchanged block hidden)
    }

  # module.db_default.module.db_instance.aws_db_instance.this[0] will be updated in-place
  ~ resource "aws_db_instance" "this" {
      + final_snapshot_identifier             = (known after apply)
        id                                    = "complete-postgresql-default"
        name                                  = "completePostgresql"
      ~ skip_final_snapshot                   = true -> false
        tags                                  = {
            "Environment" = "dev"
            "Name"        = "complete-postgresql-default"
            "Owner"       = "user"
        }
        # (44 unchanged attributes hidden)

        # (1 unchanged block hidden)
    }

  # module.db_default.module.db_instance.random_id.snapshot_identifier[0] will be created
  + resource "random_id" "snapshot_identifier" {
      + b64_std     = (known after apply)
      + b64_url     = (known after apply)
      + byte_length = 4
      + dec         = (known after apply)
      + hex         = (known after apply)
      + id          = (known after apply)
      + keepers     = {
          + "id" = "complete-postgresql-default"
        }
    }

Plan: 1 to add, 2 to change, 0 to destroy.

------------------------------------------------------------------------

Note: You didn't specify an "-out" parameter to save this plan, so Terraform
can't guarantee that exactly these actions will be performed if
"terraform apply" is subsequently run.

Diff from master to this PR for examples/complete-mssql:

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # module.db.module.db_instance.aws_db_instance.this_mssql[0] will be updated in-place
  ~ resource "aws_db_instance" "this_mssql" {
      - final_snapshot_identifier             = "complete-mssql" -> null
        id                                    = "complete-mssql"
        tags                                  = {
            "Environment" = "dev"
            "Name"        = "complete-mssql"
            "Owner"       = "user"
        }
        # (50 unchanged attributes hidden)

        # (1 unchanged block hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.

------------------------------------------------------------------------

Note: You didn't specify an "-out" parameter to save this plan, so Terraform
can't guarantee that exactly these actions will be performed if
"terraform apply" is subsequently run.

How Has This Been Tested?

  • I have tested and validated these changes using one or more of the provided examples/* projects
  • provisioned configurations in examples/complete-mssql and examples/complete-postgresql as defined in current master branch
  • switch to this PR
  • run terraform plan and inspect output (copy+pasted above)
  • run terraform apply and validate changes are as desired

@@ -101,7 +101,7 @@ module "db" {
parameters = [
{
name = "autovacuum"
value = true
value = 1
Copy link
Member Author

Choose a reason for hiding this comment

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

true works but on each subsequent plan/apply, terraform wants to change this to 1 so just setting to 1 to avoid this

@antonbabenko antonbabenko merged commit 2998de9 into terraform-aws-modules:master Mar 14, 2021
@antonbabenko
Copy link
Member

Thanks, @bryantbiggs !

This one issue was rather long and painful for many users.

v2.29.0 has been just released.

@bryantbiggs bryantbiggs deleted the fix/snapshot-and-backup-retention branch March 14, 2021 21:27
@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants