Merge pull request #397 from Xieql/log-refactor

log: refactor logging to enhance clarity
kurator-dev · Oct 23, 2023 · 051d3c5 · 051d3c5
2 parents 9524091 + 250a478
commit 051d3c5
Show file tree

Hide file tree

Showing 11 changed files with 26 additions and 33 deletions.
diff --git a/pkg/cluster-operator/attachedcluster_controller.go b/pkg/cluster-operator/attachedcluster_controller.go
@@ -29,7 +29,6 @@ import (
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 	kubeclient "k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/tools/clientcmd"
-	"k8s.io/klog/v2"
 	"sigs.k8s.io/cluster-api/util/patch"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -54,20 +53,20 @@ func (a *AttachedClusterController) SetupWithManager(ctx context.Context, mgr ct
 }
 
 func (a *AttachedClusterController) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Result, reterr error) {
-	log := ctrl.LoggerFrom(ctx)
+	log := ctrl.LoggerFrom(ctx).WithValues("attachedCluster", req.NamespacedName)
+
 	// Fetch the attachedCluster instance.
 	attachedCluster := &clusterv1alpha1.AttachedCluster{}
 	if err := a.Client.Get(ctx, req.NamespacedName, attachedCluster); err != nil {
 		if apierrors.IsNotFound(err) {
-			log.Info("attachedCluster is not exist", "attachedCluster", req)
+			log.Info("attachedCluster is not exist")
 			return ctrl.Result{}, nil
 		}
 
 		// Error reading the object - requeue the request.
 		return ctrl.Result{}, err
 	}
-	log = log.WithValues("attachedCluster", klog.KObj(attachedCluster))
-	ctx = ctrl.LoggerInto(ctx, log)
+
 	patchHelper, err := patch.NewHelper(attachedCluster, a.Client)
 	if err != nil {
 		return ctrl.Result{}, errors.Wrapf(err, "failed to init patch helper for fleet %s", req.NamespacedName)

diff --git a/pkg/cluster-operator/cluster_controller.go b/pkg/cluster-operator/cluster_controller.go
@@ -49,10 +49,6 @@ import (
 	"kurator.dev/kurator/pkg/infra/util"
 )
 
-var (
-	log = ctrl.Log.WithName("cluster-controller")
-)
-
 const (
 	// ClusterFinalizer allows ClusterController to clean up associated resources before removing it from apiserver.
 	ClusterFinalizer = "cluster.cluster.kurator.dev"
@@ -452,6 +448,8 @@ func (r *ClusterController) SetupWithManager(mgr ctrl.Manager) error {
 }
 
 func (r *ClusterController) SecretToClusterFunc(o client.Object) []ctrl.Request {
+	log := ctrl.Log.WithName("cluster-controller")
+
 	obj, ok := o.(*corev1.Secret)
 	if !ok {
 		panic(fmt.Sprintf("Expected a Secret but got a %T", o))

diff --git a/pkg/cluster-operator/customcluster_controller.go b/pkg/cluster-operator/customcluster_controller.go
@@ -25,7 +25,6 @@ import (
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
-	"k8s.io/klog/v2"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	controlplanev1 "sigs.k8s.io/cluster-api/controlplane/kubeadm/api/v1beta1"
 	capiutil "sigs.k8s.io/cluster-api/util"
@@ -143,21 +142,20 @@ func (r *CustomClusterController) SetupWithManager(ctx context.Context, mgr ctrl
 // Reconcile is part of the main kubernetes reconciliation loop which aims to
 // move the current state of the cluster closer to the desired state.
 func (r *CustomClusterController) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Result, reterr error) {
-	log := ctrl.LoggerFrom(ctx)
+	log := ctrl.LoggerFrom(ctx).WithValues("customCluster", req.NamespacedName)
 
 	// Fetch the customCluster instance.
 	customCluster := &v1alpha1.CustomCluster{}
 	if err := r.Client.Get(ctx, req.NamespacedName, customCluster); err != nil {
 		if apierrors.IsNotFound(err) {
-			log.Info("customCluster does not exist", "customCluster", req)
+			log.Info("customCluster does not exist")
 			return ctrl.Result{}, nil
 		}
-		log.Error(err, "failed to find customCluster", "customCluster", req)
+		log.Error(err, "failed to find customCluster")
 		// Error reading the object - requeue the request.
 		return ctrl.Result{}, err
 	}
-	log = log.WithValues("customCluster", klog.KObj(customCluster))
-	ctx = ctrl.LoggerInto(ctx, log)
+
 	// ensure customCluster status no nil
 	if len(customCluster.Status.Phase) == 0 {
 		customCluster.Status.Phase = v1alpha1.PendingPhase
@@ -172,7 +170,7 @@ func (r *CustomClusterController) Reconcile(ctx context.Context, req ctrl.Reques
 		}
 	}
 	if len(clusterName) == 0 {
-		log.Info("failed to get cluster from customCluster.GetOwnerReferences", "customCluster", req)
+		log.Info("failed to get cluster from customCluster.GetOwnerReferences")
 		return ctrl.Result{}, nil
 	}
 	clusterKey := client.ObjectKey{

diff --git a/pkg/cluster-operator/customcluster_scale.go b/pkg/cluster-operator/customcluster_scale.go
@@ -79,7 +79,7 @@ func (r *CustomClusterController) reconcileScaleUp(ctx context.Context, customCl
 
 		// Delete the scaleUp worker.
 		if err := r.ensureWorkerPodDeleted(ctx, customCluster, CustomClusterScaleUpAction); err != nil {
-			log.Error(err, "failed to delete scaleUp worker pod", "customCluster", customCluster.Name)
+			log.Error(err, "failed to delete scaleUp worker pod")
 			return ctrl.Result{}, err
 		}
 		conditions.MarkTrue(customCluster, v1alpha1.ScaledUpCondition)

diff --git a/pkg/cluster-operator/customcluster_upgrade.go b/pkg/cluster-operator/customcluster_upgrade.go
@@ -40,7 +40,7 @@ func (r *CustomClusterController) reconcileUpgrade(ctx context.Context, customCl
 	if err1 != nil {
 		conditions.MarkFalse(customCluster, v1alpha1.UpgradeCondition, v1alpha1.UpgradeWorkerCreateFailed,
 			clusterv1.ConditionSeverityWarning, "upgrade worker is failed to create %s/%s.", customCluster.Namespace, customCluster.Name)
-		log.Error(err1, "failed to ensure that upgrade WorkerPod is created", "customCluster", customCluster.Name)
+		log.Error(err1, "failed to ensure that upgrade WorkerPod is created")
 		return ctrl.Result{}, err1
 	}
 
@@ -63,7 +63,7 @@ func (r *CustomClusterController) reconcileUpgrade(ctx context.Context, customCl
 
 		// Delete the upgrading worker.
 		if err := r.ensureWorkerPodDeleted(ctx, customCluster, CustomClusterUpgradeAction); err != nil {
-			log.Error(err, "failed to delete upgrade worker pod", "customCluster", customCluster.Name)
+			log.Error(err, "failed to delete upgrade worker pod")
 			return ctrl.Result{}, err
 		}
 		conditions.MarkTrue(customCluster, v1alpha1.UpgradeCondition)

diff --git a/pkg/cluster-operator/custommachine_controller.go b/pkg/cluster-operator/custommachine_controller.go
@@ -50,12 +50,13 @@ func (r *CustomMachineController) SetupWithManager(ctx context.Context, mgr ctrl
 }
 
 func (r *CustomMachineController) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
-	log := ctrl.LoggerFrom(ctx)
+	log := ctrl.LoggerFrom(ctx).WithValues("customMachine", req.NamespacedName)
+
 	// Fetch the CustomMachine instance.
 	customMachine := &v1alpha1.CustomMachine{}
 	if err := r.Client.Get(ctx, req.NamespacedName, customMachine); err != nil {
 		if apierrors.IsNotFound(err) {
-			log.Info("customMachine is not exist", "customMachine", req)
+			log.Info("customMachine is not exist")
 			return ctrl.Result{}, nil
 		}
 

diff --git a/pkg/fleet-manager/application_controller.go b/pkg/fleet-manager/application_controller.go
@@ -28,7 +28,6 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
-	"k8s.io/klog/v2"
 	"sigs.k8s.io/cluster-api/util/patch"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -108,6 +107,8 @@ func (a *ApplicationManager) SetupWithManager(ctx context.Context, mgr ctrl.Mana
 }
 
 func (a *ApplicationManager) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Result, reterr error) {
+	log := ctrl.LoggerFrom(ctx).WithValues("application", req.NamespacedName)
+
 	app := &applicationapi.Application{}
 	if err := a.Get(ctx, req.NamespacedName, app); err != nil {
 		if apierrors.IsNotFound(err) {
@@ -116,9 +117,6 @@ func (a *ApplicationManager) Reconcile(ctx context.Context, req ctrl.Request) (_
 		return ctrl.Result{}, errors.Wrapf(err, "failed to get application %s", req.NamespacedName)
 	}
 
-	log := ctrl.LoggerFrom(ctx)
-	log = log.WithValues("application", klog.KObj(app))
-
 	patchHelper, err := patch.NewHelper(app, a.Client)
 	if err != nil {
 		return ctrl.Result{}, errors.Wrapf(err, "failed to init patch helper for application %s", req.NamespacedName)

diff --git a/pkg/fleet-manager/clusters.go b/pkg/fleet-manager/clusters.go
@@ -60,8 +60,7 @@ func (f *FleetManager) reconcileClusters(ctx context.Context, fleet *fleetapi.Fl
 		controlplaneSpecified = false
 	}
 
-	fleetKey := client.ObjectKeyFromObject(fleet)
-	log := ctrl.LoggerFrom(ctx).WithValues("fleet", fleetKey)
+	log := ctrl.LoggerFrom(ctx)
 	var unreadyClusters int32
 	var result ctrl.Result
 	var readyClusters []ClusterInterface
@@ -184,7 +183,7 @@ func (f *FleetManager) reconcileClusters(ctx context.Context, fleet *fleetapi.Fl
 }
 
 func (f *FleetManager) reconcileClustersOnDelete(ctx context.Context, fleet *fleetapi.Fleet) (ctrl.Result, error) {
-	log := ctrl.LoggerFrom(ctx).WithValues("fleet", types.NamespacedName{Name: fleet.Name, Namespace: fleet.Namespace})
+	log := ctrl.LoggerFrom(ctx)
 	var result ctrl.Result
 	// Loop over cluster, and add labels to the cluster
 	for _, cluster := range fleet.Spec.Clusters {

diff --git a/pkg/fleet-manager/fleet.go b/pkg/fleet-manager/fleet.go
@@ -104,9 +104,12 @@ func (f *FleetManager) objectToFleetFunc(o client.Object) []ctrl.Request {
 }
 
 func (f *FleetManager) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Result, reterr error) {
+	log := ctrl.LoggerFrom(ctx).WithValues("fleet", req.NamespacedName)
+
 	fleet := &fleetapi.Fleet{}
 	if err := f.Get(ctx, req.NamespacedName, fleet); err != nil {
 		if apiserrors.IsNotFound(err) {
+			log.Info("fleet is not exist")
 			return ctrl.Result{}, nil
 		}
 		return ctrl.Result{}, errors.Wrapf(err, "failed to get fleet %s", req.NamespacedName)
@@ -144,7 +147,6 @@ func (f *FleetManager) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.
 
 func (f *FleetManager) reconcile(ctx context.Context, fleet *fleetapi.Fleet) (ctrl.Result, error) {
 	log := ctrl.LoggerFrom(ctx)
-	log = log.WithValues("fleet", types.NamespacedName{Name: fleet.Name, Namespace: fleet.Namespace})
 
 	// Install fleet control plane
 	if err := f.reconcileControlPlane(ctx, fleet); err != nil {

diff --git a/pkg/fleet-manager/fleet_plugin_grafana.go b/pkg/fleet-manager/fleet_plugin_grafana.go
@@ -24,7 +24,6 @@ import (
 	"helm.sh/helm/v3/pkg/kube"
 	"k8s.io/apimachinery/pkg/types"
 	ctrl "sigs.k8s.io/controller-runtime"
-	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	fleetapi "kurator.dev/kurator/pkg/apis/fleet/v1alpha1"
 	"kurator.dev/kurator/pkg/fleet-manager/plugin"
@@ -34,7 +33,7 @@ import (
 // reconcileGrafanaPlugin reconciles the Grafana plugin.
 // The fleetClusters parameter is currently unused, but is included to match the function signature of other functions in reconcilePlugins.
 func (f *FleetManager) reconcileGrafanaPlugin(ctx context.Context, fleet *fleetapi.Fleet, fleetClusters map[ClusterKey]*fleetCluster) (kube.ResourceList, ctrl.Result, error) {
-	log := ctrl.LoggerFrom(ctx).WithValues("fleet", client.ObjectKeyFromObject(fleet))
+	log := ctrl.LoggerFrom(ctx)
 
 	if fleet.Spec.Plugin.Grafana == nil {
 		// reconcilePluginResources will delete all resources if plugin is nil

diff --git a/pkg/fleet-manager/fleet_plugin_metric.go b/pkg/fleet-manager/fleet_plugin_metric.go
@@ -33,7 +33,6 @@ import (
 	"k8s.io/utils/pointer"
 	capiutil "sigs.k8s.io/cluster-api/util"
 	ctrl "sigs.k8s.io/controller-runtime"
-	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	fleetapi "kurator.dev/kurator/pkg/apis/fleet/v1alpha1"
 	"kurator.dev/kurator/pkg/fleet-manager/plugin"
@@ -234,7 +233,7 @@ func (f *FleetManager) syncObjStoreSecret(ctx context.Context, fleetCluster *fle
 }
 
 func (f *FleetManager) reconcileMetricPlugin(ctx context.Context, fleet *fleetapi.Fleet, fleetClusters map[ClusterKey]*fleetCluster) (kube.ResourceList, ctrl.Result, error) {
-	log := ctrl.LoggerFrom(ctx).WithValues("fleet", client.ObjectKeyFromObject(fleet))
+	log := ctrl.LoggerFrom(ctx)
 
 	if fleet.Spec.Plugin.Metric == nil {
 		// reconcilePluginResources will delete all resources if plugin is nil