-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
CFn v2: Implement stack deletion #12576
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
base: master
Are you sure you want to change the base?
Conversation
LocalStack Community integration with Pro 2 files ± 0 2 suites ±0 1h 4m 56s ⏱️ - 37m 1s Results for commit 00c0cb1. ± Comparison against base commit 8b3dcdd. This pull request removes 2752 tests.
This pull request skips 1 test.
♻️ This comment has been updated with latest results. |
a459112
to
36039d4
Compare
60db6f4
to
ac56fa8
Compare
dc764a7
to
a3bcf96
Compare
a3bcf96
to
d7e8b94
Compare
a46547d
to
4ad07df
Compare
006a1e2
to
d71e8ab
Compare
577117b
to
0e6c490
Compare
0e6c490
to
72c5fd5
Compare
72c5fd5
to
96722da
Compare
Test Results (amd64) - Integration, Bootstrap 5 files ±0 5 suites ±0 2h 22m 37s ⏱️ +37s Results for commit 00c0cb1. ± Comparison against base commit 8b3dcdd. This pull request removes 1 and adds 3 tests. Note that renamed tests count towards both.
This pull request skips 1 test.
♻️ This comment has been updated with latest results. |
96722da
to
820bb88
Compare
820bb88
to
ecbe2a5
Compare
We create a dummy change set which contains no resources. This is then executed, therefore deleting all resources. This commit also unskips (and re-validates) an ec2 test which exercises this behaviour.
We should return the resource state if the resource is still being deleted, because the next time through the loop an item is fetched from the model.
Because we were not updating the stack template, when we tried to destroy the stack we would try to delete resources that no longer existed. Now we don't.
a99cf4d
to
7572bc8
Compare
## Motivation Currently C 8000 DK bootstrap fails for a variety of reasons. Without this, users who use CDK cannot test their applications with our new engine. ## Changes * Handle dynamic parameter lookup of SSM parameters * Set resource status on success/failure of a deployment * Handle the case where an Fn::Join is called without a list as a second argument, in the special case of an empty string, e.g. `Fn::Join: ["", ""] as the CDK does this * Add helper `find_stack_v2` function for finding v2 stacks * Implement `describe_stack_resources` to support our CDK bootstrap tests * Update the cdk bootstrap test skip reason as our lack of deletion causes test failures ## Testing I have not added any integration tests for this since its functionality is covered by our existing `TestCdkInit.test_cdk_bootstrap` (parametrized) tests. Unfortunately since we don't support deletions yet (see #12576), the tests fail when run together. Each test individually passes, so I've updated the skip reason to reflect the updated nature of the failure. To test manually, unskip the tests, and run ``` pytest tests/aws/services/cloudformation/v2/ported_from_v1/resources/test_cdk.py::TestCdkInit::test_cdk_bootstrap[10] ```
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! This is definitely one of the key pieces for the v2 engine; great to see so many test cases unblocked now.
resource_name=depends_on_resource_logical_id, node_template=self._node_template | ||
) | ||
self.visit_node_resource(node_resource) | ||
for node_resource in self._node_template.resources.resources: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we avoiding _get_node_resource_for
? It looks like the user can define depends_on
values that don't match a resource in the template
Motivation
Quite a few resource tests fail because the deletion of a stack is not correctly implemented. The tests assert that resources are deleted, but they aren't.
Changes
None
in for theafter
template and parametersvisit_node_depends_on
to not visit the depends on target twiceDELETE_IN_PROGRESS
events in the_execute_resource_action
desired_state
and some usedprevious_state
in the delete methodLimitations
While developing this change I noticed that we do not delete resources in the correct order. Namely:
The resources are correctly deployed in the right order (B then A) but they are also deleted in that order (B then A) rather than the order in CloudFormation: A then B. I will tackle this in a follow up PR.