diff --git a/pkg/kubectl/cmd/cmd_test.go b/pkg/kubectl/cmd/cmd_test.go index f0ecbfbbfdf..0d375432298 100644 --- a/pkg/kubectl/cmd/cmd_test.go +++ b/pkg/kubectl/cmd/cmd_test.go @@ -46,8 +46,8 @@ import ( ) func initTestErrorHandler(t *testing.T) { - cmdutil.BehaviorOnFatal(func(str string) { - t.Errorf("Error running command: %s", str) + cmdutil.BehaviorOnFatal(func(str string, code int) { + t.Errorf("Error running command (exit code %d): %s", code, str) }) } diff --git a/pkg/kubectl/cmd/drain_test.go b/pkg/kubectl/cmd/drain_test.go index a706de18c7f..a1261b689a1 100644 --- a/pkg/kubectl/cmd/drain_test.go +++ b/pkg/kubectl/cmd/drain_test.go @@ -177,7 +177,7 @@ func TestCordon(t *testing.T) { // Restore cmdutil behavior cmdutil.DefaultBehaviorOnFatal() }() - cmdutil.BehaviorOnFatal(func(e string) { saw_fatal = true; panic(e) }) + cmdutil.BehaviorOnFatal(func(e string, code int) { saw_fatal = true; panic(e) }) cmd.SetArgs([]string{test.arg}) cmd.Execute() }() @@ -521,7 +521,7 @@ func TestDrain(t *testing.T) { // Restore cmdutil behavior cmdutil.DefaultBehaviorOnFatal() }() - cmdutil.BehaviorOnFatal(func(e string) { saw_fatal = true; panic(e) }) + cmdutil.BehaviorOnFatal(func(e string, code int) { saw_fatal = true; panic(e) }) cmd.SetArgs(test.args) cmd.Execute() }() diff --git a/pkg/kubectl/cmd/exec.go b/pkg/kubectl/cmd/exec.go index bbfbf05b68a..b8dee1e5024 100644 --- a/pkg/kubectl/cmd/exec.go +++ b/pkg/kubectl/cmd/exec.go @@ -39,7 +39,7 @@ var ( exec_example = dedent.Dedent(` # Get output from running 'date' from pod 123456-7890, using the first container by default kubectl exec 123456-7890 date - + # Get output from running 'date' in ruby-container from pod 123456-7890 kubectl exec 123456-7890 -c ruby-container date diff --git a/pkg/kubectl/cmd/taint_test.go b/pkg/kubectl/cmd/taint_test.go index 0c9427b3823..b8087fd0f60 100644 --- a/pkg/kubectl/cmd/taint_test.go +++ b/pkg/kubectl/cmd/taint_test.go @@ -296,7 +296,7 @@ func TestTaint(t *testing.T) { // Restore cmdutil behavior cmdutil.DefaultBehaviorOnFatal() }() - cmdutil.BehaviorOnFatal(func(e string) { saw_fatal = true; panic(e) }) + cmdutil.BehaviorOnFatal(func(e string, code int) { saw_fatal = true; panic(e) }) cmd.SetArgs(test.args) cmd.Execute() }() diff --git a/pkg/kubectl/cmd/util/helpers.go b/pkg/kubectl/cmd/util/helpers.go index 3419baab3e3..7208e2f98ae 100644 --- a/pkg/kubectl/cmd/util/helpers.go +++ b/pkg/kubectl/cmd/util/helpers.go @@ -50,6 +50,7 @@ import ( const ( ApplyAnnotationsFlag = "save-config" + DefaultErrorExitCode = 1 ) type debugError interface { @@ -74,9 +75,9 @@ func AddSourceToErr(verb string, source string, err error) error { var fatalErrHandler = fatal // BehaviorOnFatal allows you to override the default behavior when a fatal -// error occurs, which is call os.Exit(1). You can pass 'panic' as a function +// error occurs, which is to call os.Exit(code). You can pass 'panic' as a function // here if you prefer the panic() over os.Exit(1). -func BehaviorOnFatal(f func(string)) { +func BehaviorOnFatal(f func(string, int)) { fatalErrHandler = f } @@ -86,19 +87,21 @@ func DefaultBehaviorOnFatal() { fatalErrHandler = fatal } -// fatal prints the message and then exits. If V(2) or greater, glog.Fatal +// fatal prints the message if set and then exits. If V(2) or greater, glog.Fatal // is invoked for extended information. -func fatal(msg string) { - // add newline if needed - if !strings.HasSuffix(msg, "\n") { - msg += "\n" - } +func fatal(msg string, code int) { + if len(msg) > 0 { + // add newline if needed + if !strings.HasSuffix(msg, "\n") { + msg += "\n" + } - if glog.V(2) { - glog.FatalDepth(2, msg) + if glog.V(2) { + glog.FatalDepth(2, msg) + } + fmt.Fprint(os.Stderr, msg) } - fmt.Fprint(os.Stderr, msg) - os.Exit(1) + os.Exit(code) } // CheckErr prints a user friendly error to STDERR and exits with a non-zero @@ -115,51 +118,49 @@ func checkErrWithPrefix(prefix string, err error) { checkErr(prefix, err, fatalErrHandler) } -func checkErr(pref string, err error, handleErr func(string)) { - if err == nil { +// checkErr formats a given error as a string and calls the passed handleErr +// func with that string and an kubectl exit code. +func checkErr(prefix string, err error, handleErr func(string, int)) { + switch { + case err == nil: return - } - - if kerrors.IsInvalid(err) { + case kerrors.IsInvalid(err): details := err.(*kerrors.StatusError).Status().Details - prefix := fmt.Sprintf("%sThe %s %q is invalid.\n", pref, details.Kind, details.Name) - errs := statusCausesToAggrError(details.Causes) - handleErr(MultilineError(prefix, errs)) - return - } - - if noMatch, ok := err.(*meta.NoResourceMatchError); ok { - switch { - case len(noMatch.PartialResource.Group) > 0 && len(noMatch.PartialResource.Version) > 0: - handleErr(fmt.Sprintf("%sthe server doesn't have a resource type %q in group %q and version %q", pref, noMatch.PartialResource.Resource, noMatch.PartialResource.Group, noMatch.PartialResource.Version)) - case len(noMatch.PartialResource.Group) > 0: - handleErr(fmt.Sprintf("%sthe server doesn't have a resource type %q in group %q", pref, noMatch.PartialResource.Resource, noMatch.PartialResource.Group)) - case len(noMatch.PartialResource.Version) > 0: - handleErr(fmt.Sprintf("%sthe server doesn't have a resource type %q in version %q", pref, noMatch.PartialResource.Resource, noMatch.PartialResource.Version)) - default: - handleErr(fmt.Sprintf("%sthe server doesn't have a resource type %q", pref, noMatch.PartialResource.Resource)) + s := fmt.Sprintf("%sThe %s %q is invalid", prefix, details.Kind, details.Name) + if len(details.Causes) > 0 { + errs := statusCausesToAggrError(details.Causes) + handleErr(MultilineError(s+": ", errs), DefaultErrorExitCode) + } else { + handleErr(s, DefaultErrorExitCode) } - return - } - - // handle multiline errors - if clientcmd.IsConfigurationInvalid(err) { - handleErr(MultilineError(fmt.Sprintf("%sError in configuration: ", pref), err)) - return - } - if agg, ok := err.(utilerrors.Aggregate); ok && len(agg.Errors()) > 0 { - handleErr(MultipleErrors(pref, agg.Errors())) - return - } - - msg, ok := StandardErrorMessage(err) - if !ok { - msg = err.Error() - if !strings.HasPrefix(msg, "error: ") { - msg = fmt.Sprintf("error: %s", msg) + case clientcmd.IsConfigurationInvalid(err): + handleErr(MultilineError(fmt.Sprintf("%sError in configuration: ", prefix), err), DefaultErrorExitCode) + default: + switch err := err.(type) { + case *meta.NoResourceMatchError: + switch { + case len(err.PartialResource.Group) > 0 && len(err.PartialResource.Version) > 0: + handleErr(fmt.Sprintf("%sthe server doesn't have a resource type %q in group %q and version %q", prefix, err.PartialResource.Resource, err.PartialResource.Group, err.PartialResource.Version), DefaultErrorExitCode) + case len(err.PartialResource.Group) > 0: + handleErr(fmt.Sprintf("%sthe server doesn't have a resource type %q in group %q", prefix, err.PartialResource.Resource, err.PartialResource.Group), DefaultErrorExitCode) + case len(err.PartialResource.Version) > 0: + handleErr(fmt.Sprintf("%sthe server doesn't have a resource type %q in version %q", prefix, err.PartialResource.Resource, err.PartialResource.Version), DefaultErrorExitCode) + default: + handleErr(fmt.Sprintf("%sthe server doesn't have a resource type %q", prefix, err.PartialResource.Resource), DefaultErrorExitCode) + } + case utilerrors.Aggregate: + handleErr(MultipleErrors(prefix, err.Errors()), DefaultErrorExitCode) + default: // for any other error type + msg, ok := StandardErrorMessage(err) + if !ok { + msg = err.Error() + if !strings.HasPrefix(msg, "error: ") { + msg = fmt.Sprintf("error: %s", msg) + } + } + handleErr(msg, DefaultErrorExitCode) } } - handleErr(fmt.Sprintf("%s%s", pref, msg)) } func statusCausesToAggrError(scs []unversioned.StatusCause) utilerrors.Aggregate { diff --git a/pkg/kubectl/cmd/util/helpers_test.go b/pkg/kubectl/cmd/util/helpers_test.go index c02d24293c0..0ba5b746935 100644 --- a/pkg/kubectl/cmd/util/helpers_test.go +++ b/pkg/kubectl/cmd/util/helpers_test.go @@ -213,91 +213,78 @@ func (f *fileHandler) ServeHTTP(res http.ResponseWriter, req *http.Request) { res.Write(f.data) } +type checkErrTestCase struct { + err error + expectedErr string + expectedCode int +} + func TestCheckInvalidErr(t *testing.T) { - tests := []struct { - err error - expected string - }{ + testCheckError(t, []checkErrTestCase{ { errors.NewInvalid(api.Kind("Invalid1"), "invalidation", field.ErrorList{field.Invalid(field.NewPath("field"), "single", "details")}), - `error: The Invalid1 "invalidation" is invalid. field: Invalid value: "single": details`, + "The Invalid1 \"invalidation\" is invalid: field: Invalid value: \"single\": details\n", + DefaultErrorExitCode, }, { errors.NewInvalid(api.Kind("Invalid2"), "invalidation", field.ErrorList{field.Invalid(field.NewPath("field1"), "multi1", "details"), field.Invalid(field.NewPath("field2"), "multi2", "details")}), - `error: The Invalid2 "invalidation" is invalid. * field1: Invalid value: "multi1": details, * field2: Invalid value: "multi2": details`, + "The Invalid2 \"invalidation\" is invalid: \n* field1: Invalid value: \"multi1\": details\n* field2: Invalid value: \"multi2\": details\n", + DefaultErrorExitCode, }, { errors.NewInvalid(api.Kind("Invalid3"), "invalidation", field.ErrorList{}), - `error: The Invalid3 "invalidation" is invalid. %!s()`, + "The Invalid3 \"invalidation\" is invalid", + DefaultErrorExitCode, }, { errors.NewInvalid(api.Kind("Invalid4"), "invalidation", field.ErrorList{field.Invalid(field.NewPath("field4"), "multi4", "details"), field.Invalid(field.NewPath("field4"), "multi4", "details")}), - `error: The Invalid4 "invalidation" is invalid. field4: Invalid value: "multi4": details`, + "The Invalid4 \"invalidation\" is invalid: field4: Invalid value: \"multi4\": details\n", + DefaultErrorExitCode, }, - } - - var errReturned string - errHandle := func(err string) { - for _, v := range strings.Split(err, "\n") { - separator := " " - if errReturned == "" || v == "" { - separator = "" - } else if !strings.HasSuffix(errReturned, ".") { - separator = ", " - } - errReturned = fmt.Sprintf("%s%s%s", errReturned, separator, v) - } - if !strings.HasPrefix(errReturned, "error: ") { - errReturned = fmt.Sprintf("error: %s", errReturned) - } - if strings.HasSuffix(errReturned, ", ") { - errReturned = errReturned[:len(errReturned)-len(" ,")] - } - } - - for _, test := range tests { - checkErr("", test.err, errHandle) - - if errReturned != test.expected { - t.Fatalf("Got: %s, expected: %s", errReturned, test.expected) - } - errReturned = "" - } + }) } func TestCheckNoResourceMatchError(t *testing.T) { - tests := []struct { - err error - expected string - }{ + testCheckError(t, []checkErrTestCase{ { &meta.NoResourceMatchError{PartialResource: unversioned.GroupVersionResource{Resource: "foo"}}, `the server doesn't have a resource type "foo"`, + DefaultErrorExitCode, }, { &meta.NoResourceMatchError{PartialResource: unversioned.GroupVersionResource{Version: "theversion", Resource: "foo"}}, `the server doesn't have a resource type "foo" in version "theversion"`, + DefaultErrorExitCode, }, { &meta.NoResourceMatchError{PartialResource: unversioned.GroupVersionResource{Group: "thegroup", Version: "theversion", Resource: "foo"}}, `the server doesn't have a resource type "foo" in group "thegroup" and version "theversion"`, + DefaultErrorExitCode, }, { &meta.NoResourceMatchError{PartialResource: unversioned.GroupVersionResource{Group: "thegroup", Resource: "foo"}}, `the server doesn't have a resource type "foo" in group "thegroup"`, + DefaultErrorExitCode, }, - } + }) +} +func testCheckError(t *testing.T, tests []checkErrTestCase) { var errReturned string - errHandle := func(err string) { + var codeReturned int + errHandle := func(err string, code int) { errReturned = err + codeReturned = code } for _, test := range tests { checkErr("", test.err, errHandle) - if errReturned != test.expected { - t.Fatalf("Got: %s, expected: %s", errReturned, test.expected) + if errReturned != test.expectedErr { + t.Fatalf("Got: %s, expected: %s", errReturned, test.expectedErr) + } + if codeReturned != test.expectedCode { + t.Fatalf("Got: %d, expected: %d", codeReturned, test.expectedCode) } } } diff --git a/pkg/util/exec/exec.go b/pkg/util/exec/exec.go index e8768455103..e1ccba0972e 100644 --- a/pkg/util/exec/exec.go +++ b/pkg/util/exec/exec.go @@ -113,7 +113,7 @@ func (cmd *cmdWrapper) Output() ([]byte, error) { func handleError(err error) error { if ee, ok := err.(*osexec.ExitError); ok { // Force a compile fail if exitErrorWrapper can't convert to ExitError. - var x ExitError = &exitErrorWrapper{ee} + var x ExitError = &ExitErrorWrapper{ee} return x } if ee, ok := err.(*osexec.Error); ok { @@ -124,14 +124,16 @@ func handleError(err error) error { return err } -// exitErrorWrapper is an implementation of ExitError in terms of os/exec ExitError. +// ExitErrorWrapper is an implementation of ExitError in terms of os/exec ExitError. // Note: standard exec.ExitError is type *os.ProcessState, which already implements Exited(). -type exitErrorWrapper struct { +type ExitErrorWrapper struct { *osexec.ExitError } +var _ ExitError = ExitErrorWrapper{} + // ExitStatus is part of the ExitError interface. -func (eew exitErrorWrapper) ExitStatus() int { +func (eew ExitErrorWrapper) ExitStatus() int { ws, ok := eew.Sys().(syscall.WaitStatus) if !ok { panic("can't call ExitStatus() on a non-WaitStatus exitErrorWrapper")