Use internal error reason more consistently

# Conflicts:
#	api/v1beta1/machine_types.go

Author: fabriziopandini

api/v1beta1/machine_types.go

@@ -98,13 +98,12 @@ const (
 	// MachineWaitingForMinReadySecondsV1Beta2Reason surfaces when a machine is ready for less than MinReadySeconds (and thus not yet available).
 	MachineWaitingForMinReadySecondsV1Beta2Reason = "WaitingForMinReadySeconds"
 
-	// MachineReadyNotYetReportedV1Beta2Reason surfaces when a machine ready is not reported yet.
-	// Note: this should never happen and it is a signal of some internal error.
-	MachineReadyNotYetReportedV1Beta2Reason = "ReadyNotYetReported"
-
 	// MachineAvailableV1Beta2Reason surfaces when a machine is ready for at least MinReadySeconds.
 	// Note: MinReadySeconds is assumed 0 until it will be implemented in v1beta2 API.
 	MachineAvailableV1Beta2Reason = AvailableV1Beta2Reason
+
+	// MachineAvailableInternalErrorV1Beta2Reason surfaces unexpected error when computing the Available condition.
+	MachineAvailableInternalErrorV1Beta2Reason = InternalErrorV1Beta2Reason
 )
 
 // Machine's Ready condition and corresponding reasons that will be used in v1Beta2 API version.
@@ -115,9 +114,8 @@ const (
 	// these conditions must be true as well.
 	MachineReadyV1Beta2Condition = ReadyV1Beta2Condition
 
-	// MachineErrorComputingReadyV1Beta2Reason surfaces when there was an error computing the ready condition.
-	// Note: this should never happen and it is a signal of some internal error.
-	MachineErrorComputingReadyV1Beta2Reason = "ErrorComputingReady"
+	// MachineReadyInternalErrorV1Beta2Reason surfaces unexpected error when computing the Ready condition.
+	MachineReadyInternalErrorV1Beta2Reason = InternalErrorV1Beta2Reason
 )
 
 // Machine's UpToDate condition and corresponding reasons that will be used in v1Beta2 API version.

internal/controllers/machine/machine_controller_status.go

@@ -114,6 +114,7 @@ func setBootstrapReadyCondition(_ context.Context, machine *clusterv1.Machine, b
 			Status:  metav1.ConditionUnknown,
 			Reason:  clusterv1.MachineBootstrapConfigInternalErrorV1Beta2Reason,
 			Message: "Please check controller logs for errors",
+			// NOTE: the error is logged by reconcileBootstrap.
 		})
 		return
 	}
@@ -169,6 +170,7 @@ func setInfrastructureReadyCondition(_ context.Context, machine *clusterv1.Machi
 			Status:  metav1.ConditionUnknown,
 			Reason:  clusterv1.MachineInfrastructureInternalErrorV1Beta2Reason,
 			Message: "Please check controller logs for errors",
+			// NOTE: the error is logged by reconcileInfrastructure.
 		})
 		return
 	}
@@ -276,13 +278,15 @@ func setNodeHealthyAndReadyConditions(ctx context.Context, machine *clusterv1.Ma
 			Status:  metav1.ConditionUnknown,
 			Reason:  clusterv1.MachineNodeInternalErrorV1Beta2Reason,
 			Message: "Please check controller logs for errors",
+			// NOTE: the error is logged by reconcileNode.
 		})
 
 		v1beta2conditions.Set(machine, metav1.Condition{
 			Type:    clusterv1.MachineNodeHealthyV1Beta2Condition,
 			Status:  metav1.ConditionUnknown,
 			Reason:  clusterv1.MachineNodeInternalErrorV1Beta2Reason,
 			Message: "Please check controller logs for errors",
+			// NOTE: the error is logged by reconcileNode.
 		})
 		return
 	}
@@ -592,14 +596,14 @@ func setReadyCondition(ctx context.Context, machine *clusterv1.Machine) {
 	}
 
 	readyCondition, err := v1beta2conditions.NewSummaryCondition(machine, clusterv1.MachineReadyV1Beta2Condition, summaryOpts...)
-	if err != nil || readyCondition == nil {
+	if err != nil {
 		// Note, this could only happen if we hit edge cases in computing the summary, which should not happen due to the fact
 		// that we are passing a non empty list of ForConditionTypes.
-		log.Error(err, "Failed to set ready condition")
+		log.Error(err, "Failed to set Ready condition")
 		readyCondition = &metav1.Condition{
 			Type:    clusterv1.MachineReadyV1Beta2Condition,
 			Status:  metav1.ConditionUnknown,
-			Reason:  clusterv1.MachineErrorComputingReadyV1Beta2Reason,
+			Reason:  clusterv1.MachineReadyInternalErrorV1Beta2Reason,
 			Message: "Please check controller logs for errors",
 		}
 	}
@@ -650,16 +654,18 @@ func calculateDeletingConditionForSummary(machine *clusterv1.Machine) v1beta2con
 	}
 }
 
-func setAvailableCondition(_ context.Context, machine *clusterv1.Machine) {
+func setAvailableCondition(ctx context.Context, machine *clusterv1.Machine) {
+	log := ctrl.LoggerFrom(ctx)
 	readyCondition := v1beta2conditions.Get(machine, clusterv1.MachineReadyV1Beta2Condition)
 
 	if readyCondition == nil {
 		// NOTE: this should never happen given that setReadyCondition is called before this method and
 		// it always add a ready condition.
+		log.Error(errors.New("Ready condition must be set before setting the available condition"), "Failed to set Available condition")
 		v1beta2conditions.Set(machine, metav1.Condition{
 			Type:    clusterv1.MachineAvailableV1Beta2Condition,
 			Status:  metav1.ConditionUnknown,
-			Reason:  clusterv1.MachineReadyNotYetReportedV1Beta2Reason,
+			Reason:  clusterv1.MachineAvailableInternalErrorV1Beta2Reason,
 			Message: "Please check controller logs for errors",
 		})
 		return

util/conditions/v1beta2/summary.go

@@ -146,7 +146,7 @@ func NewSummaryCondition(sourceObj Getter, targetConditionType string, opts ...S
 		Reason:  reason,
 		Message: message,
 		// NOTE: LastTransitionTime and ObservedGeneration will be set when this condition is added to an object by calling Set.
-	}, err
+	}, nil
 }
 
 // SetSummaryCondition is a convenience method that calls NewSummaryCondition to create a summary condition from the source object,