-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
State file "check_results" section not cleaned up after destroy, if variable validation is used #35383
Comments
I have also tested this without the validation in the variable and the behavior is as expected: "check_results" is empty. |
Hi @bczoma! Thanks for raising this. This is an interesting question! Technically variables are never "destroyed" in the sense that resource instances can be, their conditions are checked even during a destroy plan, and thus currently they do cause check results to be generated when planning and applying in destroy mode. However, I do appreciate the argument that it can be unsatisfying when a full destroy doesn't result in a truly "empty" state, and if any of the input variable checks had failed you likely wouldn't be able to reach this end state anyway. It's also a little weird that the status is given as "unknown" rather than as "pass"; that does suggest that something unusual is happening here, and perhaps we could decide that the appropriate fix here is to make sure that status gets populated correctly in this case rather than omitting the check entirely. It sounds like you were mainly interested in clarifying whether this result was a cause for concern. Is that right? If so, then I can confirm that Terraform isn't intended to consider checks when it decides whether a particular state snapshot is "empty". The definition of "empty" is focused on whether there are any resource instances and therefore the potential of any remote objects that would be "forgotten" by Terraform if the state snapshot were just discarded. |
@apparentlymart , appreciate the quick response and the explanation. Yes indeed I wanted to find out if there was anything that could/should be done at the user level. I've got my answer thank you and I'm ready to close this but will leave it to you in case this should be left open for any eventual next step. |
Thanks for confirming, @bczoma. Since you've drawn attention to the fact that the check status is getting set to "unknown" in this case, and I don't believe that's intentional, it does seem like we should probably at least understand why it behaves that way, and hopefully fix it if it's feasible to do so. I wonder if perhaps the graph pruning we do during the apply step is eliminating the variable due to it not appearing to be needed for the actions in the plan, and therefore Terraform never visits the input variable and so doesn't get a chance to decide its final check result. But I've not actually done anything to check that theory yet. If this quirk only affects the destroy case then we might decide that it's not significant enough to be worth extra complexity to resolve, but my main concern for now is whether this is indicating that something is missing more broadly, in which case fixing it might also benefit other situations beyond a full destroy. |
I think what's happening here is that the checks are not executed at all during a destroy apply operation. So the set of expected check results are loaded from the plan as This leads to another problem / issue: if resource targeting is used, any checks that are not needed by the I'm not sure what the expected behaviour should be in the event of a partial operation (ie. is it correct that checks should be set to unknown in the state if they didn't execute in the most recent operation, or should they keep the status from the last time they were executed). I guess an argument could be made for both. Either way, the root cause is the same in both issues. We register all the checks in the configuration up front to be unknown before the operation begins, and then if those checks don't operate within the operation their statuses stay as unknown. |
FWIW when I was originally designing the checks concept I intentionally made it report anything that was excluded using At the time I was doing that we only had resource and output preconditions and postconditions where "destroy" effectively removes the object that the check applies to, and so reporting it as "unknown" is arguably reasonable as a way to say that the check is declared in configuration but is not applicable to the current state. Input variables are an interesting oddity because they are not something that persists between rounds, and so it's not really meaningful to discuss "destroying" them. However, if them ending up in this "unknown" status is just a logical consequence of them following the same rules as destroyed (or un-targeted) resource instances then I'd say that's probably fine, in my opinion. I was raising it only out of concern that this might be reflecting something more sinister going on, but it sounds like it's not actually a big cause for concern. |
Given Martin's comment I will close this as working-as-designed. Given the behaviour of the other checks is behaving as originally intended, I also think the input variables being consistent with this is acceptable. |
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. |
Terraform Version
Terraform Configuration Files
Debug Output
https://gist.github.com/bczoma/89172cd0afb5f0381e1deeeab889ad82#file-terraform-log
Expected Behavior
State file contains empty "check_results" block.
Actual Behavior
State file "check_results" block is not cleaned up.
Steps to Reproduce
Use the Terraform config file:
terraform init
terraform apply
terraform destroy
Additional Context
Wanted to raise to get confirmation this is a benign behavior. I couldn't find any related information from my search.
References
No response
The text was updated successfully, but these errors were encountered: