Improve chained upgrade observability

Author: fabriziopandini

internal/controllers/topology/cluster/conditions.go

@@ -238,22 +238,21 @@ func (r *Reconciler) reconcileTopologyReconciledCondition(s *scope.Scope, cluste
 		}
 
 		// If MachineDeployments are upgrading surface it, if MachineDeployments are pending upgrades then surface the upgrade plans.
-		if s.UpgradeTracker.MachineDeployments.IsAnyUpgrading() {
-			fmt.Fprintf(msgBuilder, "\n  * %s upgrading to version %s", nameList("MachineDeployment", "MachineDeployments", s.UpgradeTracker.MachineDeployments.UpgradingNames()), *cpVersion)
+		upgradingMachineDeploymentNames, pendingMachineDeploymentNames, deferredMachineDeploymentNames := dedupNames(s.UpgradeTracker.MachineDeployments)
+		if len(upgradingMachineDeploymentNames) > 0 {
+			fmt.Fprintf(msgBuilder, "\n  * %s upgrading to version %s", nameList("MachineDeployment", "MachineDeployments", upgradingMachineDeploymentNames), *cpVersion)
 			if len(s.UpgradeTracker.ControlPlane.UpgradePlan) > 0 {
 				fmt.Fprintf(msgBuilder, " (%s pending)", strings.Join(s.UpgradeTracker.MachineDeployments.UpgradePlan, ", "))
 			}
 		}
 
-		pendingMachineDeploymentNames := sets.Set[string]{}.Insert(s.UpgradeTracker.MachineDeployments.PendingUpgradeNames()...)
-		pendingMachineDeploymentNames = pendingMachineDeploymentNames.Difference(sets.Set[string]{}.Insert(s.UpgradeTracker.MachineDeployments.DeferredUpgradeNames()...))
 		if len(pendingMachineDeploymentNames) > 0 && len(s.UpgradeTracker.MachineDeployments.UpgradePlan) > 0 {
-			fmt.Fprintf(msgBuilder, "\n  * %s pending upgrade to version %s", nameList("MachineDeployment", "MachineDeployments", pendingMachineDeploymentNames.UnsortedList()), strings.Join(s.UpgradeTracker.MachineDeployments.UpgradePlan, ", "))
+			fmt.Fprintf(msgBuilder, "\n  * %s pending upgrade to version %s", nameList("MachineDeployment", "MachineDeployments", pendingMachineDeploymentNames), strings.Join(s.UpgradeTracker.MachineDeployments.UpgradePlan, ", "))
 		}
 
 		// If MachineDeployments has been deferred or put on hold, surface it.
-		if s.UpgradeTracker.MachineDeployments.IsAnyUpgradeDeferred() {
-			fmt.Fprintf(msgBuilder, "\n  * %s upgrade to version %s deferred using topology.cluster.x-k8s.io/defer-upgrade or hold-upgrade-sequence annotations", nameList("MachineDeployment", "MachineDeployments", s.UpgradeTracker.MachineDeployments.DeferredUpgradeNames()), *cpVersion)
+		if len(deferredMachineDeploymentNames) > 0 {
+			fmt.Fprintf(msgBuilder, "\n  * %s upgrade to version %s deferred using topology.cluster.x-k8s.io/defer-upgrade or hold-upgrade-sequence annotations", nameList("MachineDeployment", "MachineDeployments", deferredMachineDeploymentNames), *cpVersion)
 			// If Deferred upgrades are blocking an upgrade, surface it.
 			// Note: Hook blocking takes the precedence on this signal.
 			if !s.HookResponseTracker.IsAnyBlocking() &&
@@ -270,22 +269,21 @@ func (r *Reconciler) reconcileTopologyReconciledCondition(s *scope.Scope, cluste
 		}
 
 		// If MachinePools are upgrading surface it, if MachinePools are pending upgrades then surface the upgrade plans.
-		if s.UpgradeTracker.MachinePools.IsAnyUpgrading() {
-			fmt.Fprintf(msgBuilder, "\n  * %s upgrading to version %s", nameList("MachinePool", "MachinePools", s.UpgradeTracker.MachinePools.UpgradingNames()), *cpVersion)
+		upgradingMachinePoolNames, pendingMachinePoolNames, deferredMachinePoolNames := dedupNames(s.UpgradeTracker.MachinePools)
+		if len(upgradingMachinePoolNames) > 0 {
+			fmt.Fprintf(msgBuilder, "\n  * %s upgrading to version %s", nameList("MachinePool", "MachinePools", upgradingMachinePoolNames), *cpVersion)
 			if len(s.UpgradeTracker.ControlPlane.UpgradePlan) > 0 {
 				fmt.Fprintf(msgBuilder, " (%s pending)", strings.Join(s.UpgradeTracker.MachinePools.UpgradePlan, ", "))
 			}
 		}
 
-		pendingMachinePoolNames := sets.Set[string]{}.Insert(s.UpgradeTracker.MachinePools.PendingUpgradeNames()...)
-		pendingMachinePoolNames = pendingMachinePoolNames.Difference(sets.Set[string]{}.Insert(s.UpgradeTracker.MachinePools.DeferredUpgradeNames()...))
 		if len(pendingMachinePoolNames) > 0 && len(s.UpgradeTracker.MachinePools.UpgradePlan) > 0 {
-			fmt.Fprintf(msgBuilder, "\n  * %s pending upgrade to version %s", nameList("MachinePool", "MachinePools", pendingMachinePoolNames.UnsortedList()), strings.Join(s.UpgradeTracker.MachinePools.UpgradePlan, ", "))
+			fmt.Fprintf(msgBuilder, "\n  * %s pending upgrade to version %s", nameList("MachinePool", "MachinePools", pendingMachinePoolNames), strings.Join(s.UpgradeTracker.MachinePools.UpgradePlan, ", "))
 		}
 
 		// If MachinePools has been deferred or put on hold, surface it.
-		if s.UpgradeTracker.MachinePools.IsAnyUpgradeDeferred() {
-			fmt.Fprintf(msgBuilder, "\n  * %s upgrade to version %s deferred using topology.cluster.x-k8s.io/defer-upgrade or hold-upgrade-sequence annotations", nameList("MachinePool", "MachinePools", s.UpgradeTracker.MachinePools.DeferredUpgradeNames()), *cpVersion)
+		if len(deferredMachinePoolNames) > 0 {
+			fmt.Fprintf(msgBuilder, "\n  * %s upgrade to version %s deferred using topology.cluster.x-k8s.io/defer-upgrade or hold-upgrade-sequence annotations", nameList("MachinePool", "MachinePools", deferredMachinePoolNames), *cpVersion)
 			// If Deferred upgrades are blocking an upgrade, surface it.
 			// Note: Hook blocking takes the precedence on this signal.
 			if !s.HookResponseTracker.IsAnyBlocking() &&
@@ -333,6 +331,17 @@ func (r *Reconciler) reconcileTopologyReconciledCondition(s *scope.Scope, cluste
 	return nil
 }
 
+// dedupNames take care of names that might exist in multiple lists.
+func dedupNames(t scope.WorkerUpgradeTracker) ([]string, []string, []string) {
+	// upgrading names are preserved
+	upgradingSet := sets.Set[string]{}.Insert(t.UpgradingNames()...)
+	// upgrading names are removed from deferred names (give precedence to the fact that it is upgrading now)
+	deferredSet := sets.Set[string]{}.Insert(t.DeferredUpgradeNames()...).Difference(upgradingSet)
+	// upgrading and deferred names are removed from pending names (it is pending if not upgrading or deferred)
+	pendingSet := sets.Set[string]{}.Insert(t.PendingUpgradeNames()...).Difference(upgradingSet).Difference(deferredSet)
+	return upgradingSet.UnsortedList(), pendingSet.UnsortedList(), deferredSet.UnsortedList()
+}
+
 // computeNameList computes list of names from the given list to be shown in conditions.
 // It shortens the list to at most 5 names and adds an ellipsis at the end if the list
 // has more than 3 elements.