Merge pull request kubernetes-sigs#11419 from fabriziopandini/refine-…

…v1beta2-mirror-conditions

🌱 Refine v1beta2 mirror conditions

api/v1beta1/cluster_types.go

@@ -126,13 +126,16 @@ const (
 	// ClusterInfrastructureReadyV1Beta2Condition mirrors Cluster's infrastructure Ready condition.
 	ClusterInfrastructureReadyV1Beta2Condition = InfrastructureReadyV1Beta2Condition
 
+	// ClusterInfrastructureReadyV1Beta2Reason surfaces when the cluster infrastructure is ready.
+	ClusterInfrastructureReadyV1Beta2Reason = ReadyV1Beta2Reason
+
+	// ClusterInfrastructureNotReadyV1Beta2Reason surfaces when the cluster infrastructure is not ready.
+	ClusterInfrastructureNotReadyV1Beta2Reason = NotReadyV1Beta2Reason
+
 	// ClusterInfrastructureInvalidConditionReportedV1Beta2Reason surfaces a infrastructure Ready condition (read from an infra cluster object) which is invalid
 	// (e.g. its status is missing).
 	ClusterInfrastructureInvalidConditionReportedV1Beta2Reason = InvalidConditionReportedV1Beta2Reason
 
-	// ClusterInfrastructureReadyNoReasonReportedV1Beta2Reason applies to a infrastructure Ready condition (read from an infra cluster object) that reports no reason.
-	ClusterInfrastructureReadyNoReasonReportedV1Beta2Reason = NoReasonReportedV1Beta2Reason
-
 	// ClusterInfrastructureInternalErrorV1Beta2Reason surfaces unexpected failures when reading an infra cluster object.
 	ClusterInfrastructureInternalErrorV1Beta2Reason = InternalErrorV1Beta2Reason
 
@@ -170,13 +173,16 @@ const (
 	// ClusterControlPlaneAvailableV1Beta2Condition is a mirror of Cluster's control plane Available condition.
 	ClusterControlPlaneAvailableV1Beta2Condition = "ControlPlaneAvailable"
 
+	// ClusterControlPlaneAvailableV1Beta2Reason surfaces when the cluster control plane is available.
+	ClusterControlPlaneAvailableV1Beta2Reason = AvailableV1Beta2Reason
+
+	// ClusterControlPlaneNotAvailableV1Beta2Reason surfaces when the cluster control plane is not available.
+	ClusterControlPlaneNotAvailableV1Beta2Reason = NotAvailableV1Beta2Reason
+
 	// ClusterControlPlaneInvalidConditionReportedV1Beta2Reason surfaces a control plane Available condition (read from a control plane object) which is invalid.
 	// (e.g. its status is missing).
 	ClusterControlPlaneInvalidConditionReportedV1Beta2Reason = InvalidConditionReportedV1Beta2Reason
 
-	// ClusterControlPlaneAvailableNoReasonReportedV1Beta2Reason applies to a control plane Available condition (read from a control plane object) that reports no reason.
-	ClusterControlPlaneAvailableNoReasonReportedV1Beta2Reason = NoReasonReportedV1Beta2Reason
-
 	// ClusterControlPlaneInternalErrorV1Beta2Reason surfaces unexpected failures when reading a control plane object.
 	ClusterControlPlaneInternalErrorV1Beta2Reason = InternalErrorV1Beta2Reason
 

api/v1beta1/machine_types.go

@@ -152,13 +152,16 @@ const (
 	// from a BoostrapConfig object referenced from the machine).
 	MachineBootstrapDataSecretProvidedV1Beta2Reason = "DataSecretProvided"
 
+	// MachineBootstrapConfigReadyV1Beta2Reason surfaces when the machine bootstrap config is ready.
+	MachineBootstrapConfigReadyV1Beta2Reason = ReadyV1Beta2Reason
+
+	// MachineBootstrapConfigNotReadyV1Beta2Reason surfaces when the machine bootstrap config is not ready.
+	MachineBootstrapConfigNotReadyV1Beta2Reason = NotReadyV1Beta2Reason
+
 	// MachineBootstrapConfigInvalidConditionReportedV1Beta2Reason surfaces a BootstrapConfig Ready condition (read from a bootstrap config object) which is invalid.
 	// (e.g. its status is missing).
 	MachineBootstrapConfigInvalidConditionReportedV1Beta2Reason = InvalidConditionReportedV1Beta2Reason
 
-	// MachineBootstrapConfigReadyNoReasonReportedV1Beta2Reason applies to a BootstrapConfig Ready condition (read from a bootstrap config object) that reports no reason.
-	MachineBootstrapConfigReadyNoReasonReportedV1Beta2Reason = NoReasonReportedV1Beta2Reason
-
 	// MachineBootstrapConfigInternalErrorV1Beta2Reason surfaces unexpected failures when reading a BootstrapConfig object.
 	MachineBootstrapConfigInternalErrorV1Beta2Reason = InternalErrorV1Beta2Reason
 
@@ -175,16 +178,19 @@ const (
 // Machine's InfrastructureReady condition and corresponding reasons that will be used in v1Beta2 API version.
 // Note: when possible, InfrastructureReady condition will use reasons surfaced from the underlying infra machine object.
 const (
-	// MachineInfrastructureReadyV1Beta2Condition mirrors the corresponding Ready condition from the Machine's Infrastructure resource.
+	// MachineInfrastructureReadyV1Beta2Condition mirrors the corresponding Ready condition from the Machine's infrastructure resource.
 	MachineInfrastructureReadyV1Beta2Condition = InfrastructureReadyV1Beta2Condition
 
+	// MachineInfrastructureReadyV1Beta2Reason surfaces when the machine infrastructure is ready.
+	MachineInfrastructureReadyV1Beta2Reason = ReadyV1Beta2Reason
+
+	// MachineInfrastructureNotReadyV1Beta2Reason surfaces when the machine infrastructure is not ready.
+	MachineInfrastructureNotReadyV1Beta2Reason = NotReadyV1Beta2Reason
+
 	// MachineInfrastructureInvalidConditionReportedV1Beta2Reason surfaces a infrastructure Ready condition (read from an infra machine object) which is invalid.
 	// (e.g. its status is missing).
 	MachineInfrastructureInvalidConditionReportedV1Beta2Reason = InvalidConditionReportedV1Beta2Reason
 
-	// MachineInfrastructureReadyNoReasonReportedV1Beta2Reason applies to a infrastructure Ready condition (read from an infra machine object) that reports no reason.
-	MachineInfrastructureReadyNoReasonReportedV1Beta2Reason = NoReasonReportedV1Beta2Reason
-
 	// MachineInfrastructureInternalErrorV1Beta2Reason surfaces unexpected failures when reading an infra machine object.
 	MachineInfrastructureInternalErrorV1Beta2Reason = InternalErrorV1Beta2Reason
 

internal/controllers/cluster/cluster_controller_status.go

@@ -254,22 +254,31 @@ func setInfrastructureReadyCondition(_ context.Context, cluster *clusterv1.Clust
 	}
 
 	if infraCluster != nil {
-		if err := v1beta2conditions.SetMirrorConditionFromUnstructured(
-			infraCluster, cluster,
+		ready, err := v1beta2conditions.NewMirrorConditionFromUnstructured(
+			infraCluster,
 			contract.InfrastructureCluster().ReadyConditionType(), v1beta2conditions.TargetConditionType(clusterv1.ClusterInfrastructureReadyV1Beta2Condition),
 			v1beta2conditions.FallbackCondition{
 				Status:  v1beta2conditions.BoolToStatus(cluster.Status.InfrastructureReady),
-				Reason:  clusterv1.ClusterInfrastructureReadyNoReasonReportedV1Beta2Reason,
+				Reason:  fallbackReason(cluster.Status.InfrastructureReady, clusterv1.ClusterInfrastructureReadyV1Beta2Reason, clusterv1.ClusterInfrastructureNotReadyV1Beta2Reason),
 				Message: infrastructureReadyFallBackMessage(cluster.Spec.InfrastructureRef.Kind, cluster.Status.InfrastructureReady),
 			},
-		); err != nil {
+		)
+		if err != nil {
 			v1beta2conditions.Set(cluster, metav1.Condition{
 				Type:    clusterv1.ClusterInfrastructureReadyV1Beta2Condition,
 				Status:  metav1.ConditionUnknown,
 				Reason:  clusterv1.ClusterInfrastructureInvalidConditionReportedV1Beta2Reason,
 				Message: err.Error(),
 			})
+			return
+		}
+
+		// In case condition has NoReasonReported and status true, we assume it is a v1beta1 condition
+		// and replace the reason with something less confusing.
+		if ready.Reason == v1beta2conditions.NoReasonReported && ready.Status == metav1.ConditionTrue {
+			ready.Reason = clusterv1.ClusterInfrastructureReadyV1Beta2Reason
 		}
+		v1beta2conditions.Set(cluster, *ready)
 		return
 	}
 
@@ -345,22 +354,31 @@ func setControlPlaneAvailableCondition(_ context.Context, cluster *clusterv1.Clu
 	}
 
 	if controlPlane != nil {
-		if err := v1beta2conditions.SetMirrorConditionFromUnstructured(
-			controlPlane, cluster,
+		available, err := v1beta2conditions.NewMirrorConditionFromUnstructured(
+			controlPlane,
 			contract.ControlPlane().AvailableConditionType(), v1beta2conditions.TargetConditionType(clusterv1.ClusterControlPlaneAvailableV1Beta2Condition),
 			v1beta2conditions.FallbackCondition{
 				Status:  v1beta2conditions.BoolToStatus(cluster.Status.ControlPlaneReady),
-				Reason:  clusterv1.ClusterControlPlaneAvailableNoReasonReportedV1Beta2Reason,
+				Reason:  fallbackReason(cluster.Status.ControlPlaneReady, clusterv1.ClusterControlPlaneAvailableV1Beta2Reason, clusterv1.ClusterControlPlaneNotAvailableV1Beta2Reason),
 				Message: controlPlaneAvailableFallBackMessage(cluster.Spec.ControlPlaneRef.Kind, cluster.Status.ControlPlaneReady),
 			},
-		); err != nil {
+		)
+		if err != nil {
 			v1beta2conditions.Set(cluster, metav1.Condition{
 				Type:    clusterv1.ClusterControlPlaneAvailableV1Beta2Condition,
 				Status:  metav1.ConditionUnknown,
 				Reason:  clusterv1.ClusterControlPlaneInvalidConditionReportedV1Beta2Reason,
 				Message: err.Error(),
 			})
+			return
 		}
+
+		// In case condition has NoReasonReported and status true, we assume it is a v1beta1 condition
+		// and replace the reason with something less confusing.
+		if available.Reason == v1beta2conditions.NoReasonReported && available.Status == metav1.ConditionTrue {
+			available.Reason = clusterv1.ClusterControlPlaneAvailableV1Beta2Reason
+		}
+		v1beta2conditions.Set(cluster, *available)
 		return
 	}
 
@@ -971,21 +989,6 @@ func setAvailableCondition(ctx context.Context, cluster *clusterv1.Cluster) {
 		summaryOpts = append(summaryOpts, v1beta2conditions.IgnoreTypesIfMissing{clusterv1.ClusterTopologyReconciledV1Beta2Condition})
 	}
 
-	// Add overrides for conditions we want to surface in the Available condition with slightly different messages.
-	var overrideConditions v1beta2conditions.OverrideConditions
-
-	if infrastructureReadyCondition := calculateInfrastructureReadyForSummary(cluster); infrastructureReadyCondition != nil {
-		overrideConditions = append(overrideConditions, *infrastructureReadyCondition)
-	}
-
-	if controlPlaneAvailableCondition := calculateControlPlaneAvailableForSummary(cluster); controlPlaneAvailableCondition != nil {
-		overrideConditions = append(overrideConditions, *controlPlaneAvailableCondition)
-	}
-
-	if len(overrideConditions) > 0 {
-		summaryOpts = append(summaryOpts, overrideConditions)
-	}
-
 	availableCondition, err := v1beta2conditions.NewSummaryCondition(cluster, clusterv1.ClusterAvailableV1Beta2Condition, summaryOpts...)
 
 	if err != nil {
@@ -1003,63 +1006,25 @@ func setAvailableCondition(ctx context.Context, cluster *clusterv1.Cluster) {
 	v1beta2conditions.Set(cluster, *availableCondition)
 }
 
-func infrastructureReadyFallBackMessage(kind string, ready bool) string {
-	return fmt.Sprintf("%s status.ready is %t", kind, ready)
-}
-
-func controlPlaneAvailableFallBackMessage(kind string, ready bool) string {
-	return fmt.Sprintf("%s status.ready is %t", kind, ready)
-}
-
-func calculateInfrastructureReadyForSummary(cluster *clusterv1.Cluster) *v1beta2conditions.ConditionWithOwnerInfo {
-	infrastructureReadyCondition := v1beta2conditions.Get(cluster, clusterv1.ClusterInfrastructureReadyV1Beta2Condition)
-
-	if infrastructureReadyCondition == nil {
-		return nil
-	}
-
-	message := infrastructureReadyCondition.Message
-	if infrastructureReadyCondition.Status == metav1.ConditionTrue && cluster.Spec.InfrastructureRef != nil && infrastructureReadyCondition.Message == infrastructureReadyFallBackMessage(cluster.Spec.InfrastructureRef.Kind, cluster.Status.InfrastructureReady) {
-		message = ""
-	}
-
-	return &v1beta2conditions.ConditionWithOwnerInfo{
-		OwnerResource: v1beta2conditions.ConditionOwnerInfo{
-			Kind: "Cluster",
-			Name: cluster.Name,
-		},
-		Condition: metav1.Condition{
-			Type:    infrastructureReadyCondition.Type,
-			Status:  infrastructureReadyCondition.Status,
-			Reason:  infrastructureReadyCondition.Reason,
-			Message: message,
-		},
+func fallbackReason(status bool, trueReason, falseReason string) string {
+	if status {
+		return trueReason
 	}
+	return falseReason
 }
 
-func calculateControlPlaneAvailableForSummary(cluster *clusterv1.Cluster) *v1beta2conditions.ConditionWithOwnerInfo {
-	controlPlaneAvailableCondition := v1beta2conditions.Get(cluster, clusterv1.ClusterControlPlaneAvailableV1Beta2Condition)
-	if controlPlaneAvailableCondition == nil {
-		return nil
-	}
-
-	message := controlPlaneAvailableCondition.Message
-	if controlPlaneAvailableCondition.Status == metav1.ConditionTrue && cluster.Spec.ControlPlaneRef != nil && controlPlaneAvailableCondition.Message == controlPlaneAvailableFallBackMessage(cluster.Spec.ControlPlaneRef.Kind, cluster.Status.ControlPlaneReady) {
-		message = ""
+func infrastructureReadyFallBackMessage(kind string, ready bool) string {
+	if ready {
+		return ""
 	}
+	return fmt.Sprintf("%s status.ready is %t", kind, ready)
+}
 
-	return &v1beta2conditions.ConditionWithOwnerInfo{
-		OwnerResource: v1beta2conditions.ConditionOwnerInfo{
-			Kind: "Cluster",
-			Name: cluster.Name,
-		},
-		Condition: metav1.Condition{
-			Type:    controlPlaneAvailableCondition.Type,
-			Status:  controlPlaneAvailableCondition.Status,
-			Reason:  controlPlaneAvailableCondition.Reason,
-			Message: message,
-		},
+func controlPlaneAvailableFallBackMessage(kind string, ready bool) string {
+	if ready {
+		return ""
 	}
+	return fmt.Sprintf("%s status.ready is %t", kind, ready)
 }
 
 func aggregateUnhealthyMachines(machines collections.Machines) string {