github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-19 09:52:49 +00:00
Code Issues Projects Releases Wiki Activity

cli: let kubectl handle error printing

Browse Source 
cli.Run was an attempt to elliminate error handling in Kubernetes
commands. However, it had to rely on heuristics that are not necessarily right
for all commands.

kubectl is one example which has its own error printing code that should be
used in all cases after a command failure. It now gets used also for
`--warnings-as-errors`. Previously, that caused the following message to be
logged at the end:

  E0110 16:56:01.987555  202060 run.go:120] "command failed" err="1 warning received"

Now it ends with:

 error: 1 warning received
This commit is contained in:
Patrick Ohly 2021-12-15 10:44:39 +01:00
parent 8fc7a9bce9
commit a5d2d6fec3
3 changed files with 59 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
cmd/kubectl/kubectl.go
View File

@ -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)
}
}

87
staging/src/k8s.io/component-base/cli/run.go
View File

@ -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
}

2
test/cmd/get.sh
View File

@ -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