Fix rad app delete does not clean up all resources when the application deployment originally failed #7081
Conversation
| if deleteErr != nil { | ||
| logger.Info("Failed to cleanup deployed output resources for resource %q. Error: %s", id.String(), deleteErr.Error()) | ||
| } | ||
|
|
There was a problem hiding this comment.
WDYT would happen in the following scenario?
- User deploys an application successfully. They are using a Recipe to create a Redis cache, and a cloud-hosted Redis cache is created successfully.
- User updates the application. The recipe runs again, but this time it fails - let's say it was on outage on the cloud-provider's part.
- Then what ???
There was a problem hiding this comment.
I think it would unnecessarily delete the cache too, with potential data loss . :(
I am trying to fix a tricky scenario where rad app delete can't clean up resources upon deployment error since we dont track o/p resources at all in that case.. I am not sure it would be an improvement if I deleted just the errored outputresource in that case?
There was a problem hiding this comment.
I think it would unnecessarily delete the cache too, with potential data loss .
Yup, that's right 😆
There was a problem hiding this comment.
Instead of deleting resource and returning error, what if we add status in deploymentOutput to track the failure state? For updating scenario (failed scenario), we can upsert to the outputresources to keep old resource references for deletion. WDYT?
| LocalID: outputResource.LocalID, | ||
| ID: outputResource.ID, | ||
| } | ||
| deployedOutputResources = append(deployedOutputResources, outputResource) |
There was a problem hiding this comment.
I can't understand why we need to add the outputResource to a map? The map will only have one item and then the function will exit based on line 302. Am I missing something?
nithyatsu
had a problem deploying
to
functional-tests
GitHub Actions
Failure
Description
We do not track the outputResources in the event of deplyment failure, but might have created some, potentially with errors.
If the deployment of outResource errors, we should cleanup these so that app can be deleted gracefully using rad app delete command.
Type of change
Fixes: #7052