terraform test: rearrange the order of destroy operations #34293
Conversation
|
I realise there is another consideration - we have an example in the docs doing exactly what the user has tried to do. It works sometimes depending on the provider so maybe we want to backport this to make the example in the docs work all the time or put another PR together that we do backport that updates the docs to add some caveats to the example. |
|
It might be a little late in the cycle for a backport the whole into v1.6. We could just classify the case in the existing docs as a bug that will be fixed via the v1.7 release along with the updated behavior. If anything, maybe we just remove the incorrect usage from the v1.6 docs? |
|
Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch. |
|
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. |
This is my second attempt at this, see #34285.
I think I overcomplicated this logic initially. Previously, we'd destroy the main state first, and then iterate through the remaining states from alternate modules in reverse order. We had documentation advising users not to create dependencies on the main state so this ordering should have been safe. I hadn't considered data sources requiring read operations during the pre-destroy refresh which leads to the referenced bug.
After this PR, we just destroy in reverse ordering with the documentation updated to simply say don't create references into later run blocks (eg. don't make a resource that depends on the creation of an earlier run block, and then reference the underlying module of that earlier run block later). I think this is actually acceptable given the way people are using the testing framework - it's much simpler this way and easier to explain. It also gives more control to the users - they can simply do a plan only run block against a module to tweak the destroy order.
A complicated extension to this would be to not only destroy during the destroy operation, but to essentially rollback states to previous run blocks until you find the first time they are created, at which point you could destroy them. I think this would be overkill for what we're trying to solve here but we could revisit that idea if this solution is inadequate.
I'm not sure if we can backport this change (given the change in the docs) but also this is quite advanced and I doubt many (if any) users will have created test configurations at the required complexity for this to be an issue yet. So maybe we could backport it? If not, we can fix this as part of the upcoming 1.7 release.
Fixes #34280
Target Release
1.7.0 or 1.6.5 - I'm not sure.
Draft CHANGELOG entry
UPGRADE NOTES
terraform test: Simplify the ordering of destroy operations during test cleanup to simple reverserunblock order.