Remove team from state if deletion failed and it does not exist #1039
Conversation
| if ghErr, ok := err.(*github.ErrorResponse); ok { | ||
| if ghErr.Response.StatusCode == http.StatusNotFound { | ||
| // If team we failed to delete does not exist, remove it from TF state. | ||
| log.Printf("[WARN] Removing team: %s from state because it no longer exists", |
There was a problem hiding this comment.
The following warning does not show up with TF_LOG=WARN. It only shows up in TF_LOG=DEBUG. Not sure if this is a general issue with this provider or just with me doing something wrong in the code:
There was a problem hiding this comment.
@jcudit the only thing I'd need some help on is the logging level as the above is only showing up in TG_LOG=DEBUG. Example line below:
2022-01-22T10:47:11.813Z [DEBUG] provider.terraform-provider-github_v4.19.1-fl.4: 2022/01/22 10:47:11 [WARN] Removing team: 5592187 from state because it no longer exists
However, when looking at a valid WARN message, it looks like this:
2022-01-22T10:47:04.978Z [WARN] ValidateProviderConfig from "provider[\"registry.terraform.io/flaconi/github\"]" changed the config value, but that value is unused
Any idea on what I'm doing wrong here?
There was a problem hiding this comment.
I am stumped. This looks very similar to what I'm seeing in hashicorp/terraform, so 😕 why it is misbehaving for you.
Does TF_LOG=JSON do anything different? Perhaps TG_LOG was used in error?
| When deleting a team and it failed, we need to check if it has already been deleted meanwhile. | ||
| This could be the case when deleting nested teams via Terraform by looping through a module | ||
| or resource and the parent team might have been deleted already. If the parent team had | ||
| been deleted already (via parallel runs), the child team is also already gone (deleted by | ||
| GitHub automatically). | ||
| So we're checking if it still exists and if not, simply remove it from TF state. |
There was a problem hiding this comment.
This is a really good comment to add to our docs on this resource as well.
I haven't checked yet how the internal tests work. Two integration tests I can think of might be written as so: 1. Positive check:
2. Negative check:
|
Remove team from state if deletion failed and it does not exist
I experienced this behaviour while continuing the work on team slug: #802 So in order not to submit all changes at once, I will make this a separete PR (it does affect current provider as well)
Current state
When deleting a team on GitHub, the API will automatically delete all of its child team. In case we want to remove a team with the terraform provider which parent team has already been deleted, the resource will fail and report back with a
404 - team not found(or slightly similar).Improvement
When deleting a team with this terraform provider and it fails, we try to fetch it and check to see if it returns a 404. If the team we tried to delete does not exist, we will simply remove it from TF state and continue grafefully.
Notes on code
As I am pretty new to golan, please give me any hints on styling, comments or best-practices which I might have done wrongly.