Skip to content

Add retry cycles for deleting stack resources#7717

Merged
dominikschubert merged 2 commits intomasterfrom
fix-cfn-stack-resource-deletion
Feb 21, 2023
Merged

Add retry cycles for deleting stack resources#7717
dominikschubert merged 2 commits intomasterfrom
fix-cfn-stack-resource-deletion

Conversation

@dominikschubert
Copy link
Member

@steffyP found an issue with stack deletion when deleting a stack that contains resources with dependencies

#7703 (comment)

Changes

  • Add retry loop around deletion of stack resources and only try to delete stack resources that are not already in a DELETE_COMPLETE state.

The loop is similar to the core deployment loop we already have. This retry now allows that first an independent resource is deleted and the dependent one is retried up to 10 times which is usually enough for the old one to be deleted as well.

There are of course still a few issues but they are quite out of scope like doing this asynchronously and not primarily relying on the loop, but instead using the resource graph.

@dominikschubert dominikschubert self-assigned this Feb 20, 2023
@dominikschubert dominikschubert temporarily deployed to localstack-ext-tests February 20, 2023 12:21 — with GitHub Actions Inactive
Copy link
Member

@steffyP steffyP left a comment

Choose a reason for hiding this comment

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

Thanks for jumping on this so quickly! 🔝
I tested the use case for RDS cluster/instance locally, and it works 😄

however, some of the tests in the CI are failing now for stack.destroy()😞

@dominikschubert dominikschubert temporarily deployed to localstack-ext-tests February 20, 2023 13:54 — with GitHub Actions Inactive
@coveralls
Copy link

Coverage Status

Coverage: 84.978% (-0.04%) from 85.016% when pulling e22f861 on fix-cfn-stack-resource-deletion into 4567080 on master.

@github-actions
Copy link

LocalStack integration with Pro

       3 files  ±0         3 suites  ±0   1h 35m 26s ⏱️ + 3m 49s
1 731 tests ±0  1 382 ✔️ ±0  349 💤 ±0  0 ±0 
2 449 runs  ±0  1 758 ✔️ ±0  691 💤 ±0  0 ±0 

Results for commit e22f861. ± Comparison against base commit 4567080.

Copy link
Member

@steffyP steffyP left a comment

Choose a reason for hiding this comment

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

LGTM 👍 😄
Running/Deleting the rds-stack sample locally seems to work, and there are no more test failures 🚀

@dominikschubert dominikschubert merged commit 7d3b73b into master Feb 21, 2023
@dominikschubert dominikschubert deleted the fix-cfn-stack-resource-deletion branch February 21, 2023 06:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants