Add -log-level flag to inject-connect (#400)

Also standardize on how we construct logger throughout commands.

CHANGELOG.md

@@ -1,5 +1,8 @@
 ## UNRELEASED
 
+IMPROVEMENTS:
+* Connect: Add `-log-level` flag to `inject-connect` command. [[GH-400](https://github.com/hashicorp/consul-k8s/pull/400)]
+
 ## 0.20.0 (November 12, 2020)
 
 FEATURES:

connect-inject/health_check_resource.go

@@ -177,6 +177,7 @@ func (h *HealthCheckResource) reconcilePod(pod *corev1.Pod) error {
 		} else if err != nil {
 			return fmt.Errorf("unable to register health check: %s", err)
 		}
+		h.Log.Debug("updating health check status", "name", pod.Name, "status", status, "reason", reason)
 		// Also update it, the reason this is separate is there is no way to set the Output field of the health check
 		// at creation time, and this is what is displayed on the UI as opposed to the Notes field.
 		err = h.updateConsulHealthCheckStatus(client, healthCheckID, status, reason)
@@ -185,6 +186,7 @@ func (h *HealthCheckResource) reconcilePod(pod *corev1.Pod) error {
 		}
 	} else if serviceCheck.Status != status {
 		// Update the healthCheck.
+		h.Log.Debug("updating health check status", "name", pod.Name, "status", status, "reason", reason)
 		err = h.updateConsulHealthCheckStatus(client, healthCheckID, status, reason)
 		if err != nil {
 			return fmt.Errorf("error updating health check: %s", err)

subcommand/common/common.go

@@ -1,6 +1,13 @@
 // Package common holds code needed by multiple commands.
 package common
 
+import (
+	"fmt"
+	"os"
+
+	"github.com/hashicorp/go-hclog"
+)
+
 const (
 	// ACLReplicationTokenName is the name used for the ACL replication policy and
 	// Kubernetes secret. It is consumed in both the server-acl-init and
@@ -11,3 +18,15 @@ const (
 	// create Kubernetes secrets.
 	ACLTokenSecretKey = "token"
 )
+
+// Logger returns an hclog instance or an error if level is invalid.
+func Logger(level string) (hclog.Logger, error) {
+	parsedLevel := hclog.LevelFromString(level)
+	if parsedLevel == hclog.NoLevel {
+		return nil, fmt.Errorf("unknown log level: %s", level)
+	}
+	return hclog.New(&hclog.LoggerOptions{
+		Level:  parsedLevel,
+		Output: os.Stderr,
+	}), nil
+}

subcommand/common/common_test.go

@@ -0,0 +1,19 @@
+package common
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestLogger_InvalidLogLevel(t *testing.T) {
+	_, err := Logger("invalid")
+	require.EqualError(t, err, "unknown log level: invalid")
+}
+
+func TestLogger(t *testing.T) {
+	lgr, err := Logger("debug")
+	require.NoError(t, err)
+	require.NotNil(t, lgr)
+	require.True(t, lgr.IsDebug())
+}

subcommand/create-federation-secret/command.go

@@ -7,7 +7,6 @@ import (
 	"flag"
 	"fmt"
 	"io/ioutil"
-	"os"
 	"strings"
 	"sync"
 	"time"
@@ -108,16 +107,11 @@ func (c *Command) Run(args []string) int {
 		return 1
 	}
 
-	// Create logger.
-	level := hclog.LevelFromString(c.flagLogLevel)
-	if level == hclog.NoLevel {
-		c.UI.Error(fmt.Sprintf("Unknown log level: %s", c.flagLogLevel))
+	logger, err := common.Logger(c.flagLogLevel)
+	if err != nil {
+		c.UI.Error(err.Error())
 		return 1
 	}
-	logger := hclog.New(&hclog.LoggerOptions{
-		Level:  level,
-		Output: os.Stderr,
-	})
 
 	// The initial secret struct. We will be filling in its data map
 	// as we continue.

subcommand/create-federation-secret/command_test.go

@@ -68,7 +68,7 @@ func TestRun_FlagValidation(t *testing.T) {
 				"-mesh-gateway-service-name=name",
 				"-log-level=invalid",
 			},
-			expErr: "Unknown log level: invalid",
+			expErr: "unknown log level: invalid",
 		},
 	}
 

subcommand/delete-completed-job/command.go

@@ -4,13 +4,12 @@ import (
 	"context"
 	"flag"
 	"fmt"
-	"os"
 	"sync"
 	"time"
 
 	"github.com/hashicorp/consul-k8s/subcommand"
+	"github.com/hashicorp/consul-k8s/subcommand/common"
 	"github.com/hashicorp/consul-k8s/subcommand/flags"
-	"github.com/hashicorp/go-hclog"
 	"github.com/mitchellh/cli"
 	v1 "k8s.io/api/batch/v1"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
@@ -19,6 +18,8 @@ import (
 	_ "k8s.io/client-go/plugin/pkg/client/auth/gcp"
 )
 
+const logLevel = "info"
+
 // Command is the command for deleting completed jobs.
 type Command struct {
 	UI cli.Ui
@@ -95,10 +96,11 @@ func (c *Command) Run(args []string) int {
 		}
 	}
 
-	logger := hclog.New(&hclog.LoggerOptions{
-		Level:  hclog.Info,
-		Output: os.Stderr,
-	})
+	logger, err := common.Logger(logLevel)
+	if err != nil {
+		c.UI.Error(err.Error())
+		return 1
+	}
 
 	// Wait for job to complete.
 	logger.Info(fmt.Sprintf("waiting for job %q to complete successfully", jobName))

subcommand/get-consul-client-ca/command.go

@@ -4,13 +4,13 @@ import (
 	"flag"
 	"fmt"
 	"io/ioutil"
-	"os"
 	"strings"
 	"sync"
 	"time"
 
 	"github.com/cenkalti/backoff"
 	godiscover "github.com/hashicorp/consul-k8s/helper/go-discover"
+	"github.com/hashicorp/consul-k8s/subcommand/common"
 	"github.com/hashicorp/consul-k8s/subcommand/flags"
 	"github.com/hashicorp/consul/api"
 	"github.com/hashicorp/go-discover"
@@ -81,16 +81,11 @@ func (c *Command) Run(args []string) int {
 		return 1
 	}
 
-	// create a logger
-	level := hclog.LevelFromString(c.flagLogLevel)
-	if level == hclog.NoLevel {
-		c.UI.Error(fmt.Sprintf("Unknown log level: %s", c.flagLogLevel))
+	logger, err := common.Logger(c.flagLogLevel)
+	if err != nil {
+		c.UI.Error(err.Error())
 		return 1
 	}
-	logger := hclog.New(&hclog.LoggerOptions{
-		Level:  level,
-		Output: os.Stderr,
-	})
 
 	// create Consul client
 	consulClient, err := c.consulClient(logger)

subcommand/get-consul-client-ca/command_test.go

@@ -44,7 +44,7 @@ func TestRun_FlagsValidation(t *testing.T) {
 				"-server-addr=foo.com",
 				"-log-level=invalid-log-level",
 			},
-			expErr: "Unknown log level: invalid-log-level",
+			expErr: "unknown log level: invalid-log-level",
 		},
 	}
 

subcommand/inject-connect/command.go

@@ -20,9 +20,9 @@ import (
 	connectinject "github.com/hashicorp/consul-k8s/connect-inject"
 	"github.com/hashicorp/consul-k8s/helper/cert"
 	"github.com/hashicorp/consul-k8s/helper/controller"
+	"github.com/hashicorp/consul-k8s/subcommand/common"
 	"github.com/hashicorp/consul-k8s/subcommand/flags"
 	"github.com/hashicorp/consul/api"
-	"github.com/hashicorp/go-hclog"
 	"github.com/mitchellh/cli"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
@@ -49,6 +49,7 @@ type Command struct {
 	flagDefaultProtocol      string // Default protocol for use with central config
 	flagConsulCACert         string // [Deprecated] Path to CA Certificate to use when communicating with Consul clients
 	flagEnvoyExtraArgs       string // Extra envoy args when starting envoy
+	flagLogLevel             string
 
 	// Flags to support namespaces
 	flagEnableNamespaces           bool     // Use namespacing on all components
@@ -140,6 +141,9 @@ func (c *Command) init() {
 	c.flagSet.StringVar(&c.flagCrossNamespaceACLPolicy, "consul-cross-namespace-acl-policy", "",
 		"[Enterprise Only] Name of the ACL policy to attach to all created Consul namespaces to allow service "+
 			"discovery across Consul namespaces. Only necessary if ACLs are enabled.")
+	c.flagSet.StringVar(&c.flagLogLevel, "log-level", "info",
+		"Log verbosity level. Supported values (in order of detail) are \"trace\", "+
+			"\"debug\", \"info\", \"warn\", and \"error\".")
 
 	// Proxy sidecar resource setting flags.
 	c.flagSet.StringVar(&c.flagDefaultSidecarProxyCPURequest, "default-sidecar-proxy-cpu-request", "", "Default sidecar proxy CPU request.")
@@ -184,9 +188,14 @@ func (c *Command) Run(args []string) int {
 		return 1
 	}
 
+	logger, err := common.Logger(c.flagLogLevel)
+	if err != nil {
+		c.UI.Error(err.Error())
+		return 1
+	}
+
 	// Proxy resources
 	var sidecarProxyCPULimit, sidecarProxyCPURequest, sidecarProxyMemoryLimit, sidecarProxyMemoryRequest resource.Quantity
-	var err error
 	if c.flagDefaultSidecarProxyCPURequest != "" {
 		sidecarProxyCPURequest, err = resource.ParseQuantity(c.flagDefaultSidecarProxyCPURequest)
 		if err != nil {
@@ -329,7 +338,7 @@ func (c *Command) Run(args []string) int {
 		EnableK8SNSMirroring:       c.flagEnableK8SNSMirroring,
 		K8SNSMirroringPrefix:       c.flagK8SNSMirroringPrefix,
 		CrossNamespaceACLPolicy:    c.flagCrossNamespaceACLPolicy,
-		Log:                        hclog.Default().Named("handler"),
+		Log:                        logger.Named("handler"),
 	}
 	mux := http.NewServeMux()
 	mux.HandleFunc("/mutate", injector.Handle)
@@ -365,15 +374,15 @@ func (c *Command) Run(args []string) int {
 		}()
 
 		healthResource := connectinject.HealthCheckResource{
-			Log:                 hclog.Default().Named("healthCheckResource"),
+			Log:                 logger.Named("healthCheckResource"),
 			KubernetesClientset: c.clientset,
 			ConsulUrl:           consulUrl,
 			Ctx:                 ctx,
 			ReconcilePeriod:     c.flagHealthChecksReconcilePeriod,
 		}
 
 		ctl := &controller.Controller{
-			Log:      hclog.Default().Named("healthCheckController"),
+			Log:      logger.Named("healthCheckController"),
 			Resource: &healthResource,
 		}
 

subcommand/inject-connect/command_test.go

@@ -23,6 +23,10 @@ func TestRun_FlagValidation(t *testing.T) {
 			flags:  []string{},
 			expErr: "-consul-k8s-image must be set",
 		},
+		{
+			flags:  []string{"-consul-k8s-image", "foo", "-log-level", "invalid"},
+			expErr: "unknown log level: invalid",
+		},
 		{
 			flags:  []string{"-consul-k8s-image", "foo", "-ca-file", "bar"},
 			expErr: "Error reading Consul's CA cert file \"bar\"",

subcommand/lifecycle-sidecar/command.go

@@ -12,8 +12,8 @@ import (
 	"syscall"
 	"time"
 
+	"github.com/hashicorp/consul-k8s/subcommand/common"
 	"github.com/hashicorp/consul-k8s/subcommand/flags"
-	"github.com/hashicorp/go-hclog"
 	"github.com/mitchellh/cli"
 )
 
@@ -74,10 +74,11 @@ func (c *Command) Run(args []string) int {
 		return 1
 	}
 
-	logger := hclog.New(&hclog.LoggerOptions{
-		Level:  hclog.LevelFromString(c.flagLogLevel),
-		Output: os.Stderr,
-	})
+	logger, err := common.Logger(c.flagLogLevel)
+	if err != nil {
+		c.UI.Error(err.Error())
+		return 1
+	}
 
 	// Log initial configuration
 	logger.Info("Command configuration", "service-config", c.flagServiceConfig,
@@ -118,7 +119,7 @@ func (c *Command) Run(args []string) int {
 	}
 }
 
-// validateFlags validates the flags and returns the logLevel.
+// validateFlags validates the flags.
 func (c *Command) validateFlags() error {
 	if c.flagServiceConfig == "" {
 		return errors.New("-service-config must be set")
@@ -142,10 +143,6 @@ func (c *Command) validateFlags() error {
 	if err != nil {
 		return fmt.Errorf("-consul-binary %q not found: %s", c.flagConsulBinary, err)
 	}
-	logLevel := hclog.LevelFromString(c.flagLogLevel)
-	if logLevel == hclog.NoLevel {
-		return fmt.Errorf("unknown log level: %s", c.flagLogLevel)
-	}
 
 	return nil
 }

subcommand/server-acl-init/command.go

@@ -6,7 +6,6 @@ import (
 	"flag"
 	"fmt"
 	"io/ioutil"
-	"os"
 	"regexp"
 	"strings"
 	"sync"
@@ -276,16 +275,12 @@ func (c *Command) Run(args []string) int {
 	// The context will only ever be intentionally ended by the timeout.
 	defer cancel()
 
-	// Configure our logger
-	level := hclog.LevelFromString(c.flagLogLevel)
-	if level == hclog.NoLevel {
-		c.UI.Error(fmt.Sprintf("Unknown log level: %s", c.flagLogLevel))
+	var err error
+	c.log, err = common.Logger(c.flagLogLevel)
+	if err != nil {
+		c.UI.Error(err.Error())
 		return 1
 	}
-	c.log = hclog.New(&hclog.LoggerOptions{
-		Level:  level,
-		Output: os.Stderr,
-	})
 
 	serverAddresses := c.flagServerAddresses
 	// Check if the provided addresses contain a cloud-auto join string.

subcommand/service-address/command.go

@@ -7,7 +7,6 @@ import (
 	"fmt"
 	"io/ioutil"
 	"net"
-	"os"
 	"sync"
 	"time"
 
@@ -79,15 +78,12 @@ func (c *Command) Run(args []string) int {
 	if c.retryDuration == 0 {
 		c.retryDuration = 1 * time.Second
 	}
-	log := hclog.New(&hclog.LoggerOptions{
-		Level:  hclog.Info,
-		Output: os.Stderr,
-	})
+	logger := hclog.Default()
 
 	// Run until we get an address from the service.
 	var address string
 	var unretryableErr error
-	backoff.Retry(withErrLogger(log, func() error {
+	backoff.Retry(withErrLogger(logger, func() error {
 		svc, err := c.k8sClient.CoreV1().Services(c.flagNamespace).Get(context.TODO(), c.flagServiceName, metav1.GetOptions{})
 		if err != nil {
 			return fmt.Errorf("getting service %s: %s", c.flagServiceName, err)