Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Always show and store planned actions and checks even when planning fails #32395

Merged
merged 6 commits into from
Jan 4, 2023

Conversation

jbardin
Copy link
Member

@jbardin jbardin commented Dec 14, 2022

Continuing on from where #31057 left off:

Some planning errors are caused by values planned successfully upstream, and so showing the partial plan created so far might potentially provide some relevant context to help understand the error message being reported.

Without showing this, today Terraform provides no way to inspect the actions planned so far, because they don't persist anywhere once Terraform exits.


Terraform used the selected providers to generate the following execution plan.
Resource actions are indicated with the following symbols:
  + create

Terraform planned the following actions, but then encountered a problem:

  # null_resource.boop[0] will be created
  + resource "null_resource" "boop" {
      + id = (known after apply)
    }

╷
│ Error: Resource precondition failed
│ 
│   on plan-time-fail-output.tf line 9, in resource "null_resource" "beep":
│    9:       condition     = length(null_resource.boop) == 0
│     ├────────────────
│     │ null_resource.boop is tuple with 1 element
│ 
│ Too many boops.
╵

After encountering an error, Terraform will now always attempt to write out a plan file with what was planned so far. This is especially useful when the failures were due to condition checks in the configuration, so that the changes recorded can be inspected to help the user determine why the conditions may have started failing.

The plan itself is marked as errored and cannot be applied, but can be inspected with terraform show -json planfile.

This is a preliminary PR for initial integration testing. The CLI cannot yet re-render a plan with an error aside from the json representation.

apparentlymart and others added 5 commits December 12, 2022 17:17
For some kinds of plan failure we will already have successfully completed
planning for at least one upstream object before encountering a downstream
error.

Since a downstream failure can be caused by an already-recorded action
from upstream, it might be helpful to inspect the actions planned so far
in order to understand better why the error occurred.

This doesn't yet make this result visible anywhere, and is backward
compatible with existing callers because they currently entirely ignore
the returned plan pointer if the diagnostics contains at least one error.
In any situation where we return a plan object along with some errors
we'll also explicitly annotate the plan object as being errored so that
we can catch if someone accidentally tries to apply that incomplete plan.

At the moment this situation is impossible to reach but in a later commit
we'll make it possible to save errored plans to disk for further
inspection, at which point it'll become important to not allow applying
them.
This is a prototype of how the CLI layer might make use of Terraform
Core's ability to produce a partial plan if it encounters an error during
planning, with two new situations:

- When using local CLI workflow, Terraform will show the partial plan
  before showing any errors.
- "terraform plan" has a new option -always-out=..., which is similar to
  the existing -out=... but additionally instructs Terraform to produce
  a plan file even if the plan is incomplete due to errors. This means
  that the plan can still be inspected by external UI implementations.

This is just a prototype to explore how these parts might fit together.
It's not a complete implementation and so should not be shipped. In
particular, it doesn't include any mention of a plan being incomplete in
the "terraform show -json" output or in the "terraform plan -json" output,
both of which would be required for a complete solution.
Make writing a plan file the default. We already create plans which have
no changes so the plan result would need to be checked in automation, so
having plans with errors should not pose a problem.

If we find workflows which cannot handle a plan that can't be applied,
we can reevaluate the need for a specialized flag. In the meantime, it
feels more logical that the plan output would always describe the result
of the plan, even if that included errors.
The status in the face of errors didn't matter before, because we never
wrote out a plan with errors.
@jbardin jbardin requested a review from a team December 14, 2022 20:15
// include errors, because we intentionally try to show a partial plan
// above even if Terraform Core encountered an error partway through
// creating it.
op.ReportResult(runningOp, diags)

if !runningOp.PlanEmpty {
op.View.PlanNextStep(op.PlanOutPath)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we still call this "next step" view function even if the plan errored? Maybe we should pass the errored status (or the entire plan) to the function so that it doesn't say "you can apply this saved plan".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PlanEmpty isn't set if there are errors, so keeps the zero value and this won't be called. There's still some more refinement to do in the CLI around errors, so we may need to revisit this anyway, but for now it doesn't interfere with the POC.

@jbardin jbardin merged commit 721df0e into main Jan 4, 2023
@jbardin jbardin deleted the jbardin/plan-output-on-error branch January 4, 2023 17:44
@github-actions
Copy link

github-actions bot commented Jan 4, 2023

Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch.

@github-actions
Copy link

github-actions bot commented Feb 4, 2023

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.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants