Ensure consistency of details and tallies in plan and apply #15884

apparentlymart · 2017-08-23T02:08:15Z

As reported in #14048, we currently have a number of inconsistencies between the following:

The detailed plan output for normal updates vs. destroying.
The plan detail vs. the tally of resources to be created, updated and destroyed.
The count of resources to destroy during plan and the count of resources destroyed during apply.

The root cause of these inconsistencies is that we have some leaky abstractions around the handling of data sources: internally we include data sources in the diff and act on them during apply to either refresh them or remove them from state (depending on what sort of apply we're doing), but in the user-facing model we think of data sources as being "read" in an apply and not appearing at all in destroy.

The goal of this PR, then, is to ensure that this abstraction stops leaking by cleaning up some places where it's not honored properly and refactoring so that these results are being computed in a more centralized place where it's less likely for future changes to cause consistency regressions.

Along the way it also adjusts some other weirdness in the plan and apply output, as a result of the refactoring.

This fixes #14048.

apparentlymart · 2017-08-23T02:12:59Z

A new (or rather, refactored) plan formatter is implemented so far, taking care of the subset of these problems that live on the planning side of things:

$ terraform plan
...

 <= data.template_file.foo
      id:           <computed>
      rendered:     <computed>
      template:     "id is ${id}"
      vars.%:       <computed>

  + null_resource.bar[0]
      id:           <computed>

  + null_resource.bar[1]
      id:           <computed>

  + null_resource.bar[2]
      id:           <computed>

  + null_resource.baz
      id:           <computed>
      triggers.%:   "1"
      triggers.foo: "bar baz"

  + null_resource.foo
      id:           <computed>

  + module.child.null_resource.child_foo
      id:           <computed>


Plan: 6 to add, 0 to change, 0 to destroy.

$ terraform plan -destroy
...

  - null_resource.bar[0]

  - null_resource.bar[1]

  - null_resource.bar[2]

  - null_resource.baz

  - null_resource.foo

  - module.child.null_resource.child_foo


Plan: 0 to add, 0 to change, 6 to destroy.

Next up is to correct the accounting of things during apply so that the cleanup of data sources from state is not presented as a destroy, and indeed isn't visible in the UI at all.

apparentlymart · 2017-08-24T00:27:51Z

$ terraform apply
null_resource.bar[1]: Creating...
null_resource.bar[2]: Creating...
null_resource.baz: Creating...
  triggers.%:   "" => "1"
  triggers.foo: "" => "bar baz"
null_resource.bar[0]: Creating...
null_resource.bar[2]: Creation complete after 0s (ID: 4726875160738392522)
null_resource.bar[0]: Creation complete after 0s (ID: 2023463361586702105)
null_resource.foo: Creating...
module.child.null_resource.child_foo: Creating...
null_resource.bar[1]: Creation complete after 0s (ID: 4539990167034217754)
null_resource.baz: Provisioning with 'local-exec'...
null_resource.baz (local-exec): Executing: ["/bin/sh" "-c" "exit 0"]
module.child.null_resource.child_foo: Creation complete after 0s (ID: 3700927677690061483)
null_resource.foo: Creation complete after 0s (ID: 5801164175881065423)
null_resource.baz: Creation complete after 0s (ID: 1453328095019012737)
data.template_file.foo: Refreshing state...

Apply complete! Resources: 6 added, 0 changed, 0 destroyed.

$ terraform destroy
null_resource.baz: Refreshing state... (ID: 1453328095019012737)
null_resource.foo: Refreshing state... (ID: 5801164175881065423)
null_resource.bar[2]: Refreshing state... (ID: 4726875160738392522)
null_resource.bar[1]: Refreshing state... (ID: 4539990167034217754)
null_resource.bar[0]: Refreshing state... (ID: 2023463361586702105)
null_resource.child_foo: Refreshing state... (ID: 3700927677690061483)
data.template_file.foo: Refreshing state...

The Terraform destroy plan has been generated and is shown below.
Resources are shown in alphabetical order for quick scanning.
Resources shown in red will be destroyed.

  - null_resource.bar[0]

  - null_resource.bar[1]

  - null_resource.bar[2]

  - null_resource.baz

  - null_resource.foo

  - module.child.null_resource.child_foo


Do you really want to destroy?
  Terraform will destroy all your managed infrastructure, as shown above.
  There is no undo. Only 'yes' will be accepted to confirm.

  Enter a value: yes

null_resource.bar[1]: Destroying... (ID: 4539990167034217754)
null_resource.baz: Destroying... (ID: 1453328095019012737)
null_resource.baz: Destruction complete after 0s
null_resource.bar[1]: Destruction complete after 0s
null_resource.bar[2]: Destroying... (ID: 4726875160738392522)
module.child.null_resource.child_foo: Destroying... (ID: 3700927677690061483)
null_resource.bar[0]: Destroying... (ID: 2023463361586702105)
module.child.null_resource.child_foo: Destruction complete after 0s
null_resource.foo: Destroying... (ID: 5801164175881065423)
null_resource.bar[2]: Destruction complete after 0s
null_resource.foo: Destruction complete after 0s
null_resource.bar[0]: Destruction complete after 0s

Destroy complete! Resources: 6 destroyed.

The implementation of ResourceAddress.Less was flawed because it was only testing each field in the "less than" direction, and falling through in cases where an earlier field compared greater than a later one. Now we test for inequality first as the selector, and only fall through if the two values for a given field are equal.

Previously the rendered plan output was constructed directly from the core plan and then annotated with counts derived from the count hook. At various places we applied little adjustments to deal with the fact that the user-facing diff model is not identical to the internal diff model, including the special handling of data source reads and destroys. Since this logic was just muddled into the rendering code, it behaved inconsistently with the tally of adds, updates and deletes. This change reworks the plan formatter so that it happens in two stages: - First, we produce a specialized Plan object that is tailored for use in the UI. This applies all the relevant logic to transform the physical model into the user model. - Second, we do a straightforward visual rendering of the display-oriented plan object. For the moment this is slightly overkill since there's only one rendering path, but it does give us the benefit of letting the counts be derived from the same data as the full detailed diff, ensuring that they'll stay consistent. Later we may choose to have other UIs for plans, such as a machine-readable output intended to drive a web UI. In that case, we'd want the web UI to consume a serialization of the _display-oriented_ plan so that it doesn't need to re-implement all of these UI special cases. This introduces to core a new diff action type for "refresh". Currently this is used _only_ in the UI layer, to represent data source reads. Later it would be good to use this type for the core diff as well, to improve consistency, but that is left for another day to keep this change focused on the UI.

Previously we were using the internal resource id syntax in the UI. Now we'll use the standard user-facing resource address syntax instead.

The fact that we clean up data source state by applying a "destroy" action for them is an implementation detail, and so should not be visible to outside callers or to the user. Signalling these as real destroys creates confusion for users because they see Terraform say things like: data.template_file.foo: Refreshing state..." ...which, to an understandably-nervous sysadmin, might make them suspect that the underlying object was deleted, rather than just Terraform's record of it.

apparentlymart · 2017-09-01T02:17:53Z

Along with the consistency improvements I also bundled in here some tweaks to the general presentation of plans. Some of the key things here are:

Use the plan symbols and colors to describe the meaning of what follows, so that the output is more friendly to those who can't see color either due to color blindness or terminal settings. This also addresses What are the different terraform plan symbol meanings? #14379 directly within the UI, rather than in the docs as discussed before. (Previously we had a long, wordy description of what each color meant and no mention of the symbols.)
Use the terminology "performing actions" rather than "executing changes" to reflect the fact that "read" is one of the possible actions, and that isn't a "change".
Use some framing characters to help split up the different steps of the plan so that it's easier to visually navigate. Previously there was a lot of information all thrown together and it was hard to quickly locate the "edges" between them.
The final call-to-action to apply the plan now spells out an exact command to run, rather than just generally referring to "the apply subcommand".

The new plan output looks like this:

$ terraform plan -out=tfplan
Refreshing Terraform state in-memory prior to plan...
The refreshed state will be used to calculate this plan, but will not be
persisted to local or remote state storage.

data.test_provider_label.foo: Refreshing state...

------------------------------------------------------------------------

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  + create
 <= read (data resources)

Terraform will perform the following actions:

 <= module.child.data.template_file.foo[0]
      id:          <computed>
      rendered:    <computed>
      template:    "foo 0 yes"

 <= module.child.data.template_file.foo[1]
      id:          <computed>
      rendered:    <computed>
      template:    "foo 1 yes"

  + module.child.null_resource.foo[0]
      id:          <computed>
      triggers.%:  "1"
      triggers.in: "yes"

  + module.child.null_resource.foo[1]
      id:          <computed>
      triggers.%:  "1"
      triggers.in: "yes"


Plan: 2 to add, 0 to change, 0 to destroy.

------------------------------------------------------------------------

This plan was saved to: tfplan

To perform exactly these actions, run the following command to apply:
    terraform apply "tfplan"

The plan output produced by terraform apply -auto-approve=false is also updated to use this same presentation in preparation for it becoming the default/primary workflow in the next major release.

Data source destroy is an implementation detail and not something that external callers should see or expect.

The previous diff presentation was rather "wordy", and not very friendly to those who can't see color either because they have color-blindness or because they don't have a color-supporting terminal. This new presentation uses the actual symbols used in the plan output and tries to be more concise. It also uses some framing characters to try to separate the different stages of "terraform plan" to make it easier to visually navigate. The apply command also adopts this new plan presentation, in preparation for "terraform apply" (with interactive plan confirmation) becoming the primary, safe workflow in the next major release. Finally, we standardize on the terminology "perform" and "actions" rather than "execute" and "changes" to reflect the fact that reading is now an action and that isn't actually a _change_.

In #15884 we adjusted the plan output to give an explicit command to run to apply a plan, whereas before this command was just alluded to in the prose. Since releasing that, we've got good feedback that it's confusing to include such instructions when Terraform is running in a workflow automation tool, because such tools usually abstract away exactly what commands are run and require users to take different actions to proceed through the workflow. To accommodate such environments while retaining helpful messages for normal CLI usage, here we introduce a new environment variable TF_IN_AUTOMATION which, when set to a non-empty value, is a hint to Terraform that it isn't being run in an interactive command shell and it should thus tone down the "next steps" messaging. The documentation for this setting is included as part of the "...in automation" guide since it's not generally useful in other cases. We also intentionally disclaim comprehensive support for this since we want to avoid creating an extreme number of "if running in automation..." codepaths that would increase the testing matrix and hurt maintainability. The focus is specifically on the output of the three commands we give in the automation guide, which at present means the following two situations: * "terraform init" does not include the final paragraphs that suggest running "terraform plan" and tell you in what situations you might need to re-run "terraform init". * "terraform plan" does not include the final paragraphs that either warn about not specifying "-out=..." or instruct to run "terraform apply" with the generated plan file.

In hashicorp#15884 we adjusted the plan output to give an explicit command to run to apply a plan, whereas before this command was just alluded to in the prose. Since releasing that, we've got good feedback that it's confusing to include such instructions when Terraform is running in a workflow automation tool, because such tools usually abstract away exactly what commands are run and require users to take different actions to proceed through the workflow. To accommodate such environments while retaining helpful messages for normal CLI usage, here we introduce a new environment variable TF_IN_AUTOMATION which, when set to a non-empty value, is a hint to Terraform that it isn't being run in an interactive command shell and it should thus tone down the "next steps" messaging. The documentation for this setting is included as part of the "...in automation" guide since it's not generally useful in other cases. We also intentionally disclaim comprehensive support for this since we want to avoid creating an extreme number of "if running in automation..." codepaths that would increase the testing matrix and hurt maintainability. The focus is specifically on the output of the three commands we give in the automation guide, which at present means the following two situations: * "terraform init" does not include the final paragraphs that suggest running "terraform plan" and tell you in what situations you might need to re-run "terraform init". * "terraform plan" does not include the final paragraphs that either warn about not specifying "-out=..." or instruct to run "terraform apply" with the generated plan file.

apparentlymart added bug cli labels Aug 23, 2017

apparentlymart force-pushed the b-plan-apply-consistency branch from 99a2697 to ed18454 Compare August 23, 2017 23:23

apparentlymart added 2 commits August 31, 2017 15:43

apparentlymart force-pushed the b-plan-apply-consistency branch from bd2f6e8 to 01a37a7 Compare August 31, 2017 22:44

apparentlymart added 2 commits August 31, 2017 17:16

command: show resource actions using resource addresses

d09b4a0

Previously we were using the internal resource id syntax in the UI. Now we'll use the standard user-facing resource address syntax instead.

apparentlymart force-pushed the b-plan-apply-consistency branch from 01a37a7 to 09a4219 Compare September 1, 2017 00:17

apparentlymart force-pushed the b-plan-apply-consistency branch from 839010c to 1d921fa Compare September 1, 2017 02:19

apparentlymart added 3 commits August 31, 2017 19:26

core: add testHook for testing correct interaction with hooks

0dcdb94

core: test that we skip hooks for data source destroy

f7125b3

Data source destroy is an implementation detail and not something that external callers should see or expect.

apparentlymart force-pushed the b-plan-apply-consistency branch from 1d921fa to cbbcce8 Compare September 1, 2017 02:26

apparentlymart changed the title ~~[WIP] Ensure consistency of details and tallies in plan and apply~~ Ensure consistency of details and tallies in plan and apply Sep 1, 2017

apparentlymart requested a review from jbardin September 1, 2017 02:27

jbardin approved these changes Sep 1, 2017

View reviewed changes

apparentlymart merged commit 83414be into master Sep 2, 2017

apparentlymart deleted the b-plan-apply-consistency branch September 2, 2017 00:55

This was referenced Sep 2, 2017

What are the different terraform plan symbol meanings? #14379

Closed

terraform plan -destroy lists data resources #14048

Closed

Destroy tries to destroy "data" type resources #15996

Closed

apparentlymart mentioned this pull request Sep 9, 2017

cli: allow disabling "next steps" message in terraform plan #16059

Merged

mo-hit mentioned this pull request Sep 18, 2017

issue with atlantis comment colors / spacing hootsuite/atlantis#137

Closed

jbardin mentioned this pull request Oct 2, 2017

Better display for set diffs, drop unchanged elements #15265

Closed

apparentlymart mentioned this pull request Nov 2, 2017

terraform plan -destroy mistakenly shows an additional item #16534

Closed

ghost locked as resolved and limited conversation to collaborators Mar 13, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ensure consistency of details and tallies in plan and apply #15884

Ensure consistency of details and tallies in plan and apply #15884

apparentlymart commented Aug 23, 2017 •

edited

Loading

apparentlymart commented Aug 23, 2017 •

edited

Loading

apparentlymart commented Aug 24, 2017 •

edited

Loading

apparentlymart commented Sep 1, 2017

Ensure consistency of details and tallies in plan and apply #15884

Ensure consistency of details and tallies in plan and apply #15884

Conversation

apparentlymart commented Aug 23, 2017 • edited Loading

apparentlymart commented Aug 23, 2017 • edited Loading

apparentlymart commented Aug 24, 2017 • edited Loading

apparentlymart commented Sep 1, 2017

apparentlymart commented Aug 23, 2017 •

edited

Loading

apparentlymart commented Aug 23, 2017 •

edited

Loading

apparentlymart commented Aug 24, 2017 •

edited

Loading