More comments

Author: fabriziopandini

api/v1beta1/machine_types.go

@@ -143,7 +143,7 @@ const (
 	MachineBootstrapConfigInvalidConditionReportedV1Beta2Reason = InvalidConditionReportedV1Beta2Reason
 
 	// MachineBootstrapConfigReadyNoReasonReportedV1Beta2Reason applies to a BootstrapConfig Ready condition (read from a bootstrap config object) that reports no reason.
-	MachineBootstrapConfigReadyNoReasonReportedV1Beta2Reason = NoV1Beta2ReasonReported
+	MachineBootstrapConfigReadyNoReasonReportedV1Beta2Reason = NoReasonReportedV1Beta2Reason
 
 	// MachineBootstrapConfigInternalErrorV1Beta2Reason surfaces unexpected failures when reading a BootstrapConfig object.
 	MachineBootstrapConfigInternalErrorV1Beta2Reason = InternalErrorV1Beta2Reason
@@ -169,7 +169,7 @@ const (
 	MachineInfrastructureInvalidConditionReportedV1Beta2Reason = InvalidConditionReportedV1Beta2Reason
 
 	// MachineInfrastructureReadyNoReasonReportedV1Beta2Reason applies to a infrastructure Ready condition (read from an infra machine object) that reports no reason.
-	MachineInfrastructureReadyNoReasonReportedV1Beta2Reason = NoV1Beta2ReasonReported
+	MachineInfrastructureReadyNoReasonReportedV1Beta2Reason = NoReasonReportedV1Beta2Reason
 
 	// MachineInfrastructureInternalErrorV1Beta2Reason surfaces unexpected failures when reading a infra machine object.
 	MachineInfrastructureInternalErrorV1Beta2Reason = InternalErrorV1Beta2Reason

api/v1beta1/v1beta2_condition_consts.go

@@ -91,9 +91,9 @@ const (
 	// (e.g. its status is missing).
 	InvalidConditionReportedV1Beta2Reason = "InvalidConditionReported"
 
-	// NoV1Beta2ReasonReported applies to a condition, usually read from an external object, that reports no reason.
+	// NoReasonReportedV1Beta2Reason applies to a condition, usually read from an external object, that reports no reason.
 	// Note: this could happen e.g. when an external object still uses Cluster API v1beta1 Conditions.
-	NoV1Beta2ReasonReported = "NoReasonReported"
+	NoReasonReportedV1Beta2Reason = "NoReasonReported"
 
 	// InternalErrorV1Beta2Reason surfaces unexpected errors reporting by controllers.
 	// In most cases, it will be required to look at controllers logs to properly triage those issues.

internal/controllers/machine/machine_controller_status.go

@@ -234,7 +234,7 @@ func setNodeHealthyAndReadyConditions(ctx context.Context, machine *clusterv1.Ma
 			}
 			reason := condition.Reason
 			if reason == "" {
-				reason = clusterv1.NoV1Beta2ReasonReported
+				reason = clusterv1.NoReasonReportedV1Beta2Reason
 			}
 			nodeReady = &metav1.Condition{
 				Type:               clusterv1.MachineNodeReadyV1Beta2Condition,
@@ -461,7 +461,7 @@ func (c machineConditionCustomMergeStrategy) Merge(conditions []v1beta2condition
 			}
 			// Note: MachineNodeReadyV1Beta2Condition is not relevant for the summary.
 		}
-		return v1beta2conditions.GetDefaultMergePriority(nil)(condition)
+		return v1beta2conditions.GetDefaultMergePriorityFunc(nil)(condition)
 	}).Merge(conditions, conditionTypes)
 }
 

internal/controllers/machine/machine_controller_status_test.go

