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

Make the pre-destroy refresh a full plan #32208

Merged
merged 1 commit into from Nov 17, 2022
Merged

Conversation

jbardin
Copy link
Member

@jbardin jbardin commented Nov 11, 2022

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 before evalCheckRules is called, and lets us change the diagnosticSeverity of 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

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 jbardin merged commit b5168eb into main Nov 17, 2022
@jbardin jbardin deleted the jbardin/pre-desstroy-refresh branch November 17, 2022 19:18
@github-actions
Copy link

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

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 Dec 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
1.3-backport If you add this label to a PR before merging, backport-assistant will open a new PR once merged
Projects
None yet
2 participants