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

add delete_automated_backups option for rds instances #8461

Merged
merged 4 commits into from
Feb 11, 2020

Conversation

MrLunar
Copy link
Contributor

@MrLunar MrLunar commented Apr 26, 2019

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request

Fixes #6507 and #6512

Changes proposed in this pull request:

  • Ability to retain snapshots for RDS instances after database deletion

Output from acceptance testing:

$ make testAcc TEST=./aws TESTARGS='-run=TestAccAWSDBInstance_NoDeleteAutomatedBackups'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -parallel 20 -run=TestAccAWSDBInstance_NoDeleteAutomatedBackups -timeout 120m
=== RUN   TestAccAWSDBInstance_NoDeleteAutomatedBackups
=== PAUSE TestAccAWSDBInstance_NoDeleteAutomatedBackups
=== CONT  TestAccAWSDBInstance_NoDeleteAutomatedBackups
--- PASS: TestAccAWSDBInstance_NoDeleteAutomatedBackups (652.17s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       653.778s

@ghost ghost added size/M Managed by automation to categorize the size of a PR. service/rds Issues and PRs that pertain to the rds service. documentation Introduces or discusses updates to documentation. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. labels Apr 26, 2019
@tomaszdudek7
Copy link

Any ideas when and if this gets merged? Would be great to have.

@aeschright aeschright requested a review from a team June 26, 2019 16:41
@nywilken nywilken added the enhancement Requests to existing resources that expand the functionality or scope. label Oct 2, 2019
@aeschright
Copy link
Contributor

Hi @MrLunar 👋 Thanks for going ahead with this! I think this is the right approach, but we'll need to add a couple of things so it works smoothly.

Item 1 is a state migration: https://github.com/terraform-providers/terraform-provider-aws/blob/master/.github/CONTRIBUTING.md#resource-contribution-guidelines

Implements State Migration When Adding New Virtual Attribute: For new "virtual" attributes (those only in Terraform and not in the API), the schema should implement State Migration to prevent differences for existing configurations that upgrade.

Item 2 is import support: https://github.com/terraform-providers/terraform-provider-aws/blob/master/.github/CONTRIBUTING.md#adding-resource-import-support

Let us know if you're still available to work on this. I appreciate the start!

@alex
Copy link
Contributor

alex commented Jan 29, 2020

@aeschright Thanks for taking a look at this PR. If the patch author doesn't have the time to take a look in the next few days, I'm more than happy to take those two things over. Will give it a few days before I dive in

@MrLunar
Copy link
Contributor Author

MrLunar commented Jan 29, 2020

@aeschright

No problem, I will take a look at those (probably at the weekend) and get them added in. Cheers!

@alex Thanks for the support! If I struggle to get this completed in a reasonable timeframe, I'll let you know.

@alex
Copy link
Contributor

alex commented Jan 29, 2020

Awesome! We'd very much like to have this at $work, so please let me know if I can help out!

@MrLunar MrLunar force-pushed the rds-retain-automated-backups branch from 5ba5b18 to 9e93f3b Compare February 2, 2020 20:57
to support new virtual attribute "delete_automated_backups"
@ghost ghost added size/XL Managed by automation to categorize the size of a PR. and removed size/M Managed by automation to categorize the size of a PR. labels Feb 9, 2020
@MrLunar
Copy link
Contributor Author

MrLunar commented Feb 9, 2020

  • 1. State migration to v1 added
  • 2. Import support added

@bflad bflad self-assigned this Feb 11, 2020
@bflad bflad added this to the v2.49.0 milestone Feb 11, 2020
Copy link
Contributor

@bflad bflad left a comment

Choose a reason for hiding this comment

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

This looks great, thanks so much, @MrLunar 🚀

Output from acceptance testing:

--- PASS: TestAccAWSDBInstance_AllowMajorVersionUpgrade (504.57s)
--- PASS: TestAccAWSDBInstance_basic (559.89s)
--- PASS: TestAccAWSDBInstance_CACertificateIdentifier (690.35s)
--- PASS: TestAccAWSDBInstance_cloudwatchLogsExportConfiguration (527.87s)
--- PASS: TestAccAWSDBInstance_cloudwatchLogsExportConfigurationUpdate (778.72s)
--- PASS: TestAccAWSDBInstance_DeletionProtection (544.94s)
--- PASS: TestAccAWSDBInstance_EnabledCloudwatchLogsExports_Oracle (736.63s)
--- PASS: TestAccAWSDBInstance_EnabledCloudwatchLogsExports_Postgresql (584.79s)
--- PASS: TestAccAWSDBInstance_FinalSnapshotIdentifier (832.16s)
--- PASS: TestAccAWSDBInstance_FinalSnapshotIdentifier_SkipFinalSnapshot (771.31s)
--- PASS: TestAccAWSDBInstance_generatedName (589.19s)
--- PASS: TestAccAWSDBInstance_iamAuth (498.14s)
--- PASS: TestAccAWSDBInstance_IsAlreadyBeingDeleted (499.57s)
--- PASS: TestAccAWSDBInstance_kmsKey (495.75s)
--- PASS: TestAccAWSDBInstance_MaxAllocatedStorage (624.72s)
--- PASS: TestAccAWSDBInstance_MinorVersion (442.63s)
--- PASS: TestAccAWSDBInstance_MonitoringInterval (994.22s)
--- PASS: TestAccAWSDBInstance_MonitoringRoleArn_EnabledToDisabled (747.67s)
--- PASS: TestAccAWSDBInstance_MonitoringRoleArn_EnabledToRemoved (774.43s)
--- PASS: TestAccAWSDBInstance_MonitoringRoleArn_RemovedToEnabled (643.71s)
--- PASS: TestAccAWSDBInstance_MSSQL_Domain (3138.70s)
--- PASS: TestAccAWSDBInstance_MSSQL_DomainSnapshotRestore (3194.44s)
--- PASS: TestAccAWSDBInstance_MSSQL_TZ (1494.84s)
--- PASS: TestAccAWSDBInstance_MySQL_SnapshotRestoreWithEngineVersion (1970.10s)
--- PASS: TestAccAWSDBInstance_namePrefix (568.95s)
--- PASS: TestAccAWSDBInstance_NoDeleteAutomatedBackups (705.29s)
--- PASS: TestAccAWSDBInstance_optionGroup (518.53s)
--- PASS: TestAccAWSDBInstance_Password (582.04s)
--- PASS: TestAccAWSDBInstance_portUpdate (528.25s)
--- PASS: TestAccAWSDBInstance_ReplicateSourceDb (1480.14s)
--- PASS: TestAccAWSDBInstance_ReplicateSourceDb_AllowMajorVersionUpgrade (1753.52s)
--- PASS: TestAccAWSDBInstance_ReplicateSourceDb_AutoMinorVersionUpgrade (1611.87s)
--- PASS: TestAccAWSDBInstance_ReplicateSourceDb_AvailabilityZone (1541.21s)
--- PASS: TestAccAWSDBInstance_ReplicateSourceDb_BackupRetentionPeriod (1793.14s)
--- PASS: TestAccAWSDBInstance_ReplicateSourceDb_BackupWindow (1526.01s)
--- PASS: TestAccAWSDBInstance_ReplicateSourceDb_CACertificateIdentifier (1475.11s)
--- PASS: TestAccAWSDBInstance_ReplicateSourceDb_IamDatabaseAuthenticationEnabled (1626.78s)
--- PASS: TestAccAWSDBInstance_ReplicateSourceDb_MaintenanceWindow (1414.19s)
--- PASS: TestAccAWSDBInstance_ReplicateSourceDb_MaxAllocatedStorage (1414.77s)
--- PASS: TestAccAWSDBInstance_ReplicateSourceDb_Monitoring (1793.90s)
--- PASS: TestAccAWSDBInstance_ReplicateSourceDb_MultiAZ (2234.03s)
--- PASS: TestAccAWSDBInstance_ReplicateSourceDb_ParameterGroupName (1646.73s)
--- PASS: TestAccAWSDBInstance_ReplicateSourceDb_PerformanceInsightsEnabled (1757.92s)
--- PASS: TestAccAWSDBInstance_ReplicateSourceDb_Port (1535.96s)
--- PASS: TestAccAWSDBInstance_ReplicateSourceDb_VpcSecurityGroupIds (1477.33s)
--- PASS: TestAccAWSDBInstance_separate_iops_update (781.13s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifier (1201.52s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifier_AllocatedStorage (1534.42s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifier_AllowMajorVersionUpgrade (2485.20s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifier_AutoMinorVersionUpgrade (1221.27s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifier_AvailabilityZone (1183.16s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifier_BackupRetentionPeriod (1607.65s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifier_BackupRetentionPeriod_Unset (1837.74s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifier_BackupWindow (1271.59s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifier_DeletionProtection (1297.40s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifier_IamDatabaseAuthenticationEnabled (1352.30s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifier_Io1Storage (1625.67s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifier_MaintenanceWindow (1151.10s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifier_MaxAllocatedStorage (1242.04s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifier_Monitoring (1424.87s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifier_MultiAZ (2043.46s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifier_MultiAZ_SQLServer (4215.52s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifier_ParameterGroupName (1403.09s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifier_PerformanceInsightsEnabled (1544.76s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifier_Port (1321.98s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifier_Tags (1402.55s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifier_VpcSecurityGroupIds (1132.69s)
--- PASS: TestAccAWSDBInstance_SnapshotIdentifier_VpcSecurityGroupIds_Tags (1232.98s)
--- PASS: TestAccAWSDBInstance_subnetGroup (913.45s)
--- PASS: TestAccAWSRDSDBInstance_PerformanceInsightsEnabled_DisabledToEnabled (630.08s)
--- PASS: TestAccAWSRDSDBInstance_PerformanceInsightsEnabled_EnabledToDisabled (792.31s)
--- PASS: TestAccAWSRDSDBInstance_PerformanceInsightsKmsKeyId (1053.46s)
--- PASS: TestAccAWSRDSDBInstance_PerformanceInsightsRetentionPeriod (873.09s)
--- PASS: TestResourceAwsDbInstanceStateUpgradeV0 (0.00s)
--- SKIP: TestAccAWSDBInstance_ec2Classic (1.15s)
--- SKIP: TestAccAWSDBInstance_ReplicateSourceDb_DeletionProtection (0.00s)
--- SKIP: TestAccAWSDBInstance_SnapshotIdentifier_Tags_Unset (0.00s)

"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
)

func resourceAwsDbInstanceResourceV0() *schema.Resource {
Copy link
Contributor

Choose a reason for hiding this comment

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

Verified all attributes/types match pre state upgrade schema. 👍

}
}

func TestResourceAwsDbInstanceStateUpgradeV0(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

💯

})
}

func testAccCheckAWSDBInstanceAutomatedBackups(s *terraform.State) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice! 😄

@bflad bflad merged commit 9729804 into hashicorp:master Feb 11, 2020
bflad added a commit that referenced this pull request Feb 11, 2020
@nathanielks
Copy link
Contributor

Thanks, @MrLunar!

@ghost
Copy link

ghost commented Feb 14, 2020

This has been released in version 2.49.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks!

@ghost
Copy link

ghost commented Mar 27, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

@ghost ghost locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Introduces or discusses updates to documentation. enhancement Requests to existing resources that expand the functionality or scope. service/rds Issues and PRs that pertain to the rds service. size/XL Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: Support disabling delation of RDS automated backups
7 participants