@@ -132,8 +132,12 @@ func TestSetBootstrapReadyCondition(t *testing.T) {
 			},
 		},
 		{
-			name:    "Use status.BoostrapReady flag as a fallback Ready condition from bootstrap config is missing (ready true)",
-			machine: defaultMachine.DeepCopy(),
+			name: "Use status.BoostrapReady flag as a fallback Ready condition from bootstrap config is missing (ready true)",
+			machine: func() *clusterv1.Machine {
+				m := defaultMachine.DeepCopy()
+				m.Status.BootstrapReady = true
+				return m
+			}(),
 			bootstrapConfig: &unstructured.Unstructured{Object: map[string]interface{}{
 				"kind":       "GenericBootstrapConfig",
 				"apiVersion": "bootstrap.cluster.x-k8s.io/v1beta1",
@@ -148,9 +152,9 @@ func TestSetBootstrapReadyCondition(t *testing.T) {
 			bootstrapConfigIsNotFound: false,
 			expectCondition: metav1.Condition{
 				Type:    clusterv1.MachineBootstrapConfigReadyV1Beta2Condition,
-				Status:  metav1.ConditionFalse,
+				Status:  metav1.ConditionTrue,
 				Reason:  clusterv1.MachineBootstrapConfigReadyNoReasonReportedV1Beta2Reason,
-				Message: "GenericBootstrapConfig status.ready is false",
+				Message: "GenericBootstrapConfig status.ready is true",
 			},
 		},
 		{
@@ -322,8 +326,12 @@ func TestSetInfrastructureReadyCondition(t *testing.T) {
 			},
 		},
 		{
-			name:    "Use status.InfrastructureReady flag as a fallback Ready condition from infra machine is missing (ready true)",
-			machine: defaultMachine.DeepCopy(),
+			name: "Use status.InfrastructureReady flag as a fallback Ready condition from infra machine is missing (ready true)",
+			machine: func() *clusterv1.Machine {
+				m := defaultMachine.DeepCopy()
+				m.Status.InfrastructureReady = true
+				return m
+			}(),
 			infraMachine: &unstructured.Unstructured{Object: map[string]interface{}{
 				"kind":       "GenericInfrastructureMachine",
 				"apiVersion": "infrastructure.cluster.x-k8s.io/v1beta1",
@@ -338,9 +346,9 @@ func TestSetInfrastructureReadyCondition(t *testing.T) {
 			infraMachineIsNotFound: false,
 			expectCondition: metav1.Condition{
 				Type:    clusterv1.MachineInfrastructureReadyV1Beta2Condition,
-				Status:  metav1.ConditionFalse,
+				Status:  metav1.ConditionTrue,
 				Reason:  clusterv1.MachineInfrastructureReadyNoReasonReportedV1Beta2Reason,
-				Message: "GenericInfrastructureMachine status.ready is false",
+				Message: "GenericInfrastructureMachine status.ready is true",
 			},
 		},
 		{

util/conditions/v1beta2/merge_strategies.go

@@ -82,16 +82,16 @@ func DefaultMergeStrategyWithCustomPriority(getPriorityFunc func(condition metav
 
 func newDefaultMergeStrategy(negativePolarityConditionTypes sets.Set[string]) MergeStrategy {
 	return &defaultMergeStrategy{
-		getPriorityFunc: GetDefaultMergePriority(negativePolarityConditionTypes),
+		getPriorityFunc: GetDefaultMergePriorityFunc(negativePolarityConditionTypes),
 	}
 }
 
-// GetDefaultMergePriority returns the merge priority for each condition.
+// GetDefaultMergePriorityFunc returns the merge priority for each condition.
 // It assigns following priority values to conditions:
 // - issues: conditions with positive polarity (normal True) and status False or conditions with negative polarity (normal False) and status True.
 // - unknown: conditions with status unknown.
 // - info: conditions with positive polarity (normal True) and status True or conditions with negative polarity (normal False) and status False.
-func GetDefaultMergePriority(negativePolarityConditionTypes sets.Set[string]) func(condition metav1.Condition) MergePriority {
+func GetDefaultMergePriorityFunc(negativePolarityConditionTypes sets.Set[string]) func(condition metav1.Condition) MergePriority {
 	return func(condition metav1.Condition) MergePriority {
 		switch condition.Status {
 		case metav1.ConditionTrue:

util/conditions/v1beta2/merge_strategies_test.go

@@ -108,7 +108,7 @@ func TestSplitConditionsByPriority(t *testing.T) {
 		{OwnerResource: ConditionOwnerInfo{Name: "baz"}, Condition: metav1.Condition{Type: "!C", Status: metav1.ConditionFalse}},   // info
 	}
 
-	issueConditions, unknownConditions, infoConditions := splitConditionsByPriority(conditions, GetDefaultMergePriority(sets.New[string]("!C")))
+	issueConditions, unknownConditions, infoConditions := splitConditionsByPriority(conditions, GetDefaultMergePriorityFunc(sets.New[string]("!C")))
 
 	// Check condition are grouped as expected and order is preserved.
 
@@ -196,7 +196,7 @@ func TestDefaultMergePriority(t *testing.T) {
 			if tt.negativePolarity {
 				negativePolarityConditionTypes.Insert(tt.condition.Type)
 			}
-			gotPriority := GetDefaultMergePriority(negativePolarityConditionTypes)(tt.condition)
+			gotPriority := GetDefaultMergePriorityFunc(negativePolarityConditionTypes)(tt.condition)
 
 			g.Expect(gotPriority).To(Equal(tt.wantPriority))
 		})