stacks: skip resource instance hooks during refresh plans #37043
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
liamcervante
added
the
no-changelog-needed
dsa0x
previously approved these changes
May 14, 2025
liamcervante
force-pushed
the
liamcervante/stacks/refresh-and-destroy-hooks
branch
2 times, most recently
from
7ab94a7 to
781a068
Compare
dsa0x
approved these changes
May 15, 2025
Contributor
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
During Stacks destroy operations each component does a refresh operation before calling destroy. This was actually causing the number of resources to be double-counted by the stacks progress events as the refresh operation was triggering the pre- and post- diff hooks to be counted by Stacks.
This PR updates the stacks component planning so the hook implementations are taken out of the code shared by the refresh and planning operations, and are instead called directly by the stack graph nodes where they are needed. This ensures the destroy-refresh operation doesn't emit status updates for resources anymore.
I've added a big testing helper function to help test the stacks hooks implementation, and added a test that uses it to expose the problem fixed here. Because most of the tests share the same core testing code, this implementation does also ensure that many additional tests are also validating the hooks process is adding things in the right order even if they're not all directly validating the exact hooks that are being called.