allow service to be reused #578
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
Integralist
requested review from
a team and
triblondon
and removed request for
phamann and
a team
triblondon
approved these changes
Aug 10, 2022
| }) | ||
| if err != nil { | ||
| return diag.FromErr(err) | ||
| if !d.Get("reuse").(bool) { |
There was a problem hiding this comment.
Would it be clearer to make this a check for force_destroy instead of not reuse?
There was a problem hiding this comment.
No, because if the user doesn't specify anything then this breaks the current behaviour.
e.g. currently, if the user has a deactivated service and they don't specify force_destroy, then the service will successfully be deleted. By changing the condition to be d.Get("force_destroy").(bool) we're effectively saying they have to add force_destroy to delete their deactivated service. By negating the reuse check we can keep the existing behaviour while supporting the new behaviour.
Co-authored-by: Andrew Betts <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #515
Summary: add
reuseattribute tofastly_service_computeandfastly_service_vclthat causesterraform destroyto not fully delete the service.See my notes for the full details and my manual validation of this change set, and the various caveats:
#515 (comment)
NOTE: I've manually tested this feature, and found it to work (see comment) but there are no tests written because the Terraform test framework expects resources to be destroyed at the end of each e2e test, and this PR implements a feature that sits outside that scenario, and so any test we write would fail unless written in a really janky way and additionally we'd need to implement a post-test process to then clean-up the service resource that was set to persist being destroyed.
Flow Examples
terraform destroy: service not activatedforce_destroy== no deactivation service API call, delete service API call (successful)reuse== no deactivation service API call, no delete service API call (basically a no-op)terraform destroy: service activatedforce_destroy== deactivate service API call (successful), delete service API call (successful)reuse== deactivate service API call (successful)