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

(aws-rds): Can't destroy a stack with a database that has removalPolicy property set to RemovalPolicy.RETAIN #22141

Closed
maxd opened this issue Sep 20, 2022 · 9 comments · Fixed by #28660
Assignees
Labels
@aws-cdk/aws-rds Related to Amazon Relational Database bug This issue is a bug. effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md p2

Comments

@maxd
Copy link

maxd commented Sep 20, 2022

Describe the bug

I have a stack that creates a database with parameters group:

const parameterGroup = new ParameterGroup(this, 'ParameterGroup', {
    ...
}

const database = new DatabaseInstance(this, 'DatabaseInstance', {
    parameterGroup: parameterGroup,
    removalPolicy: RemovalPolicy.RETAIN,
    ...
})

As you can see the database has removalPolicy property set to RETAIN because I want to leave the database after destroying this stack.

When I destroy this stack I see the following errors:

2:04:24 PM | DELETE_FAILED        | AWS::RDS::DBParameterGroup                  | ParameterGroup5E32DECB
One or more database instances are still members of this parameter group xxx-database-parametergroup5e32decb-daetrwpaqpgw, so the group cannot be deleted (Service: Rd
s, Status Code: 400, Request ID: 389b18db-ea82-482b-a0e6-f64887da6f82)

2:19:21 PM | DELETE_FAILED        | AWS::EC2::SecurityGroup                     | DatabaseInstanceSecurityGroup8BDF0112
resource sg-0bfc8aacb3d3e3d4a has a dependent object (Service: AmazonEC2; Status Code: 400; Error Code: DependencyViolation; Request ID: 1eac5393-83df-48cf-bd75-41f25abb04
7a; Proxy: null)

As I understand the CF leaves the database but tries to destroy ParameterGroup and SecurtyGroup. I suppose this is the wrong behavior and CF should leave related ParameterGroup and SecurityGroup too.

I can create a custom SecurityGroup and change it remove policy use the applyRemovalPolicy method:

    const securityGroup = new SecurityGroup(this, 'SecurityGroup', {
      vpc: props.vpc,
      allowAllOutbound: true,
    })
    securityGroup.applyRemovalPolicy(removalPolicy(SecurityGroup))

BUT I can't change the removal policy for a ParameterGroup in the same way because parameterGroup.applyRemovalPolicy(RemovalPolicy.RETAIN) raises the following error:

Error: Cannot apply RemovalPolicy: no child or not a CfnResource. Apply the removal policy on the CfnResource directly.
    at ParameterGroup.applyRemovalPolicy (/Users/user/Projects/work/my-project/node_modules/aws-cdk-lib/core/lib/resource.js:1:2872)
    at Database.createDatabase (/Users/user/Projects/work/my-project/src/stacks/billfold/database/index.ts:78:20)
    at new Database (/Users/user/Projects/work/my-project/src/stacks/billfold/database/index.ts:36:27)
    at new Billfold (/Users/user/Projects/work/my-project/src/stages/billfold.ts:66:22)
    at Object.<anonymous> (/Users/user/Projects/work/my-project/src/apps/billfold.ts:10:1)
    at Module._compile (node:internal/modules/cjs/loader:1105:14)
    at Module.m._compile (/Users/user/Projects/work/my-project/node_modules/ts-node/src/index.ts:1618:23)
    at Module._extensions..js (node:internal/modules/cjs/loader:1159:10)
    at Object.require.extensions.<computed> [as .ts] (/Users/user/Projects/work/my-project/node_modules/ts-node/src/index.ts:1621:12)
    at Module.load (node:internal/modules/cjs/loader:981:32)

Expected Behavior

I can destroy a stack with a database that has removalPolicy property set to RemovalPolicy.RETAIN and has a relation to a parameter and security groups.

Current Behavior

The CF leaves the database but tries to destroy the related parameter group and security group and fails with the errors:

2:04:24 PM | DELETE_FAILED        | AWS::RDS::DBParameterGroup                  | ParameterGroup5E32DECB
One or more database instances are still members of this parameter group billfold-database-parametergroup5e32decb-daetrwpaqpgw, so the group cannot be deleted (Service: Rd
s, Status Code: 400, Request ID: 389b18db-ea82-482b-a0e6-f64887da6f82)

2:19:21 PM | DELETE_FAILED        | AWS::EC2::SecurityGroup                     | DatabaseInstanceSecurityGroup8BDF0112
resource sg-0bfc8aacb3d3e3d4a has a dependent object (Service: AmazonEC2; Status Code: 400; Error Code: DependencyViolation; Request ID: 1eac5393-83df-48cf-bd75-41f25abb04
7a; Proxy: null)

Reproduction Steps

  1. Create a stack with database and parameter group using the following template:
const parameterGroup = new ParameterGroup(this, 'ParameterGroup', {
    ...
}

const database = new DatabaseInstance(this, 'DatabaseInstance', {
    parameterGroup: parameterGroup,
    removalPolicy: RemovalPolicy.RETAIN,
    ...
})
  1. Deploy this stack
  2. Destroy this stack

Possible Solution

I suppose:

  1. The DatabaseInstance and DatabaseInstanceFromSnapshot constructs should propagate the removal policy to the related parameters and security groups.
  2. The ParametersGroup#applyRemovalPolicy method should work and doesn't raise the error.

Additional Information/Context

No response

CDK CLI Version

2.39.0 (build e36bfe5)

Framework Version

No response

Node.js Version

v16.15.1

OS

macOS 12.4 (21F79)

Language

Typescript

Language Version

4.7.4

Other information

Looks like the #20649 issue is similar to this.

@maxd maxd added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Sep 20, 2022
@github-actions github-actions bot added the @aws-cdk/aws-rds Related to Amazon Relational Database label Sep 20, 2022
@corymhall
Copy link
Contributor

It looks like applyRemovalPolicy fails on ParameterGroup because there is no defaultChild set.

The solution is to set the defaultChild in both the bindToCluster and bindToInstance methods

i.e.

this.node.defaultChild = this.clusterCfnGroup;

@corymhall corymhall added p2 effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md and removed needs-triage This issue or PR still needs to be triaged. labels Sep 20, 2022
@corymhall corymhall removed their assignment Sep 20, 2022
@maxd
Copy link
Author

maxd commented Sep 20, 2022

@corymhall Thank you for you quick response. But I don't think that this solution will work because:

  1. Look at this example:

    const parameterGroup = new ParameterGroup(this, 'ParameterGroup', {
        ...
    }
    parameterGroup.applyRemovalPolicy(cdk.RemovalPolicy.RETAIN)
    
    const database = new DatabaseInstance(this, 'DatabaseInstance', {
        parameterGroup: parameterGroup,
        removalPolicy: RemovalPolicy.RETAIN,
        ...
    })
    

    As you can see I can call applyRemovalPolicy before binding it to a DB instance or a DB cluster.

  2. I've found the following test:

    test('creates 2 parameter groups when bound to a cluster and an instance', () => {
    ...
    

    Looks like ParameterGroup can be binded to DB instance an cluster simultaneously but node can have only one defaultChild. Right?

How about the following solution?:

  1. We can add removalPolicy property to the ParameterGroupProps class

  2. Then we can save value from ParameterGroupProps#removalPolicy to the removalPolicy filed in the constructor

  3. After that we can use value from removalPolicy filed to set removal policy in bindToCluster and bindToInstance methods e.g.:

    ...
    this.instanceCfnGroup.applyRemovalPolicy(this.removalPolicy);
    ...
    
  4. And we should override ParameterGroup#applyRemovalPolicy method something like this:

    public applyRemovalPolicy(policy: cdk.RemovalPolicy): void {
        this.removalPolicy = policy;
        if (this.clusterCfnGroup) {
            this.clusterCfnGroup.applyRemovalPolicy(policy);
        }
        if (this.instanceCfnGroup) {
            this.instanceCfnGroup.applyRemovalPolicy(policy);
        }
    }
    

    As you can see this overrode method will update removalPolicy field and resources if they are already exists.

So, what do you think?

@corymhall
Copy link
Contributor

So, what do you think?

Looks good to me!

@mowemcfc
Copy link

Hi @corymhall @maxd does this issue still exist? I'm happy to pick it up if so as I have encountered the same in my project.

@maxd
Copy link
Author

maxd commented Mar 20, 2023

I suppose it still exists.

@ChrisLane
Copy link

It would be great to have this fixed.
I just came across this issue in my use-case where I want to create a database via CDK and then immediately orphan it so that it can be maintained manually via AWS management console; this bug is stopping me from being able to continue.

@GavinZZ GavinZZ self-assigned this Jan 9, 2024
@chris-allen
Copy link

I experience the same behavior even when the database has a removalPolicy of RemovalPolicy.SNAPSHOT.

My only solution has been to manually update the database to use a default parameter group and delete the one to be cleaned up.

@GavinZZ
Copy link
Contributor

GavinZZ commented Jan 10, 2024

Hi all, thanks for creating an issue. Going to implement a fix to this problem as @maxd described in this comment.

@mergify mergify bot closed this as completed in #28660 Jan 11, 2024
mergify bot pushed a commit that referenced this issue Jan 11, 2024
…28660)

>  Can't destroy a stack that includes a rds database and rds parameter group where the database has removalPolicy property set to RemovalPolicy.RETAIN

### The following is the current behaviour:
```
const parameterGroup = new ParameterGroup(this, 'ParameterGroup', {
    ...
}

const database = new DatabaseInstance(this, 'DatabaseInstance', {
    parameterGroup: parameterGroup,
    removalPolicy: RemovalPolicy.RETAIN,
    ...
})
```

When destroying the stack
```
When I destroy this stack I see the following errors:

2:04:24 PM | DELETE_FAILED        | AWS::RDS::DBParameterGroup                  | ParameterGroup5E32DECB
One or more database instances are still members of this parameter group xxx-database-parametergroup5e32decb-daetrwpaqpgw, so the group cannot be deleted (Service: Rd
s, Status Code: 400, Request ID: 389b18db-ea82-482b-a0e6-f64887da6f82)

2:19:21 PM | DELETE_FAILED        | AWS::EC2::SecurityGroup                     | DatabaseInstanceSecurityGroup8BDF0112
resource sg-0bfc8aacb3d3e3d4a has a dependent object (Service: AmazonEC2; Status Code: 400; Error Code: DependencyViolation; Request ID: 1eac5393-83df-48cf-bd75-41f25abb04
7a; Proxy: null)

```

As pointed out in the issue linked below, we cannot simply use the clusterRds' or instanceRds' removal policy because the parameter group can be simultaneously binded to a cluster and an instance. 

### New behaviour:
Add an optional property `removalPolicy` to the L2 Parameter Group resource and set the deletion policy to the generated L1 Parameter Group (Either cluster or instance) depending on the usage. 

Added unit test and integration test to verify that it works as expected.

Closes #22141

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-rds Related to Amazon Relational Database bug This issue is a bug. effort/small Small work item – less than a day of effort good first issue Related to contributions. See CONTRIBUTING.md p2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants