Move etcdLogger to reconcile struct

Author: dmvolod

controlplane/kubeadm/controllers/alias.go

@@ -20,7 +20,7 @@ import (
 	"context"
 	"time"
 
-	"go.uber.org/zap/zapcore"
+	"go.uber.org/zap"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
@@ -37,7 +37,7 @@ type KubeadmControlPlaneReconciler struct {
 
 	EtcdDialTimeout time.Duration
 	EtcdCallTimeout time.Duration
-	EtcdLogLevel    zapcore.Level
+	EtcdLogger      *zap.Logger
 
 	// WatchFilterValue is the label value used to filter events prior to reconciliation.
 	WatchFilterValue string
@@ -53,7 +53,7 @@ func (r *KubeadmControlPlaneReconciler) SetupWithManager(ctx context.Context, mg
 		ClusterCache:                r.ClusterCache,
 		EtcdDialTimeout:             r.EtcdDialTimeout,
 		EtcdCallTimeout:             r.EtcdCallTimeout,
-		EtcdLogLevel:                r.EtcdLogLevel,
+		EtcdLogger:                  r.EtcdLogger,
 		WatchFilterValue:            r.WatchFilterValue,
 		RemoteConditionsGracePeriod: r.RemoteConditionsGracePeriod,
 	}).SetupWithManager(ctx, mgr, options)

controlplane/kubeadm/internal/cluster.go

@@ -24,6 +24,7 @@ import (
 	"time"
 
 	"github.com/pkg/errors"
+	"go.uber.org/zap"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/client-go/rest"
@@ -51,6 +52,7 @@ type Management struct {
 	ClusterCache        clustercache.ClusterCache
 	EtcdDialTimeout     time.Duration
 	EtcdCallTimeout     time.Duration
+	EtcdLogger          *zap.Logger
 }
 
 // RemoteClusterConnectionError represents a failure to connect to a remote cluster.
@@ -152,7 +154,7 @@ func (m *Management) GetWorkloadCluster(ctx context.Context, clusterKey client.O
 		restConfig:          restConfig,
 		Client:              c,
 		CoreDNSMigrator:     &CoreDNSMigrator{},
-		etcdClientGenerator: NewEtcdClientGenerator(restConfig, tlsConfig, m.EtcdDialTimeout, m.EtcdCallTimeout),
+		etcdClientGenerator: NewEtcdClientGenerator(restConfig, tlsConfig, m.EtcdDialTimeout, m.EtcdCallTimeout, m.EtcdLogger),
 	}, nil
 }
 

controlplane/kubeadm/internal/controllers/controller.go

@@ -25,8 +25,7 @@ import (
 
 	"github.com/blang/semver/v4"
 	"github.com/pkg/errors"
-	"go.etcd.io/etcd/client/pkg/v3/logutil"
-	"go.uber.org/zap/zapcore"
+	"go.uber.org/zap"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -47,7 +46,6 @@ import (
 	clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2"
 	"sigs.k8s.io/cluster-api/controllers/clustercache"
 	"sigs.k8s.io/cluster-api/controlplane/kubeadm/internal"
-	"sigs.k8s.io/cluster-api/controlplane/kubeadm/internal/etcd"
 	"sigs.k8s.io/cluster-api/feature"
 	"sigs.k8s.io/cluster-api/internal/contract"
 	"sigs.k8s.io/cluster-api/internal/util/ssa"
@@ -87,7 +85,7 @@ type KubeadmControlPlaneReconciler struct {
 
 	EtcdDialTimeout time.Duration
 	EtcdCallTimeout time.Duration
-	EtcdLogLevel    zapcore.Level
+	EtcdLogger      *zap.Logger
 
 	// WatchFilterValue is the label value used to filter events prior to reconciliation.
 	WatchFilterValue string
@@ -113,12 +111,6 @@ func (r *KubeadmControlPlaneReconciler) SetupWithManager(ctx context.Context, mg
 	}
 
 	predicateLog := ctrl.LoggerFrom(ctx).WithValues("controller", "kubeadmcontrolplane")
-	etcdLogger, err := logutil.CreateDefaultZapLogger(r.EtcdLogLevel)
-	if err != nil {
-		return errors.Wrap(err, "failed to create ETCD client zap logger")
-	}
-	etcd.SetLogger(etcdLogger)
-
 	c, err := ctrl.NewControllerManagedBy(mgr).
 		For(&controlplanev1.KubeadmControlPlane{}).
 		Owns(&clusterv1.Machine{}, builder.WithPredicates(predicates.ResourceIsChanged(mgr.GetScheme(), predicateLog))).
@@ -156,6 +148,7 @@ func (r *KubeadmControlPlaneReconciler) SetupWithManager(ctx context.Context, mg
 			ClusterCache:        r.ClusterCache,
 			EtcdDialTimeout:     r.EtcdDialTimeout,
 			EtcdCallTimeout:     r.EtcdCallTimeout,
+			EtcdLogger:          r.EtcdLogger,
 		}
 	}
 

controlplane/kubeadm/internal/etcd/etcd.go

@@ -24,10 +24,8 @@ import (
 
 	"github.com/pkg/errors"
 	"go.etcd.io/etcd/api/v3/etcdserverpb"
-	"go.etcd.io/etcd/client/pkg/v3/logutil"
 	clientv3 "go.etcd.io/etcd/client/v3"
 	"go.uber.org/zap"
-	"go.uber.org/zap/zapcore"
 	"google.golang.org/grpc"
 	kerrors "k8s.io/apimachinery/pkg/util/errors"
 
@@ -134,17 +132,7 @@ type ClientConfiguration struct {
 	TLSConfig   *tls.Config
 	DialTimeout time.Duration
 	CallTimeout time.Duration
-}
-
-var (
-	// Create the etcdClientLogger only once. Otherwise every call of clientv3.New
-	// would create its own logger which leads to a lot of memory allocations.
-	etcdClientLogger, _ = logutil.CreateDefaultZapLogger(zapcore.InfoLevel)
-)
-
-// SetLogger allows to redefine ETCD client logger.
-func SetLogger(logger *zap.Logger) {
-	etcdClientLogger = logger
+	Logger      *zap.Logger
 }
 
 // NewClient creates a new etcd client with the given configuration.
@@ -161,7 +149,7 @@ func NewClient(ctx context.Context, config ClientConfiguration) (*Client, error)
 			grpc.WithContextDialer(dialer.DialContextWithAddr),
 		},
 		TLS:    config.TLSConfig,
-		Logger: etcdClientLogger,
+		Logger: config.Logger,
 	})
 	if err != nil {
 		return nil, errors.Wrap(err, "unable to create etcd client")

controlplane/kubeadm/internal/etcd_client_generator.go

@@ -23,6 +23,7 @@ import (
 	"time"
 
 	"github.com/pkg/errors"
+	"go.uber.org/zap"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	kerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/apimachinery/pkg/util/sets"
@@ -44,7 +45,7 @@ type clientCreator func(ctx context.Context, endpoint string) (*etcd.Client, err
 var errEtcdNodeConnection = errors.New("failed to connect to etcd node")
 
 // NewEtcdClientGenerator returns a new etcdClientGenerator instance.
-func NewEtcdClientGenerator(restConfig *rest.Config, tlsConfig *tls.Config, etcdDialTimeout, etcdCallTimeout time.Duration) *EtcdClientGenerator {
+func NewEtcdClientGenerator(restConfig *rest.Config, tlsConfig *tls.Config, etcdDialTimeout, etcdCallTimeout time.Duration, etcdLogger *zap.Logger) *EtcdClientGenerator {
 	ecg := &EtcdClientGenerator{restConfig: restConfig, tlsConfig: tlsConfig}
 
 	ecg.createClient = func(ctx context.Context, endpoint string) (*etcd.Client, error) {
@@ -60,6 +61,7 @@ func NewEtcdClientGenerator(restConfig *rest.Config, tlsConfig *tls.Config, etcd
 			TLSConfig:   tlsConfig,
 			DialTimeout: etcdDialTimeout,
 			CallTimeout: etcdCallTimeout,
+			Logger:      etcdLogger,
 		})
 	}
 

controlplane/kubeadm/internal/etcd_client_generator_test.go

@@ -25,20 +25,23 @@ import (
 	. "github.com/onsi/gomega"
 	"github.com/pkg/errors"
 	"go.etcd.io/etcd/api/v3/etcdserverpb"
+	"go.etcd.io/etcd/client/pkg/v3/logutil"
 	clientv3 "go.etcd.io/etcd/client/v3"
+	"go.uber.org/zap/zapcore"
 	"k8s.io/client-go/rest"
 
 	"sigs.k8s.io/cluster-api/controlplane/kubeadm/internal/etcd"
 	etcdfake "sigs.k8s.io/cluster-api/controlplane/kubeadm/internal/etcd/fake"
 )
 
 var (
-	subject *EtcdClientGenerator
+	subject             *EtcdClientGenerator
+	etcdClientLogger, _ = logutil.CreateDefaultZapLogger(zapcore.InfoLevel)
 )
 
 func TestNewEtcdClientGenerator(t *testing.T) {
 	g := NewWithT(t)
-	subject = NewEtcdClientGenerator(&rest.Config{}, &tls.Config{MinVersion: tls.VersionTLS12}, 0, 0)
+	subject = NewEtcdClientGenerator(&rest.Config{}, &tls.Config{MinVersion: tls.VersionTLS12}, 0, 0, etcdClientLogger)
 	g.Expect(subject.createClient).To(Not(BeNil()))
 }
 
@@ -90,7 +93,7 @@ func TestFirstAvailableNode(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			g := NewWithT(t)
-			subject = NewEtcdClientGenerator(&rest.Config{}, &tls.Config{MinVersion: tls.VersionTLS12}, 0, 0)
+			subject = NewEtcdClientGenerator(&rest.Config{}, &tls.Config{MinVersion: tls.VersionTLS12}, 0, 0, etcdClientLogger)
 			subject.createClient = tt.cc
 
 			client, err := subject.forFirstAvailableNode(ctx, tt.nodes)
@@ -212,7 +215,7 @@ func TestForLeader(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			g := NewWithT(t)
 
-			subject = NewEtcdClientGenerator(&rest.Config{}, &tls.Config{MinVersion: tls.VersionTLS12}, 0, 0)
+			subject = NewEtcdClientGenerator(&rest.Config{}, &tls.Config{MinVersion: tls.VersionTLS12}, 0, 0, etcdClientLogger)
 			subject.createClient = tt.cc
 
 			client, err := subject.forLeader(ctx, tt.nodes)

controlplane/kubeadm/main.go

@@ -27,6 +27,7 @@ import (
 
 	"github.com/pkg/errors"
 	"github.com/spf13/pflag"
+	"go.etcd.io/etcd/client/pkg/v3/logutil"
 	"go.uber.org/zap/zapcore"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
@@ -101,7 +102,7 @@ var (
 	skipCRDMigrationPhases         []string
 	etcdDialTimeout                time.Duration
 	etcdCallTimeout                time.Duration
-	etcdLogLevel                   int8
+	etcdLogLevel                   string
 )
 
 func init() {
@@ -192,8 +193,8 @@ func InitFlags(fs *pflag.FlagSet) {
 	fs.DurationVar(&etcdCallTimeout, "etcd-call-timeout-duration", etcd.DefaultCallTimeout,
 		"Duration that the etcd client waits at most for read and write operations to etcd.")
 
-	fs.Int8Var(&etcdLogLevel, "etcd-client-log-level", int8(zapcore.InfoLevel),
-		"Logging level for etcd client.")
+	fs.StringVar(&etcdLogLevel, "etcd-client-log-level", zapcore.InfoLevel.String(),
+		"Logging level for etcd client. Possible values are: debug, info, warn, error, dpanic, panic, fatal.")
 
 	flags.AddManagerOptions(fs, &managerOptions)
 
@@ -423,14 +424,22 @@ func setupReconcilers(ctx context.Context, mgr ctrl.Manager) {
 		os.Exit(1)
 	}
 
+	zapLogLevel, err := zapcore.ParseLevel(etcdLogLevel)
+	if err != nil {
+		setupLog.Error(err, "Unable to parse etcd log level")
+	}
+	etcdLogger, err := logutil.CreateDefaultZapLogger(zapLogLevel)
+	if err != nil {
+		setupLog.Error(err, "unable to create etcd logger")
+	}
 	if err := (&kubeadmcontrolplanecontrollers.KubeadmControlPlaneReconciler{
 		Client:                      mgr.GetClient(),
 		SecretCachingClient:         secretCachingClient,
 		ClusterCache:                clusterCache,
 		WatchFilterValue:            watchFilterValue,
 		EtcdDialTimeout:             etcdDialTimeout,
 		EtcdCallTimeout:             etcdCallTimeout,
-		EtcdLogLevel:                zapcore.Level(etcdLogLevel),
+		EtcdLogger:                  etcdLogger,
 		RemoteConditionsGracePeriod: remoteConditionsGracePeriod,
 	}).SetupWithManager(ctx, mgr, concurrency(kubeadmControlPlaneConcurrency)); err != nil {
 		setupLog.Error(err, "unable to create controller", "controller", "KubeadmControlPlane")