terraform test: refactor graph edge calculation #37357
Conversation
liamcervante
added
the
no-changelog-needed
| // will be handled later. | ||
| if node.parallel && refNode.parallel { | ||
| g.Connect(dag.BasicEdge(refNode, node)) | ||
| } |
There was a problem hiding this comment.
Isn't the sequential order of cleanup nodes also important?
There was a problem hiding this comment.
Yep, but I think that's all included in the depStateKeys metadata and doesn't need to be handled separately.
You definitely know this, but for other potential readers: If the run blocks creating the to-be-destroyed resources executed in sequence, then we want to destroy them in sequence as well (but reversed).
I think those run blocks will have had edges between them in the original graph indicating the sequential nature of operation that would have also been captured in the depStateKeys map. That map is built here, and it just uses the edges in the original graph. Those edges will contain both the requirements for sequential execution and parallel execution, I don't think we need to compute the sequential edges separately - we just want to reverse what was in the original graph and all of those edges are captured together.
There was a problem hiding this comment.
Ah. I see. Thanks!. Then we can remove other references to that .parallel attribute
There was a problem hiding this comment.
And also the parallel attribute of the NodeStateCleanup struct
There was a problem hiding this comment.
Nice, I hadn't noticed it was only referenced there. Done!
This PR uses the newly introduced
priorRunsconcept to simply connecting edges between runs within the Terraform Test graph. In turn, we can now simply the edges calculation between cleanup nodes as the dependencies (including references and parallelisation information) are now embedded into the graph already, and we can retrieve this from the dependency information instead of recalculating it.In addition, we simplify the state key logic by precomputing it during the config parsing. This in turn let's users actually write into the main state from modules which might be something they want to do.