Destroy Failed Deployments #3602
Conversation
There was a problem hiding this comment.
I can't comment on line 79/80 b/c it didn't change in this PR, but should we change this:
}); err != nil {
c.ui.Output("Error destroying the deployment: %s", err.Error(), terminal.WithErrorStyle())
return ErrSentinel
}to simply print the error and continue destroying other deployments, instead of returning?
internal/cli/deployment_destroy.go
Outdated
| } | ||
| } | ||
| if err != nil { |
There was a problem hiding this comment.
This err is now the err from line 44 and not any error returned from app.Destroy, I don't believe that is what we want.
If we plan to continue destroy attempts we probably need to collect the errors in a slice? I'm no longer sure what the intended behavior is
There was a problem hiding this comment.
@catsby I think it'd be best to remove the error check/return here. You're right that this is actually the err from line 44, and we already catch/return that earlier (as we should).
I think collecting the errors in a slice is a good idea. After the loop is finished, we can check if there's > 0 errors in the slice, and if there are, return an error signifying that one or more destroy-ments failed. The output message will already have warned the user of which ones failed so no need to parse through the slice for anything more specific.
…plete deployment.
Co-authored-by: Clint <[email protected]>
…other deployments, even if one or more errors while being destroyed.
paladin-devops
force-pushed
the
533-incomplete-deployment-destroy
branch
from
6f6e344 to
322dd27
Compare
Update
waypoint deployment destroyto continue with deleting the specified deployment from the database, while also warning the user that the resources created from the failed deployment may not be fully destroyed.Addresses #533.