Improve Machine remediation logs

Author: fabriziopandini

internal/controllers/machinehealthcheck/machinehealthcheck_controller.go

@@ -377,10 +377,11 @@ func (r *Reconciler) patchUnhealthyTargets(ctx context.Context, logger logr.Logg
 	// mark for remediation
 	errList := []error{}
 	for _, t := range unhealthy {
+		logger := logger.WithValues("Machine", klog.KObj(t.Machine), "Node", klog.KObj(t.Node))
 		condition := conditions.Get(t.Machine, clusterv1.MachineHealthCheckSucceededCondition)
 
 		if annotations.IsPaused(cluster, t.Machine) {
-			logger.Info("Machine has failed health check, but machine is paused so skipping remediation", "target", t.string(), "reason", condition.Reason, "message", condition.Message)
+			logger.Info("Machine has failed health check, but machine is paused so skipping remediation", "reason", condition.Reason, "message", condition.Message)
 		} else {
 			if m.Spec.RemediationTemplate != nil {
 				// If external remediation request already exists,
@@ -424,15 +425,15 @@ func (r *Reconciler) patchUnhealthyTargets(ctx context.Context, logger logr.Logg
 				// the same Machine, users are in charge of setting health checks and remediation properly.
 				to.SetName(t.Machine.Name)
 
-				logger.Info("Target has failed health check, creating an external remediation request", "remediation request name", to.GetName(), "target", t.string(), "reason", condition.Reason, "message", condition.Message)
+				logger.Info("Machine has failed health check, creating an external remediation request", "remediation request name", to.GetName(), "reason", condition.Reason, "message", condition.Message)
 				// Create the external clone.
 				if err := r.Client.Create(ctx, to); err != nil {
 					conditions.MarkFalse(m, clusterv1.ExternalRemediationRequestAvailableCondition, clusterv1.ExternalRemediationRequestCreationFailedReason, clusterv1.ConditionSeverityError, err.Error())
 					errList = append(errList, errors.Wrapf(err, "error creating remediation request for machine %q in namespace %q within cluster %q", t.Machine.Name, t.Machine.Namespace, t.Machine.Spec.ClusterName))
 					return errList
 				}
 			} else {
-				logger.Info("Target has failed health check, marking for remediation", "target", t.string(), "reason", condition.Reason, "message", condition.Message)
+				logger.Info("Machine has failed health check, marking for remediation", "reason", condition.Reason, "message", condition.Message)
 				// NOTE: MHC is responsible for creating MachineOwnerRemediatedCondition if missing or to trigger another remediation if the previous one is completed;
 				// instead, if a remediation is in already progress, the remediation owner is responsible for completing the process and MHC should not overwrite the condition.
 				if !conditions.Has(t.Machine, clusterv1.MachineOwnerRemediatedCondition) || conditions.IsTrue(t.Machine, clusterv1.MachineOwnerRemediatedCondition) {
@@ -449,8 +450,9 @@ func (r *Reconciler) patchUnhealthyTargets(ctx context.Context, logger logr.Logg
 			t.Machine,
 			corev1.EventTypeNormal,
 			EventMachineMarkedUnhealthy,
-			"Machine %v has been marked as unhealthy",
-			t.string(),
+			"Machine %s has been marked as unhealthy by %s",
+			klog.KObj(t.Machine),
+			klog.KObj(t.MHC),
 		)
 	}
 	return errList

internal/controllers/machinehealthcheck/machinehealthcheck_targets.go

@@ -63,15 +63,6 @@ type healthCheckTarget struct {
 	nodeMissing bool
 }
 
-func (t *healthCheckTarget) string() string {
-	return fmt.Sprintf("%s/%s/%s/%s",
-		t.MHC.GetNamespace(),
-		t.MHC.GetName(),
-		t.Machine.GetName(),
-		t.nodeName(),
-	)
-}
-
 // Get the node name if the target has a node.
 func (t *healthCheckTarget) nodeName() string {
 	if t.Node != nil {
@@ -302,7 +293,7 @@ func (r *Reconciler) healthCheckTargets(targets []healthCheckTarget, logger logr
 	var healthy []healthCheckTarget
 
 	for _, t := range targets {
-		logger := logger.WithValues("target", t.string())
+		logger := logger.WithValues("Machine", klog.KObj(t.Machine), "Node", klog.KObj(t.Node))
 		logger.V(3).Info("Health checking target")
 		needsRemediation, nextCheck := t.needsRemediation(logger, timeoutForMachineToHaveNode)
 
@@ -312,13 +303,13 @@ func (r *Reconciler) healthCheckTargets(targets []healthCheckTarget, logger logr
 		}
 
 		if nextCheck > 0 {
-			logger.V(3).Info("Target is likely to go unhealthy", "timeUntilUnhealthy", nextCheck.Truncate(time.Second).String())
+			logger.V(3).Info("Machine is likely to go unhealthy", "timeUntilUnhealthy", nextCheck.Truncate(time.Second).String())
 			r.recorder.Eventf(
 				t.Machine,
 				corev1.EventTypeNormal,
 				EventDetectedUnhealthy,
-				"Machine %v has unhealthy node %v",
-				t.string(),
+				"Machine %s has unhealthy node %s",
+				t.Machine.Name,
 				t.nodeName(),
 			)
 			nextCheckTimes = append(nextCheckTimes, nextCheck)