From 3c475430586cefb60f9d2bbb4b2b89cfaa7b282b Mon Sep 17 00:00:00 2001
From: fabriziopandini <fpandini@vmware.com>
Date: Thu, 14 Nov 2024 19:57:03 +0100
Subject: [PATCH] Refine v1beta2 condition order

---
 .../cluster/cluster_controller_status.go      |  4 ++--
 .../cluster/cluster_controller_status_test.go |  8 +++----
 util/conditions/v1beta2/sort.go               | 24 ++++++++++++-------
 util/conditions/v1beta2/sort_test.go          |  4 ++--
 4 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/internal/controllers/cluster/cluster_controller_status.go b/internal/controllers/cluster/cluster_controller_status.go
index dc71ff1d4b2b..0a97e1a8c269 100644
--- a/internal/controllers/cluster/cluster_controller_status.go
+++ b/internal/controllers/cluster/cluster_controller_status.go
@@ -960,12 +960,12 @@ func setAvailableCondition(ctx context.Context, cluster *clusterv1.Cluster) {
 	log := ctrl.LoggerFrom(ctx)
 
 	forConditionTypes := v1beta2conditions.ForConditionTypes{
+		clusterv1.ClusterDeletingV1Beta2Condition,
+		clusterv1.ClusterRemoteConnectionProbeV1Beta2Condition,
 		clusterv1.ClusterInfrastructureReadyV1Beta2Condition,
 		clusterv1.ClusterControlPlaneAvailableV1Beta2Condition,
 		clusterv1.ClusterWorkersAvailableV1Beta2Condition,
-		clusterv1.ClusterRemoteConnectionProbeV1Beta2Condition,
 		clusterv1.ClusterTopologyReconciledV1Beta2Condition,
-		clusterv1.ClusterDeletingV1Beta2Condition,
 	}
 	for _, g := range cluster.Spec.AvailabilityGates {
 		forConditionTypes = append(forConditionTypes, g.ConditionType)
diff --git a/internal/controllers/cluster/cluster_controller_status_test.go b/internal/controllers/cluster/cluster_controller_status_test.go
index 09270cd84258..54489efdecf8 100644
--- a/internal/controllers/cluster/cluster_controller_status_test.go
+++ b/internal/controllers/cluster/cluster_controller_status_test.go
@@ -1730,11 +1730,11 @@ func TestSetAvailableCondition(t *testing.T) {
 				Type:   clusterv1.ClusterAvailableV1Beta2Condition,
 				Status: metav1.ConditionUnknown,
 				Reason: clusterv1.ClusterAvailableUnknownV1Beta2Reason,
-				Message: "* InfrastructureReady: Condition not yet reported\n" +
-					"* ControlPlaneAvailable: Condition not yet reported\n" +
-					"* WorkersAvailable: Condition not yet reported\n" +
+				Message: "* Deleting: Condition not yet reported\n" +
 					"* RemoteConnectionProbe: Condition not yet reported\n" +
-					"* Deleting: Condition not yet reported",
+					"* InfrastructureReady: Condition not yet reported\n" +
+					"* ControlPlaneAvailable: Condition not yet reported\n" +
+					"* WorkersAvailable: Condition not yet reported",
 			},
 		},
 		{
diff --git a/util/conditions/v1beta2/sort.go b/util/conditions/v1beta2/sort.go
index 74803c4efaf6..885dfdcadcc4 100644
--- a/util/conditions/v1beta2/sort.go
+++ b/util/conditions/v1beta2/sort.go
@@ -36,17 +36,15 @@ func init() {
 // and all the other conditions are sorted by Type.
 func defaultSortLessFunc(i, j metav1.Condition) bool {
 	fi, oki := orderMap[i.Type]
+	if !oki {
+		fi = orderMap[readinessAndAvailabilityGates]
+	}
 	fj, okj := orderMap[j.Type]
-	switch {
-	case oki && !okj:
-		return true
-	case !oki && okj:
-		return false
-	case oki && okj:
-		return fi < fj
+	if !okj {
+		fj = orderMap[readinessAndAvailabilityGates]
 	}
-
-	return i.Type < j.Type
+	return fi < fj ||
+		(fi == fj && i.Type < j.Type)
 }
 
 // The order array below leads to the following condition ordering:
@@ -84,6 +82,8 @@ func defaultSortLessFunc(i, j metav1.Condition) bool {
 // | -- Aggregated from Machines -- |         |     |    |    |    |         |
 // | MachinesReady                  | x       | x   | x  | x  | x  |         |
 // | MachinesUpToDate               | x       | x   | x  | x  | x  |         |
+// | -- From other controllers --   |         |     |    |    |    |         |
+// | Readiness/Availability gates   | x       |     |    |    |    | x       |
 // | -- Misc --                     |         |     |    |    |    |         |
 // | Paused                         | x       | x   | x  | x  | x  | x       |
 // | Deleting                       | x       | x   | x  | x  | x  | x       |
@@ -117,10 +117,16 @@ var order = []string{
 	clusterv1.ScalingUpV1Beta2Condition,
 	clusterv1.MachinesReadyV1Beta2Condition,
 	clusterv1.MachinesUpToDateV1Beta2Condition,
+	readinessAndAvailabilityGates,
 	clusterv1.PausedV1Beta2Condition,
 	clusterv1.DeletingV1Beta2Condition,
 }
 
+// Constants defining a placeholder for readiness and availability gates.
+const (
+	readinessAndAvailabilityGates = ""
+)
+
 // Constants inlined for ordering (we want to avoid importing the KCP API package).
 const (
 	kubeadmControlPlaneCertificatesAvailableV1Beta2Condition              = "CertificatesAvailable"
diff --git a/util/conditions/v1beta2/sort_test.go b/util/conditions/v1beta2/sort_test.go
index 9bc8d40e458d..2badfcd5313e 100644
--- a/util/conditions/v1beta2/sort_test.go
+++ b/util/conditions/v1beta2/sort_test.go
@@ -46,10 +46,10 @@ func TestDefaultSortLessFunc(t *testing.T) {
 	g.Expect(conditions).To(Equal([]metav1.Condition{
 		{Type: clusterv1.AvailableV1Beta2Condition},
 		{Type: clusterv1.ReadyV1Beta2Condition},
-		{Type: clusterv1.PausedV1Beta2Condition},
-		{Type: clusterv1.DeletingV1Beta2Condition},
 		{Type: "A"},
 		{Type: "B"},
 		{Type: "C!"},
+		{Type: clusterv1.PausedV1Beta2Condition},
+		{Type: clusterv1.DeletingV1Beta2Condition},
 	}))
 }