Move klog initialization to cli.Run()

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
 }

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

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

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

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