Use global slog logger instead of log

Signed-off-by: Richard Wall <[email protected]>

Author: wallrj

pkg/agent/config.go

@@ -3,11 +3,11 @@ package agent
 import (
 	"fmt"
 	"io"
-	"log"
 	"net/url"
 	"os"
 	"time"
 
+	"github.com/go-logr/logr"
 	"github.com/hashicorp/go-multierror"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
@@ -355,7 +355,8 @@ type CombinedConfig struct {
 // The error returned may be a multierror.Error. Use multierror.Prefix(err,
 // "context:") rather than fmt.Errorf("context: %w", err) when wrapping the
 // error.
-func ValidateAndCombineConfig(log *log.Logger, cfg Config, flags AgentCmdFlags) (CombinedConfig, client.Client, error) {
+func ValidateAndCombineConfig(log logr.Logger, cfg Config, flags AgentCmdFlags) (CombinedConfig, client.Client, error) {
+
 	res := CombinedConfig{}
 	var errs error
 
@@ -364,23 +365,23 @@ func ValidateAndCombineConfig(log *log.Logger, cfg Config, flags AgentCmdFlags)
 		switch {
 		case flags.VenafiCloudMode && flags.CredentialsPath != "":
 			mode = VenafiCloudKeypair
-			log.Printf("Using the %s auth mode since --venafi-cloud and --credentials-path were specified.", mode)
+			log.Info(fmt.Sprintf("Using the %s auth mode since --venafi-cloud and --credentials-path were specified.", mode))
 		case flags.ClientID != "" && flags.PrivateKeyPath != "":
 			mode = VenafiCloudKeypair
-			log.Printf("Using the %s auth mode since --client-id and --private-key-path were specified.", mode)
+			log.Info(fmt.Sprintf("Using the %s auth mode since --client-id and --private-key-path were specified.", mode))
 		case flags.ClientID != "":
 			return CombinedConfig{}, nil, fmt.Errorf("if --client-id is specified, --private-key-path must also be specified")
 		case flags.PrivateKeyPath != "":
 			return CombinedConfig{}, nil, fmt.Errorf("--private-key-path is specified, --client-id must also be specified")
 		case flags.VenConnName != "":
 			mode = VenafiCloudVenafiConnection
-			log.Printf("Using the %s auth mode since --venafi-connection was specified.", mode)
+			log.Info(fmt.Sprintf("Using the %s auth mode since --venafi-connection was specified.", mode))
 		case flags.APIToken != "":
 			mode = JetstackSecureAPIToken
-			log.Printf("Using the %s auth mode since --api-token was specified.", mode)
+			log.Info(fmt.Sprintf("Using the %s auth mode since --api-token was specified.", mode))
 		case !flags.VenafiCloudMode && flags.CredentialsPath != "":
 			mode = JetstackSecureOAuth
-			log.Printf("Using the %s auth mode since --credentials-file was specified without --venafi-cloud.", mode)
+			log.Info(fmt.Sprintf("Using the %s auth mode since --credentials-file was specified without --venafi-cloud.", mode))
 		default:
 			return CombinedConfig{}, nil, fmt.Errorf("no auth mode specified. You can use one of four auth modes:\n" +
 				" - Use (--venafi-cloud with --credentials-file) or (--client-id with --private-key-path) to use the " + string(VenafiCloudKeypair) + " mode.\n" +
@@ -403,10 +404,10 @@ func ValidateAndCombineConfig(log *log.Logger, cfg Config, flags AgentCmdFlags)
 		case hasServerField && hasEndpointField:
 			// The `server` field takes precedence over the deprecated
 			// `endpoint` field.
-			log.Printf("The `server` and `endpoint` fields are both set in the config; using the `server` field.")
+			log.Info("The `server` and `endpoint` fields are both set in the config; using the `server` field.")
 			server = cfg.Server
 		case !hasServerField && hasEndpointField:
-			log.Printf("Using deprecated Endpoint configuration. User Server instead.")
+			log.Info("Using deprecated Endpoint configuration. User Server instead.")
 			if cfg.Endpoint.Protocol == "" && cfg.Server == "" {
 				cfg.Endpoint.Protocol = "http"
 			}
@@ -424,7 +425,7 @@ func ValidateAndCombineConfig(log *log.Logger, cfg Config, flags AgentCmdFlags)
 			errs = multierror.Append(errs, fmt.Errorf("server %q is not a valid URL", server))
 		}
 		if res.AuthMode == VenafiCloudVenafiConnection && server != "" {
-			log.Printf("ignoring the server field specified in the config file. In %s mode, this field is not needed.", VenafiCloudVenafiConnection)
+			log.Info(fmt.Sprintf("Ignoring the server field specified in the config file. In %s mode, this field is not needed.", VenafiCloudVenafiConnection))
 			server = ""
 		}
 		res.Server = server
@@ -454,7 +455,7 @@ func ValidateAndCombineConfig(log *log.Logger, cfg Config, flags AgentCmdFlags)
 			// change this value with the new --venafi-connection flag, and this
 			// field is simply ignored.
 			if cfg.VenafiCloud != nil && cfg.VenafiCloud.UploadPath != "" {
-				log.Printf(`ignoring the venafi-cloud.upload_path field in the config file. In %s mode, this field is not needed.`, res.AuthMode)
+				log.Info(fmt.Sprintf(`Ignoring the venafi-cloud.upload_path field in the config file. In %s mode, this field is not needed.`, res.AuthMode))
 			}
 			uploadPath = ""
 		}
@@ -472,7 +473,7 @@ func ValidateAndCombineConfig(log *log.Logger, cfg Config, flags AgentCmdFlags)
 	// https://venafi.atlassian.net/browse/VC-35385 is done.
 	{
 		if cfg.VenafiCloud != nil && cfg.VenafiCloud.UploaderID != "" {
-			log.Printf(`ignoring the venafi-cloud.uploader_id field in the config file. This field is not needed in %s mode.`, res.AuthMode)
+			log.Info(fmt.Sprintf(`Ignoring the venafi-cloud.uploader_id field in the config file. This field is not needed in %s mode.`, res.AuthMode))
 		}
 	}
 
@@ -524,13 +525,13 @@ func ValidateAndCombineConfig(log *log.Logger, cfg Config, flags AgentCmdFlags)
 		case flags.Period == 0 && cfg.Period == 0:
 			errs = multierror.Append(errs, fmt.Errorf("period must be set using --period or -p, or using the 'period' field in the config file"))
 		case flags.Period == 0 && cfg.Period > 0:
-			log.Printf("Using period from config %s", cfg.Period)
+			log.Info("Using period from config", "period", cfg.Period)
 			period = cfg.Period
 		case flags.Period > 0 && cfg.Period == 0:
 			period = flags.Period
 		case flags.Period > 0 && cfg.Period > 0:
 			// The flag takes precedence.
-			log.Printf("Both the 'period' field and --period are set. Using the value provided with --period.")
+			log.Info("Both the 'period' field and --period are set. Using the value provided with --period.")
 			period = flags.Period
 		}
 		res.Period = period
@@ -599,7 +600,7 @@ func ValidateAndCombineConfig(log *log.Logger, cfg Config, flags AgentCmdFlags)
 // The error returned may be a multierror.Error. Use multierror.Prefix(err,
 // "context:") rather than fmt.Errorf("context: %w", err) when wrapping the
 // error.
-func validateCredsAndCreateClient(log *log.Logger, flagCredentialsPath, flagClientID, flagPrivateKeyPath, flagAPIToken string, cfg CombinedConfig) (client.Client, error) {
+func validateCredsAndCreateClient(log logr.Logger, flagCredentialsPath, flagClientID, flagPrivateKeyPath, flagAPIToken string, cfg CombinedConfig) (client.Client, error) {
 	var errs error
 
 	var preflightClient client.Client
@@ -719,7 +720,7 @@ func ValidateDataGatherers(dataGatherers []DataGatherer) error {
 
 // The error returned may be a multierror.Error. Instead of adding context to
 // the error with fmt.Errorf("%w", err), use multierror.Prefix(err, "context").
-func createCredentialClient(log *log.Logger, credentials client.Credentials, cfg CombinedConfig, agentMetadata *api.AgentMetadata) (client.Client, error) {
+func createCredentialClient(log logr.Logger, credentials client.Credentials, cfg CombinedConfig, agentMetadata *api.AgentMetadata) (client.Client, error) {
 	switch creds := credentials.(type) {
 	case *client.VenafiSvcAccountCredentials:
 		// The uploader ID isn't actually used in the backend, let's use an
@@ -730,7 +731,7 @@ func createCredentialClient(log *log.Logger, credentials client.Credentials, cfg
 		if cfg.AuthMode == VenafiCloudKeypair {
 			// We don't do this for the VenafiCloudVenafiConnection mode because
 			// the upload_path field is ignored in that mode.
-			log.Println("Loading upload_path from \"venafi-cloud\" configuration.")
+			log.Info("Loading upload_path from \"venafi-cloud\" configuration.")
 			uploadPath = cfg.UploadPath
 		}
 		return client.NewVenafiCloudClient(agentMetadata, creds, cfg.Server, uploaderID, uploadPath, cfg.DisableCompression)

pkg/agent/config_test.go

@@ -6,15 +6,16 @@ import (
 	"context"
 	"fmt"
 	"io"
-	"log"
 	"net/http"
 	"os"
 	"testing"
 	"time"
 
+	"github.com/go-logr/logr"
 	"github.com/spf13/cobra"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"k8s.io/klog/v2/ktesting"
 
 	"github.com/jetstack/preflight/pkg/client"
 	"github.com/jetstack/preflight/pkg/testutil"
@@ -86,7 +87,7 @@ func Test_ValidateAndCombineConfig(t *testing.T) {
 
 	t.Run("--period flag takes precedence over period field in config, shows warning", func(t *testing.T) {
 		t.Setenv("POD_NAMESPACE", "venafi")
-		log, gotLogs := recordLogs()
+		log, gotLogs := recordLogs(t)
 		got, _, err := ValidateAndCombineConfig(log,
 			withConfig(testutil.Undent(`
 				server: https://api.venafi.eu
@@ -97,8 +98,8 @@ func Test_ValidateAndCombineConfig(t *testing.T) {
 			withCmdLineFlags("--period", "99m", "--credentials-file", fakeCredsPath))
 		require.NoError(t, err)
 		assert.Equal(t, testutil.Undent(`
-			Using the Jetstack Secure OAuth auth mode since --credentials-file was specified without --venafi-cloud.
-			Both the 'period' field and --period are set. Using the value provided with --period.
+			INFO Using the Jetstack Secure OAuth auth mode since --credentials-file was specified without --venafi-cloud.
+			INFO Both the 'period' field and --period are set. Using the value provided with --period.
 		`), gotLogs.String())
 		assert.Equal(t, 99*time.Minute, got.Period)
 	})
@@ -573,7 +574,7 @@ func Test_ValidateAndCombineConfig(t *testing.T) {
 	t.Run("venafi-cloud-workload-identity-auth: warning about server, venafi-cloud.uploader_id, and venafi-cloud.upload_path being skipped", func(t *testing.T) {
 		t.Setenv("POD_NAMESPACE", "venafi")
 		t.Setenv("KUBECONFIG", withFile(t, fakeKubeconfig))
-		log, gotLogs := recordLogs()
+		log, gotLogs := recordLogs(t)
 		got, gotCl, err := ValidateAndCombineConfig(log,
 			withConfig(testutil.Undent(`
 				server: https://api.venafi.eu
@@ -587,11 +588,11 @@ func Test_ValidateAndCombineConfig(t *testing.T) {
 		)
 		require.NoError(t, err)
 		assert.Equal(t, testutil.Undent(`
-			Using the Venafi Cloud VenafiConnection auth mode since --venafi-connection was specified.
-			ignoring the server field specified in the config file. In Venafi Cloud VenafiConnection mode, this field is not needed.
-			ignoring the venafi-cloud.upload_path field in the config file. In Venafi Cloud VenafiConnection mode, this field is not needed.
-			ignoring the venafi-cloud.uploader_id field in the config file. This field is not needed in Venafi Cloud VenafiConnection mode.
-			Using period from config 1h0m0s
+			INFO Using the Venafi Cloud VenafiConnection auth mode since --venafi-connection was specified.
+			INFO Ignoring the server field specified in the config file. In Venafi Cloud VenafiConnection mode, this field is not needed.
+			INFO Ignoring the venafi-cloud.upload_path field in the config file. In Venafi Cloud VenafiConnection mode, this field is not needed.
+			INFO Ignoring the venafi-cloud.uploader_id field in the config file. This field is not needed in Venafi Cloud VenafiConnection mode.
+			INFO Using period from config period="1h0m0s"
 		`), gotLogs.String())
 		assert.Equal(t, VenafiCloudVenafiConnection, got.AuthMode)
 		assert.IsType(t, &client.VenConnClient{}, gotCl)
@@ -994,13 +995,15 @@ func withFile(t testing.TB, content string) string {
 	return f.Name()
 }
 
-func recordLogs() (*log.Logger, *bytes.Buffer) {
-	b := bytes.Buffer{}
-	return log.New(&b, "", 0), &b
+func recordLogs(t *testing.T) (logr.Logger, ktesting.Buffer) {
+	log := ktesting.NewLogger(t, ktesting.NewConfig(ktesting.BufferLogs(true)))
+	testingLogger, ok := log.GetSink().(ktesting.Underlier)
+	require.True(t, ok)
+	return log, testingLogger.GetBuffer()
 }
 
-func discardLogs() *log.Logger {
-	return log.New(io.Discard, "", 0)
+func discardLogs() logr.Logger {
+	return logr.Discard()
 }
 
 // Shortcut for ParseConfig.

pkg/agent/run.go

@@ -7,13 +7,15 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"log/slog"
 	"net"
 	"net/http"
 	"os"
 	"strings"
 	"time"
 
 	"github.com/cenkalti/backoff"
+	"github.com/go-logr/logr"
 	"github.com/hashicorp/go-multierror"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/promhttp"
@@ -33,7 +35,6 @@ import (
 	"github.com/jetstack/preflight/pkg/client"
 	"github.com/jetstack/preflight/pkg/datagatherer"
 	"github.com/jetstack/preflight/pkg/kubeconfig"
-	"github.com/jetstack/preflight/pkg/logs"
 	"github.com/jetstack/preflight/pkg/version"
 
 	"net/http/pprof"
@@ -51,7 +52,6 @@ const schemaVersion string = "v2.0.0"
 
 // Run starts the agent process
 func Run(cmd *cobra.Command, args []string) error {
-	logs.Log.Printf("Preflight agent version: %s (%s)", version.PreflightVersion, version.Commit)
 	ctx, cancel := context.WithCancel(
 		klog.NewContext(
 			context.Background(),
@@ -60,6 +60,8 @@ func Run(cmd *cobra.Command, args []string) error {
 	)
 	defer cancel()
 
+	slog.Info("Starting agent", "version", version.PreflightVersion, "commit", version.Commit)
+
 	file, err := os.Open(Flags.ConfigFilePath)
 	if err != nil {
 		return fmt.Errorf("Failed to load config file for agent from: %s", Flags.ConfigFilePath)
@@ -76,7 +78,7 @@ func Run(cmd *cobra.Command, args []string) error {
 		return fmt.Errorf("Failed to parse config file: %s", err)
 	}
 
-	config, preflightClient, err := ValidateAndCombineConfig(logs.Log, cfg, Flags)
+	config, preflightClient, err := ValidateAndCombineConfig(logr.FromSlogHandler(slog.Default().Handler()), cfg, Flags)
 	if err != nil {
 		return fmt.Errorf("While evaluating configuration: %v", err)
 	}
@@ -179,7 +181,7 @@ func Run(cmd *cobra.Command, args []string) error {
 			return fmt.Errorf("failed to instantiate %q data gatherer  %q: %v", kind, dgConfig.Name, err)
 		}
 
-		logs.Log.Printf("starting %q datagatherer", dgConfig.Name)
+		slog.Info("Starting datagatherer", "gatherer", dgConfig.Name)
 
 		// start the data gatherers and wait for the cache sync
 		group.Go(func() error {
@@ -214,7 +216,7 @@ func Run(cmd *cobra.Command, args []string) error {
 		// the run.
 		if err := dg.WaitForCacheSync(bootCtx.Done()); err != nil {
 			// log sync failure, this might recover in future
-			logs.Log.Printf("failed to complete initial sync of %q data gatherer %q: %v", dgConfig.Kind, dgConfig.Name, err)
+			slog.Warn("Failed to complete initial sync", "kind", dgConfig.Kind, "gatherer", dgConfig.Name, "reason", err)
 		}
 	}
 
@@ -259,7 +261,7 @@ func newEventf(installNS string) (Eventf, error) {
 	var eventf Eventf
 	if os.Getenv("POD_NAME") == "" {
 		eventf = func(eventType, reason, msg string, args ...interface{}) {}
-		logs.Log.Printf("error messages will not show in the pod's events because the POD_NAME environment variable is empty")
+		slog.Warn("Error messages will not show in the pod's events because the POD_NAME environment variable is empty")
 	} else {
 		podName := os.Getenv("POD_NAME")
 
@@ -285,7 +287,7 @@ func gatherAndOutputData(eventf Eventf, config CombinedConfig, preflightClient c
 	var readings []*api.DataReading
 
 	if config.InputPath != "" {
-		logs.Log.Printf("Reading data from local file: %s", config.InputPath)
+		slog.Info("Reading data from local file", "path", config.InputPath)
 		data, err := os.ReadFile(config.InputPath)
 		if err != nil {
 			return fmt.Errorf("failed to read local data file: %s", err)
@@ -311,7 +313,7 @@ func gatherAndOutputData(eventf Eventf, config CombinedConfig, preflightClient c
 		if err != nil {
 			return fmt.Errorf("failed to output to local file: %s", err)
 		}
-		logs.Log.Printf("Data saved to local file: %s", config.OutputPath)
+		slog.Info("Data saved to local file", "path", config.OutputPath)
 	} else {
 		backOff := backoff.NewExponentialBackOff()
 		backOff.InitialInterval = 30 * time.Second
@@ -322,7 +324,7 @@ func gatherAndOutputData(eventf Eventf, config CombinedConfig, preflightClient c
 		}
 		err := backoff.RetryNotify(post, backOff, func(err error, t time.Duration) {
 			eventf("Warning", "PushingErr", "retrying in %v after error: %s", t, err)
-			logs.Log.Printf("retrying in %v after error: %s", t, err)
+			slog.Warn("Pushing error", "backoffInterval", t, "reason", err)
 		})
 		if err != nil {
 			return fmt.Errorf("Exiting due to fatal error uploading: %v", err)
@@ -343,11 +345,8 @@ func gatherData(config CombinedConfig, dataGatherers map[string]datagatherer.Dat
 			continue
 		}
 
-		if count >= 0 {
-			logs.Log.Printf("successfully gathered %d items from %q datagatherer", count, k)
-		} else {
-			logs.Log.Printf("successfully gathered data from %q datagatherer", k)
-		}
+		slog.Info("Successfully gathered data", "gatherer", k, "count", count)
+
 		readings = append(readings, &api.DataReading{
 			ClusterID:     config.ClusterID,
 			DataGatherer:  k,
@@ -379,7 +378,7 @@ func gatherData(config CombinedConfig, dataGatherers map[string]datagatherer.Dat
 func postData(config CombinedConfig, preflightClient client.Client, readings []*api.DataReading) error {
 	baseURL := config.Server
 
-	logs.Log.Println("Posting data to:", baseURL)
+	slog.Info("Posting data", "to", baseURL)
 
 	if config.AuthMode == VenafiCloudKeypair || config.AuthMode == VenafiCloudVenafiConnection {
 		// orgID and clusterID are not required for Venafi Cloud auth
@@ -390,7 +389,7 @@ func postData(config CombinedConfig, preflightClient client.Client, readings []*
 		if err != nil {
 			return fmt.Errorf("post to server failed: %+v", err)
 		}
-		logs.Log.Println("Data sent successfully.")
+		slog.Info("Data sent successfully")
 
 		return nil
 	}
@@ -406,7 +405,7 @@ func postData(config CombinedConfig, preflightClient client.Client, readings []*
 			prometheus.Labels{"organization": config.OrganizationID, "cluster": config.ClusterID},
 		)
 		metric.Set(float64(len(data)))
-		logs.Log.Printf("Data readings upload size: %d", len(data))
+		slog.Info("Data readings", "uploadSize", len(data))
 		path := config.EndpointPath
 		if path == "" {
 			path = "/api/v1/datareadings"
@@ -426,7 +425,7 @@ func postData(config CombinedConfig, preflightClient client.Client, readings []*
 
 			return fmt.Errorf("received response with status code %d. Body: [%s]", code, errorContent)
 		}
-		logs.Log.Println("Data sent successfully.")
+		slog.Info("Data sent successfully")
 		return err
 	}
 
@@ -438,7 +437,7 @@ func postData(config CombinedConfig, preflightClient client.Client, readings []*
 	if err != nil {
 		return fmt.Errorf("post to server failed: %+v", err)
 	}
-	logs.Log.Println("Data sent successfully.")
+	slog.Info("Data sent successfully")
 
 	return nil
 }