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

Destroy with deletion_protection returns a exit code of ` #7869

Closed
hadim opened this issue Nov 22, 2020 · 13 comments
Closed

Destroy with deletion_protection returns a exit code of ` #7869

hadim opened this issue Nov 22, 2020 · 13 comments
Assignees
Labels

Comments

@hadim
Copy link

hadim commented Nov 22, 2020

When running terraform destroy on a config with deletion_protection set to true for a google_sql_database_instance, tf will delete all the resources except the google_sql_database_instance one and its parent dependencies.

But the terraform destroy command returns an exit code of 1 with the following error message:

Error: Error, failed to delete instance because deletion_protection is set to true. Set it to false to proceed with instance deletion

It seems to me this behaviour is not correct and the command should return 0 instead since there is no error and not destroying the resource was the correct behaviour.

Am I missing something?

@hadim
Copy link
Author

hadim commented Nov 22, 2020

(this behaviour can have consequence in a CI setup for example, where a build could be marked as failed while in fact, it succeded)

@upodroid
Copy link
Contributor

Hey, you need to set it to false or it won't delete. There is a note at the top of the docs about this.

https://registry.terraform.io/providers/hashicorp/google/latest/docs/resources/sql_database_instance

@hadim
Copy link
Author

hadim commented Nov 23, 2020

Sorry I wasn't clear. I actually DONT want the db to be destroyed. My point is that in that case the destroy command should return no error and an exit code of 0.

@upodroid
Copy link
Contributor

It is not written like that because terraform destroy must remove the resource from the platform. To prevent accidental deletions, that field is in place.

What you are describing is equivalent to terraform state rm google_sql_database_instance.foobar which you can already do but not as part of destroy.

@hadim
Copy link
Author

hadim commented Nov 23, 2020

I am not saying the behaviour of deletion_protection is wrong and actually, it behaves exactly as I want. I am just wondering why the terraform destroy command returns an exit code of 1 (failure) while IMO it should return an exit code of 0 (success).

@edwardmedia edwardmedia self-assigned this Nov 23, 2020
@edwardmedia
Copy link
Contributor

edwardmedia commented Nov 23, 2020

Using fmt.Info instead?

return fmt.Errorf("Error, failed to delete instance because deletion_protection is set to true. Set it to false to proceed with instance deletion")

@hadim
Copy link
Author

hadim commented Nov 23, 2020

An info log makes sense to me. Maybe something like "The instance has not been deleted because deletion_protection is set to true. Set it to false to proceed with instance deletion"

@nat-henderson
Copy link
Contributor

Hi,

Sorry for the inconvenience, but I feel that the behavior is correct, including the return code. I think that "I ran terraform destroy but it did not destroy all my resources" merits a "failure" rather than a "success" exit code. This is also important to stop terraform from destroying upstream resources. I propose that if you want to check, in ci, whether terraform destroy was "successful", you could check [[ $(terraform state list | wc -l) < "2" ]].

Will that help?

@rileykarson
Copy link
Collaborator

To add to @ndmckinley's response (mostly because they got to this right before I did as a response to #7868):

I feel that Terraform returning an exit code of 1 here is appropriate because the plan stage of terraform apply indicated that it would destroy the resource, but Terraform failed to do so.

@hadim
Copy link
Author

hadim commented Nov 23, 2020

I disagree since a failure (at least to me) is when something unexcepted happens such as an error. In that particular case, terraform destroy is doing the right thing since he didn't destroy something I specifically ask to not destroy. So I would except the command to return a success code.

Looks like you guys see that from a different perspective and I got your point :-)

All good and thanks again for the assistance.

@hadim
Copy link
Author

hadim commented Nov 23, 2020

you could check [[ $(terraform state list | wc -l) < "2" ]].

Yes, there is plenty of ways to hack this of course. It's just less robust and less readable.

@hadim
Copy link
Author

hadim commented Nov 28, 2020

I think that "I ran terraform destroy but it did not destroy all my resources" merits a "failure" rather than a "success" exit code.

For context please see also hashicorp/terraform#3874

@ghost
Copy link

ghost commented Dec 24, 2020

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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!

@ghost ghost locked as resolved and limited conversation to collaborators Dec 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

6 participants