From d627b1686c9d346a40f733dcb35661758215d8a1 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Wed, 28 Dec 2022 22:28:05 +0100 Subject: [PATCH 1/2] logs: support standard flag.FlagSet This is useful for binaries that don't use pflag and cannot migrate to it because they have to support the traditional single-dash command line parsing. pflag is a drop-in replacement at the source code level, but the behavior of flag parsing in the resulting binary is different. --- .../component-base/logs/api/v1/options.go | 35 +++++++++++++++++++ .../logs/api/v1/options_test.go | 27 ++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/staging/src/k8s.io/component-base/logs/api/v1/options.go b/staging/src/k8s.io/component-base/logs/api/v1/options.go index 9990e31fff2..a5e11f7d864 100644 --- a/staging/src/k8s.io/component-base/logs/api/v1/options.go +++ b/staging/src/k8s.io/component-base/logs/api/v1/options.go @@ -220,6 +220,41 @@ func apply(c *LoggingConfiguration, options *LoggingOptions, featureGate feature // AddFlags adds command line flags for the configuration. func AddFlags(c *LoggingConfiguration, fs *pflag.FlagSet) { + addFlags(c, fs) +} + +// AddGoFlags is a variant of AddFlags for a standard FlagSet. +func AddGoFlags(c *LoggingConfiguration, fs *flag.FlagSet) { + addFlags(c, goFlagSet{FlagSet: fs}) +} + +// flagSet is the interface implemented by pflag.FlagSet, with +// just those methods defined which are needed by addFlags. +type flagSet interface { + BoolVar(p *bool, name string, value bool, usage string) + DurationVar(p *time.Duration, name string, value time.Duration, usage string) + StringVar(p *string, name string, value string, usage string) + Var(value pflag.Value, name string, usage string) + VarP(value pflag.Value, name, shorthand, usage string) +} + +// goFlagSet implements flagSet for a stdlib flag.FlagSet. +type goFlagSet struct { + *flag.FlagSet +} + +func (fs goFlagSet) Var(value pflag.Value, name string, usage string) { + fs.FlagSet.Var(value, name, usage) +} + +func (fs goFlagSet) VarP(value pflag.Value, name, shorthand, usage string) { + // Ignore shorthand, it's not needed and not supported. + fs.FlagSet.Var(value, name, usage) +} + +// addFlags can be used with both flag.FlagSet and pflag.FlagSet. The internal +// interface definition avoids duplicating this code. +func addFlags(c *LoggingConfiguration, fs flagSet) { formats := logRegistry.list() fs.StringVar(&c.Format, "logging-format", c.Format, fmt.Sprintf("Sets the log format. Permitted formats: %s.", formats)) // No new log formats should be added after generation is of flag options diff --git a/staging/src/k8s.io/component-base/logs/api/v1/options_test.go b/staging/src/k8s.io/component-base/logs/api/v1/options_test.go index d70fdda2e8d..f07f09f86ac 100644 --- a/staging/src/k8s.io/component-base/logs/api/v1/options_test.go +++ b/staging/src/k8s.io/component-base/logs/api/v1/options_test.go @@ -152,6 +152,33 @@ $`, buffer.String()) $`, buffer.String()) }) + t.Run("AddGoFlags", func(t *testing.T) { + newOptions := NewLoggingConfiguration() + var fs flag.FlagSet + var buffer bytes.Buffer + AddGoFlags(newOptions, &fs) + fs.SetOutput(&buffer) + fs.PrintDefaults() + // In contrast to copying through VisitAll, the type of some options is now + // known: + // -log-flush-frequency duration + // Maximum number of seconds between log flushes (default 5s) + // -logging-format string + // Sets the log format. Permitted formats: "text". (default "text") + // -v value + // number for the log level verbosity + // -vmodule value + // comma-separated list of pattern=N settings for file-filtered logging (only works for text log format) + assert.Regexp(t, `^.*-log-flush-frequency.*duration.* +.*default 5s.* +.*-logging-format.*string.* +.*default.*text.* +.*-v.* +.* +.*-vmodule.* +.* +$`, buffer.String()) + }) } func TestContextualLogging(t *testing.T) { From 6f3e5e30e5bb4c745338dd47daea02810e68f36d Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Tue, 24 Jan 2023 14:58:37 +0100 Subject: [PATCH 2/2] logs: consolidate unit tests TestFlags got superseded by TestFlagSet/pflag in 8251a632692ad6d996bf3fff49002c5ead23c318. --- .../component-base/logs/api/v1/options_test.go | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/staging/src/k8s.io/component-base/logs/api/v1/options_test.go b/staging/src/k8s.io/component-base/logs/api/v1/options_test.go index f07f09f86ac..2bdf4388e04 100644 --- a/staging/src/k8s.io/component-base/logs/api/v1/options_test.go +++ b/staging/src/k8s.io/component-base/logs/api/v1/options_test.go @@ -32,23 +32,6 @@ import ( "k8s.io/klog/v2" ) -func TestFlags(t *testing.T) { - c := NewLoggingConfiguration() - fs := pflag.NewFlagSet("addflagstest", pflag.ContinueOnError) - output := bytes.Buffer{} - AddFlags(c, fs) - fs.SetOutput(&output) - fs.PrintDefaults() - want := ` --log-flush-frequency duration Maximum number of seconds between log flushes (default 5s) - --logging-format string Sets the log format. Permitted formats: "text". (default "text") - -v, --v Level number for the log level verbosity - --vmodule pattern=N,... comma-separated list of pattern=N settings for file-filtered logging (only works for text log format) -` - if !assert.Equal(t, want, output.String()) { - t.Errorf("Wrong list of flags. expect %q, got %q", want, output.String()) - } -} - func TestOptions(t *testing.T) { newOptions := NewLoggingConfiguration() testcases := []struct {