From c9c4357c0339d815db37280eec21f5130e3d4a5a Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Thu, 23 Sep 2021 13:37:53 +0200 Subject: [PATCH 1/4] cli: centralize command execution Many commands have the same boilerplate code for setting up randomization and logging. The new k8s.io/component-base/cli.Run method contains that common code and simplifies the code of commands which are using it. --- staging/src/k8s.io/component-base/cli/run.go | 74 ++++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 staging/src/k8s.io/component-base/cli/run.go diff --git a/staging/src/k8s.io/component-base/cli/run.go b/staging/src/k8s.io/component-base/cli/run.go new file mode 100644 index 00000000000..0d430e04ddc --- /dev/null +++ b/staging/src/k8s.io/component-base/cli/run.go @@ -0,0 +1,74 @@ +/* +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" + + "k8s.io/component-base/logs" +) + +// 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. +func Run(cmd *cobra.Command) int { + rand.Seed(time.Now().UnixNano()) + defer logs.FlushLogs() + + // 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 { + // The error may be about the command line ("unknown shorthand + // flag: 'e' in -elp" in kube-scheduler). Printing that without + // any decoration like the one that klog would add ("E0923 + // 23:02:03.219216 4168816 run.go:61] unknown shorthand flag") + // is more readable. + // + // We also don't know in which state logging (configuration may + // have failed) is whereas os.Stderr always should be usable. + fmt.Fprintf(os.Stderr, "%v\n", err) + return 1 + } + return 0 +} From 21d1bcd6b8498370832fa76f680d3de56837bc83 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Thu, 16 Sep 2021 18:18:35 +0200 Subject: [PATCH 2/4] initialize logging after flag parsing It wasn't documented that InitLogs already uses the log flush frequency, so some commands have called it before parsing (for example, kubectl in the original code for logs.go). The flag never had an effect in such commands. Fixing this turned into a major refactoring of how commands set up flags and run their Cobra command: - component-base/logs: implicitely registering flags during package init is an anti-pattern that makes it impossible to use the package in commands which want full control over their command line. Logging flags must be added explicitly now, something that the new cli.Run does automatically. - component-base/logs: AddFlags would have crashed in kubectl-convert if it had been called because it relied on the global pflag.CommandLine. This has been fixed and kubectl-convert now has the same --log-flush-frequency flag as other commands. - component-base/logs/testinit: an exception are tests where flag.CommandLine has to be used. This new package can be imported to add flags to that once per test program. - Normalization of the klog command line flags was inconsistent. Some commands unintentionally didn't normalize to the recommended format with hyphens. This gets fixed for sample programs, but not for production programs because it would be a breaking change. This refactoring has the following user-visible effects: - The validation error for `go run ./cmd/kube-apiserver --logging-format=json --add-dir-header` now references `add-dir-header` instead of `add_dir_header`. - `staging/src/k8s.io/cloud-provider/sample` uses flags with hyphen instead of underscore. - `--log-flush-frequency` is not listed anymore in the --logging-format flag's `non-default formats don't honor these flags` usage text because it will also work for non-default formats once it is needed. - `cmd/kubelet`: the description of `--logging-format` uses hyphens instead of underscores for the flags, which now matches what the command is using. - `staging/src/k8s.io/component-base/logs/example/cmd`: added logging flags. - `apiextensions-apiserver` no longer prints a useless stack trace for `main` when command line parsing raises an error. --- cmd/cloud-controller-manager/main.go | 15 +---- cmd/kube-apiserver/apiserver.go | 15 +---- .../app/options/globalflags_test.go | 2 + .../controller-manager.go | 15 +---- cmd/kube-proxy/app/server.go | 6 +- cmd/kube-proxy/proxy.go | 22 ++----- cmd/kube-scheduler/app/server.go | 3 + cmd/kube-scheduler/scheduler.go | 31 +--------- cmd/kubectl-convert/kubectl-convert.go | 21 ++----- cmd/kubectl/kubectl.go | 22 ++----- cmd/kubelet/app/options/globalflags.go | 9 --- cmd/kubelet/app/options/options.go | 1 + cmd/kubelet/app/server.go | 3 + cmd/kubelet/kubelet.go | 23 +++++-- cmd/kubemark/hollow-node.go | 26 +++----- pkg/kubectl/cmd/convert/convert.go | 2 + staging/publishing/import-restrictions.yaml | 2 +- .../k8s.io/apiextensions-apiserver/main.go | 14 +---- .../integration/conversion/conversion_test.go | 1 + .../test/integration/helpers.go | 1 + .../cloud-provider/app/controllermanager.go | 3 + .../cloud-provider/sample/basic_main.go | 21 ++----- .../cli/globalflag/globalflags.go | 14 ----- .../cli/globalflag/globalflags_test.go | 2 + .../src/k8s.io/component-base/logs/config.go | 60 +++++++++++------- .../component-base/logs/example/cmd/logger.go | 18 ++++-- .../src/k8s.io/component-base/logs/logs.go | 51 ++++++++++++--- .../component-base/logs/options_test.go | 2 +- .../component-base/logs/testinit/testinit.go | 32 ++++++++++ .../k8s.io/component-base/logs/validate.go | 39 +++--------- staging/src/k8s.io/kube-aggregator/main.go | 14 +---- staging/src/k8s.io/kubectl/pkg/cmd/cmd.go | 19 +++--- .../src/k8s.io/kubectl/pkg/util/logs/logs.go | 62 ------------------- .../cmd/webhook/server/server.go | 2 + .../cmd/webhook/webhook.go | 26 ++------ staging/src/k8s.io/sample-apiserver/go.mod | 2 +- staging/src/k8s.io/sample-apiserver/main.go | 15 ++--- test/e2e/e2e.go | 3 + test/e2e_node/e2e_node_suite_test.go | 2 + test/images/agnhost/agnhost.go | 10 ++- test/integration/framework/flags.go | 22 +++++++ vendor/modules.txt | 3 +- 42 files changed, 270 insertions(+), 386 deletions(-) create mode 100644 staging/src/k8s.io/component-base/logs/testinit/testinit.go delete mode 100644 staging/src/k8s.io/kubectl/pkg/util/logs/logs.go create mode 100644 test/integration/framework/flags.go diff --git a/cmd/cloud-controller-manager/main.go b/cmd/cloud-controller-manager/main.go index 7c452b74800..5eb38d73605 100644 --- a/cmd/cloud-controller-manager/main.go +++ b/cmd/cloud-controller-manager/main.go @@ -25,9 +25,7 @@ limitations under the License. package main import ( - "math/rand" "os" - "time" "github.com/spf13/pflag" @@ -36,8 +34,8 @@ import ( "k8s.io/cloud-provider/app" cloudcontrollerconfig "k8s.io/cloud-provider/app/config" "k8s.io/cloud-provider/options" + "k8s.io/component-base/cli" cliflag "k8s.io/component-base/cli/flag" - "k8s.io/component-base/logs" _ "k8s.io/component-base/metrics/prometheus/clientgo" // load all the prometheus client-go plugins _ "k8s.io/component-base/metrics/prometheus/version" // for version metric registration "k8s.io/klog/v2" @@ -46,8 +44,6 @@ import ( ) func main() { - rand.Seed(time.Now().UnixNano()) - pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) ccmOptions, err := options.NewCloudControllerManagerOptions() @@ -79,13 +75,8 @@ func main() { } command := app.NewCloudControllerManagerCommand(ccmOptions, cloudInitializer, controllerInitializers, fss, wait.NeverStop) - - logs.InitLogs() - defer logs.FlushLogs() - - if err := command.Execute(); err != nil { - os.Exit(1) - } + code := cli.Run(command) + os.Exit(code) } func cloudInitializer(config *cloudcontrollerconfig.CompletedConfig) cloudprovider.Interface { diff --git a/cmd/kube-apiserver/apiserver.go b/cmd/kube-apiserver/apiserver.go index 1ccff72a18b..fd61264a737 100644 --- a/cmd/kube-apiserver/apiserver.go +++ b/cmd/kube-apiserver/apiserver.go @@ -19,14 +19,12 @@ limitations under the License. package main import ( - "math/rand" "os" - "time" "github.com/spf13/pflag" + "k8s.io/component-base/cli" cliflag "k8s.io/component-base/cli/flag" - "k8s.io/component-base/logs" _ "k8s.io/component-base/logs/json/register" // for JSON log format registration _ "k8s.io/component-base/metrics/prometheus/clientgo" // load all the prometheus client-go plugins _ "k8s.io/component-base/metrics/prometheus/version" // for version metric registration @@ -34,16 +32,9 @@ import ( ) func main() { - rand.Seed(time.Now().UnixNano()) - pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) command := app.NewAPIServerCommand() - - logs.InitLogs() - defer logs.FlushLogs() - - if err := command.Execute(); err != nil { - os.Exit(1) - } + code := cli.Run(command) + os.Exit(code) } diff --git a/cmd/kube-apiserver/app/options/globalflags_test.go b/cmd/kube-apiserver/app/options/globalflags_test.go index f4f6dac6c3e..20d21c0f411 100644 --- a/cmd/kube-apiserver/app/options/globalflags_test.go +++ b/cmd/kube-apiserver/app/options/globalflags_test.go @@ -27,6 +27,7 @@ import ( cliflag "k8s.io/component-base/cli/flag" "k8s.io/component-base/cli/globalflag" + "k8s.io/component-base/logs" ) func TestAddCustomGlobalFlags(t *testing.T) { @@ -48,6 +49,7 @@ func TestAddCustomGlobalFlags(t *testing.T) { // Get all flags from flags.CommandLine, except flag `test.*`. wantedFlag := []string{"help"} pflag.CommandLine.AddGoFlagSet(flag.CommandLine) + logs.AddFlags(pflag.CommandLine) normalizeFunc := nfs.GetNormalizeFunc() pflag.VisitAll(func(flag *pflag.Flag) { if !strings.Contains(flag.Name, "test.") { diff --git a/cmd/kube-controller-manager/controller-manager.go b/cmd/kube-controller-manager/controller-manager.go index 39d1363deb6..df6a2c7fa09 100644 --- a/cmd/kube-controller-manager/controller-manager.go +++ b/cmd/kube-controller-manager/controller-manager.go @@ -21,14 +21,12 @@ limitations under the License. package main import ( - "math/rand" "os" - "time" "github.com/spf13/pflag" + "k8s.io/component-base/cli" cliflag "k8s.io/component-base/cli/flag" - "k8s.io/component-base/logs" _ "k8s.io/component-base/logs/json/register" // for JSON log format registration _ "k8s.io/component-base/metrics/prometheus/clientgo" // load all the prometheus client-go plugin _ "k8s.io/component-base/metrics/prometheus/version" // for version metric registration @@ -36,16 +34,9 @@ import ( ) func main() { - rand.Seed(time.Now().UnixNano()) - pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) command := app.NewControllerManagerCommand() - - logs.InitLogs() - defer logs.FlushLogs() - - if err := command.Execute(); err != nil { - os.Exit(1) - } + code := cli.Run(command) + os.Exit(code) } diff --git a/cmd/kube-proxy/app/server.go b/cmd/kube-proxy/app/server.go index f399c5a1b73..d01dc81a28a 100644 --- a/cmd/kube-proxy/app/server.go +++ b/cmd/kube-proxy/app/server.go @@ -20,6 +20,7 @@ package app import ( "errors" + goflag "flag" "fmt" "io/ioutil" "net" @@ -509,6 +510,7 @@ with the apiserver API to configure the proxy.`, return nil }, } + cmd.SetGlobalNormalizationFunc(pflag.CommandLine.GetNormalizeFunc()) var err error opts.config, err = opts.ApplyDefaults(opts.config) @@ -518,7 +520,9 @@ with the apiserver API to configure the proxy.`, os.Exit(1) } - opts.AddFlags(cmd.Flags()) + fs := cmd.Flags() + opts.AddFlags(fs) + fs.AddGoFlagSet(goflag.CommandLine) // for --boot-id-file and --machine-id-file // TODO handle error cmd.MarkFlagFilename("config", "yaml", "yml", "json") diff --git a/cmd/kube-proxy/proxy.go b/cmd/kube-proxy/proxy.go index 8e3002c2cc0..d668c8da42d 100644 --- a/cmd/kube-proxy/proxy.go +++ b/cmd/kube-proxy/proxy.go @@ -17,35 +17,21 @@ limitations under the License. package main import ( - goflag "flag" - "math/rand" "os" - "time" "github.com/spf13/pflag" + "k8s.io/component-base/cli" cliflag "k8s.io/component-base/cli/flag" - "k8s.io/component-base/logs" _ "k8s.io/component-base/metrics/prometheus/restclient" // for client metric registration _ "k8s.io/component-base/metrics/prometheus/version" // for version metric registration "k8s.io/kubernetes/cmd/kube-proxy/app" ) func main() { - rand.Seed(time.Now().UnixNano()) + pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) command := app.NewProxyCommand() - - // TODO: once we switch everything over to Cobra commands, we can go back to calling - // utilflag.InitFlags() (by removing its pflag.Parse() call). For now, we have to set the - // normalize func and add the go flag set by hand. - pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) - pflag.CommandLine.AddGoFlagSet(goflag.CommandLine) - // utilflag.InitFlags() - logs.InitLogs() - defer logs.FlushLogs() - - if err := command.Execute(); err != nil { - os.Exit(1) - } + code := cli.Run(command) + os.Exit(code) } diff --git a/cmd/kube-scheduler/app/server.go b/cmd/kube-scheduler/app/server.go index ee5b5a462e1..0be5f9c5146 100644 --- a/cmd/kube-scheduler/app/server.go +++ b/cmd/kube-scheduler/app/server.go @@ -25,6 +25,7 @@ import ( goruntime "runtime" "github.com/spf13/cobra" + "github.com/spf13/pflag" utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apiserver/pkg/authentication/authenticator" @@ -99,6 +100,8 @@ for more information about scheduling and the kube-scheduler component.`, return nil }, } + cmd.SetGlobalNormalizationFunc(pflag.CommandLine.GetNormalizeFunc()) + fs := cmd.Flags() verflag.AddFlags(namedFlagSets.FlagSet("global")) globalflag.AddGlobalFlags(namedFlagSets.FlagSet("global"), cmd.Name()) diff --git a/cmd/kube-scheduler/scheduler.go b/cmd/kube-scheduler/scheduler.go index b6ef3a83b9f..912e484bc73 100644 --- a/cmd/kube-scheduler/scheduler.go +++ b/cmd/kube-scheduler/scheduler.go @@ -17,15 +17,12 @@ limitations under the License. package main import ( - "fmt" - "math/rand" "os" - "time" "github.com/spf13/pflag" + "k8s.io/component-base/cli" cliflag "k8s.io/component-base/cli/flag" - "k8s.io/component-base/logs" _ "k8s.io/component-base/logs/json/register" // for JSON log format registration _ "k8s.io/component-base/metrics/prometheus/clientgo" _ "k8s.io/component-base/metrics/prometheus/version" // for version metric registration @@ -33,31 +30,9 @@ import ( ) func main() { - if err := runSchedulerCmd(); err != nil { - fmt.Fprintf(os.Stderr, "%v\n", err) - os.Exit(1) - } -} - -func runSchedulerCmd() error { - rand.Seed(time.Now().UnixNano()) - pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) command := app.NewSchedulerCommand() - - logs.InitLogs() - defer logs.FlushLogs() - - err := command.ParseFlags(os.Args[1:]) - if err != nil { - // when fail to parse flags, return error with the usage message. - return fmt.Errorf("%v\n%s", err, command.UsageString()) - } - - if err := command.Execute(); err != nil { - return err - } - - return nil + code := cli.Run(command) + os.Exit(code) } diff --git a/cmd/kubectl-convert/kubectl-convert.go b/cmd/kubectl-convert/kubectl-convert.go index d030ae8bf3a..7b7610f4be4 100644 --- a/cmd/kubectl-convert/kubectl-convert.go +++ b/cmd/kubectl-convert/kubectl-convert.go @@ -17,20 +17,21 @@ limitations under the License. package main import ( - goflag "flag" "os" "github.com/spf13/pflag" "k8s.io/cli-runtime/pkg/genericclioptions" + "k8s.io/component-base/cli" cliflag "k8s.io/component-base/cli/flag" cmdutil "k8s.io/kubectl/pkg/cmd/util" - "k8s.io/kubectl/pkg/util/logs" "k8s.io/kubernetes/pkg/kubectl/cmd/convert" ) func main() { + pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) flags := pflag.NewFlagSet("kubectl-convert", pflag.ExitOnError) + flags.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) pflag.CommandLine = flags kubeConfigFlags := genericclioptions.NewConfigFlags(true).WithDeprecatedPasswordFlag() @@ -38,20 +39,8 @@ func main() { matchVersionKubeConfigFlags := cmdutil.NewMatchVersionFlags(kubeConfigFlags) f := cmdutil.NewFactory(matchVersionKubeConfigFlags) - - // TODO: once we switch everything over to Cobra commands, we can go back to calling - // cliflag.InitFlags() (by removing its pflag.Parse() call). For now, we have to set the - // normalize func and add the go flag set by hand. - pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) - pflag.CommandLine.AddGoFlagSet(goflag.CommandLine) - // cliflag.InitFlags() - logs.InitLogs() - defer logs.FlushLogs() - cmd := convert.NewCmdConvert(f, genericclioptions.IOStreams{In: os.Stdin, Out: os.Stdout, ErrOut: os.Stderr}) matchVersionKubeConfigFlags.AddFlags(cmd.PersistentFlags()) - if err := cmd.Execute(); err != nil { - os.Exit(1) - } - + code := cli.Run(cmd) + os.Exit(code) } diff --git a/cmd/kubectl/kubectl.go b/cmd/kubectl/kubectl.go index a5e24493a41..67576646e5f 100644 --- a/cmd/kubectl/kubectl.go +++ b/cmd/kubectl/kubectl.go @@ -17,36 +17,22 @@ limitations under the License. package main import ( - goflag "flag" - "math/rand" "os" - "time" "github.com/spf13/pflag" + "k8s.io/component-base/cli" cliflag "k8s.io/component-base/cli/flag" "k8s.io/kubectl/pkg/cmd" - "k8s.io/kubectl/pkg/util/logs" // Import to initialize client auth plugins. _ "k8s.io/client-go/plugin/pkg/client/auth" ) func main() { - rand.Seed(time.Now().UnixNano()) + pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) command := cmd.NewDefaultKubectlCommand() - - // TODO: once we switch everything over to Cobra commands, we can go back to calling - // cliflag.InitFlags() (by removing its pflag.Parse() call). For now, we have to set the - // normalize func and add the go flag set by hand. - pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) - pflag.CommandLine.AddGoFlagSet(goflag.CommandLine) - // cliflag.InitFlags() - logs.InitLogs() - defer logs.FlushLogs() - - if err := command.Execute(); err != nil { - os.Exit(1) - } + code := cli.Run(command) + os.Exit(code) } diff --git a/cmd/kubelet/app/options/globalflags.go b/cmd/kubelet/app/options/globalflags.go index fdc20d18177..2b0cc506191 100644 --- a/cmd/kubelet/app/options/globalflags.go +++ b/cmd/kubelet/app/options/globalflags.go @@ -27,7 +27,6 @@ import ( // libs that provide registration functions "k8s.io/component-base/logs" "k8s.io/component-base/version/verflag" - "k8s.io/klog/v2" // ensure libs have a chance to globally register their flags _ "k8s.io/kubernetes/pkg/credentialprovider/azure" @@ -38,7 +37,6 @@ import ( // against the global flagsets from "flag" and "github.com/spf13/pflag". // We do this in order to prevent unwanted flags from leaking into the Kubelet's flagset. func AddGlobalFlags(fs *pflag.FlagSet) { - addKlogFlags(fs) addCadvisorFlags(fs) addCredentialProviderFlags(fs) verflag.AddFlags(fs) @@ -88,10 +86,3 @@ func addCredentialProviderFlags(fs *pflag.FlagSet) { fs.AddFlagSet(local) } - -// addKlogFlags adds flags from k8s.io/klog -func addKlogFlags(fs *pflag.FlagSet) { - local := flag.NewFlagSet(os.Args[0], flag.ExitOnError) - klog.InitFlags(local) - fs.AddGoFlagSet(local) -} diff --git a/cmd/kubelet/app/options/options.go b/cmd/kubelet/app/options/options.go index 527057d35d4..0343cd6d820 100644 --- a/cmd/kubelet/app/options/options.go +++ b/cmd/kubelet/app/options/options.go @@ -383,6 +383,7 @@ func (f *KubeletFlags) AddFlags(mainfs *pflag.FlagSet) { // AddKubeletConfigFlags adds flags for a specific kubeletconfig.KubeletConfiguration to the specified FlagSet func AddKubeletConfigFlags(mainfs *pflag.FlagSet, c *kubeletconfig.KubeletConfiguration) { fs := pflag.NewFlagSet("", pflag.ExitOnError) + fs.SetNormalizeFunc(mainfs.GetNormalizeFunc()) defer func() { // All KubeletConfiguration flags are now deprecated, and any new flags that point to // KubeletConfiguration fields are deprecated-on-creation. When removing flags at the end diff --git a/cmd/kubelet/app/server.go b/cmd/kubelet/app/server.go index f831a434fbe..0b8d8e67fce 100644 --- a/cmd/kubelet/app/server.go +++ b/cmd/kubelet/app/server.go @@ -259,6 +259,9 @@ HTTP server: The kubelet can also listen for HTTP and respond to a simple API } } + // Config and flags parsed, now we can initialize logging. + logs.InitLogs() + // construct a KubeletServer from kubeletFlags and kubeletConfig kubeletServer := &options.KubeletServer{ KubeletFlags: *kubeletFlags, diff --git a/cmd/kubelet/kubelet.go b/cmd/kubelet/kubelet.go index 16b6dc9dc4e..0a6cfd17aa4 100644 --- a/cmd/kubelet/kubelet.go +++ b/cmd/kubelet/kubelet.go @@ -26,6 +26,9 @@ import ( "os" "time" + "github.com/spf13/cobra" + + cliflag "k8s.io/component-base/cli/flag" "k8s.io/component-base/logs" _ "k8s.io/component-base/logs/json/register" // for JSON log format registration _ "k8s.io/component-base/metrics/prometheus/restclient" @@ -34,13 +37,23 @@ import ( ) func main() { + command := app.NewKubeletCommand() + + // kubelet uses a config file and does its own special + // parsing of flags and that config file. It initializes + // logging after it is done with that. Therefore it does + // not use cli.Run like other, simpler commands. + code := run(command) + os.Exit(code) +} + +func run(command *cobra.Command) int { + defer logs.FlushLogs() rand.Seed(time.Now().UnixNano()) - command := app.NewKubeletCommand() - logs.InitLogs() - defer logs.FlushLogs() - + command.SetGlobalNormalizationFunc(cliflag.WordSepNormalizeFunc) if err := command.Execute(); err != nil { - os.Exit(1) + return 1 } + return 0 } diff --git a/cmd/kubemark/hollow-node.go b/cmd/kubemark/hollow-node.go index e0a1e6974a1..df302d8ed3a 100644 --- a/cmd/kubemark/hollow-node.go +++ b/cmd/kubemark/hollow-node.go @@ -20,7 +20,6 @@ import ( "errors" goflag "flag" "fmt" - "math/rand" "os" "time" @@ -36,8 +35,8 @@ import ( restclient "k8s.io/client-go/rest" "k8s.io/client-go/tools/clientcmd" "k8s.io/client-go/tools/events" + "k8s.io/component-base/cli" cliflag "k8s.io/component-base/cli/flag" - "k8s.io/component-base/logs" _ "k8s.io/component-base/metrics/prometheus/restclient" // for client metric registration _ "k8s.io/component-base/metrics/prometheus/version" // for version metric registration "k8s.io/component-base/version" @@ -131,22 +130,11 @@ func (c *hollowNodeConfig) createHollowKubeletOptions() *kubemark.HollowKubletOp } func main() { - rand.Seed(time.Now().UnixNano()) + pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) command := newHollowNodeCommand() - - // TODO: once we switch everything over to Cobra commands, we can go back to calling - // cliflag.InitFlags() (by removing its pflag.Parse() call). For now, we have to set the - // normalize func and add the go flag set by hand. - pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) - pflag.CommandLine.AddGoFlagSet(goflag.CommandLine) - // cliflag.InitFlags() - logs.InitLogs() - defer logs.FlushLogs() - - if err := command.Execute(); err != nil { - os.Exit(1) - } + code := cli.Run(command) + os.Exit(code) } // newControllerManagerCommand creates a *cobra.Command object with default parameters @@ -172,7 +160,11 @@ func newHollowNodeCommand() *cobra.Command { return nil }, } - s.addFlags(cmd.Flags()) + cmd.SetGlobalNormalizationFunc(pflag.CommandLine.GetNormalizeFunc()) + + fs := cmd.Flags() + fs.AddGoFlagSet(goflag.CommandLine) // for flags like --docker-only + s.addFlags(fs) return cmd } diff --git a/pkg/kubectl/cmd/convert/convert.go b/pkg/kubectl/cmd/convert/convert.go index 74b16ccbbe4..3557704e27b 100644 --- a/pkg/kubectl/cmd/convert/convert.go +++ b/pkg/kubectl/cmd/convert/convert.go @@ -20,6 +20,7 @@ import ( "fmt" "github.com/spf13/cobra" + "github.com/spf13/pflag" "k8s.io/klog/v2" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -100,6 +101,7 @@ func NewCmdConvert(f cmdutil.Factory, ioStreams genericclioptions.IOStreams) *co cmdutil.CheckErr(o.RunConvert()) }, } + cmd.SetGlobalNormalizationFunc(pflag.CommandLine.GetNormalizeFunc()) cmd.Flags().BoolVar(&o.local, "local", o.local, "If true, convert will NOT try to contact api-server but run locally.") cmd.Flags().StringVar(&o.OutputVersion, "output-version", o.OutputVersion, i18n.T("Output the formatted object with the given group version (for ex: 'extensions/v1beta1').")) diff --git a/staging/publishing/import-restrictions.yaml b/staging/publishing/import-restrictions.yaml index ee437f47c51..6710e73662a 100644 --- a/staging/publishing/import-restrictions.yaml +++ b/staging/publishing/import-restrictions.yaml @@ -279,5 +279,5 @@ - k8s.io/pod-security-admission - k8s.io/component-base/featuregate - k8s.io/component-base/logs - - k8s.io/component-base/cli/flag + - k8s.io/component-base/cli - k8s.io/utils diff --git a/staging/src/k8s.io/apiextensions-apiserver/main.go b/staging/src/k8s.io/apiextensions-apiserver/main.go index a4784e14ff4..ce08f04fab9 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/main.go +++ b/staging/src/k8s.io/apiextensions-apiserver/main.go @@ -17,24 +17,16 @@ limitations under the License. package main import ( - "flag" "os" - "k8s.io/klog/v2" - "k8s.io/apiextensions-apiserver/pkg/cmd/server" genericapiserver "k8s.io/apiserver/pkg/server" - "k8s.io/component-base/logs" + "k8s.io/component-base/cli" ) func main() { - logs.InitLogs() - defer logs.FlushLogs() - stopCh := genericapiserver.SetupSignalHandler() cmd := server.NewServerCommand(os.Stdout, os.Stderr, stopCh) - cmd.Flags().AddGoFlagSet(flag.CommandLine) - if err := cmd.Execute(); err != nil { - klog.Fatal(err) - } + code := cli.Run(cmd) + os.Exit(code) } diff --git a/staging/src/k8s.io/apiextensions-apiserver/test/integration/conversion/conversion_test.go b/staging/src/k8s.io/apiextensions-apiserver/test/integration/conversion/conversion_test.go index 23d1c6bd3ee..43a0a3dc451 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/test/integration/conversion/conversion_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/test/integration/conversion/conversion_test.go @@ -40,6 +40,7 @@ import ( "k8s.io/apimachinery/pkg/util/wait" etcd3watcher "k8s.io/apiserver/pkg/storage/etcd3" "k8s.io/client-go/dynamic" + _ "k8s.io/component-base/logs/testinit" // enable logging flags apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" diff --git a/staging/src/k8s.io/apiextensions-apiserver/test/integration/helpers.go b/staging/src/k8s.io/apiextensions-apiserver/test/integration/helpers.go index 59134f7768c..811ff9a9369 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/test/integration/helpers.go +++ b/staging/src/k8s.io/apiextensions-apiserver/test/integration/helpers.go @@ -29,6 +29,7 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/client-go/dynamic" + _ "k8s.io/component-base/logs/testinit" // enable logging flags ) var swaggerMetadataDescriptions = metav1.ObjectMeta{}.SwaggerDoc() diff --git a/staging/src/k8s.io/cloud-provider/app/controllermanager.go b/staging/src/k8s.io/cloud-provider/app/controllermanager.go index 820fd4f32a7..628e3a97cef 100644 --- a/staging/src/k8s.io/cloud-provider/app/controllermanager.go +++ b/staging/src/k8s.io/cloud-provider/app/controllermanager.go @@ -25,6 +25,8 @@ import ( "time" "github.com/spf13/cobra" + "github.com/spf13/pflag" + "k8s.io/apimachinery/pkg/runtime/schema" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/sets" @@ -101,6 +103,7 @@ the cloud specific control loops shipped with Kubernetes.`, return nil }, } + cmd.SetGlobalNormalizationFunc(pflag.CommandLine.GetNormalizeFunc()) fs := cmd.Flags() namedFlagSets := s.Flags(ControllerNames(controllerInitFuncConstructors), ControllersDisabledByDefault.List()) diff --git a/staging/src/k8s.io/cloud-provider/sample/basic_main.go b/staging/src/k8s.io/cloud-provider/sample/basic_main.go index c3821156932..4d6f820fefb 100644 --- a/staging/src/k8s.io/cloud-provider/sample/basic_main.go +++ b/staging/src/k8s.io/cloud-provider/sample/basic_main.go @@ -21,9 +21,7 @@ limitations under the License. package main import ( - "math/rand" "os" - "time" "github.com/spf13/pflag" "k8s.io/apimachinery/pkg/util/wait" @@ -31,8 +29,8 @@ import ( "k8s.io/cloud-provider/app" "k8s.io/cloud-provider/app/config" "k8s.io/cloud-provider/options" + "k8s.io/component-base/cli" cliflag "k8s.io/component-base/cli/flag" - "k8s.io/component-base/logs" _ "k8s.io/component-base/metrics/prometheus/clientgo" // load all the prometheus client-go plugins _ "k8s.io/component-base/metrics/prometheus/version" // for version metric registration "k8s.io/klog/v2" @@ -41,7 +39,7 @@ import ( ) func main() { - rand.Seed(time.Now().UnixNano()) + pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) ccmOptions, err := options.NewCloudControllerManagerOptions() if err != nil { @@ -50,19 +48,8 @@ func main() { fss := cliflag.NamedFlagSets{} command := app.NewCloudControllerManagerCommand(ccmOptions, cloudInitializer, controllerInitializers(), fss, wait.NeverStop) - - // TODO: once we switch everything over to Cobra commands, we can go back to calling - // utilflag.InitFlags() (by removing its pflag.Parse() call). For now, we have to set the - // normalize func and add the go flag set by hand. - // Here is an sample - pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) - // utilflag.InitFlags() - logs.InitLogs() - defer logs.FlushLogs() - - if err := command.Execute(); err != nil { - os.Exit(1) - } + code := cli.Run(command) + os.Exit(code) } // If custom ClientNames are used, as below, then the controller will not use diff --git a/staging/src/k8s.io/component-base/cli/globalflag/globalflags.go b/staging/src/k8s.io/component-base/cli/globalflag/globalflags.go index 7a5640be743..5f5b4b560d7 100644 --- a/staging/src/k8s.io/component-base/cli/globalflag/globalflags.go +++ b/staging/src/k8s.io/component-base/cli/globalflag/globalflags.go @@ -19,34 +19,20 @@ package globalflag import ( "flag" "fmt" - "os" "github.com/spf13/pflag" "k8s.io/component-base/logs" - "k8s.io/klog/v2" ) // AddGlobalFlags explicitly registers flags that libraries (klog, verflag, etc.) register // against the global flagsets from "flag" and "k8s.io/klog/v2". // We do this in order to prevent unwanted flags from leaking into the component's flagset. func AddGlobalFlags(fs *pflag.FlagSet, name string) { - addKlogFlags(fs) logs.AddFlags(fs) fs.BoolP("help", "h", false, fmt.Sprintf("help for %s", name)) } -// addKlogFlags adds flags from k8s.io/klog -func addKlogFlags(fs *pflag.FlagSet) { - local := flag.NewFlagSet(os.Args[0], flag.ExitOnError) - klog.InitFlags(local) - normalizeFunc := fs.GetNormalizeFunc() - local.VisitAll(func(fl *flag.Flag) { - fl.Name = string(normalizeFunc(fs, fl.Name)) - fs.AddGoFlag(fl) - }) -} - // Register adds a flag to local that targets the Value associated with the Flag named globalName in flag.CommandLine. func Register(local *pflag.FlagSet, globalName string) { if f := flag.CommandLine.Lookup(globalName); f != nil { diff --git a/staging/src/k8s.io/component-base/cli/globalflag/globalflags_test.go b/staging/src/k8s.io/component-base/cli/globalflag/globalflags_test.go index 27780c76864..0ed462e6720 100644 --- a/staging/src/k8s.io/component-base/cli/globalflag/globalflags_test.go +++ b/staging/src/k8s.io/component-base/cli/globalflag/globalflags_test.go @@ -26,6 +26,7 @@ import ( "github.com/spf13/pflag" cliflag "k8s.io/component-base/cli/flag" + "k8s.io/component-base/logs" ) func TestAddGlobalFlags(t *testing.T) { @@ -42,6 +43,7 @@ func TestAddGlobalFlags(t *testing.T) { // Get all flags from flags.CommandLine, except flag `test.*`. wantedFlag := []string{"help"} pflag.CommandLine.AddGoFlagSet(flag.CommandLine) + logs.AddFlags(pflag.CommandLine) normalizeFunc := nfs.GetNormalizeFunc() pflag.VisitAll(func(flag *pflag.Flag) { if !strings.Contains(flag.Name, "test.") { diff --git a/staging/src/k8s.io/component-base/logs/config.go b/staging/src/k8s.io/component-base/logs/config.go index 8a029fa7cc5..296cfb9ce6b 100644 --- a/staging/src/k8s.io/component-base/logs/config.go +++ b/staging/src/k8s.io/component-base/logs/config.go @@ -19,6 +19,7 @@ package logs import ( "flag" "fmt" + "sort" "strings" "github.com/spf13/pflag" @@ -36,9 +37,17 @@ const ( // LogRegistry is new init LogFormatRegistry struct var LogRegistry = NewLogFormatRegistry() +// loggingFlags captures the state of the logging flags, in particular their default value +// before flag parsing. It is used by UnsupportedLoggingFlags. +var loggingFlags pflag.FlagSet + func init() { // Text format is default klog format LogRegistry.Register(DefaultLogFormat, nil) + + var fs flag.FlagSet + klog.InitFlags(&fs) + loggingFlags.AddGoFlagSet(&fs) } // List of logs (k8s.io/klog + k8s.io/component-base/logs) flags supported by all logging formats @@ -49,11 +58,7 @@ var supportedLogsFlags = map[string]struct{}{ // BindLoggingFlags binds the Options struct fields to a flagset func BindLoggingFlags(c *config.LoggingConfiguration, fs *pflag.FlagSet) { - normalizeFunc := func(name string) string { - f := fs.GetNormalizeFunc() - return string(f(fs, name)) - } - unsupportedFlags := fmt.Sprintf("--%s", strings.Join(UnsupportedLoggingFlags(normalizeFunc), ", --")) + unsupportedFlags := strings.Join(unsupportedLoggingFlagNames(fs.GetNormalizeFunc()), ", ") formats := fmt.Sprintf(`"%s"`, strings.Join(LogRegistry.List(), `", "`)) fs.StringVar(&c.Format, "logging-format", c.Format, fmt.Sprintf("Sets the log format. Permitted formats: %s.\nNon-default formats don't honor these flags: %s.\nNon-default choices are currently alpha and subject to change without warning.", formats, unsupportedFlags)) // No new log formats should be added after generation is of flag options @@ -62,30 +67,37 @@ func BindLoggingFlags(c *config.LoggingConfiguration, fs *pflag.FlagSet) { Runtime log sanitization may introduce significant computation overhead and therefore should not be enabled in production.`) } -// UnsupportedLoggingFlags lists unsupported logging flags -func UnsupportedLoggingFlags(normalizeFunc func(name string) string) []string { - allFlags := []string{} - - // k8s.io/klog flags - fs := &flag.FlagSet{} - klog.InitFlags(fs) - fs.VisitAll(func(flag *flag.Flag) { +// UnsupportedLoggingFlags lists unsupported logging flags. The normalize +// function is optional. +func UnsupportedLoggingFlags(normalizeFunc func(f *pflag.FlagSet, name string) pflag.NormalizedName) []*pflag.Flag { + // k8s.io/component-base/logs and klog flags + pfs := &pflag.FlagSet{} + loggingFlags.VisitAll(func(flag *pflag.Flag) { if _, found := supportedLogsFlags[flag.Name]; !found { - name := flag.Name - if normalizeFunc != nil { - name = normalizeFunc(name) - } - allFlags = append(allFlags, name) + // Normalization changes flag.Name, so make a copy. + clone := *flag + pfs.AddFlag(&clone) } }) - // k8s.io/component-base/logs flags - pfs := &pflag.FlagSet{} - AddFlags(pfs) + // Apply normalization. + pfs.SetNormalizeFunc(normalizeFunc) + + var allFlags []*pflag.Flag pfs.VisitAll(func(flag *pflag.Flag) { - if _, found := supportedLogsFlags[flag.Name]; !found { - allFlags = append(allFlags, flag.Name) - } + allFlags = append(allFlags, flag) }) return allFlags } + +// unsupportedLoggingFlagNames lists unsupported logging flags by name, with +// optional normalization and sorted. +func unsupportedLoggingFlagNames(normalizeFunc func(f *pflag.FlagSet, name string) pflag.NormalizedName) []string { + unsupportedFlags := UnsupportedLoggingFlags(normalizeFunc) + names := make([]string, 0, len(unsupportedFlags)) + for _, f := range unsupportedFlags { + names = append(names, "--"+f.Name) + } + sort.Strings(names) + return names +} diff --git a/staging/src/k8s.io/component-base/logs/example/cmd/logger.go b/staging/src/k8s.io/component-base/logs/example/cmd/logger.go index c098def7437..0e63e565b74 100644 --- a/staging/src/k8s.io/component-base/logs/example/cmd/logger.go +++ b/staging/src/k8s.io/component-base/logs/example/cmd/logger.go @@ -22,7 +22,10 @@ import ( "os" "github.com/spf13/cobra" + "github.com/spf13/pflag" + "k8s.io/component-base/cli" + cliflag "k8s.io/component-base/cli/flag" "k8s.io/component-base/logs" "k8s.io/klog/v2" @@ -30,13 +33,14 @@ import ( ) func main() { - command := NewLoggerCommand() - logs.InitLogs() - defer logs.FlushLogs() + // NamedFlagSets use the global pflag.CommandLine. We don't use those + // in this command (yet), but set it globally anyway for consistency + // with other commands. + pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) - if err := command.Execute(); err != nil { - os.Exit(1) - } + command := NewLoggerCommand() + code := cli.Run(command) + os.Exit(code) } func NewLoggerCommand() *cobra.Command { @@ -52,6 +56,8 @@ func NewLoggerCommand() *cobra.Command { runLogger() }, } + cmd.SetGlobalNormalizationFunc(pflag.CommandLine.GetNormalizeFunc()) + o.AddFlags(cmd.Flags()) return cmd } diff --git a/staging/src/k8s.io/component-base/logs/logs.go b/staging/src/k8s.io/component-base/logs/logs.go index 073e0312fb1..04b1c4f98df 100644 --- a/staging/src/k8s.io/component-base/logs/logs.go +++ b/staging/src/k8s.io/component-base/logs/logs.go @@ -14,6 +14,9 @@ See the License for the specific language governing permissions and limitations under the License. */ +// Package logs contains support for logging options, flags and setup. +// Commands must explicitly enable command line flags. They no longer +// get added automatically when importing this package. package logs import ( @@ -29,16 +32,42 @@ import ( const logFlushFreqFlagName = "log-flush-frequency" -var logFlushFreq = pflag.Duration(logFlushFreqFlagName, 5*time.Second, "Maximum number of seconds between log flushes") +var ( + packageFlags = flag.NewFlagSet("logging", flag.ContinueOnError) + logFlushFreq time.Duration +) func init() { - klog.InitFlags(flag.CommandLine) + klog.InitFlags(packageFlags) + packageFlags.DurationVar(&logFlushFreq, logFlushFreqFlagName, 5*time.Second, "Maximum number of seconds between log flushes") } -// AddFlags registers this package's flags on arbitrary FlagSets, such that they point to the -// same value as the global flags. +// AddFlags registers this package's flags on arbitrary FlagSets. This includes +// the klog flags, with the original underscore as separator between. If +// commands want hyphens as separators, they can set +// k8s.io/component-base/cli/flag/WordSepNormalizeFunc as normalization +// function on the flag set before calling AddFlags. +// +// May be called more than once. func AddFlags(fs *pflag.FlagSet) { - fs.AddFlag(pflag.Lookup(logFlushFreqFlagName)) + // Determine whether the flags are already present by looking up one + // which always should exist. + if f := fs.Lookup("v"); f != nil { + return + } + fs.AddGoFlagSet(packageFlags) +} + +// AddGoFlags is a variant of AddFlags for traditional Go flag.FlagSet. +// Commands should use pflag whenever possible for the sake of consistency. +// Cases where this function is needed include tests (they have to set up flags +// in flag.CommandLine) and commands that for historic reasons use Go +// flag.Parse and cannot change to pflag because it would break their command +// line interface. +func AddGoFlags(fs *flag.FlagSet) { + packageFlags.VisitAll(func(f *flag.Flag) { + fs.Var(f.Value, f.Name, f.Usage) + }) } // KlogWriter serves as a bridge between the standard log package and the glog package. @@ -50,15 +79,19 @@ func (writer KlogWriter) Write(data []byte) (n int, err error) { return len(data), nil } -// InitLogs initializes logs the way we want for kubernetes. +// InitLogs initializes logs the way we want for Kubernetes. +// It should be called after parsing flags. If called before that, +// it will use the default log settings. func InitLogs() { log.SetOutput(KlogWriter{}) log.SetFlags(0) - // The default glog flush interval is 5 seconds. - go wait.Forever(klog.Flush, *logFlushFreq) + // The default klog flush interval is 5 seconds. + go wait.Forever(klog.Flush, logFlushFreq) } -// FlushLogs flushes logs immediately. +// FlushLogs flushes logs immediately. This should be called at the end of +// the main function via defer to ensure that all pending log messages +// are printed before exiting the program. func FlushLogs() { klog.Flush() } diff --git a/staging/src/k8s.io/component-base/logs/options_test.go b/staging/src/k8s.io/component-base/logs/options_test.go index 7be7cdb22cb..3472f949d89 100644 --- a/staging/src/k8s.io/component-base/logs/options_test.go +++ b/staging/src/k8s.io/component-base/logs/options_test.go @@ -37,7 +37,7 @@ func TestFlags(t *testing.T) { want := ` --experimental-logging-sanitization [Experimental] When enabled prevents logging of fields tagged as sensitive (passwords, keys, tokens). Runtime log sanitization may introduce significant computation overhead and therefore should not be enabled in production. --logging-format string Sets the log format. Permitted formats: "text". - Non-default formats don't honor these flags: --add_dir_header, --alsologtostderr, --log_backtrace_at, --log_dir, --log_file, --log_file_max_size, --logtostderr, --one_output, --skip_headers, --skip_log_headers, --stderrthreshold, --vmodule, --log-flush-frequency. + Non-default formats don't honor these flags: --add_dir_header, --alsologtostderr, --log_backtrace_at, --log_dir, --log_file, --log_file_max_size, --logtostderr, --one_output, --skip_headers, --skip_log_headers, --stderrthreshold, --vmodule. Non-default choices are currently alpha and subject to change without warning. (default "text") ` if !assert.Equal(t, want, output.String()) { diff --git a/staging/src/k8s.io/component-base/logs/testinit/testinit.go b/staging/src/k8s.io/component-base/logs/testinit/testinit.go new file mode 100644 index 00000000000..41ea89310c4 --- /dev/null +++ b/staging/src/k8s.io/component-base/logs/testinit/testinit.go @@ -0,0 +1,32 @@ +/* +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 testinit adds logging flags to a Ginkgo or Go test program during +// initialization, something that the logs package itself no longer does. +// +// Normal commands should not use this and instead manage logging flags with +// logs.Options and/or cli.Run. +package testinit + +import ( + "flag" + + "k8s.io/component-base/logs" +) + +func init() { + logs.AddGoFlags(flag.CommandLine) +} diff --git a/staging/src/k8s.io/component-base/logs/validate.go b/staging/src/k8s.io/component-base/logs/validate.go index 86f1cecf34e..c6858af49ae 100644 --- a/staging/src/k8s.io/component-base/logs/validate.go +++ b/staging/src/k8s.io/component-base/logs/validate.go @@ -17,23 +17,22 @@ limitations under the License. package logs import ( - "flag" "fmt" - "strings" - - "github.com/spf13/pflag" "k8s.io/apimachinery/pkg/util/validation/field" + cliflag "k8s.io/component-base/cli/flag" "k8s.io/component-base/config" ) func ValidateLoggingConfiguration(c *config.LoggingConfiguration, fldPath *field.Path) field.ErrorList { errs := field.ErrorList{} if c.Format != DefaultLogFormat { - allFlags := UnsupportedLoggingFlags(hyphensToUnderscores) - for _, fname := range allFlags { - if flagIsSet(fname, hyphensToUnderscores) { - errs = append(errs, field.Invalid(fldPath.Child("format"), c.Format, fmt.Sprintf("Non-default format doesn't honor flag: %s", fname))) + // WordSepNormalizeFunc is just a guess. Most commands use it, + // but we cannot know for sure. + allFlags := UnsupportedLoggingFlags(cliflag.WordSepNormalizeFunc) + for _, f := range allFlags { + if f.DefValue != f.Value.String() { + errs = append(errs, field.Invalid(fldPath.Child("format"), c.Format, fmt.Sprintf("Non-default format doesn't honor flag: %s", f.Name))) } } } @@ -42,27 +41,3 @@ func ValidateLoggingConfiguration(c *config.LoggingConfiguration, fldPath *field } return errs } - -// hyphensToUnderscores replaces hyphens with underscores -// we should always use underscores instead of hyphens when validate flags -func hyphensToUnderscores(s string) string { - return strings.Replace(s, "-", "_", -1) -} - -func flagIsSet(name string, normalizeFunc func(name string) string) bool { - f := flag.Lookup(name) - if f != nil { - return f.DefValue != f.Value.String() - } - if normalizeFunc != nil { - f = flag.Lookup(normalizeFunc(name)) - if f != nil { - return f.DefValue != f.Value.String() - } - } - pf := pflag.Lookup(name) - if pf != nil { - return pf.DefValue != pf.Value.String() - } - panic("failed to lookup unsupported log flag") -} diff --git a/staging/src/k8s.io/kube-aggregator/main.go b/staging/src/k8s.io/kube-aggregator/main.go index e7575e96d0c..ed8d45f6a35 100644 --- a/staging/src/k8s.io/kube-aggregator/main.go +++ b/staging/src/k8s.io/kube-aggregator/main.go @@ -17,13 +17,10 @@ limitations under the License. package main import ( - "flag" "os" - "k8s.io/klog/v2" - genericapiserver "k8s.io/apiserver/pkg/server" - "k8s.io/component-base/logs" + "k8s.io/component-base/cli" "k8s.io/kube-aggregator/pkg/cmd/server" // force compilation of packages we'll later rely upon @@ -35,14 +32,9 @@ import ( ) func main() { - logs.InitLogs() - defer logs.FlushLogs() - stopCh := genericapiserver.SetupSignalHandler() options := server.NewDefaultOptions(os.Stdout, os.Stderr) cmd := server.NewCommandStartAggregator(options, stopCh) - cmd.Flags().AddGoFlagSet(flag.CommandLine) - if err := cmd.Execute(); err != nil { - klog.Fatal(err) - } + code := cli.Run(cmd) + os.Exit(code) } diff --git a/staging/src/k8s.io/kubectl/pkg/cmd/cmd.go b/staging/src/k8s.io/kubectl/pkg/cmd/cmd.go index fcdc8574bc3..52f1042e69e 100644 --- a/staging/src/k8s.io/kubectl/pkg/cmd/cmd.go +++ b/staging/src/k8s.io/kubectl/pkg/cmd/cmd.go @@ -17,7 +17,6 @@ limitations under the License. package cmd import ( - "flag" "fmt" "io" "net/http" @@ -255,13 +254,11 @@ func NewKubectlCommand(in io.Reader, out, err io.Writer) *cobra.Command { return nil }, } + // From this point and forward we get warnings on flags that contain "_" separators + // when adding them with hyphen instead of the original name. + cmds.SetGlobalNormalizationFunc(cliflag.WarnWordSepNormalizeFunc) flags := cmds.PersistentFlags() - flags.SetNormalizeFunc(cliflag.WarnWordSepNormalizeFunc) // Warn for "_" flags - - // Normalize all flags that are coming from other packages or pre-configurations - // a.k.a. change all "_" to "-". e.g. glog package - flags.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) addProfilingFlags(flags) @@ -270,12 +267,10 @@ func NewKubectlCommand(in io.Reader, out, err io.Writer) *cobra.Command { kubeConfigFlags := genericclioptions.NewConfigFlags(true).WithDeprecatedPasswordFlag() kubeConfigFlags.AddFlags(flags) matchVersionKubeConfigFlags := cmdutil.NewMatchVersionFlags(kubeConfigFlags) - matchVersionKubeConfigFlags.AddFlags(cmds.PersistentFlags()) + matchVersionKubeConfigFlags.AddFlags(flags) // Updates hooks to add kubectl command headers: SIG CLI KEP 859. addCmdHeaderHooks(cmds, kubeConfigFlags) - cmds.PersistentFlags().AddGoFlagSet(flag.CommandLine) - f := cmdutil.NewFactory(matchVersionKubeConfigFlags) // Sending in 'nil' for the getLanguageFn() results in using @@ -285,9 +280,6 @@ func NewKubectlCommand(in io.Reader, out, err io.Writer) *cobra.Command { // the language, instead of just loading from the LANG env. variable. i18n.LoadTranslations("kubectl", nil) - // From this point and forward we get warnings on flags that contain "_" separators - cmds.SetGlobalNormalizationFunc(cliflag.WarnWordSepNormalizeFunc) - ioStreams := genericclioptions.IOStreams{In: in, Out: out, ErrOut: err} // Proxy command is incompatible with CommandHeaderRoundTripper, so @@ -392,6 +384,9 @@ func NewKubectlCommand(in io.Reader, out, err io.Writer) *cobra.Command { cmds.AddCommand(apiresources.NewCmdAPIResources(f, ioStreams)) cmds.AddCommand(options.NewCmdOptions(ioStreams.Out)) + // Stop warning about normalization of flags. That makes it possible to + // add the klog flags later. + cmds.SetGlobalNormalizationFunc(cliflag.WordSepNormalizeFunc) return cmds } diff --git a/staging/src/k8s.io/kubectl/pkg/util/logs/logs.go b/staging/src/k8s.io/kubectl/pkg/util/logs/logs.go deleted file mode 100644 index 538a1597e22..00000000000 --- a/staging/src/k8s.io/kubectl/pkg/util/logs/logs.go +++ /dev/null @@ -1,62 +0,0 @@ -/* -Copyright 2014 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 logs - -import ( - "flag" - "log" - "time" - - "github.com/spf13/pflag" - "k8s.io/apimachinery/pkg/util/wait" - "k8s.io/klog/v2" -) - -var logFlushFreq = pflag.Duration("log-flush-frequency", 5*time.Second, "Maximum number of seconds between log flushes") - -// TODO(thockin): This is temporary until we agree on log dirs and put those into each cmd. -func init() { - klog.InitFlags(flag.CommandLine) - flag.Set("logtostderr", "true") -} - -// KlogWriter serves as a bridge between the standard log package and the glog package. -type KlogWriter struct{} - -// Write implements the io.Writer interface. -func (writer KlogWriter) Write(data []byte) (n int, err error) { - klog.InfoDepth(1, string(data)) - return len(data), nil -} - -// InitLogs initializes logs the way we want for kubernetes. -func InitLogs() { - log.SetOutput(KlogWriter{}) - log.SetFlags(0) - // The default glog flush interval is 5 seconds. - go wait.Until(klog.Flush, *logFlushFreq, wait.NeverStop) -} - -// FlushLogs flushes logs immediately. -func FlushLogs() { - klog.Flush() -} - -// NewLogger creates a new log.Logger which sends logs to klog.Info. -func NewLogger(prefix string) *log.Logger { - return log.New(KlogWriter{}, prefix, 0) -} diff --git a/staging/src/k8s.io/pod-security-admission/cmd/webhook/server/server.go b/staging/src/k8s.io/pod-security-admission/cmd/webhook/server/server.go index d922529e03a..e8ea13022ba 100644 --- a/staging/src/k8s.io/pod-security-admission/cmd/webhook/server/server.go +++ b/staging/src/k8s.io/pod-security-admission/cmd/webhook/server/server.go @@ -27,6 +27,7 @@ import ( "time" "github.com/spf13/cobra" + "github.com/spf13/pflag" admissionv1 "k8s.io/api/admission/v1" utilerrors "k8s.io/apimachinery/pkg/util/errors" @@ -58,6 +59,7 @@ Security Standards.`, }, Args: cobra.NoArgs, } + cmd.SetGlobalNormalizationFunc(pflag.CommandLine.GetNormalizeFunc()) opts.AddFlags(cmd.Flags()) return cmd diff --git a/staging/src/k8s.io/pod-security-admission/cmd/webhook/webhook.go b/staging/src/k8s.io/pod-security-admission/cmd/webhook/webhook.go index 8c6642b670f..874184d56a6 100644 --- a/staging/src/k8s.io/pod-security-admission/cmd/webhook/webhook.go +++ b/staging/src/k8s.io/pod-security-admission/cmd/webhook/webhook.go @@ -17,34 +17,18 @@ limitations under the License. package main import ( - "flag" - "math/rand" "os" - "time" - "github.com/spf13/pflag" - - cliflag "k8s.io/component-base/cli/flag" - "k8s.io/component-base/logs" + "k8s.io/component-base/cli" _ "k8s.io/component-base/logs/json/register" // for JSON log format registration - "k8s.io/klog/v2" "k8s.io/pod-security-admission/cmd/webhook/server" ) func main() { - rand.Seed(time.Now().UnixNano()) - - pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) + // Due to historic reasons, this command does *not* normalize command + // line flags. command := server.NewServerCommand() - - logs.InitLogs() - defer logs.FlushLogs() - logFlags := flag.NewFlagSet("klog", flag.ContinueOnError) - klog.InitFlags(logFlags) - command.Flags().AddGoFlagSet(logFlags) - - if err := command.Execute(); err != nil { - os.Exit(1) - } + code := cli.Run(command) + os.Exit(code) } diff --git a/staging/src/k8s.io/sample-apiserver/go.mod b/staging/src/k8s.io/sample-apiserver/go.mod index 3264ea0af9b..b0801662214 100644 --- a/staging/src/k8s.io/sample-apiserver/go.mod +++ b/staging/src/k8s.io/sample-apiserver/go.mod @@ -7,12 +7,12 @@ go 1.16 require ( github.com/google/gofuzz v1.1.0 github.com/spf13/cobra v1.2.1 + github.com/spf13/pflag v1.0.5 k8s.io/apimachinery v0.0.0 k8s.io/apiserver v0.0.0 k8s.io/client-go v0.0.0 k8s.io/code-generator v0.0.0 k8s.io/component-base v0.0.0 - k8s.io/klog/v2 v2.20.0 k8s.io/kube-openapi v0.0.0-20210817084001-7fbd8d59e5b8 k8s.io/utils v0.0.0-20210819203725-bdf08cb9a70a ) diff --git a/staging/src/k8s.io/sample-apiserver/main.go b/staging/src/k8s.io/sample-apiserver/main.go index 1142988e42f..a926f0cee4b 100644 --- a/staging/src/k8s.io/sample-apiserver/main.go +++ b/staging/src/k8s.io/sample-apiserver/main.go @@ -17,25 +17,22 @@ limitations under the License. package main import ( - "flag" "os" - "k8s.io/klog/v2" + "github.com/spf13/pflag" genericapiserver "k8s.io/apiserver/pkg/server" - "k8s.io/component-base/logs" + "k8s.io/component-base/cli" + cliflag "k8s.io/component-base/cli/flag" "k8s.io/sample-apiserver/pkg/cmd/server" ) func main() { - logs.InitLogs() - defer logs.FlushLogs() + pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) stopCh := genericapiserver.SetupSignalHandler() options := server.NewWardleServerOptions(os.Stdout, os.Stderr) cmd := server.NewCommandStartWardleServer(options, stopCh) - cmd.Flags().AddGoFlagSet(flag.CommandLine) - if err := cmd.Execute(); err != nil { - klog.Fatal(err) - } + code := cli.Run(cmd) + os.Exit(code) } diff --git a/test/e2e/e2e.go b/test/e2e/e2e.go index c979e583947..92660febc4a 100644 --- a/test/e2e/e2e.go +++ b/test/e2e/e2e.go @@ -61,6 +61,9 @@ import ( _ "k8s.io/kubernetes/test/e2e/framework/providers/kubemark" _ "k8s.io/kubernetes/test/e2e/framework/providers/openstack" _ "k8s.io/kubernetes/test/e2e/framework/providers/vsphere" + + // Ensure that logging flags are part of the command line. + _ "k8s.io/component-base/logs/testinit" ) const ( diff --git a/test/e2e_node/e2e_node_suite_test.go b/test/e2e_node/e2e_node_suite_test.go index 9594b5c67d8..81da69d0b78 100644 --- a/test/e2e_node/e2e_node_suite_test.go +++ b/test/e2e_node/e2e_node_suite_test.go @@ -42,6 +42,7 @@ import ( utilyaml "k8s.io/apimachinery/pkg/util/yaml" clientset "k8s.io/client-go/kubernetes" cliflag "k8s.io/component-base/cli/flag" + "k8s.io/component-base/logs" "k8s.io/kubernetes/pkg/util/rlimit" commontest "k8s.io/kubernetes/test/e2e/common" "k8s.io/kubernetes/test/e2e/framework" @@ -105,6 +106,7 @@ func TestMain(m *testing.M) { e2econfig.CopyFlags(e2econfig.Flags, flag.CommandLine) framework.RegisterCommonFlags(flag.CommandLine) registerNodeFlags(flag.CommandLine) + logs.AddFlags(pflag.CommandLine) pflag.CommandLine.AddGoFlagSet(flag.CommandLine) // Mark the run-services-mode flag as hidden to prevent user from using it. pflag.CommandLine.MarkHidden("run-services-mode") diff --git a/test/images/agnhost/agnhost.go b/test/images/agnhost/agnhost.go index 2c517e5be95..3f50dc9cff5 100644 --- a/test/images/agnhost/agnhost.go +++ b/test/images/agnhost/agnhost.go @@ -17,11 +17,11 @@ limitations under the License. package main import ( - "flag" + "os" "github.com/spf13/cobra" - "k8s.io/klog/v2" + "k8s.io/component-base/cli" auditproxy "k8s.io/kubernetes/test/images/agnhost/audit-proxy" "k8s.io/kubernetes/test/images/agnhost/connect" crdconvwebhook "k8s.io/kubernetes/test/images/agnhost/crd-conversion-webhook" @@ -85,8 +85,6 @@ func main() { // NOTE(claudiub): Some tests are passing logging related flags, so we need to be able to // accept them. This will also include them in the printed help. - loggingFlags := &flag.FlagSet{} - klog.InitFlags(loggingFlags) - rootCmd.PersistentFlags().AddGoFlagSet(loggingFlags) - rootCmd.Execute() + code := cli.Run(rootCmd) + os.Exit(code) } diff --git a/test/integration/framework/flags.go b/test/integration/framework/flags.go new file mode 100644 index 00000000000..d79b3e1e0d5 --- /dev/null +++ b/test/integration/framework/flags.go @@ -0,0 +1,22 @@ +/* +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 framework + +import ( + // All integration tests are expected to have logging flags. + _ "k8s.io/component-base/logs/testinit" +) diff --git a/vendor/modules.txt b/vendor/modules.txt index 18b3f39ef90..a75d1552264 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -1935,6 +1935,7 @@ k8s.io/code-generator/pkg/util k8s.io/code-generator/third_party/forked/golang/reflect # k8s.io/component-base v0.0.0 => ./staging/src/k8s.io/component-base ## explicit +k8s.io/component-base/cli k8s.io/component-base/cli/flag k8s.io/component-base/cli/globalflag k8s.io/component-base/codec @@ -1952,6 +1953,7 @@ k8s.io/component-base/logs/json k8s.io/component-base/logs/json/register k8s.io/component-base/logs/logreduction k8s.io/component-base/logs/sanitization +k8s.io/component-base/logs/testinit k8s.io/component-base/metrics k8s.io/component-base/metrics/legacyregistry k8s.io/component-base/metrics/prometheus/clientgo @@ -2148,7 +2150,6 @@ k8s.io/kubectl/pkg/util/fieldpath k8s.io/kubectl/pkg/util/hash k8s.io/kubectl/pkg/util/i18n k8s.io/kubectl/pkg/util/interrupt -k8s.io/kubectl/pkg/util/logs k8s.io/kubectl/pkg/util/openapi k8s.io/kubectl/pkg/util/openapi/testing k8s.io/kubectl/pkg/util/openapi/validation From 00e4a599f6f3510644549b7e3a3c0ee741ce264a Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Tue, 28 Sep 2021 20:59:15 +0200 Subject: [PATCH 3/4] command lines: always show flags with hyphens All Kubernetes commands should show flags with hyphens in their help text even when the flag originally was defined with underscore. Converting a command to this style is not breaking its command line API because the old-style parameter with underscore is accepted as alias. The easiest solution to achieve this is to set normalization shortly before running the command in the new central cli.Run or the few places where that function isn't used yet. There may be some texts which depends on normalization at flag definition time, like the --logging-format usage warning. Those get generated assuming that hyphens will be used. --- cmd/cloud-controller-manager/main.go | 4 ---- cmd/kube-apiserver/apiserver.go | 5 ----- cmd/kube-controller-manager/controller-manager.go | 5 ----- cmd/kube-proxy/app/server.go | 1 - cmd/kube-proxy/proxy.go | 5 ----- cmd/kube-scheduler/app/server.go | 2 -- cmd/kube-scheduler/scheduler.go | 5 ----- cmd/kubectl-convert/kubectl-convert.go | 3 --- cmd/kubectl/kubectl.go | 5 ----- cmd/kubelet/app/options/options.go | 1 - cmd/kubemark/hollow-node.go | 3 --- pkg/kubectl/cmd/convert/convert.go | 2 -- .../src/k8s.io/cloud-provider/app/controllermanager.go | 2 -- .../src/k8s.io/cloud-provider/app/testing/testserver.go | 3 --- staging/src/k8s.io/cloud-provider/sample/basic_main.go | 3 --- staging/src/k8s.io/component-base/cli/run.go | 7 ++++++- staging/src/k8s.io/component-base/logs/config.go | 6 +++++- .../src/k8s.io/component-base/logs/example/cmd/logger.go | 9 --------- staging/src/k8s.io/component-base/logs/options_test.go | 2 +- staging/src/k8s.io/component-base/logs/validate.go | 2 +- .../pod-security-admission/cmd/webhook/server/server.go | 2 -- .../k8s.io/pod-security-admission/cmd/webhook/webhook.go | 3 --- staging/src/k8s.io/sample-apiserver/go.mod | 1 - staging/src/k8s.io/sample-apiserver/main.go | 5 ----- 24 files changed, 13 insertions(+), 73 deletions(-) diff --git a/cmd/cloud-controller-manager/main.go b/cmd/cloud-controller-manager/main.go index 5eb38d73605..048ef9ad486 100644 --- a/cmd/cloud-controller-manager/main.go +++ b/cmd/cloud-controller-manager/main.go @@ -27,8 +27,6 @@ package main import ( "os" - "github.com/spf13/pflag" - "k8s.io/apimachinery/pkg/util/wait" cloudprovider "k8s.io/cloud-provider" "k8s.io/cloud-provider/app" @@ -44,8 +42,6 @@ import ( ) func main() { - pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) - ccmOptions, err := options.NewCloudControllerManagerOptions() if err != nil { klog.Fatalf("unable to initialize command options: %v", err) diff --git a/cmd/kube-apiserver/apiserver.go b/cmd/kube-apiserver/apiserver.go index fd61264a737..42956a9bf6e 100644 --- a/cmd/kube-apiserver/apiserver.go +++ b/cmd/kube-apiserver/apiserver.go @@ -21,10 +21,7 @@ package main import ( "os" - "github.com/spf13/pflag" - "k8s.io/component-base/cli" - cliflag "k8s.io/component-base/cli/flag" _ "k8s.io/component-base/logs/json/register" // for JSON log format registration _ "k8s.io/component-base/metrics/prometheus/clientgo" // load all the prometheus client-go plugins _ "k8s.io/component-base/metrics/prometheus/version" // for version metric registration @@ -32,8 +29,6 @@ import ( ) func main() { - pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) - command := app.NewAPIServerCommand() code := cli.Run(command) os.Exit(code) diff --git a/cmd/kube-controller-manager/controller-manager.go b/cmd/kube-controller-manager/controller-manager.go index df6a2c7fa09..2a34cfcdf3e 100644 --- a/cmd/kube-controller-manager/controller-manager.go +++ b/cmd/kube-controller-manager/controller-manager.go @@ -23,10 +23,7 @@ package main import ( "os" - "github.com/spf13/pflag" - "k8s.io/component-base/cli" - cliflag "k8s.io/component-base/cli/flag" _ "k8s.io/component-base/logs/json/register" // for JSON log format registration _ "k8s.io/component-base/metrics/prometheus/clientgo" // load all the prometheus client-go plugin _ "k8s.io/component-base/metrics/prometheus/version" // for version metric registration @@ -34,8 +31,6 @@ import ( ) func main() { - pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) - command := app.NewControllerManagerCommand() code := cli.Run(command) os.Exit(code) diff --git a/cmd/kube-proxy/app/server.go b/cmd/kube-proxy/app/server.go index d01dc81a28a..a5c90f0ad03 100644 --- a/cmd/kube-proxy/app/server.go +++ b/cmd/kube-proxy/app/server.go @@ -510,7 +510,6 @@ with the apiserver API to configure the proxy.`, return nil }, } - cmd.SetGlobalNormalizationFunc(pflag.CommandLine.GetNormalizeFunc()) var err error opts.config, err = opts.ApplyDefaults(opts.config) diff --git a/cmd/kube-proxy/proxy.go b/cmd/kube-proxy/proxy.go index d668c8da42d..86ef9dff68e 100644 --- a/cmd/kube-proxy/proxy.go +++ b/cmd/kube-proxy/proxy.go @@ -19,18 +19,13 @@ package main import ( "os" - "github.com/spf13/pflag" - "k8s.io/component-base/cli" - cliflag "k8s.io/component-base/cli/flag" _ "k8s.io/component-base/metrics/prometheus/restclient" // for client metric registration _ "k8s.io/component-base/metrics/prometheus/version" // for version metric registration "k8s.io/kubernetes/cmd/kube-proxy/app" ) func main() { - pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) - command := app.NewProxyCommand() code := cli.Run(command) os.Exit(code) diff --git a/cmd/kube-scheduler/app/server.go b/cmd/kube-scheduler/app/server.go index 0be5f9c5146..1bda9d0e579 100644 --- a/cmd/kube-scheduler/app/server.go +++ b/cmd/kube-scheduler/app/server.go @@ -25,7 +25,6 @@ import ( goruntime "runtime" "github.com/spf13/cobra" - "github.com/spf13/pflag" utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apiserver/pkg/authentication/authenticator" @@ -100,7 +99,6 @@ for more information about scheduling and the kube-scheduler component.`, return nil }, } - cmd.SetGlobalNormalizationFunc(pflag.CommandLine.GetNormalizeFunc()) fs := cmd.Flags() verflag.AddFlags(namedFlagSets.FlagSet("global")) diff --git a/cmd/kube-scheduler/scheduler.go b/cmd/kube-scheduler/scheduler.go index 912e484bc73..71739808dd2 100644 --- a/cmd/kube-scheduler/scheduler.go +++ b/cmd/kube-scheduler/scheduler.go @@ -19,10 +19,7 @@ package main import ( "os" - "github.com/spf13/pflag" - "k8s.io/component-base/cli" - cliflag "k8s.io/component-base/cli/flag" _ "k8s.io/component-base/logs/json/register" // for JSON log format registration _ "k8s.io/component-base/metrics/prometheus/clientgo" _ "k8s.io/component-base/metrics/prometheus/version" // for version metric registration @@ -30,8 +27,6 @@ import ( ) func main() { - pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) - command := app.NewSchedulerCommand() code := cli.Run(command) os.Exit(code) diff --git a/cmd/kubectl-convert/kubectl-convert.go b/cmd/kubectl-convert/kubectl-convert.go index 7b7610f4be4..d47b120515b 100644 --- a/cmd/kubectl-convert/kubectl-convert.go +++ b/cmd/kubectl-convert/kubectl-convert.go @@ -23,15 +23,12 @@ import ( "k8s.io/cli-runtime/pkg/genericclioptions" "k8s.io/component-base/cli" - cliflag "k8s.io/component-base/cli/flag" cmdutil "k8s.io/kubectl/pkg/cmd/util" "k8s.io/kubernetes/pkg/kubectl/cmd/convert" ) func main() { - pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) flags := pflag.NewFlagSet("kubectl-convert", pflag.ExitOnError) - flags.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) pflag.CommandLine = flags kubeConfigFlags := genericclioptions.NewConfigFlags(true).WithDeprecatedPasswordFlag() diff --git a/cmd/kubectl/kubectl.go b/cmd/kubectl/kubectl.go index 67576646e5f..7c7924a5f5f 100644 --- a/cmd/kubectl/kubectl.go +++ b/cmd/kubectl/kubectl.go @@ -19,10 +19,7 @@ package main import ( "os" - "github.com/spf13/pflag" - "k8s.io/component-base/cli" - cliflag "k8s.io/component-base/cli/flag" "k8s.io/kubectl/pkg/cmd" // Import to initialize client auth plugins. @@ -30,8 +27,6 @@ import ( ) func main() { - pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) - command := cmd.NewDefaultKubectlCommand() code := cli.Run(command) os.Exit(code) diff --git a/cmd/kubelet/app/options/options.go b/cmd/kubelet/app/options/options.go index 0343cd6d820..527057d35d4 100644 --- a/cmd/kubelet/app/options/options.go +++ b/cmd/kubelet/app/options/options.go @@ -383,7 +383,6 @@ func (f *KubeletFlags) AddFlags(mainfs *pflag.FlagSet) { // AddKubeletConfigFlags adds flags for a specific kubeletconfig.KubeletConfiguration to the specified FlagSet func AddKubeletConfigFlags(mainfs *pflag.FlagSet, c *kubeletconfig.KubeletConfiguration) { fs := pflag.NewFlagSet("", pflag.ExitOnError) - fs.SetNormalizeFunc(mainfs.GetNormalizeFunc()) defer func() { // All KubeletConfiguration flags are now deprecated, and any new flags that point to // KubeletConfiguration fields are deprecated-on-creation. When removing flags at the end diff --git a/cmd/kubemark/hollow-node.go b/cmd/kubemark/hollow-node.go index df302d8ed3a..de254acb964 100644 --- a/cmd/kubemark/hollow-node.go +++ b/cmd/kubemark/hollow-node.go @@ -130,8 +130,6 @@ func (c *hollowNodeConfig) createHollowKubeletOptions() *kubemark.HollowKubletOp } func main() { - pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) - command := newHollowNodeCommand() code := cli.Run(command) os.Exit(code) @@ -160,7 +158,6 @@ func newHollowNodeCommand() *cobra.Command { return nil }, } - cmd.SetGlobalNormalizationFunc(pflag.CommandLine.GetNormalizeFunc()) fs := cmd.Flags() fs.AddGoFlagSet(goflag.CommandLine) // for flags like --docker-only diff --git a/pkg/kubectl/cmd/convert/convert.go b/pkg/kubectl/cmd/convert/convert.go index 3557704e27b..74b16ccbbe4 100644 --- a/pkg/kubectl/cmd/convert/convert.go +++ b/pkg/kubectl/cmd/convert/convert.go @@ -20,7 +20,6 @@ import ( "fmt" "github.com/spf13/cobra" - "github.com/spf13/pflag" "k8s.io/klog/v2" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -101,7 +100,6 @@ func NewCmdConvert(f cmdutil.Factory, ioStreams genericclioptions.IOStreams) *co cmdutil.CheckErr(o.RunConvert()) }, } - cmd.SetGlobalNormalizationFunc(pflag.CommandLine.GetNormalizeFunc()) cmd.Flags().BoolVar(&o.local, "local", o.local, "If true, convert will NOT try to contact api-server but run locally.") cmd.Flags().StringVar(&o.OutputVersion, "output-version", o.OutputVersion, i18n.T("Output the formatted object with the given group version (for ex: 'extensions/v1beta1').")) diff --git a/staging/src/k8s.io/cloud-provider/app/controllermanager.go b/staging/src/k8s.io/cloud-provider/app/controllermanager.go index 628e3a97cef..a458c4e0236 100644 --- a/staging/src/k8s.io/cloud-provider/app/controllermanager.go +++ b/staging/src/k8s.io/cloud-provider/app/controllermanager.go @@ -25,7 +25,6 @@ import ( "time" "github.com/spf13/cobra" - "github.com/spf13/pflag" "k8s.io/apimachinery/pkg/runtime/schema" utilruntime "k8s.io/apimachinery/pkg/util/runtime" @@ -103,7 +102,6 @@ the cloud specific control loops shipped with Kubernetes.`, return nil }, } - cmd.SetGlobalNormalizationFunc(pflag.CommandLine.GetNormalizeFunc()) fs := cmd.Flags() namedFlagSets := s.Flags(ControllerNames(controllerInitFuncConstructors), ControllersDisabledByDefault.List()) diff --git a/staging/src/k8s.io/cloud-provider/app/testing/testserver.go b/staging/src/k8s.io/cloud-provider/app/testing/testserver.go index 9e334bf5524..16181464ddb 100644 --- a/staging/src/k8s.io/cloud-provider/app/testing/testserver.go +++ b/staging/src/k8s.io/cloud-provider/app/testing/testserver.go @@ -25,7 +25,6 @@ import ( "strings" "time" - "github.com/spf13/pflag" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/kubernetes" restclient "k8s.io/client-go/rest" @@ -33,7 +32,6 @@ import ( "k8s.io/cloud-provider/app" "k8s.io/cloud-provider/app/config" "k8s.io/cloud-provider/options" - cliflag "k8s.io/component-base/cli/flag" ) // TearDownFunc is to be called to tear down a test server. @@ -104,7 +102,6 @@ func StartTestServer(t Logger, customFlags []string) (result TestServer, err err } fss := cliflag.NamedFlagSets{} command := app.NewCloudControllerManagerCommand(s, cloudInitializer, app.DefaultInitFuncConstructors, fss, stopCh) - pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) commandArgs := []string{} listeners := []net.Listener{} diff --git a/staging/src/k8s.io/cloud-provider/sample/basic_main.go b/staging/src/k8s.io/cloud-provider/sample/basic_main.go index 4d6f820fefb..10af37ada12 100644 --- a/staging/src/k8s.io/cloud-provider/sample/basic_main.go +++ b/staging/src/k8s.io/cloud-provider/sample/basic_main.go @@ -23,7 +23,6 @@ package main import ( "os" - "github.com/spf13/pflag" "k8s.io/apimachinery/pkg/util/wait" cloudprovider "k8s.io/cloud-provider" "k8s.io/cloud-provider/app" @@ -39,8 +38,6 @@ import ( ) func main() { - pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) - ccmOptions, err := options.NewCloudControllerManagerOptions() if err != nil { klog.Fatalf("unable to initialize command options: %v", err) diff --git a/staging/src/k8s.io/component-base/cli/run.go b/staging/src/k8s.io/component-base/cli/run.go index 0d430e04ddc..8456b178a68 100644 --- a/staging/src/k8s.io/component-base/cli/run.go +++ b/staging/src/k8s.io/component-base/cli/run.go @@ -25,15 +25,20 @@ import ( "github.com/spf13/cobra" "k8s.io/component-base/logs" + cliflag "k8s.io/component-base/cli/flag" ) // 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 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) + // This is idempotent. logs.AddFlags(cmd.PersistentFlags()) diff --git a/staging/src/k8s.io/component-base/logs/config.go b/staging/src/k8s.io/component-base/logs/config.go index 296cfb9ce6b..3bec829a238 100644 --- a/staging/src/k8s.io/component-base/logs/config.go +++ b/staging/src/k8s.io/component-base/logs/config.go @@ -24,6 +24,7 @@ import ( "github.com/spf13/pflag" + cliflag "k8s.io/component-base/cli/flag" "k8s.io/component-base/config" "k8s.io/klog/v2" ) @@ -58,7 +59,10 @@ var supportedLogsFlags = map[string]struct{}{ // BindLoggingFlags binds the Options struct fields to a flagset func BindLoggingFlags(c *config.LoggingConfiguration, fs *pflag.FlagSet) { - unsupportedFlags := strings.Join(unsupportedLoggingFlagNames(fs.GetNormalizeFunc()), ", ") + // The help text is generated assuming that flags will eventually use + // hyphens, even if currently no normalization function is set for the + // flag set yet. + unsupportedFlags := strings.Join(unsupportedLoggingFlagNames(cliflag.WordSepNormalizeFunc), ", ") formats := fmt.Sprintf(`"%s"`, strings.Join(LogRegistry.List(), `", "`)) fs.StringVar(&c.Format, "logging-format", c.Format, fmt.Sprintf("Sets the log format. Permitted formats: %s.\nNon-default formats don't honor these flags: %s.\nNon-default choices are currently alpha and subject to change without warning.", formats, unsupportedFlags)) // No new log formats should be added after generation is of flag options diff --git a/staging/src/k8s.io/component-base/logs/example/cmd/logger.go b/staging/src/k8s.io/component-base/logs/example/cmd/logger.go index 0e63e565b74..c3ee30f1082 100644 --- a/staging/src/k8s.io/component-base/logs/example/cmd/logger.go +++ b/staging/src/k8s.io/component-base/logs/example/cmd/logger.go @@ -22,10 +22,8 @@ import ( "os" "github.com/spf13/cobra" - "github.com/spf13/pflag" "k8s.io/component-base/cli" - cliflag "k8s.io/component-base/cli/flag" "k8s.io/component-base/logs" "k8s.io/klog/v2" @@ -33,11 +31,6 @@ import ( ) func main() { - // NamedFlagSets use the global pflag.CommandLine. We don't use those - // in this command (yet), but set it globally anyway for consistency - // with other commands. - pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) - command := NewLoggerCommand() code := cli.Run(command) os.Exit(code) @@ -56,8 +49,6 @@ func NewLoggerCommand() *cobra.Command { runLogger() }, } - cmd.SetGlobalNormalizationFunc(pflag.CommandLine.GetNormalizeFunc()) - o.AddFlags(cmd.Flags()) return cmd } diff --git a/staging/src/k8s.io/component-base/logs/options_test.go b/staging/src/k8s.io/component-base/logs/options_test.go index 3472f949d89..f33d060f2d4 100644 --- a/staging/src/k8s.io/component-base/logs/options_test.go +++ b/staging/src/k8s.io/component-base/logs/options_test.go @@ -37,7 +37,7 @@ func TestFlags(t *testing.T) { want := ` --experimental-logging-sanitization [Experimental] When enabled prevents logging of fields tagged as sensitive (passwords, keys, tokens). Runtime log sanitization may introduce significant computation overhead and therefore should not be enabled in production. --logging-format string Sets the log format. Permitted formats: "text". - Non-default formats don't honor these flags: --add_dir_header, --alsologtostderr, --log_backtrace_at, --log_dir, --log_file, --log_file_max_size, --logtostderr, --one_output, --skip_headers, --skip_log_headers, --stderrthreshold, --vmodule. + Non-default formats don't honor these flags: --add-dir-header, --alsologtostderr, --log-backtrace-at, --log-dir, --log-file, --log-file-max-size, --logtostderr, --one-output, --skip-headers, --skip-log-headers, --stderrthreshold, --vmodule. Non-default choices are currently alpha and subject to change without warning. (default "text") ` if !assert.Equal(t, want, output.String()) { diff --git a/staging/src/k8s.io/component-base/logs/validate.go b/staging/src/k8s.io/component-base/logs/validate.go index c6858af49ae..f0ed8cde1ce 100644 --- a/staging/src/k8s.io/component-base/logs/validate.go +++ b/staging/src/k8s.io/component-base/logs/validate.go @@ -27,7 +27,7 @@ import ( func ValidateLoggingConfiguration(c *config.LoggingConfiguration, fldPath *field.Path) field.ErrorList { errs := field.ErrorList{} if c.Format != DefaultLogFormat { - // WordSepNormalizeFunc is just a guess. Most commands use it, + // WordSepNormalizeFunc is just a guess. Commands should use it, // but we cannot know for sure. allFlags := UnsupportedLoggingFlags(cliflag.WordSepNormalizeFunc) for _, f := range allFlags { diff --git a/staging/src/k8s.io/pod-security-admission/cmd/webhook/server/server.go b/staging/src/k8s.io/pod-security-admission/cmd/webhook/server/server.go index e8ea13022ba..d922529e03a 100644 --- a/staging/src/k8s.io/pod-security-admission/cmd/webhook/server/server.go +++ b/staging/src/k8s.io/pod-security-admission/cmd/webhook/server/server.go @@ -27,7 +27,6 @@ import ( "time" "github.com/spf13/cobra" - "github.com/spf13/pflag" admissionv1 "k8s.io/api/admission/v1" utilerrors "k8s.io/apimachinery/pkg/util/errors" @@ -59,7 +58,6 @@ Security Standards.`, }, Args: cobra.NoArgs, } - cmd.SetGlobalNormalizationFunc(pflag.CommandLine.GetNormalizeFunc()) opts.AddFlags(cmd.Flags()) return cmd diff --git a/staging/src/k8s.io/pod-security-admission/cmd/webhook/webhook.go b/staging/src/k8s.io/pod-security-admission/cmd/webhook/webhook.go index 874184d56a6..0297288b36a 100644 --- a/staging/src/k8s.io/pod-security-admission/cmd/webhook/webhook.go +++ b/staging/src/k8s.io/pod-security-admission/cmd/webhook/webhook.go @@ -25,9 +25,6 @@ import ( ) func main() { - // Due to historic reasons, this command does *not* normalize command - // line flags. - command := server.NewServerCommand() code := cli.Run(command) os.Exit(code) diff --git a/staging/src/k8s.io/sample-apiserver/go.mod b/staging/src/k8s.io/sample-apiserver/go.mod index b0801662214..2a1dbd2803f 100644 --- a/staging/src/k8s.io/sample-apiserver/go.mod +++ b/staging/src/k8s.io/sample-apiserver/go.mod @@ -7,7 +7,6 @@ go 1.16 require ( github.com/google/gofuzz v1.1.0 github.com/spf13/cobra v1.2.1 - github.com/spf13/pflag v1.0.5 k8s.io/apimachinery v0.0.0 k8s.io/apiserver v0.0.0 k8s.io/client-go v0.0.0 diff --git a/staging/src/k8s.io/sample-apiserver/main.go b/staging/src/k8s.io/sample-apiserver/main.go index a926f0cee4b..708a2a43125 100644 --- a/staging/src/k8s.io/sample-apiserver/main.go +++ b/staging/src/k8s.io/sample-apiserver/main.go @@ -19,17 +19,12 @@ package main import ( "os" - "github.com/spf13/pflag" - genericapiserver "k8s.io/apiserver/pkg/server" "k8s.io/component-base/cli" - cliflag "k8s.io/component-base/cli/flag" "k8s.io/sample-apiserver/pkg/cmd/server" ) func main() { - pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) - stopCh := genericapiserver.SetupSignalHandler() options := server.NewWardleServerOptions(os.Stdout, os.Stderr) cmd := server.NewCommandStartWardleServer(options, stopCh) From 1957fb6508c4dbf629f13c5bbf5e36b6bffa6a61 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Wed, 29 Sep 2021 08:45:43 +0200 Subject: [PATCH 4/4] command lines: harmonize command line parse error handling The recommendation from #sig-cli was to print usage, then the error. Extra care is taken to only print the usage instruction when the error really was about flag parsing. Taking kube-scheduler as example: $ _output/bin/kube-scheduler I0929 09:42:42.289039 149029 serving.go:348] Generated self-signed cert in-memory ... W0929 09:42:42.489255 149029 client_config.go:620] error creating inClusterConfig, falling back to default config: unable to load in-cluster configuration, KUBERNETES_SERVICE_HOST and KUBERNETES_SERVICE_PORT must be defined E0929 09:42:42.489366 149029 run.go:98] "command failed" err="invalid configuration: no configuration has been provided, try setting KUBERNETES_MASTER environment variable" $ _output/bin/kube-scheduler --xxx Usage: kube-scheduler [flags] ... --vmodule moduleSpec comma-separated list of pattern=N settings for file-filtered logging Error: unknown flag: --xxx The kubectl behavior doesn't change: $ _output/bin/kubectl get nodes Unable to connect to the server: dial tcp: lookup xxxx: No address associated with hostname $ _output/bin/kubectl --xxx Error: unknown flag: --xxx See 'kubectl --help' for usage. --- cmd/kube-scheduler/app/server.go | 2 - .../cloud-provider/app/testing/testserver.go | 1 + staging/src/k8s.io/component-base/cli/run.go | 62 ++++++++++++++++--- test/cmd/get.sh | 2 +- 4 files changed, 56 insertions(+), 11 deletions(-) diff --git a/cmd/kube-scheduler/app/server.go b/cmd/kube-scheduler/app/server.go index 1bda9d0e579..004d0f20e9c 100644 --- a/cmd/kube-scheduler/app/server.go +++ b/cmd/kube-scheduler/app/server.go @@ -88,8 +88,6 @@ for more information about scheduling and the kube-scheduler component.`, } return nil }, - SilenceErrors: true, - SilenceUsage: true, Args: func(cmd *cobra.Command, args []string) error { for _, arg := range args { if len(arg) > 0 { diff --git a/staging/src/k8s.io/cloud-provider/app/testing/testserver.go b/staging/src/k8s.io/cloud-provider/app/testing/testserver.go index 16181464ddb..bd7a7b2c8ee 100644 --- a/staging/src/k8s.io/cloud-provider/app/testing/testserver.go +++ b/staging/src/k8s.io/cloud-provider/app/testing/testserver.go @@ -32,6 +32,7 @@ import ( "k8s.io/cloud-provider/app" "k8s.io/cloud-provider/app/config" "k8s.io/cloud-provider/options" + cliflag "k8s.io/component-base/cli/flag" ) // TearDownFunc is to be called to tear down a test server. diff --git a/staging/src/k8s.io/component-base/cli/run.go b/staging/src/k8s.io/component-base/cli/run.go index 8456b178a68..cc2c977e386 100644 --- a/staging/src/k8s.io/component-base/cli/run.go +++ b/staging/src/k8s.io/component-base/cli/run.go @@ -24,8 +24,9 @@ import ( "github.com/spf13/cobra" - "k8s.io/component-base/logs" 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. @@ -39,6 +40,44 @@ func Run(cmd *cobra.Command) int { 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()) @@ -64,15 +103,22 @@ func Run(cmd *cobra.Command) int { } if err := cmd.Execute(); err != nil { - // The error may be about the command line ("unknown shorthand - // flag: 'e' in -elp" in kube-scheduler). Printing that without - // any decoration like the one that klog would add ("E0923 + // 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 more readable. + // is less readable. Using klog.Fatal is even worse because it + // dumps a stack trace that isn't about the error. // - // We also don't know in which state logging (configuration may - // have failed) is whereas os.Stderr always should be usable. - fmt.Fprintf(os.Stderr, "%v\n", err) + // 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/test/cmd/get.sh b/test/cmd/get.sh index d9b9107c80f..7f7ceb0c611 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}" 'Error: 1 warning received' + kube::test::if_has_string "${output_message}" 'err="1 warning received"' set +o nounset set +o errexit