terraform graph: By default, produce a compact graph with only resources in it#34288
Merged
apparentlymart merged 3 commits intomainfrom Nov 28, 2023
Merged
terraform graph: By default, produce a compact graph with only resources in it#34288apparentlymart merged 3 commits intomainfrom
apparentlymart merged 3 commits intomainfrom
Conversation
dceed78 to
5888ad4
Compare
This is another contribution to our slowly-growing library of data structures of address types that implement UniqueKeyer. This is currently just a wrapper around *dag.AcyclicGraph which avoids the need for the caller to type-assert the results and which has hopefully less confusing method names for analyzing the edges. However, that's just an implementation detail and we should not expose that in the public API of this type.
This helper method derives from an execution graph a typically-smaller graph which describes only the relationships between resources. This could then be used for analyses that only care about the relationships between resources and don't need to consider all of the other objects that might indirectly glue the resources together.
Unless a user specifically requests a real operation graph using the -type option, we'll by default present a simplified graph which only represents the relationships between resources, since resources are the main side-effects and so the ordering of these is more interesting than the ordering of Terraform's internal implementation details.
5888ad4 to
82e573f
Compare
kmoe
approved these changes
Nov 28, 2023
Contributor
|
Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch. |
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. |
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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
This fixes #14511.
I actually wrote the first two commits here as part of a new round of work on #30937, but after implementing
Graph.ResourceGraphI remembered that a graph just of the relationships between resources -- without all of the other intermediaries -- was the main idea I heard about when researching #14511 many years ago.Given that the
terraform graphcommand is an ancillary debugging/learning tool rather than part of the main Terraform workflow, I find it pretty unlikely that we'd prioritize any work directly on it in the near future, but since my work here already got most of the way there I figured I might as well make a small effort to close that old issue at the same time, while I'm working in this area anyway.The effect of this PR, then, is to change the default behavior of
terraform graphto produce a reduced graph that includes only the resources (bothresourceanddata) described in the configuration, grouped by which module they belong to. Any intermediate nodes such as input variables and output values are not directly included, but indirect dependencies through them are preserved and shown as if they were direct dependencies between resources.The idea of prioritizing resources here is to roughly align with how we prioritize information in other parts of the CLI, such as in
terraform planandterraform showwhich both spend most of their output showing data about resources and de-emphasize (or ignore entirely) most other kinds of objects in Terraform.This also incorporates another very old
terraform graphimprovement idea, which is to arrange the graph in reverse order of the dependency arrows so that a sighted viewer can read the image from left to right to see approximately what order the operations might happen in, with the first operations on the far left and the last operations on the far right. This aligns with the left-to-right reading order of most languages using the latin alphabet, which is the alphabet Terraform uses for its own identifiers/etc. Anyone who would prefer to have it oriented the other way (with the first operations on the far right) can change therankdirargument to be"LR"instead after they've generated a graph.This will change behavior for anyone who is routinely running
terraform graphwith no other options. That command was intentionally excluded from the Terraform v1.x compatibility promises in anticipation of making changes like this one, and the old default behavior is still available here asterraform graph -type=plan. If we move forward with this, I'll add an upgrade note to the changelog describing how to obtain the previous default behavior.While in the area anyway I also did a small amount of modernization of the
terraform graphcommand and its tests, now using the "streams" abstraction to write to stdout instead of using the legacy "Ui" object.I mentioned above that the first two commits here are really for #30937. The connection there is that resources are the main object type that performs actions that can potentially need to be deferred due to unknown values, and so having a summarized graph with just the relationships between resources makes it more efficient to ask the question "does this resource depend on any other resources that have already been deferred?". But more on that will follow in later PRs, once I've made more progress on the details there.
Because this was a "while I was in the area" sort of thing, I don't expect to have time to respond to non-trivial feedback, but of course if anyone has any big concerns about this then we can always put it back on the shelf and try again another time!