Tolerate 404 errors during resource deletion to handle already delete… #2590
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
alexsomesan
reviewed
Sep 23, 2024
| @@ -564,16 +575,24 @@ func (s *RawProviderServer) ApplyResourceChange(ctx context.Context, req *tfprot | |||
| _, err := rs.Get(ctxDeadline, rname, metav1.GetOptions{}) | |||
| if err != nil { | |||
| if apierrors.IsNotFound(err) { | |||
| s.logger.Trace("[ApplyResourceChange][Delete]", "Resource is deleted") | |||
| s.logger.Trace("[ApplyResourceChange][Delete]", "Resource is already deleted") | |||
There was a problem hiding this comment.
We actually don't need the change over here.
I this case, receiving a 404 while waiting for a resource to actually be deleted is a confirmation of the actual deletion, not an indication that the resource was not actually present to begin with (in that case the 404 will have shown up as response to the original .Delete() call).
| Summary: fmt.Sprintf("Resource %q was already deleted", rname), | ||
| Detail: fmt.Sprintf("The resource %q was not found in the Kubernetes API. This may be due to the resource being already deleted.", rname), | ||
| }) | ||
| return resp, nil |
There was a problem hiding this comment.
Since this return statement is identical in both branches of the if statement, you can actually move it right after it to de-duplicate.
|
Also, please add a CHANGELOG entry (see workflow checks failure). |
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.
Description
Fixes #2188
This PR introduces changes to handle "404 Not Found" errors during the deletion of Kubernetes resources, particularly in cases where the resource may have already been deleted by an operator managing the CRD before Terraform attempts to delete it.
Changes Introduced:
Added logic to check for IsNotFound(err) when calling Delete on resources.
If a "404 Not Found" error is encountered, it is now treated as a successful deletion and logged as "Resource
already deleted, ignoring 404 error".
Other errors during deletion will continue to be handled as before.
Acceptance tests
Output from acceptance testing:
Warning: Resource "elasticsearch" was already deleted
│
│ The resource "elasticsearch" was not found in the Kubernetes API. This may be due to the resource being already deleted.
╵
╷
│ Warning: Resource "elasticsearch-es-elastic-user" was already deleted
│
│ The resource "elasticsearch-es-elastic-user" was not found in the Kubernetes API. This may be due to the resource being already deleted.
╵
Destroy complete! Resources: 3 destroyed.
Release Note
Release note for CHANGELOG:
References
Community Note