[Feature Request](Stack Dependencies): stack dependency should also be hornoured during "cdk destroy"

jiem-ying commented 1 year ago

Describe the feature

If users claim stacks' variables in an order that is different from Stack Dependencies via addDependency/add_dependency, "cdk destroy --all" will incur dependency-related "Delete_failed" stacks.

Use Case

Code examples in Python

app.py:

stack2 = CdkPythonStackotherStack(app, "cdk-python-second-stack", env = env) 
stack1 = CdkPythonStack2Stack(app, "cdk-python-first-stack", env = env)  
stack3 = CdkPythonStackThirdStack(app, "cdk-python-third-stack", env = env)

stack2.add_dependency(stack1)
stack3.add_dependency(stack1)

Additional restriction to trigger error if order wrong. In stack1 I am exporting an output

        CfnOutput(self, "output", value = snsALL.topic_arn, export_name="snsGlobal")

In stack2 and stack3, I am importing the stack1-export to create CFN-side explicit dependency

        snsALL=sns.Topic.from_topic_arn(self, 'topic', Fn.import_value('snsGlobal'))

As you can see app.py stack variables are claimed in order:

stack2 
stack1 
stack3

Whereas, the Stack Dependency is created as

 stack1 
   |___ stack2
   |___ stack3

During the cdk deploy --all, the stack1 always to be created first then following stack2/3.

However, during the cdk destroy --all, the only order destroy process is following is reverse order of the variables:

(.venv) Admin:~/environment/cdk_python_stack2 (master) $ cdk destroy --all
Are you sure you want to delete: cdk-python-third-stack, cdk-python-first-stack, cdk-python-second-stack (y/n)? y

it appears to be expected given current public async destroy function is coded here as:

    // The stacks will have been ordered for deployment, so reverse them for deletion.
    stacks = stacks.reversed();

Which gives rise to error:

cdk-python-third-stack: destroying... [1/3]

 ✅  cdk-python-third-stack: destroyed
cdk-python-first-stack: destroying... [2/3]

 ❌  cdk-python-first-stack: destroy failed Error: Failed to destroy cdk-python-first-stack: CREATE_COMPLETE (Export snsGlobal cannot be deleted as it is in use by cdk-python-second-stack)
    at destroyStack (/home/ec2-user/.nvm/versions/node/v16.13.1/lib/node_modules/aws-cdk/lib/index.js:426:1796)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async CdkToolkit.destroy (/home/ec2-user/.nvm/versions/node/v16.13.1/lib/node_modules/aws-cdk/lib/index.js:429:161194)
    at async exec4 (/home/ec2-user/.nvm/versions/node/v16.13.1/lib/node_modules/aws-cdk/lib/index.js:504:52657)

Failed to destroy cdk-python-first-stack: CREATE_COMPLETE (Export snsGlobal cannot be deleted as it is in use by cdk-python-second-stack)

Proposed Solution

Update the the destroy function, to reverse the stacks order that deploy function used based on Stack Dependencies:

      const stacksAndTheirAssetManifests = stacks.flatMap(stack => [
        stack,
        ...stack.dependencies.filter(cxapi.AssetManifestArtifact.isAssetManifestArtifact),
      ]);
      const workGraph = new WorkGraphBuilder(prebuildAssets).build(stacksAndTheirAssetManifests);

Other Information

Maybe also consider update the Stack Dependencies documentation with a highlighted text states that currently the "destroy" is not following the dependency but merely reverse var-declaration order in code.

Acknowledgements

[ ] I may be able to implement this feature request
[ ] This feature might incur a breaking change

CDK version used

2.88.0

Environment details (OS name and version, etc.)

Cloud9 latest

pahud commented 1 year ago

I think this is a limit from cloudformation. When the export value of stack1 is used by stack2 and stack3, cloudformation does not allow you to delete stack1 first. However, having stack2 and stack3 depend on stack1 means stack1 should be deleted first, which causes a dilemma here.

janeklb commented 7 months ago

However, having stack2 and stack3 depend on stack1 means stack1 should be deleted first, which causes a dilemma here.

If stack 2 and stack 3 depend on stack 1, then they need to be deleted 1st.

jiem-ying commented 6 months ago

Hi team, this issue is commonly impacting complex CDK project's destroy. A number of skip delete and manually removal required from CloudFormation side. Hope this FR can get higher priority for a better user experience.

aws / aws-cdk