From 94888e653c6f8d0554f251256cd1dd9eafea4368 Mon Sep 17 00:00:00 2001 From: Mike Dame Date: Fri, 10 Dec 2021 12:00:11 -0500 Subject: [PATCH] Move klog initialization to cli.Run() --- cmd/descheduler/app/server.go | 4 - cmd/descheduler/descheduler.go | 13 +-- vendor/k8s.io/component-base/cli/OWNERS | 12 +++ vendor/k8s.io/component-base/cli/run.go | 125 ++++++++++++++++++++++++ vendor/modules.txt | 1 + 5 files changed, 142 insertions(+), 13 deletions(-) create mode 100644 vendor/k8s.io/component-base/cli/OWNERS create mode 100644 vendor/k8s.io/component-base/cli/run.go diff --git a/cmd/descheduler/app/server.go b/cmd/descheduler/app/server.go index c34dc2483..0975e3ec1 100644 --- a/cmd/descheduler/app/server.go +++ b/cmd/descheduler/app/server.go @@ -19,7 +19,6 @@ package app import ( "context" - "flag" "io" "sigs.k8s.io/descheduler/cmd/descheduler/app/options" @@ -30,7 +29,6 @@ import ( apiserver "k8s.io/apiserver/pkg/server" "k8s.io/apiserver/pkg/server/mux" restclient "k8s.io/client-go/rest" - aflag "k8s.io/component-base/cli/flag" "k8s.io/component-base/metrics/legacyregistry" "k8s.io/klog/v2" ) @@ -82,8 +80,6 @@ func NewDeschedulerCommand(out io.Writer) *cobra.Command { } cmd.SetOut(out) flags := cmd.Flags() - flags.SetNormalizeFunc(aflag.WordSepNormalizeFunc) - flags.AddGoFlagSet(flag.CommandLine) s.AddFlags(flags) return cmd } diff --git a/cmd/descheduler/descheduler.go b/cmd/descheduler/descheduler.go index 3f3d5c0f0..24068d905 100644 --- a/cmd/descheduler/descheduler.go +++ b/cmd/descheduler/descheduler.go @@ -17,9 +17,9 @@ limitations under the License. package main import ( - "fmt" - "k8s.io/component-base/logs" "os" + + "k8s.io/component-base/cli" "sigs.k8s.io/descheduler/cmd/descheduler/app" ) @@ -28,11 +28,6 @@ func main() { cmd := app.NewDeschedulerCommand(out) cmd.AddCommand(app.NewVersionCommand()) - logs.InitLogs() - defer logs.FlushLogs() - - if err := cmd.Execute(); err != nil { - fmt.Println(err) - os.Exit(1) - } + code := cli.Run(cmd) + os.Exit(code) } diff --git a/vendor/k8s.io/component-base/cli/OWNERS b/vendor/k8s.io/component-base/cli/OWNERS new file mode 100644 index 000000000..22bdeb45f --- /dev/null +++ b/vendor/k8s.io/component-base/cli/OWNERS @@ -0,0 +1,12 @@ +# See the OWNERS docs at https://go.k8s.io/owners + +# Currently assigned cli to sig-cli since: +# (a) its literally named "cli" +# (b) flags are the bread-and-butter of cli tools. + +approvers: +- sig-cli-maintainers +reviewers: +- sig-cli +labels: +- sig/cli diff --git a/vendor/k8s.io/component-base/cli/run.go b/vendor/k8s.io/component-base/cli/run.go new file mode 100644 index 000000000..cc2c977e3 --- /dev/null +++ b/vendor/k8s.io/component-base/cli/run.go @@ -0,0 +1,125 @@ +/* +Copyright 2021 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package cli + +import ( + "fmt" + "math/rand" + "os" + "time" + + "github.com/spf13/cobra" + + cliflag "k8s.io/component-base/cli/flag" + "k8s.io/component-base/logs" + "k8s.io/klog/v2" +) + +// Run provides the common boilerplate code around executing a cobra command. +// For example, it ensures that logging is set up properly. Logging +// 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. +func Run(cmd *cobra.Command) int { + rand.Seed(time.Now().UnixNano()) + defer logs.FlushLogs() + + cmd.SetGlobalNormalizationFunc(cliflag.WordSepNormalizeFunc) + + // When error printing is enabled for the Cobra command, a flag parse + // error gets printed first, then optionally the often long usage + // text. This is very unreadable in a console because the last few + // lines that will be visible on screen don't include the error. + // + // The recommendation from #sig-cli was to print the usage text, then + // the error. We implement this consistently for all commands here. + // However, we don't want to print the usage text when command + // execution fails for other reasons than parsing. We detect this via + // the FlagParseError callback. + // + // A variable is used instead of wrapping the error with a special + // error type because the variable is simpler and less fragile: the + // original FlagErrorFunc might replace the wrapped error. + parsingFailed := false + if cmd.SilenceUsage { + // Some commands, like kubectl, already deal with this themselves. + // We don't change the behavior for those and just track whether + // parsing failed for the error output below. + flagErrorFunc := cmd.FlagErrorFunc() + cmd.SetFlagErrorFunc(func(c *cobra.Command, err error) error { + parsingFailed = true + return flagErrorFunc(c, err) + }) + } else { + cmd.SilenceUsage = true + cmd.SetFlagErrorFunc(func(c *cobra.Command, err error) error { + parsingFailed = true + + // Re-enable usage printing. + c.SilenceUsage = false + return err + }) + } + + // In all cases error printing is done below. + cmd.SilenceErrors = true + + // This is idempotent. + logs.AddFlags(cmd.PersistentFlags()) + + // Inject logs.InitLogs after command line parsing into one of the + // PersistentPre* functions. + switch { + case cmd.PersistentPreRun != nil: + pre := cmd.PersistentPreRun + cmd.PersistentPreRun = func(cmd *cobra.Command, args []string) { + logs.InitLogs() + pre(cmd, args) + } + case cmd.PersistentPreRunE != nil: + pre := cmd.PersistentPreRunE + cmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error { + logs.InitLogs() + return pre(cmd, args) + } + default: + cmd.PersistentPreRun = func(cmd *cobra.Command, args []string) { + logs.InitLogs() + } + } + + 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. + // + // We can distinguish these two cases depending on whether + // our FlagErrorFunc above was called. + if parsingFailed { + fmt.Fprintf(os.Stderr, "Error: %v\n", err) + } else { + klog.ErrorS(err, "command failed") + } + return 1 + } + return 0 +} diff --git a/vendor/modules.txt b/vendor/modules.txt index 26f8c6bdd..378e589ed 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -1049,6 +1049,7 @@ k8s.io/code-generator/pkg/util k8s.io/code-generator/third_party/forked/golang/reflect # k8s.io/component-base v0.23.0 ## explicit; go 1.16 +k8s.io/component-base/cli k8s.io/component-base/cli/flag k8s.io/component-base/config k8s.io/component-base/config/v1alpha1