Use zap sugared logger

We use zap from controller runtime, returning logr.Logger. Replace it with zap SugaredLoger which is much nicer to use, supporting both structured and formatted logging and named log levels. We use the default development configuration which require no special setup. All the logs using string concatenation or fmt.Sprintf() replace with log.Infof() and log.Errof() and use %q format for more clear logs. Signed-off-by: Nir Soffer <[email protected]>
RamenDR · Nov 29, 2024 · fc9145d · fc9145d
1 parent 7dacd3b
commit fc9145d
Show file tree

Hide file tree

Showing 19 changed files with 104 additions and 107 deletions.
diff --git a/e2e/deployers/applicationset.go b/e2e/deployers/applicationset.go
@@ -13,7 +13,7 @@ type ApplicationSet struct{}
 func (a ApplicationSet) Deploy(w types.Workload) error {
 	name := GetCombinedName(a, w)
 	namespace := util.ArgocdNamespace
-	log := util.Ctx.Log.WithName(name)
+	log := util.Ctx.Log.Named(name)
 
 	log.Info("Deploying workload")
 
@@ -43,7 +43,7 @@ func (a ApplicationSet) Deploy(w types.Workload) error {
 func (a ApplicationSet) Undeploy(w types.Workload) error {
 	name := GetCombinedName(a, w)
 	namespace := util.ArgocdNamespace
-	log := util.Ctx.Log.WithName(name)
+	log := util.Ctx.Log.Named(name)
 
 	log.Info("Undeploying workload")
 

diff --git a/e2e/deployers/crud.go b/e2e/deployers/crud.go
@@ -11,9 +11,9 @@ import (
 	"os/exec"
 	"strings"
 
-	"github.com/go-logr/logr"
 	"github.com/ramendr/ramen/e2e/types"
 	"github.com/ramendr/ramen/e2e/util"
+	"go.uber.org/zap"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	k8stypes "k8s.io/apimachinery/pkg/types"
@@ -61,7 +61,7 @@ func CreateManagedClusterSetBinding(name, namespace string) error {
 	return nil
 }
 
-func DeleteManagedClusterSetBinding(name, namespace string, log logr.Logger) error {
+func DeleteManagedClusterSetBinding(name, namespace string, log *zap.SugaredLogger) error {
 	mcsb := &ocmv1b2.ManagedClusterSetBinding{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      name,
@@ -75,13 +75,13 @@ func DeleteManagedClusterSetBinding(name, namespace string, log logr.Logger) err
 			return err
 		}
 
-		log.Info("ManagedClusterSetBinding " + name + " not found")
+		log.Infof("ManagedClusterSetBinding %q not found", name)
 	}
 
 	return nil
 }
 
-func CreatePlacement(name, namespace string, log logr.Logger) error {
+func CreatePlacement(name, namespace string, log *zap.SugaredLogger) error {
 	labels := make(map[string]string)
 	labels[AppLabelKey] = name
 	clusterSet := []string{ClusterSetName}
@@ -111,7 +111,7 @@ func CreatePlacement(name, namespace string, log logr.Logger) error {
 	return nil
 }
 
-func DeletePlacement(name, namespace string, log logr.Logger) error {
+func DeletePlacement(name, namespace string, log *zap.SugaredLogger) error {
 	placement := &ocmv1b1.Placement{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      name,
@@ -131,7 +131,7 @@ func DeletePlacement(name, namespace string, log logr.Logger) error {
 	return nil
 }
 
-func CreateSubscription(s Subscription, w types.Workload, log logr.Logger) error {
+func CreateSubscription(s Subscription, w types.Workload, log *zap.SugaredLogger) error {
 	name := GetCombinedName(s, w)
 	namespace := name
 
@@ -176,7 +176,7 @@ func CreateSubscription(s Subscription, w types.Workload, log logr.Logger) error
 	err := util.Ctx.Hub.CtrlClient.Create(context.Background(), subscription)
 	if err != nil {
 		if !errors.IsAlreadyExists(err) {
-			log.Info(fmt.Sprintf("create subscription with error: %v", err))
+			log.Infof("Create subscription with error: %s", err)
 
 			return err
 		}
@@ -187,7 +187,7 @@ func CreateSubscription(s Subscription, w types.Workload, log logr.Logger) error
 	return nil
 }
 
-func DeleteSubscription(s Subscription, w types.Workload, log logr.Logger) error {
+func DeleteSubscription(s Subscription, w types.Workload, log *zap.SugaredLogger) error {
 	name := GetCombinedName(s, w)
 	namespace := name
 
@@ -240,7 +240,7 @@ func getSubscription(client client.Client, namespace, name string) (*subscriptio
 	return subscription, nil
 }
 
-func CreatePlacementDecisionConfigMap(cmName string, cmNamespace string, log logr.Logger) error {
+func CreatePlacementDecisionConfigMap(cmName string, cmNamespace string, log *zap.SugaredLogger) error {
 	object := metav1.ObjectMeta{Name: cmName, Namespace: cmNamespace}
 
 	data := map[string]string{
@@ -258,13 +258,13 @@ func CreatePlacementDecisionConfigMap(cmName string, cmNamespace string, log log
 			return fmt.Errorf("could not create configMap %q", cmName)
 		}
 
-		log.Info("ConfigMap " + cmName + " already Exists")
+		log.Infof("ConfigMap %q already Exists", cmName)
 	}
 
 	return nil
 }
 
-func DeleteConfigMap(cmName string, cmNamespace string, log logr.Logger) error {
+func DeleteConfigMap(cmName string, cmNamespace string, log *zap.SugaredLogger) error {
 	object := metav1.ObjectMeta{Name: cmName, Namespace: cmNamespace}
 
 	configMap := &corev1.ConfigMap{
@@ -277,14 +277,14 @@ func DeleteConfigMap(cmName string, cmNamespace string, log logr.Logger) error {
 			return fmt.Errorf("could not delete configMap %q", cmName)
 		}
 
-		log.Info("ConfigMap " + cmName + " not found")
+		log.Infof("ConfigMap %q not found", cmName)
 	}
 
 	return nil
 }
 
 // nolint:funlen
-func CreateApplicationSet(a ApplicationSet, w types.Workload, log logr.Logger) error {
+func CreateApplicationSet(a ApplicationSet, w types.Workload, log *zap.SugaredLogger) error {
 	var requeueSeconds int64 = 180
 
 	name := GetCombinedName(a, w)
@@ -362,7 +362,7 @@ func CreateApplicationSet(a ApplicationSet, w types.Workload, log logr.Logger) e
 	return nil
 }
 
-func DeleteApplicationSet(a ApplicationSet, w types.Workload, log logr.Logger) error {
+func DeleteApplicationSet(a ApplicationSet, w types.Workload, log *zap.SugaredLogger) error {
 	name := GetCombinedName(a, w)
 	namespace := util.ArgocdNamespace
 
@@ -386,7 +386,7 @@ func DeleteApplicationSet(a ApplicationSet, w types.Workload, log logr.Logger) e
 }
 
 // check if only the last appset is in the argocd namespace
-func isLastAppsetInArgocdNs(namespace string, log logr.Logger) (bool, error) {
+func isLastAppsetInArgocdNs(namespace string, log *zap.SugaredLogger) (bool, error) {
 	appsetList := &argocdv1alpha1hack.ApplicationSetList{}
 
 	err := util.Ctx.Hub.CtrlClient.List(
@@ -400,7 +400,7 @@ func isLastAppsetInArgocdNs(namespace string, log logr.Logger) (bool, error) {
 	return len(appsetList.Items) == 1, nil
 }
 
-func DeleteDiscoveredApps(w types.Workload, namespace, cluster string, log logr.Logger) error {
+func DeleteDiscoveredApps(w types.Workload, namespace, cluster string, log *zap.SugaredLogger) error {
 	tempDir, err := os.MkdirTemp("", "ramen-")
 	if err != nil {
 		return err

diff --git a/e2e/deployers/discoveredapps.go b/e2e/deployers/discoveredapps.go
@@ -20,7 +20,7 @@ func (d DiscoveredApps) GetName() string {
 func (d DiscoveredApps) Deploy(w types.Workload) error {
 	name := GetCombinedName(d, w)
 	namespace := name
-	log := util.Ctx.Log.WithName(name)
+	log := util.Ctx.Log.Named(name)
 
 	log.Info("Deploying workload")
 
@@ -68,7 +68,7 @@ func (d DiscoveredApps) Deploy(w types.Workload) error {
 func (d DiscoveredApps) Undeploy(w types.Workload) error {
 	name := GetCombinedName(d, w)
 	namespace := name // this namespace is in dr clusters
-	log := util.Ctx.Log.WithName(name)
+	log := util.Ctx.Log.Named(name)
 
 	log.Info("Undeploying workload")
 
@@ -77,27 +77,27 @@ func (d DiscoveredApps) Undeploy(w types.Workload) error {
 		return err
 	}
 
-	log.Info("Deleting discovered apps on " + drpolicy.Spec.DRClusters[0])
+	log.Infof("Deleting discovered apps on cluster %q", drpolicy.Spec.DRClusters[0])
 
 	// delete app on both clusters
 	if err := DeleteDiscoveredApps(w, namespace, drpolicy.Spec.DRClusters[0], log); err != nil {
 		return err
 	}
 
-	log.Info("Deletting discovered apps on " + drpolicy.Spec.DRClusters[1])
+	log.Infof("Deletting discovered apps on cluster %q", drpolicy.Spec.DRClusters[1])
 
 	if err := DeleteDiscoveredApps(w, namespace, drpolicy.Spec.DRClusters[1], log); err != nil {
 		return err
 	}
 
-	log.Info("Deleting namespace " + namespace + " on " + drpolicy.Spec.DRClusters[0])
+	log.Infof("Deleting namespace %q on cluster %q", namespace, drpolicy.Spec.DRClusters[0])
 
 	// delete namespace on both clusters
 	if err := util.DeleteNamespace(util.Ctx.C1.CtrlClient, namespace, log); err != nil {
 		return err
 	}
 
-	log.Info("Deleting namespace " + namespace + " on " + drpolicy.Spec.DRClusters[1])
+	log.Infof("Deleting namespace %q on cluster %q", namespace, drpolicy.Spec.DRClusters[1])
 
 	if err := util.DeleteNamespace(util.Ctx.C2.CtrlClient, namespace, log); err != nil {
 		return err

diff --git a/e2e/deployers/retry.go b/e2e/deployers/retry.go
@@ -7,14 +7,14 @@ import (
 	"fmt"
 	"time"
 
-	"github.com/go-logr/logr"
 	"github.com/ramendr/ramen/e2e/types"
 	"github.com/ramendr/ramen/e2e/util"
+	"go.uber.org/zap"
 	subscriptionv1 "open-cluster-management.io/multicloud-operators-subscription/pkg/apis/apps/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
-func waitSubscriptionPhase(namespace, name string, phase subscriptionv1.SubscriptionPhase, log logr.Logger) error {
+func waitSubscriptionPhase(namespace, name string, phase subscriptionv1.SubscriptionPhase, log *zap.SugaredLogger) error {
 	startTime := time.Now()
 
 	for {
@@ -25,7 +25,7 @@ func waitSubscriptionPhase(namespace, name string, phase subscriptionv1.Subscrip
 
 		currentPhase := sub.Status.Phase
 		if currentPhase == phase {
-			log.Info(fmt.Sprintf("Subscription phase is %s", phase))
+			log.Infof("Subscription phase is %s", phase)
 
 			return nil
 		}
@@ -38,7 +38,7 @@ func waitSubscriptionPhase(namespace, name string, phase subscriptionv1.Subscrip
 	}
 }
 
-func WaitWorkloadHealth(client client.Client, namespace string, w types.Workload, log logr.Logger) error {
+func WaitWorkloadHealth(client client.Client, namespace string, w types.Workload, log *zap.SugaredLogger) error {
 	startTime := time.Now()
 
 	for {

diff --git a/e2e/deployers/subscription.go b/e2e/deployers/subscription.go
@@ -27,7 +27,7 @@ func (s Subscription) Deploy(w types.Workload) error {
 	// Address namespace/label/suffix as needed for various resources
 	name := GetCombinedName(s, w)
 	namespace := name
-	log := util.Ctx.Log.WithName(name)
+	log := util.Ctx.Log.Named(name)
 
 	log.Info("Deploying workload")
 
@@ -59,7 +59,7 @@ func (s Subscription) Deploy(w types.Workload) error {
 func (s Subscription) Undeploy(w types.Workload) error {
 	name := GetCombinedName(s, w)
 	namespace := name
-	log := util.Ctx.Log.WithName(name)
+	log := util.Ctx.Log.Named(name)
 
 	log.Info("Undeploying workload")
 

diff --git a/e2e/dractions/actions.go b/e2e/dractions/actions.go
@@ -7,11 +7,11 @@ import (
 	"strings"
 	"time"
 
-	"github.com/go-logr/logr"
 	ramen "github.com/ramendr/ramen/api/v1alpha1"
 	"github.com/ramendr/ramen/e2e/deployers"
 	"github.com/ramendr/ramen/e2e/types"
 	"github.com/ramendr/ramen/e2e/util"
+	"go.uber.org/zap"
 	"k8s.io/client-go/util/retry"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
@@ -36,7 +36,7 @@ func EnableProtection(w types.Workload, d types.Deployer) error {
 
 	name := GetCombinedName(d, w)
 	namespace := GetNamespace(d, w)
-	log := util.Ctx.Log.WithName(name)
+	log := util.Ctx.Log.Named(name)
 
 	log.Info("Protecting workload")
 
@@ -51,7 +51,7 @@ func EnableProtection(w types.Workload, d types.Deployer) error {
 	}
 
 	clusterName := placementDecision.Status.Decisions[0].ClusterName
-	log.Info("Workload running on " + clusterName)
+	log.Infof("Workload running on cluster %q", clusterName)
 
 	log.Info("Annotating placement")
 
@@ -98,7 +98,7 @@ func DisableProtection(w types.Workload, d types.Deployer) error {
 
 	name := GetCombinedName(d, w)
 	namespace := GetNamespace(d, w)
-	log := util.Ctx.Log.WithName(name)
+	log := util.Ctx.Log.Named(name)
 
 	log.Info("Unprotecting workload")
 
@@ -116,7 +116,7 @@ func DisableProtection(w types.Workload, d types.Deployer) error {
 
 func Failover(w types.Workload, d types.Deployer) error {
 	name := GetCombinedName(d, w)
-	log := util.Ctx.Log.WithName(name)
+	log := util.Ctx.Log.Named(name)
 
 	log.Info("Failing over workload")
 
@@ -133,7 +133,7 @@ func Failover(w types.Workload, d types.Deployer) error {
 // Update DRPC
 func Relocate(w types.Workload, d types.Deployer) error {
 	name := GetCombinedName(d, w)
-	log := util.Ctx.Log.WithName(name)
+	log := util.Ctx.Log.Named(name)
 
 	log.Info("Relocating workload")
 
@@ -144,7 +144,7 @@ func Relocate(w types.Workload, d types.Deployer) error {
 	return failoverRelocate(w, d, ramen.ActionRelocate, log)
 }
 
-func failoverRelocate(w types.Workload, d types.Deployer, action ramen.DRAction, log logr.Logger) error {
+func failoverRelocate(w types.Workload, d types.Deployer, action ramen.DRAction, log *zap.SugaredLogger) error {
 	name := GetCombinedName(d, w)
 	namespace := GetNamespace(d, w)
 
@@ -162,7 +162,7 @@ func failoverRelocate(w types.Workload, d types.Deployer, action ramen.DRAction,
 	return waitDRPC(client, namespace, name, ramen.Relocated, log)
 }
 
-func waitAndUpdateDRPC(client client.Client, namespace, drpcName string, action ramen.DRAction, log logr.Logger) error {
+func waitAndUpdateDRPC(client client.Client, namespace, drpcName string, action ramen.DRAction, log *zap.SugaredLogger) error {
 	// here we expect drpc should be ready before action
 	if err := waitDRPCReady(client, namespace, drpcName, log); err != nil {
 		return err