diff --git a/cmd/kubectl/kubectl.go b/cmd/kubectl/kubectl.go index 7c7924a5f5f..09c18cfa209 100644 --- a/cmd/kubectl/kubectl.go +++ b/cmd/kubectl/kubectl.go @@ -17,10 +17,9 @@ limitations under the License. package main import ( - "os" - "k8s.io/component-base/cli" "k8s.io/kubectl/pkg/cmd" + "k8s.io/kubectl/pkg/cmd/util" // Import to initialize client auth plugins. _ "k8s.io/client-go/plugin/pkg/client/auth" @@ -28,6 +27,8 @@ import ( func main() { command := cmd.NewDefaultKubectlCommand() - code := cli.Run(command) - os.Exit(code) + if err := cli.RunNoErrOutput(command); err != nil { + // Pretty-print the error and exit with an error. + util.CheckErr(err) + } } diff --git a/staging/src/k8s.io/component-base/cli/run.go b/staging/src/k8s.io/component-base/cli/run.go index 06fc367dd92..c72b303e2dc 100644 --- a/staging/src/k8s.io/component-base/cli/run.go +++ b/staging/src/k8s.io/component-base/cli/run.go @@ -34,7 +34,58 @@ import ( // flags get added to the command line if not added already. Flags get normalized // so that help texts show them with hyphens. Underscores are accepted // as alternative for the command parameters. +// +// Run tries to be smart about how to print errors that are returned by the +// command: before logging is known to be set up, it prints them as plain text +// to stderr. This covers command line flag parse errors and unknown commands. +// Afterwards it logs them. This covers runtime errors. +// +// Commands like kubectl where logging is not normally part of the runtime output +// should use RunNoErrOutput instead and deal with the returned error themselves. func Run(cmd *cobra.Command) int { + if logsInitialized, err := run(cmd); err != nil { + // If the error is about flag parsing, then printing that error + // with the decoration that klog would add ("E0923 + // 23:02:03.219216 4168816 run.go:61] unknown shorthand flag") + // is less readable. Using klog.Fatal is even worse because it + // dumps a stack trace that isn't about the error. + // + // But if it is some other error encountered at runtime, then + // we want to log it as error, at least in most commands because + // their output is a log event stream. + // + // We can distinguish these two cases depending on whether + // we got to logs.InitLogs() above. + // + // This heuristic might be problematic for command line + // tools like kubectl where the output is carefully controlled + // and not a log by default. They should use RunNoErrOutput + // instead. + // + // The usage of klog is problematic also because we don't know + // whether the command has managed to configure it. This cannot + // be checked right now, but may become possible when the early + // logging proposal from + // https://github.com/kubernetes/enhancements/pull/3078 + // ("contextual logging") is implemented. + if !logsInitialized { + fmt.Fprintf(os.Stderr, "Error: %v\n", err) + } else { + klog.ErrorS(err, "command failed") + } + return 1 + } + return 0 +} + +// RunNoErrOutput is a version of Run which returns the cobra command error +// instead of printing it. +func RunNoErrOutput(cmd *cobra.Command) error { + _, err := run(cmd) + return err +} + +func run(cmd *cobra.Command) (logsInitialized bool, err error) { rand.Seed(time.Now().UnixNano()) defer logs.FlushLogs() @@ -70,7 +121,6 @@ func Run(cmd *cobra.Command) int { // Inject logs.InitLogs after command line parsing into one of the // PersistentPre* functions. - logsInitialized := false switch { case cmd.PersistentPreRun != nil: pre := cmd.PersistentPreRun @@ -93,37 +143,6 @@ func Run(cmd *cobra.Command) int { } } - if err := cmd.Execute(); err != nil { - // If the error is about flag parsing, then printing that error - // with the decoration that klog would add ("E0923 - // 23:02:03.219216 4168816 run.go:61] unknown shorthand flag") - // is less readable. Using klog.Fatal is even worse because it - // dumps a stack trace that isn't about the error. - // - // But if it is some other error encountered at runtime, then - // we want to log it as error, at least in most commands because - // their output is a log event stream. - // - // We can distinguish these two cases depending on whether - // we got to logs.InitLogs() above. - // - // This heuristic might be problematic for command line - // tools like kubectl where the output is carefully controlled - // and not a log by default. It works because kubectl has - // its own error handling once a command runs. - // - // The usage of klog is problematic also because we don't know - // whether the command has managed to configure it. This cannot - // be checked right now, but may become possible when the early - // logging proposal from - // https://github.com/kubernetes/enhancements/pull/3078 - // ("contextual logging") is implemented. - if !logsInitialized { - fmt.Fprintf(os.Stderr, "Error: %v\n", err) - } else { - klog.ErrorS(err, "command failed") - } - return 1 - } - return 0 + err = cmd.Execute() + return } diff --git a/test/cmd/get.sh b/test/cmd/get.sh index 36e1472eb24..be24af42c10 100755 --- a/test/cmd/get.sh +++ b/test/cmd/get.sh @@ -421,7 +421,7 @@ run_deprecated_api_tests() { kube::test::if_has_string "${output_message}" 'PodSecurityPolicy is deprecated' output_message=$(! kubectl get podsecuritypolicies.v1beta1.policy --warnings-as-errors 2>&1 "${kube_flags[@]}") kube::test::if_has_string "${output_message}" 'PodSecurityPolicy is deprecated' - kube::test::if_has_string "${output_message}" 'err="1 warning received"' + kube::test::if_has_string "${output_message}" 'error: 1 warning received' set +o nounset set +o errexit