prevent cycles when connecting destroy nodes#31857
Conversation
jbardin
added
the
1.3-backport
jbardin
force-pushed
the
jbardin/destroy-edge-cycles
branch
from
a1e5af5 to
abbfa0c
Compare
alisdair
left a comment
alisdair
left a comment
There was a problem hiding this comment.
The code change makes sense to me given the detailed explanation in the comments, thanks!
It might be worth expanding the comments in the test case to explain why the newly added objects cause a cycle without this change. It took me a bit of reading up to see from the inlined HCL config that the other provider references module.mod.
When adding destroy edges between resources from different providers, and a provider itself depends on the other provider's resources, we can get cycles in the final dependency graph. The problem is a little deeper than simply not connecting these nodes, since the edges are still needed when doing a full destroy operation. For now we can get by assuming the edges are required, and reverting them only if they result in a cycle. This works because destroy edges are the last edges added to managed resources during graph building. This was rarely a problem before v1.3, because noop nodes were not added to the apply graph, and unused values were aggressively pruned. In v1.3 however all nodes are kept in the graph so that postcondition blocks are always evaluated during apply, increasing the chances of the cycles appearing.
jbardin
force-pushed
the
jbardin/destroy-edge-cycles
branch
from
abbfa0c to
ce02344
Compare
|
Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch. |
ghost
mentioned this pull request
|
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. |
3 participants
When adding destroy edges between resources from different providers, and a provider itself depends on the other provider's resources, we can get cycles in the final dependency graph.
The problem is a little deeper than simply not connecting these nodes, since the edges are still needed when doing a full destroy operation. For now we can get by assuming the edges are required, and reverting them only if they result in a cycle. This works because destroy edges are the last edges added to managed resources during graph building.
This was rarely a problem before v1.3, because noop nodes were not added to the apply graph at all, and unused values were aggressively pruned. In v1.3 however all nodes are kept in the graph so that postcondition blocks are always evaluated during apply, increasing the chances of the cycles appearing.
Fixes #31843
Target Release
1.3.1