From 82d6af204abcc4df7a3b4fa25eb92ab352510b02 Mon Sep 17 00:00:00 2001 From: Luke Kysow <1034429+lkysow@users.noreply.github.com> Date: Fri, 2 Oct 2020 13:36:05 -0700 Subject: [PATCH 1/3] Add -log-level flag to controller --- go.mod | 2 ++ subcommand/controller/command.go | 44 ++++++++++++++++++++++++++- subcommand/controller/command_test.go | 4 +++ 3 files changed, 49 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index e5e4c00309..7fc8e6836b 100644 --- a/go.mod +++ b/go.mod @@ -29,6 +29,7 @@ require ( github.com/radovskyb/watcher v1.0.2 github.com/stretchr/testify v1.5.1 go.opencensus.io v0.22.0 // indirect + go.uber.org/zap v1.10.0 golang.org/x/net v0.0.0-20200625001655-4c5254603344 // indirect golang.org/x/oauth2 v0.0.0-20191202225959-858c2ad4c8b6 // indirect golang.org/x/sync v0.0.0-20200625203802-6e8e738ad208 // indirect @@ -40,6 +41,7 @@ require ( k8s.io/api v0.18.6 k8s.io/apimachinery v0.18.6 k8s.io/client-go v0.18.6 + k8s.io/klog/v2 v2.0.0 sigs.k8s.io/controller-runtime v0.6.3 ) diff --git a/subcommand/controller/command.go b/subcommand/controller/command.go index 8d14c00d81..511bc85ea5 100644 --- a/subcommand/controller/command.go +++ b/subcommand/controller/command.go @@ -10,9 +10,11 @@ import ( "github.com/hashicorp/consul-k8s/controller" "github.com/hashicorp/consul-k8s/subcommand/flags" "github.com/mitchellh/cli" + "go.uber.org/zap/zapcore" "k8s.io/apimachinery/pkg/runtime" utilruntime "k8s.io/apimachinery/pkg/util/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "k8s.io/klog/v2" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/log/zap" "sigs.k8s.io/controller-runtime/pkg/webhook" @@ -29,6 +31,7 @@ type Command struct { flagEnableLeaderElection bool flagEnableWebhooks bool flagDatacenter string + flagLogLevel string // Flags to support Consul Enterprise namespaces. flagEnableNamespaces bool @@ -46,6 +49,13 @@ var ( setupLog = ctrl.Log.WithName("setup") ) +const ( + LogLevelDebug = "debug" + LogLevelInfo = "info" + LogLevelWarn = "warn" + LogLevelError = "error" +) + func init() { utilruntime.Must(clientgoscheme.AddToScheme(scheme)) utilruntime.Must(v1alpha1.AddToScheme(scheme)) @@ -75,6 +85,9 @@ func (c *Command) init() { "Directory that contains the TLS cert and key required for the webhook. The cert and key files must be named 'tls.crt' and 'tls.key' respectively.") c.flagSet.BoolVar(&c.flagEnableWebhooks, "enable-webhooks", true, "Enable webhooks. Disable when running locally since Kube API server won't be able to route to local server.") + c.flagSet.StringVar(&c.flagLogLevel, "log-level", LogLevelInfo, + fmt.Sprintf("Log verbosity level. Supported values (in order of detail) are "+ + "%q, %q, %q, and %q.", LogLevelDebug, LogLevelInfo, LogLevelWarn, LogLevelError)) c.httpFlags = &flags.HTTPFlags{} flags.Merge(c.flagSet, c.httpFlags.Flags()) @@ -82,7 +95,6 @@ func (c *Command) init() { } func (c *Command) Run(args []string) int { - ctrl.SetLogger(zap.New(zap.UseDevMode(true))) c.once.Do(c.init) if err := c.flagSet.Parse(args); err != nil { c.UI.Error(fmt.Sprintf("Parsing flagset: %s", err.Error())) @@ -101,11 +113,23 @@ func (c *Command) Run(args []string) int { return 1 } + zapLevel, err := toLevel(c.flagLogLevel) + if err != nil { + c.UI.Error(err.Error()) + return 1 + } + // We set UseDevMode to true because we don't want our logs json + // formatted. + logger := zap.New(zap.UseDevMode(true), zap.Level(zapLevel)) + ctrl.SetLogger(logger) + klog.SetLogger(logger) + mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ Scheme: scheme, Port: 9443, LeaderElection: c.flagEnableLeaderElection, LeaderElectionID: "consul.hashicorp.com", + Logger: logger, }) if err != nil { setupLog.Error(err, "unable to start manager") @@ -231,6 +255,24 @@ func (c *Command) Run(args []string) int { return 0 } +// toLevel returns the zapcore level and an error if lvl is not supported. +func toLevel(lvl string) (zapcore.Level, error) { + switch lvl { + case LogLevelDebug: + return zapcore.DebugLevel, nil + case LogLevelInfo: + return zapcore.InfoLevel, nil + case LogLevelWarn: + return zapcore.WarnLevel, nil + case LogLevelError: + return zapcore.ErrorLevel, nil + default: + return zapcore.DebugLevel, + fmt.Errorf("invalid -log-level %q, must be one of %q, %q, %q, %q", + lvl, LogLevelDebug, LogLevelInfo, LogLevelWarn, LogLevelError) + } +} + func (c *Command) Help() string { c.once.Do(c.init) return c.help diff --git a/subcommand/controller/command_test.go b/subcommand/controller/command_test.go index b5bf76619e..4aaa772a78 100644 --- a/subcommand/controller/command_test.go +++ b/subcommand/controller/command_test.go @@ -26,6 +26,10 @@ func TestRun_FlagValidation(t *testing.T) { flags: []string{"-webhook-tls-cert-dir", "/foo"}, expErr: "-datacenter must be set", }, + { + flags: []string{"-webhook-tls-cert-dir", "/foo", "-datacenter", "foo", "-log-level", "invalid"}, + expErr: `invalid -log-level "invalid", must be one of "debug", "info", "warn", "error"`, + }, } for _, c := range cases { From 8b684be20602d0978cc0c55df3db89e7ae8c1e2c Mon Sep 17 00:00:00 2001 From: Luke Kysow <1034429+lkysow@users.noreply.github.com> Date: Wed, 7 Oct 2020 11:47:35 -0700 Subject: [PATCH 2/3] Switch to using zapcore.UnmarshalText --- subcommand/controller/command.go | 24 +++--------------------- subcommand/controller/command_test.go | 2 +- 2 files changed, 4 insertions(+), 22 deletions(-) diff --git a/subcommand/controller/command.go b/subcommand/controller/command.go index 511bc85ea5..3d5443cb72 100644 --- a/subcommand/controller/command.go +++ b/subcommand/controller/command.go @@ -113,9 +113,9 @@ func (c *Command) Run(args []string) int { return 1 } - zapLevel, err := toLevel(c.flagLogLevel) - if err != nil { - c.UI.Error(err.Error()) + var zapLevel zapcore.Level + if err := zapLevel.UnmarshalText([]byte(c.flagLogLevel)); err != nil { + c.UI.Error(fmt.Sprintf("Error parsing -log-level %q: %s", c.flagLogLevel, err.Error())) return 1 } // We set UseDevMode to true because we don't want our logs json @@ -255,24 +255,6 @@ func (c *Command) Run(args []string) int { return 0 } -// toLevel returns the zapcore level and an error if lvl is not supported. -func toLevel(lvl string) (zapcore.Level, error) { - switch lvl { - case LogLevelDebug: - return zapcore.DebugLevel, nil - case LogLevelInfo: - return zapcore.InfoLevel, nil - case LogLevelWarn: - return zapcore.WarnLevel, nil - case LogLevelError: - return zapcore.ErrorLevel, nil - default: - return zapcore.DebugLevel, - fmt.Errorf("invalid -log-level %q, must be one of %q, %q, %q, %q", - lvl, LogLevelDebug, LogLevelInfo, LogLevelWarn, LogLevelError) - } -} - func (c *Command) Help() string { c.once.Do(c.init) return c.help diff --git a/subcommand/controller/command_test.go b/subcommand/controller/command_test.go index 4aaa772a78..50ba5b963a 100644 --- a/subcommand/controller/command_test.go +++ b/subcommand/controller/command_test.go @@ -28,7 +28,7 @@ func TestRun_FlagValidation(t *testing.T) { }, { flags: []string{"-webhook-tls-cert-dir", "/foo", "-datacenter", "foo", "-log-level", "invalid"}, - expErr: `invalid -log-level "invalid", must be one of "debug", "info", "warn", "error"`, + expErr: `Error parsing -log-level "invalid": unrecognized level: "invalid"`, }, } From 5cfc6b53561e699494a5e53046b77969d2aef86b Mon Sep 17 00:00:00 2001 From: Luke Kysow <1034429+lkysow@users.noreply.github.com> Date: Wed, 7 Oct 2020 11:50:21 -0700 Subject: [PATCH 3/3] Use zapcore constants --- subcommand/controller/command.go | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/subcommand/controller/command.go b/subcommand/controller/command.go index 3d5443cb72..de1349bdc3 100644 --- a/subcommand/controller/command.go +++ b/subcommand/controller/command.go @@ -49,13 +49,6 @@ var ( setupLog = ctrl.Log.WithName("setup") ) -const ( - LogLevelDebug = "debug" - LogLevelInfo = "info" - LogLevelWarn = "warn" - LogLevelError = "error" -) - func init() { utilruntime.Must(clientgoscheme.AddToScheme(scheme)) utilruntime.Must(v1alpha1.AddToScheme(scheme)) @@ -85,9 +78,9 @@ func (c *Command) init() { "Directory that contains the TLS cert and key required for the webhook. The cert and key files must be named 'tls.crt' and 'tls.key' respectively.") c.flagSet.BoolVar(&c.flagEnableWebhooks, "enable-webhooks", true, "Enable webhooks. Disable when running locally since Kube API server won't be able to route to local server.") - c.flagSet.StringVar(&c.flagLogLevel, "log-level", LogLevelInfo, + c.flagSet.StringVar(&c.flagLogLevel, "log-level", zapcore.InfoLevel.String(), fmt.Sprintf("Log verbosity level. Supported values (in order of detail) are "+ - "%q, %q, %q, and %q.", LogLevelDebug, LogLevelInfo, LogLevelWarn, LogLevelError)) + "%q, %q, %q, and %q.", zapcore.DebugLevel.String(), zapcore.InfoLevel.String(), zapcore.WarnLevel.String(), zapcore.ErrorLevel.String())) c.httpFlags = &flags.HTTPFlags{} flags.Merge(c.flagSet, c.httpFlags.Flags())