Make the pre-destroy refresh a full plan #32208
Merged
Conversation
This file contains 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
In order to complete the terraform destroy command, a refresh must first be done to update state and remove any instances which have already been deleted externally. This was being done with a refresh plan, which will avoid any condition evaluations and avoid planning new instances. That however can fail due to invalid references from resources that are already missing from the state. A new plan type to handle the concept of the pre-destroy-refresh is needed here, which should probably be incorporated directly into the destroy plan, just like the original refresh walk was incorporated into the normal planning process. That however is major refactoring that is not appropriate for a patch release. Instead we make two discrete changes here to prevent blocking a destroy plan. The first is to use a normal plan to refresh, which will enable evaluation because missing and inconsistent instances will be planned for creation and updates, allowing them to be evaluated. That is not optimal of course, but does revert to the method used by previous Terraform releases until a better method can be implemented. The second change is adding a preDestroyRefresh flag to the planning process. This is checked in any location which evalCheckRules is called, and lets us change the diagnosticSeverity of the output to only be warnings, matching the behavior of a normal refresh plan.
jbardin
added
the
1.3-backport
apparentlymart
approved these changes
Nov 17, 2022
|
Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch. |
|
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. |
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.
In order to complete the terraform destroy command, a refresh must first be done to update state and remove any instances which have already been deleted externally. This was being done with a refresh plan, which will avoid any condition evaluations and avoid planning new instances. That however can fail due to invalid references from resources that are already missing from the state.
A new plan type to handle the concept of the pre-destroy-refresh is needed here, which should probably be incorporated directly into the destroy plan, just like the original refresh walk was incorporated into the normal planning process. That however is major refactoring that is not appropriate for a patch release.
Instead we make two discrete changes here to prevent blocking a destroy plan. The first is to use a normal plan to refresh, which will enable evaluation because missing and inconsistent instances will be planned for creation and updates, allowing them to be evaluated. That is not optimal of course, but does revert to the method used by previous Terraform releases until a better method can be implemented.
The second change is adding a
preDestroyRefreshflag to the planning process. This is checked beforeevalCheckRulesis called, and lets us change thediagnosticSeverityof the output to only be warnings, matching the behavior of a normal refresh plan.The combination of these two changes should prevent the vast majority of problems which could block a destroy plan. Remaining evaluation problems during the pre-destroy refresh can be avoided by using
-refresh=false.Fixes #32185
Fixes #32126