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

core: Keep old value on error even for delete #21033

Merged
merged 1 commit into from
Apr 17, 2019

Conversation

apparentlymart
Copy link
Contributor

When an operation fails, providers may return a null new value rather than returning a partial state. In that case, we'd prefer to keep the old value so that we stand the best chance of being able to retry on a subsequent run.

Previously we were making an exception for the delete action, allowing the result of that to be null even when an error is returned. In practice that was a bad idea because it would cause Terraform to lose track of the object even though it might not actually have been deleted.

Now we'll retain the old object even in the delete case. Providers can still return partial new objects if they were able to partially complete a delete operation, in which case we'll discard what we had before, but if the result is null with errors then we'll assume the delete failed entirely and so just keep the old state as-is, giving us the opportunity to refresh it on the next run to see if anything actually happened after all.

This fixes #20968.

(This also includes a new resource in the test provider which isn't used by the patch but was useful for some manual UX testing here, so I thought I'd include it in case it's similarly useful in future, given how simple its implementation is.)

When an operation fails, providers may return a null new value rather than
returning a partial state. In that case, we'd prefer to keep the old value
so that we stand the best chance of being able to retry on a subsequent
run.

Previously we were making an exception for the delete action, allowing
the result of that to be null even when an error is returned. In practice
that was a bad idea because it would cause Terraform to lose track of the
object even though it might not actually have been deleted.

Now we'll retain the old object even in the delete case. Providers can
still return partial new objects if they were able to partially complete
a delete operation, in which case we'll discard what we had before, but
if the result is null with errors then we'll assume the delete failed
entirely and so just keep the old state as-is, giving us the opportunity
to refresh it on the next run to see if anything actually happened after
all.

(This also includes a new resource in the test provider which isn't used
by the patch but was useful for some manual UX testing here, so I thought
I'd include it in case it's similarly useful in future, given how simple
its implementation is.)
@apparentlymart apparentlymart requested a review from a team April 16, 2019 23:52
@apparentlymart apparentlymart self-assigned this Apr 16, 2019
@apparentlymart apparentlymart added this to the v0.12.0 milestone Apr 16, 2019
@apparentlymart apparentlymart marked this pull request as ready for review April 16, 2019 23:52
Copy link
Contributor

@mildwonkey mildwonkey left a comment

Choose a reason for hiding this comment

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

This LGTM! I restarted the build for good measure, but I'll approve this either way.

@apparentlymart apparentlymart merged commit ff2de9c into master Apr 17, 2019
@apparentlymart apparentlymart deleted the b-delete-drop-on-error branch April 17, 2019 14:47
@ghost
Copy link

ghost commented Jul 26, 2019

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Jul 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

terraform 0.12 beta Resource is deleted from state when destroy ends in error
2 participants