core: change taint semantics #6600
Conversation
|
@mitchellh @phinze I noticed this one needs to be rebased again... But (again) it would be very nice if we can move this one along... Thx! |
This means it’s shown correctly in a plan and takes into account any actions that are dependant on the tainted resource and, vice verse, any actions that the tainted resource depends on. So this changes the behaviour from saying this resource is tainted so just forget about it and make sure it gets deleted in the background, to saying I want that resource to be recreated (taking into account the existing resource and it’s place in the graph).
|
@phinze @mitchellh just rebased and fixed the PR so it is merge-able again... I know you guys are busy, but I would really appreciate if we can move on (discuss/merge/whatever) with this one so I don't have to rebase and fix the PR for a 4th time 😉 Thx! |
|
@svanharmelen Sorry for the delay! I will pick this up this week. 👍 |
|
That would be great... Thx! |
|
This is great, @svanharmelen! I've done a top to bottom review of the code and also played with the behavior a bit locally. The user experience is much nicer with this change!! Doing a final rebase and merge locally. 👍 |
|
Merged in #6894! |
|
Thanks for the review and merge @phinze! 🎉 |
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
ghost
locked and limited conversation to collaborators
Change tainted resources to act as a normal (first class citizens) resource. This means it’s shown correctly in a plan/diff and takes into account any actions that are dependant on the tainted resource and, vice verse, any actions that the tainted resource depends on.
So this changes the behaviour from saying this resource is tainted so just forget about it and make sure it gets deleted in the background, to saying I want that resource to be recreated (taking into account the
existing resource and it’s place in the graph).
An example of what this fixes is if you taint a resource (say an instance in a network) and then call
terraform destroy. Before this would not work properly because both the instance and the network would be destroyed in parallel (so no deps taking into account). Now the graph will be correct and so the destroy will succeed properly.Also it brings a little better UX:
Before:
After:
This change alters the state file layout, but I don't think a migration is needed. I noticed there is another version of the state being prepared, so I think this change should tag along on that change.