support multiple exit codes based on what went wrong/right #1135
Conversation
= at least one failure, 4 = no failures but at least 1 warn 1 as a catch all, 2 for invalid input etc ref #1131
|
TODO: docs update: done replicatedhq/troubleshoot.sh#489 |
|
TODO: fix what I did with |
|
docs PR - replicatedhq/troubleshoot.sh#489 |
(concatenate it into the string, wrap/unwrap accordingly)
|
Ready for review 👍 Tests pass and docs are done replicatedhq/troubleshoot.sh#489 |
There was a problem hiding this comment.
I feel like a more go-like pattern for exiting with different codes would be to define an interface
type ExitError interface {
Error() string
ExitStatus() int
}
func main() {
err := RootCmd().Execute()
fmt.Fprintln(os.Stderr, err.Error())
switch err := errors.Unwrap(err).(type) {
case ExitError:
os.Exit(err.ExitStatus())
default:
os.Exit(1)
}
}
I actually considered something like this, and even went to the extent of a custom error type (struct) that implements the The idea of using I did almost have some success again today with a custom error struct (to the point I got it to build), but it was still just a generic error being returned not my custom type. If I change the I think for now in light of wanting to get this merged ASAP for use, we should retain the delimiter approach and as a future improvement try refactor things here. |
this is still hacky, but it gets us by for today
|
@xavpaice also mentioned this example using I've tried this approach also - the downside is it'll never run our |
|
Apologies for not testing the code in my suggestion. This was more difficult than I had thought. Ive created a quick prototype for returning errors with exit codes here. |
had to modify it slightly to work around nil pointer references on errors, but it works :)
no worries thanks @emosbaugh :) |
Co-authored-by: Ethan Mosbaugh <ethan@replicated.com>
Co-authored-by: Ethan Mosbaugh <ethan@replicated.com>
|
@emosbaugh the downside to always printing an error is we end up with the below, you get |
I overlooked this logic on my previous review. All errors should have a message. You should set the error message to something like "preflight checks failed with errors" or "preflight checks failed with warnings". If you dont want to output a message in this case I would suggest another error type PreflightCheckResultsError and have another case in InitAndExecute with errors.As where you do not print the error. |
if there's errors in processing (wasn't happening before), and we don't return an error if all is well (for an exit code 0 situation)
|
OK I feel like this might be the best we're going to get... keen to get this merged and then improve on it later if we need to though :) |
|
|
Co-authored-by: Ethan Mosbaugh <ethan@replicated.com>
Co-authored-by: Ethan Mosbaugh <ethan@replicated.com>
Co-authored-by: Ethan Mosbaugh <ethan@replicated.com>
Description, Motivation and Context
support multiple exit codes based on what went wrong/right
main motivator initially: failing helm preflights in an elegant way
0 = all passed, 3 = at least one failure, 4 = no failures but at least 1 warn
1 as a catch all, 2 for invalid input etc
resolves #1131
Checklist
Does this PR introduce a breaking change?