-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
CFNv2: Support stack sets #12909
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
CFNv2: Support stack sets #12909
Conversation
ec4b9b9 to
2f3aa84
Compare
LocalStack Community integration with Pro 2 files ± 0 2 suites ±0 23m 23s ⏱️ - 1h 21m 38s Results for commit 2f3aa84. ± Comparison against base commit 34049aa. This pull request removes 4040 and adds 2 tests. Note that renamed tests count towards both.This pull request removes 213 skipped tests and adds 2 skipped tests. Note that renamed tests count towards both. |
Test Results (amd64) - Integration, Bootstrap 5 files 5 suites 32m 47s ⏱️ Results for commit 2f3aa84. |
pinzon
left a comment
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.
LGTM. Just a minor comment.
| # wait for completion of stack | ||
| for stack_name, account_id, region_name in stacks_to_await: | ||
| client = connect_to( | ||
| aws_access_key_id=account_id, region_name=region_name | ||
| ).cloudformation | ||
| client.get_waiter("stack_create_complete").wait(StackName=stack_name) |
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.
comment: is this correct? Maybe a better approach would be to put this code in a separate thread and update the stack set operation status depending on the result of the waits.
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.
I think you are right but I think you have two suggestions here:
- run each stack instance concurrently in a background thread
- don't assume success and handle the error
I don't think point 1. is worth tackling at this stage, however point 2 is very relevant. I would like to tackle this in a follow up since this PR does not support successfully deploying stack instances. See the note here.
I have started a PR to create a validated stack sets test. I think the step after that is to handle error cases. Thanks!
Motivation
The CloudFormation V1 provider had basic support for stack sets, with a single test covering the functionality. This functionality should be ported over to the new provider.
Changes
Testing
Unfortunately there is manual set up required for testing stack sets. See the docs here: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/stacksets-prereqs-self-managed.html so this change does not test/assert the behaviour of deployed resources though they were manually verified.