Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom

Apply graph failure handling #30199

Merged
merged 2 commits into from
Dec 17, 2021
Merged

Apply graph failure handling #30199

merged 2 commits into from
Dec 17, 2021

Conversation

jbardin
Copy link
Member

@jbardin jbardin commented Dec 17, 2021 •
edited
Loading

This PR resolves 2 issues around failures during the core Apply step.

  • The first is that the graph building diagnostics were lost, and no error was being returned upon failure, allowing terraform to continue.
  • The second issue is that while an apply error normally means that there could be a partial state to store, in the event of an error before any apply operations have started we have no state, and should not overwrite the existing state.

Fixes #30178

@jbardin jbardin added the 1.1-backport If you add this label to a PR before merging, backport-assistant will open a new PR once merged label Dec 17, 2021
@jbardin jbardin requested a review from a team December 17, 2021 17:52
@jbardin jbardin self-assigned this Dec 17, 2021
@jbardin
return graph errors from Context.Apply
58e001c 
errors from building during apply were lost
@jbardin
don't persist a nil state from Apply
8c4031e 
Apply should not return a nil state to be persisted.
@jbardin jbardin force-pushed the jbardin/apply-failure-diags branch from 512df5b to 8c4031e Compare December 17, 2021 19:01
@jbardin jbardin merged commit 2c8edfb into main Dec 17, 2021
@jbardin jbardin deleted the jbardin/apply-failure-diags branch December 17, 2021 19:08
@teamterraform teamterraform mentioned this pull request Dec 17, 2021
Closed
@jbardin jbardin mentioned this pull request Dec 17, 2021
Merged
@Nothing4You Nothing4You mentioned this pull request Dec 22, 2021
Closed
@caiodelgadonew caiodelgadonew mentioned this pull request Dec 27, 2021
Closed
@github-actions
Copy link
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.
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 Jan 17, 2022
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Reviewers

@alisdair alisdair alisdair approved these changes

Assignees

@jbardin jbardin

Labels
1.1-backport If you add this label to a PR before merging, backport-assistant will open a new PR once merged
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Terraform does not apply generated plan and clears out the state
2 participants
@jbardin @alisdair