fixup

sbueringer · sbueringer · commit 2d144d150560 · 2022-02-22T13:07:09.000+01:00
diff --git a/bootstrap/kubeadm/internal/controllers/kubeadmconfig_controller.go b/bootstrap/kubeadm/internal/controllers/kubeadmconfig_controller.go
@@ -113,7 +113,7 @@ func (r *KubeadmConfigReconciler) SetupWithManager(ctx context.Context, mgr ctrl
 		r.TokenTTL = DefaultTokenTTL
 	}
 
-	tr := tlog.Reconciler(r, "controllers.KubeadmConfigReconciler", "kubeadmconfig")
+	tr := tlog.Reconciler(r)
 	b := ctrl.NewControllerManagedBy(mgr).
 		For(&bootstrapv1.KubeadmConfig{}).
 		WithOptions(options).
diff --git a/controlplane/kubeadm/internal/controllers/controller.go b/controlplane/kubeadm/internal/controllers/controller.go
@@ -79,7 +79,7 @@ type KubeadmControlPlaneReconciler struct {
 }
 
 func (r *KubeadmControlPlaneReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
-	tr := tlog.Reconciler(r, "controllers.KubeadmControlPlaneReconciler", "kubeadmcontrolplane")
+	tr := tlog.Reconciler(r)
 	c, err := ctrl.NewControllerManagedBy(mgr).
 		For(&controlplanev1.KubeadmControlPlane{}).
 		Owns(&clusterv1.Machine{}).
@@ -253,6 +253,7 @@ func patchKubeadmControlPlane(ctx context.Context, patchHelper *patch.Helper, kc
 // reconcile handles KubeadmControlPlane reconciliation.
 func (r *KubeadmControlPlaneReconciler) reconcile(ctx context.Context, cluster *clusterv1.Cluster, kcp *controlplanev1.KubeadmControlPlane) (res ctrl.Result, reterr error) {
 	log := ctrl.LoggerFrom(ctx)
+	log.Info("Reconcile KubeadmControlPlane")
 
 	// Make sure to reconcile the external infrastructure reference.
 	if err := r.reconcileExternalReference(ctx, cluster, &kcp.Spec.MachineTemplate.InfrastructureRef); err != nil {
diff --git a/hack/observability/promtail/values.yaml b/hack/observability/promtail/values.yaml
@@ -6,6 +6,7 @@ config:
 
   snippets:
     pipelineStages:
+    # Parse cluster and machine to make them available as labels.
     - cri: { }
     - json:
         expressions:
diff --git a/internal/controllers/cluster/cluster_controller.go b/internal/controllers/cluster/cluster_controller.go
@@ -76,7 +76,7 @@ type Reconciler struct {
 }
 
 func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
-	tr := tlog.Reconciler(r, "controllers.ClusterReconciler", "cluster")
+	tr := tlog.Reconciler(r)
 	controller, err := ctrl.NewControllerManagedBy(mgr).
 		For(&clusterv1.Cluster{}).
 		Watches(
diff --git a/internal/controllers/clusterclass/clusterclass_controller.go b/internal/controllers/clusterclass/clusterclass_controller.go
@@ -57,7 +57,7 @@ type Reconciler struct {
 }
 
 func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
-	tr := tlog.Reconciler(r, "controllers.ClusterClassReconciler", "clusterclass")
+	tr := tlog.Reconciler(r)
 	err := ctrl.NewControllerManagedBy(mgr).
 		For(&clusterv1.ClusterClass{}).
 		Named("clusterclass").
diff --git a/internal/controllers/machine/machine_controller.go b/internal/controllers/machine/machine_controller.go
@@ -103,7 +103,7 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt
 		r.nodeDeletionRetryTimeout = 10 * time.Second
 	}
 
-	tr := log.Reconciler(r, "controllers.MachineReconciler", "machine")
+	tr := log.Reconciler(r)
 	controller, err := ctrl.NewControllerManagedBy(mgr).
 		For(&clusterv1.Machine{}).
 		WithOptions(options).
@@ -297,7 +297,7 @@ func (r *Reconciler) reconcileDelete(ctx context.Context, cluster *clusterv1.Clu
 	if err != nil {
 		switch err {
 		case errNoControlPlaneNodes, errLastControlPlaneNode, errNilNodeRef, errClusterIsBeingDeleted, errControlPlaneIsBeingDeleted:
-			log.Info("Deleting Kubernetes Node associated with Machine is not allowed", "node", m.Status.NodeRef, "cause", err.Error())
+			log.Info("Deleting Kubernetes Node associated with Machine is not allowed", "node", klog.KRef("", m.Status.NodeRef.Name), "cause", err.Error())
 		default:
 			return ctrl.Result{}, errors.Wrapf(err, "failed to check if Kubernetes Node deletion is allowed")
 		}
@@ -319,7 +319,7 @@ func (r *Reconciler) reconcileDelete(ctx context.Context, cluster *clusterv1.Clu
 				return ctrl.Result{}, err
 			}
 
-			log.Info("Draining node", "node", m.Status.NodeRef.Name)
+			log.Info("Draining node", "node", klog.KRef("", m.Status.NodeRef.Name))
 			// The DrainingSucceededCondition never exists before the node is drained for the first time,
 			// so its transition time can be used to record the first time draining.
 			// This `if` condition prevents the transition time to be changed more than once.
@@ -351,7 +351,7 @@ func (r *Reconciler) reconcileDelete(ctx context.Context, cluster *clusterv1.Clu
 					r.recorder.Eventf(m, corev1.EventTypeWarning, "FailedWaitForVolumeDetach", "error wait for volume detach, node %q: %v", m.Status.NodeRef.Name, err)
 					return ctrl.Result{}, err
 				}
-				log.Info("Waiting for node volumes to be detached", "node", m.Status.NodeRef.Name)
+				log.Info("Waiting for node volumes to be detached", "node", klog.KRef("", m.Status.NodeRef.Name))
 				return ctrl.Result{}, nil
 			}
 			conditions.MarkTrue(m, clusterv1.VolumeDetachSucceededCondition)
@@ -391,7 +391,7 @@ func (r *Reconciler) reconcileDelete(ctx context.Context, cluster *clusterv1.Clu
 	// We only delete the node after the underlying infrastructure is gone.
 	// https://github.com/kubernetes-sigs/cluster-api/issues/2565
 	if isDeleteNodeAllowed {
-		log.Info("Deleting node", "node", m.Status.NodeRef.Name)
+		log.Info("Deleting node", "node", klog.KRef("", m.Status.NodeRef.Name))
 
 		var deleteNodeErr error
 		waitErr := wait.PollImmediate(2*time.Second, r.nodeDeletionRetryTimeout, func() (bool, error) {
@@ -401,7 +401,7 @@ func (r *Reconciler) reconcileDelete(ctx context.Context, cluster *clusterv1.Clu
 			return true, nil
 		})
 		if waitErr != nil {
-			log.Error(deleteNodeErr, "Timed out deleting node", "node", m.Status.NodeRef.Name)
+			log.Error(deleteNodeErr, "Timed out deleting node", "node", klog.KRef("", m.Status.NodeRef.Name))
 			conditions.MarkFalse(m, clusterv1.MachineNodeHealthyCondition, clusterv1.DeletionFailedReason, clusterv1.ConditionSeverityWarning, "")
 			r.recorder.Eventf(m, corev1.EventTypeWarning, "FailedDeleteNode", "error deleting Machine's node: %v", deleteNodeErr)
 
@@ -449,7 +449,7 @@ func (r *Reconciler) nodeDrainTimeoutExceeded(machine *clusterv1.Machine) bool {
 // isDeleteNodeAllowed returns nil only if the Machine's NodeRef is not nil
 // and if the Machine is not the last control plane node in the cluster.
 func (r *Reconciler) isDeleteNodeAllowed(ctx context.Context, cluster *clusterv1.Cluster, machine *clusterv1.Machine) error {
-	log := ctrl.LoggerFrom(ctx, "cluster", klog.KObj(cluster).String())
+	log := ctrl.LoggerFrom(ctx)
 	// Return early if the cluster is being deleted.
 	if !cluster.DeletionTimestamp.IsZero() {
 		return errClusterIsBeingDeleted
@@ -509,7 +509,7 @@ func (r *Reconciler) isDeleteNodeAllowed(ctx context.Context, cluster *clusterv1
 }
 
 func (r *Reconciler) drainNode(ctx context.Context, cluster *clusterv1.Cluster, nodeName string) (ctrl.Result, error) {
-	log := ctrl.LoggerFrom(ctx, "node", nodeName)
+	log := ctrl.LoggerFrom(ctx, "node", klog.KRef("", nodeName))
 
 	restConfig, err := remote.RESTConfig(ctx, controllerName, r.Client, util.ObjectKey(cluster))
 	if err != nil {
@@ -581,7 +581,7 @@ func (r *Reconciler) drainNode(ctx context.Context, cluster *clusterv1.Cluster,
 // because if the node is deleted before detach success, then the underline VMDK will be deleted together with the Machine
 // so after node draining we need to check if all volumes are detached before deleting the node.
 func (r *Reconciler) shouldWaitForNodeVolumes(ctx context.Context, cluster *clusterv1.Cluster, nodeName string) (bool, error) {
-	log := ctrl.LoggerFrom(ctx, "node", nodeName)
+	log := ctrl.LoggerFrom(ctx, "node", klog.KRef("", nodeName))
 
 	remoteClient, err := r.Tracker.GetClient(ctx, util.ObjectKey(cluster))
 	if err != nil {
diff --git a/internal/controllers/machine/machine_controller_noderef.go b/internal/controllers/machine/machine_controller_noderef.go
@@ -23,6 +23,7 @@ import (
 	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/klog/v2"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
@@ -188,7 +189,7 @@ func (r *Reconciler) getNode(ctx context.Context, c client.Reader, providerID *n
 			for key, node := range nl.Items {
 				nodeProviderID, err := noderefutil.NewProviderID(node.Spec.ProviderID)
 				if err != nil {
-					log.Error(err, "Failed to parse ProviderID", "node", client.ObjectKeyFromObject(&nl.Items[key]).String())
+					log.Error(err, "Failed to parse ProviderID", "node", klog.KRef("", nl.Items[key].GetName()).String())
 					continue
 				}
 
diff --git a/internal/controllers/machinedeployment/machinedeployment_controller.go b/internal/controllers/machinedeployment/machinedeployment_controller.go
@@ -74,7 +74,7 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt
 		return err
 	}
 
-	tr := tlog.Reconciler(r, "controllers.MachineDeploymentReconciler", "machinedeployment")
+	tr := tlog.Reconciler(r)
 	c, err := ctrl.NewControllerManagedBy(mgr).
 		For(&clusterv1.MachineDeployment{}).
 		Owns(&clusterv1.MachineSet{}).
diff --git a/internal/controllers/machinehealthcheck/machinehealthcheck_controller.go b/internal/controllers/machinehealthcheck/machinehealthcheck_controller.go
@@ -86,7 +86,7 @@ type Reconciler struct {
 }
 
 func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
-	tr := tlog.Reconciler(r, "controllers.MachineHealthCheck", "machinehealthcheck")
+	tr := tlog.Reconciler(r)
 	controller, err := ctrl.NewControllerManagedBy(mgr).
 		For(&clusterv1.MachineHealthCheck{}).
 		Watches(
diff --git a/internal/controllers/machineset/machineset_controller.go b/internal/controllers/machineset/machineset_controller.go
@@ -87,7 +87,7 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt
 		return err
 	}
 
-	tr := tlog.Reconciler(r, "controllers.MachineSetReconciler", "machineset")
+	tr := tlog.Reconciler(r)
 	c, err := ctrl.NewControllerManagedBy(mgr).
 		For(&clusterv1.MachineSet{}).
 		Owns(&clusterv1.Machine{}).
diff --git a/internal/controllers/topology/cluster/cluster_controller.go b/internal/controllers/topology/cluster/cluster_controller.go
@@ -75,7 +75,7 @@ type Reconciler struct {
 }
 
 func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
-	tr := tlog.Reconciler(r, "controllers.topology.ClusterReconciler", "cluster")
+	tr := tlog.Reconciler(r)
 	c, err := ctrl.NewControllerManagedBy(mgr).
 		For(&clusterv1.Cluster{}, builder.WithPredicates(
 			// Only reconcile Cluster with topology.
diff --git a/internal/controllers/topology/machinedeployment/machinedeployment_controller.go b/internal/controllers/topology/machinedeployment/machinedeployment_controller.go
@@ -53,7 +53,7 @@ type Reconciler struct {
 }
 
 func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
-	tr := tlog.Reconciler(r, "controllers.topology.MachineDeploymentReconciler", "machinedeployment")
+	tr := tlog.Reconciler(r)
 	err := ctrl.NewControllerManagedBy(mgr).
 		For(&clusterv1.MachineDeployment{}).
 		Named("topology/machinedeployment").
diff --git a/internal/controllers/topology/machineset/machineset_controller.go b/internal/controllers/topology/machineset/machineset_controller.go
@@ -54,7 +54,7 @@ type Reconciler struct {
 }
 
 func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
-	tr := tlog.Reconciler(r, "controllers.topology.MachineSetReconciler", "machineset")
+	tr := tlog.Reconciler(r)
 	err := ctrl.NewControllerManagedBy(mgr).
 		For(&clusterv1.MachineSet{}).
 		Named("topology/machineset").
diff --git a/internal/log/log.go b/internal/log/log.go
@@ -45,26 +45,22 @@ func LoggerCustomizer(log logr.Logger, controllerName, kind string) func(_ logr.
 
 // logReconciler adds logging.
 type logReconciler struct {
-	Reconciler     reconcile.Reconciler
-	controllerName string
-	kind           string
-	randSource     *rand.Rand
+	Reconciler reconcile.Reconciler
+	randSource *rand.Rand
 }
 
 // Reconciler creates a reconciles which wraps the current reconciler and adds logs & traces.
-func Reconciler(reconciler reconcile.Reconciler, reconcilerName, kind string) reconcile.Reconciler {
+func Reconciler(reconciler reconcile.Reconciler) reconcile.Reconciler {
 	var rngSeed int64
 	_ = binary.Read(crand.Reader, binary.LittleEndian, &rngSeed)
 	return &logReconciler{
-		Reconciler:     reconciler,
-		controllerName: reconcilerName,
-		kind:           kind,
-		randSource:     rand.New(rand.NewSource(rngSeed)), //nolint:gosec // math/rand is enough, we don't need crypto/rand.
+		Reconciler: reconciler,
+		randSource: rand.New(rand.NewSource(rngSeed)), //nolint:gosec // math/rand is enough, we don't need crypto/rand.
 	}
 }
 
 // Reconcile
-// FIXME: Open issue: we should really make sure the log.Error in CR gets the trace id too, by either:
+// FIXME: We should really make sure the log.Error in CR gets our k/v pairs too (currently reconcileID).
 // * creating this span in CR.
 // * disabling the error log in CR and doing it ourselves.
 func (r *logReconciler) Reconcile(ctx context.Context, req ctrl.Request) (_ ctrl.Result, reterr error) {
diff --git a/test/infrastructure/docker/internal/controllers/dockercluster_controller.go b/test/infrastructure/docker/internal/controllers/dockercluster_controller.go
@@ -193,7 +193,7 @@ func (r *DockerClusterReconciler) reconcileDelete(ctx context.Context, dockerClu
 
 // SetupWithManager will add watches for this controller.
 func (r *DockerClusterReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
-	tr := tlog.Reconciler(r, "controllers.DockerClusterReconciler", "dockercluster")
+	tr := tlog.Reconciler(r)
 	c, err := ctrl.NewControllerManagedBy(mgr).
 		For(&infrav1.DockerCluster{}).
 		WithOptions(options).
diff --git a/test/infrastructure/docker/internal/controllers/dockermachine_controller.go b/test/infrastructure/docker/internal/controllers/dockermachine_controller.go
@@ -364,7 +364,7 @@ func (r *DockerMachineReconciler) SetupWithManager(ctx context.Context, mgr ctrl
 		return err
 	}
 
-	tr := tlog.Reconciler(r, "controllers.DockerMachineReconciler", "dockermachine")
+	tr := tlog.Reconciler(r)
 	c, err := ctrl.NewControllerManagedBy(mgr).
 		For(&infrav1.DockerMachine{}).
 		WithOptions(options).

Original file line number	Diff line number	Diff line change
`@@ -113,7 +113,7 @@ func (r *KubeadmConfigReconciler) SetupWithManager(ctx context.Context, mgr ctrl`
`113`	`113`	`r.TokenTTL = DefaultTokenTTL`
`114`	`114`	`}`
`115`	`115`
`116`		`- tr := tlog.Reconciler(r, "controllers.KubeadmConfigReconciler", "kubeadmconfig")`
	`116`	`+ tr := tlog.Reconciler(r)`
`117`	`117`	`b := ctrl.NewControllerManagedBy(mgr).`
`118`	`118`	`For(&bootstrapv1.KubeadmConfig{}).`
`119`	`119`	`WithOptions(options).`
Original file line number	Diff line number	Diff line change
`@@ -76,7 +76,7 @@ type Reconciler struct {`
`76`	`76`	`}`
`77`	`77`
`78`	`78`	`func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {`
`79`		`- tr := tlog.Reconciler(r, "controllers.ClusterReconciler", "cluster")`
	`79`	`+ tr := tlog.Reconciler(r)`
`80`	`80`	`controller, err := ctrl.NewControllerManagedBy(mgr).`
`81`	`81`	`For(&clusterv1.Cluster{}).`
`82`	`82`	`Watches(`
Original file line number	Diff line number	Diff line change
`@@ -57,7 +57,7 @@ type Reconciler struct {`
`57`	`57`	`}`
`58`	`58`
`59`	`59`	`func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {`
`60`		`- tr := tlog.Reconciler(r, "controllers.ClusterClassReconciler", "clusterclass")`
	`60`	`+ tr := tlog.Reconciler(r)`
`61`	`61`	`err := ctrl.NewControllerManagedBy(mgr).`
`62`	`62`	`For(&clusterv1.ClusterClass{}).`
`63`	`63`	`Named("clusterclass").`
Original file line number	Diff line number	Diff line change
`@@ -74,7 +74,7 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt`
`74`	`74`	`return err`
`75`	`75`	`}`
`76`	`76`
`77`		`- tr := tlog.Reconciler(r, "controllers.MachineDeploymentReconciler", "machinedeployment")`
	`77`	`+ tr := tlog.Reconciler(r)`
`78`	`78`	`c, err := ctrl.NewControllerManagedBy(mgr).`
`79`	`79`	`For(&clusterv1.MachineDeployment{}).`
`80`	`80`	`Owns(&clusterv1.MachineSet{}).`
Original file line number	Diff line number	Diff line change
`@@ -86,7 +86,7 @@ type Reconciler struct {`
`86`	`86`	`}`
`87`	`87`
`88`	`88`	`func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {`
`89`		`- tr := tlog.Reconciler(r, "controllers.MachineHealthCheck", "machinehealthcheck")`
	`89`	`+ tr := tlog.Reconciler(r)`
`90`	`90`	`controller, err := ctrl.NewControllerManagedBy(mgr).`
`91`	`91`	`For(&clusterv1.MachineHealthCheck{}).`
`92`	`92`	`Watches(`
Original file line number	Diff line number	Diff line change
`@@ -87,7 +87,7 @@ func (r *Reconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, opt`
`87`	`87`	`return err`
`88`	`88`	`}`
`89`	`89`
`90`		`- tr := tlog.Reconciler(r, "controllers.MachineSetReconciler", "machineset")`
	`90`	`+ tr := tlog.Reconciler(r)`
`91`	`91`	`c, err := ctrl.NewControllerManagedBy(mgr).`
`92`	`92`	`For(&clusterv1.MachineSet{}).`
`93`	`93`	`Owns(&clusterv1.Machine{}).`