Merge pull request #2937 from jluhrsen/CORENET-6572-codex

CORENET-6572: only report Progressing for active network rollouts

Author: openshift-merge-bot[bot]

pkg/controller/statusmanager/machineconfig_status.go

@@ -140,55 +140,43 @@ func (status *StatusManager) SetFromMachineConfigPool(mcPools []mcfgv1.MachineCo
 	// No degraded pools, so clear degraded status
 	status.setNotDegraded(MachineConfig)
 
-	// Now check for progressing and process machine configs
 	for role, machineConfigs := range status.renderedMachineConfigs {
 		pools, err := status.findMachineConfigPoolsForLabel(mcPools, map[string]string{names.MachineConfigLabelRoleKey: role})
 		if err != nil {
 			klog.Errorf("failed to get machine config pools for the role %s: %v", role, err)
 		}
+		for _, machineConfig := range machineConfigs.UnsortedList() {
+			mcSet := sets.New[string](machineConfig)
+			beingRemoved := status.machineConfigsBeingRemoved[role].Has(machineConfig)
 
-		progressingPool := status.isAnyMachineConfigPoolProgressing(pools)
-		if progressingPool != "" {
-			status.setProgressing(MachineConfig, "MachineConfig", fmt.Sprintf("%s machine config pool in progressing state", progressingPool))
-			return nil
-		}
-		for _, pool := range pools {
-			if pool.Spec.Paused {
-				// When a machine config pool is in paused state, then it is expected that mco doesn't process any machine configs for the pool.
-				// so if we report network status as progressing state then it blocks networking upgrade until machine config pool is changed
-				// into unpaused state. so let's not consider the pool for reporting status.
-				continue
-			}
-			for _, machineConfig := range machineConfigs.UnsortedList() {
-				added := true
-				removed := true
-				mcSet := sets.Set[string]{}
-				mcSet.Insert(machineConfig)
-				if mcsBeingRemoved, ok := status.machineConfigsBeingRemoved[role]; ok && mcsBeingRemoved.Has(machineConfig) {
-					removed = mcutil.AreMachineConfigsRemovedFromPool(pool.Status, mcSet)
-					if removed {
-						status.machineConfigsBeingRemoved[role].Delete(machineConfig)
-						// Delete map entry from status cache if role doesn't have machine configs. By deleting the entry,
-						// there won't be any unnecessary processing of pools in the reconcile loop when it's not dealing
-						// with network operator machine configs anymore.
-						if status.machineConfigsBeingRemoved[role].Len() == 0 {
-							delete(status.machineConfigsBeingRemoved, role)
-						}
-						status.renderedMachineConfigs[role].Delete(machineConfig)
-						if status.renderedMachineConfigs[role].Len() == 0 {
-							delete(status.renderedMachineConfigs, role)
-						}
-						if err := status.setLastRenderedMachineConfigState(status.renderedMachineConfigs); err != nil {
-							return fmt.Errorf("failed to update rendered machine config state: %v", err)
-						}
+			sawNonPausedPool := false
+			for _, pool := range pools {
+				if pool.Spec.Paused {
+					// When a machine config pool is in paused state, then it is expected that mco doesn't process any machine configs for the pool.
+					// so if we report network status as progressing state then it blocks networking upgrade until machine config pool is changed
+					// into unpaused state. so let's not consider the pool for reporting status.
+					continue
+				}
+				sawNonPausedPool = true
+
+				if beingRemoved {
+					if mcutil.AreMachineConfigsRemovedFromPoolSource(pool.Status, mcSet) {
+						continue
 					}
-				} else {
-					added = mcutil.AreMachineConfigsRenderedOnPool(pool.Status, mcSet)
+				} else if mcutil.AreMachineConfigsRenderedOnPoolSource(pool.Status, mcSet) {
+					continue
 				}
-				if !added || !removed {
-					status.setProgressing(MachineConfig, "MachineConfig",
-						fmt.Sprintf("%s machine config pool is still processing %s machine config", pool.Name, machineConfig))
-					return nil
+
+				status.setProgressing(MachineConfig, "MachineConfig",
+					fmt.Sprintf("%s machine config pool is still processing %s machine config", pool.Name, machineConfig))
+				return nil
+			}
+
+			// Wait to prune cached removal state until every non-paused pool for
+			// this role reflects the updated rendered source.
+			if beingRemoved && sawNonPausedPool {
+				if err := status.forgetRemovedMachineConfig(role, machineConfig); err != nil {
+					return err
 				}
 			}
 		}
@@ -197,6 +185,24 @@ func (status *StatusManager) SetFromMachineConfigPool(mcPools []mcfgv1.MachineCo
 	return nil
 }
 
+func (status *StatusManager) forgetRemovedMachineConfig(role, machineConfig string) error {
+	status.machineConfigsBeingRemoved[role].Delete(machineConfig)
+	// Delete map entry from status cache if role doesn't have machine configs. By deleting the entry,
+	// there won't be any unnecessary processing of pools in the reconcile loop when it's not dealing
+	// with network operator machine configs anymore.
+	if status.machineConfigsBeingRemoved[role].Len() == 0 {
+		delete(status.machineConfigsBeingRemoved, role)
+	}
+	status.renderedMachineConfigs[role].Delete(machineConfig)
+	if status.renderedMachineConfigs[role].Len() == 0 {
+		delete(status.renderedMachineConfigs, role)
+	}
+	if err := status.setLastRenderedMachineConfigState(status.renderedMachineConfigs); err != nil {
+		return fmt.Errorf("failed to update rendered machine config state: %v", err)
+	}
+	return nil
+}
+
 func (status *StatusManager) getLastRenderedMachineConfigState() (map[string]sets.Set[string], error) {
 	renderedMachineConfigs := map[string]sets.Set[string]{}
 	co := &configv1.ClusterOperator{ObjectMeta: metav1.ObjectMeta{Name: status.name}}
@@ -250,17 +256,6 @@ func (status *StatusManager) isAnyMachineConfigPoolDegraded(pools []mcfgv1.Machi
 	return degradedPool
 }
 
-func (status *StatusManager) isAnyMachineConfigPoolProgressing(pools []mcfgv1.MachineConfigPool) string {
-	var progressingPool string
-	for _, pool := range pools {
-		if mcomcfgv1.IsMachineConfigPoolConditionTrue(pool.Status.Conditions, mcfgv1.MachineConfigPoolUpdating) {
-			progressingPool = pool.Name
-			break
-		}
-	}
-	return progressingPool
-}
-
 func (status *StatusManager) findMachineConfigPoolsForLabel(mcPools []mcfgv1.MachineConfigPool, mcLabel labels.Set) ([]mcfgv1.MachineConfigPool, error) {
 	var mcps []mcfgv1.MachineConfigPool
 	for _, mcPool := range mcPools {