From fcc29894c2e929d07aac3ec22db1d9b6aab15422 Mon Sep 17 00:00:00 2001 From: Mark McLoughlin Date: Mon, 14 Jan 2019 13:09:36 +0000 Subject: [PATCH] config: multiple logging fixes First, don't panic with invalid --log-outputs. For example: $> ./bin/etcd --log-outputs foo 2018-12-20 15:05:47.988652 C | embed: unknown log-output "foo" (only supports "default", "stderr", "stdout") panic: unknown log-output "foo" (only supports "default", "stderr", "stdout") goroutine 1 [running]: go.etcd.io/etcd/vendor/github.com/coreos/pkg/capnslog.(*PackageLogger).Panicf(0xc000294b00, 0x10fe067, 0x30, 0xc0001fa398, 0x4, 0x4) go.etcd.io/etcd/vendor/github.com/coreos/pkg/capnslog/pkg_logger.go:75 +0x161 go.etcd.io/etcd/embed.(*Config).setupLogging(0xc000291400, 0xc0002a85b0, 0x1) go.etcd.io/etcd/embed/config_logging.go:120 +0x1939 ... Or: $> ./bin/etcd --log-outputs foo,default --logger zap panic: multi logoutput for "default" is not supported yet goroutine 1 [running]: go.etcd.io/etcd/embed.(*Config).setupLogging(0xc000314500, 0xc0001b2f70, 0x1) go.etcd.io/etcd/embed/config_logging.go:129 +0x2437 go.etcd.io/etcd/embed.(*Config).Validate(0xc000314500, 0xc000268a98, 0x127e440) go.etcd.io/etcd/embed/config.go:543 +0x43 Second, don't exit in embed.setupLogging(). Before: $> ./bin/etcd --log-outputs foo,bar --logger=capnslog supports only 1 value in '--log-outputs', got ["bar" "foo"] and after: $> ./bin/etcd --log-outputs foo,bar 2018-12-20 15:10:24.317982 E | etcdmain: error verifying flags, --logger=capnslog supports only 1 value in '--log-outputs', got ["bar" "foo"]. See 'etcd --help'. Third, remove duplicated unique strings code. UniqueStringsFromFlag() is already available to return a sorted slice of values, so just use that. Lastly, fix a tiny logging typo in config. --- embed/config_logging.go | 7 +++---- etcdmain/config.go | 20 +++----------------- 2 files changed, 6 insertions(+), 21 deletions(-) diff --git a/embed/config_logging.go b/embed/config_logging.go index fc6406697..37b7ee421 100644 --- a/embed/config_logging.go +++ b/embed/config_logging.go @@ -103,8 +103,7 @@ func (cfg *Config) setupLogging() error { } if len(cfg.LogOutputs) != 1 { - fmt.Printf("--logger=capnslog supports only 1 value in '--log-outputs', got %q\n", cfg.LogOutputs) - os.Exit(1) + return fmt.Errorf("--logger=capnslog supports only 1 value in '--log-outputs', got %q", cfg.LogOutputs) } // capnslog initially SetFormatter(NewDefaultFormatter(os.Stderr)) // where NewDefaultFormatter returns NewJournaldFormatter when syscall.Getppid() == 1 @@ -117,7 +116,7 @@ func (cfg *Config) setupLogging() error { capnslog.SetFormatter(capnslog.NewPrettyFormatter(os.Stdout, cfg.Debug)) case DefaultLogOutput: default: - plog.Panicf(`unknown log-output %q (only supports %q, %q, %q)`, output, DefaultLogOutput, StdErrLogOutput, StdOutLogOutput) + return fmt.Errorf("unknown log-output %q (only supports %q, %q, %q)", output, DefaultLogOutput, StdErrLogOutput, StdOutLogOutput) } case "zap": @@ -127,7 +126,7 @@ func (cfg *Config) setupLogging() error { if len(cfg.LogOutputs) > 1 { for _, v := range cfg.LogOutputs { if v == DefaultLogOutput { - panic(fmt.Errorf("multi logoutput for %q is not supported yet", DefaultLogOutput)) + return fmt.Errorf("multi logoutput for %q is not supported yet", DefaultLogOutput) } } } diff --git a/etcdmain/config.go b/etcdmain/config.go index 045ab33de..78700e74d 100644 --- a/etcdmain/config.go +++ b/etcdmain/config.go @@ -24,7 +24,6 @@ import ( "net/url" "os" "runtime" - "sort" "strings" "go.etcd.io/etcd/embed" @@ -284,7 +283,7 @@ func (cfg *config) parse(arguments []string) error { err = cfg.configFromFile(cfg.configFile) if lg := cfg.ec.GetLogger(); lg != nil { lg.Info( - "loaded server configuraionl, other configuration command line flags and environment variables will be ignored if provided", + "loaded server configuration, other configuration command line flags and environment variables will be ignored if provided", zap.String("path", cfg.configFile), ) } else { @@ -315,21 +314,8 @@ func (cfg *config) configFromCmdLine() error { cfg.ec.CipherSuites = flags.StringsFromFlag(cfg.cf.flagSet, "cipher-suites") // TODO: remove this in v3.5 - output := flags.UniqueStringsMapFromFlag(cfg.cf.flagSet, "log-output") - oss1 := make([]string, 0, len(output)) - for v := range output { - oss1 = append(oss1, v) - } - sort.Strings(oss1) - cfg.ec.DeprecatedLogOutput = oss1 - - outputs := flags.UniqueStringsMapFromFlag(cfg.cf.flagSet, "log-outputs") - oss2 := make([]string, 0, len(outputs)) - for v := range outputs { - oss2 = append(oss2, v) - } - sort.Strings(oss2) - cfg.ec.LogOutputs = oss2 + cfg.ec.DeprecatedLogOutput = flags.UniqueStringsFromFlag(cfg.cf.flagSet, "log-output") + cfg.ec.LogOutputs = flags.UniqueStringsFromFlag(cfg.cf.flagSet, "log-outputs") cfg.ec.ClusterState = cfg.cf.clusterState.String() cfg.cp.Fallback = cfg.cf.fallback.String()