CORENET-6572: only report Progressing for active network rollouts

Keep pod-based Progressing tied to an actual CNO rollout instead of temporary unavailability during node reboot churn. Persist the rollout-active state so Progressing stays true until the rollout is fully available, while still ignoring plain reboot churn after install.\n\nFor machine config status, stop treating generic MCP node convergence as a CNO rollout signal. Reuse shared mcutil helpers for source-only checks so routine MCO updates do not flip network Progressing to true.

Signed-off-by: Jamo Luhrsen <[email protected]>

Author: jluhrsen

pkg/controller/statusmanager/machineconfig_status.go

@@ -140,18 +140,11 @@ 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)
 		}
-
-		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.
@@ -165,7 +158,7 @@ func (status *StatusManager) SetFromMachineConfigPool(mcPools []mcfgv1.MachineCo
 				mcSet := sets.Set[string]{}
 				mcSet.Insert(machineConfig)
 				if mcsBeingRemoved, ok := status.machineConfigsBeingRemoved[role]; ok && mcsBeingRemoved.Has(machineConfig) {
-					removed = mcutil.AreMachineConfigsRemovedFromPool(pool.Status, mcSet)
+					removed = mcutil.AreMachineConfigsRemovedFromPoolSource(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,
@@ -183,7 +176,7 @@ func (status *StatusManager) SetFromMachineConfigPool(mcPools []mcfgv1.MachineCo
 						}
 					}
 				} else {
-					added = mcutil.AreMachineConfigsRenderedOnPool(pool.Status, mcSet)
+					added = mcutil.AreMachineConfigsRenderedOnPoolSource(pool.Status, mcSet)
 				}
 				if !added || !removed {
 					status.setProgressing(MachineConfig, "MachineConfig",
@@ -250,17 +243,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 {

pkg/controller/statusmanager/pod_status.go

@@ -38,6 +38,7 @@ type podState struct {
 	DaemonsetStates   []daemonsetState
 	DeploymentStates  []deploymentState
 	StatefulsetStates []statefulsetState
+	InstallComplete   bool
 }
 
 // daemonsetState is the internal state we use to check if a rollout has
@@ -47,6 +48,7 @@ type daemonsetState struct {
 
 	LastSeenStatus appsv1.DaemonSetStatus
 	LastChangeTime time.Time
+	RolloutActive  bool
 }
 
 // deploymentState is the same as daemonsetState.. but for deployments!
@@ -55,6 +57,7 @@ type deploymentState struct {
 
 	LastSeenStatus appsv1.DeploymentStatus
 	LastChangeTime time.Time
+	RolloutActive  bool
 }
 
 // statefulsetState is the same as daemonsetState.. but for statefulsets!
@@ -63,6 +66,7 @@ type statefulsetState struct {
 
 	LastSeenStatus appsv1.StatefulSetStatus
 	LastChangeTime time.Time
+	RolloutActive  bool
 }
 
 // SetFromPods sets the operator Degraded/Progressing/Available status, based on
@@ -79,14 +83,20 @@ func (status *StatusManager) SetFromPods() {
 	progressing := []string{}
 	hung := []string{}
 
-	daemonsetStates, deploymentStates, statefulsetStates := status.getLastPodState()
+	daemonsetStates, deploymentStates, statefulsetStates, installComplete := status.getLastPodState()
+	if !status.installComplete && installComplete {
+		status.installComplete = true
+	}
 
 	if (len(daemonSets) + len(deployments) + len(statefulSets)) == 0 {
 		progressing = append(progressing, "Deploying")
 	}
 
 	for _, ds := range daemonSets {
 		dsName := NewClusteredName(ds)
+		dsState, hadState := daemonsetStates[dsName]
+		dsState.RolloutActive = daemonSetRolloutActive(ds, dsState.RolloutActive, status.installComplete)
+		dsRolloutActive := dsState.RolloutActive
 
 		dsProgressing := false
 
@@ -97,13 +107,14 @@ func (status *StatusManager) SetFromPods() {
 			progressing = append(progressing, fmt.Sprintf("DaemonSet %q update is rolling out (%d out of %d updated)", dsName.String(), ds.Status.UpdatedNumberScheduled, ds.Status.DesiredNumberScheduled))
 			dsProgressing = true
 		} else if ds.Status.NumberUnavailable > 0 {
-			progressing = append(progressing, fmt.Sprintf("DaemonSet %q is not available (awaiting %d nodes)", dsName.String(), ds.Status.NumberUnavailable))
-			dsProgressing = true
-			// Check for any pods in CrashLoopBackOff state and mark the operator as degraded if so.
+			if dsRolloutActive {
+				progressing = append(progressing, fmt.Sprintf("DaemonSet %q is not available (awaiting %d nodes)", dsName.String(), ds.Status.NumberUnavailable))
+				dsProgressing = true
+			}
 			if !isNonCritical(ds) {
 				hung = append(hung, status.CheckCrashLoopBackOffPods(dsName, ds.Spec.Selector.MatchLabels, "DaemonSet")...)
 			}
-		} else if ds.Status.NumberAvailable == 0 && ds.Status.DesiredNumberScheduled > 0 {
+		} else if ds.Status.NumberAvailable == 0 && dsRolloutActive {
 			progressing = append(progressing, fmt.Sprintf("DaemonSet %q is not yet scheduled on any nodes", dsName.String()))
 			dsProgressing = true
 		} else if ds.Generation > ds.Status.ObservedGeneration {
@@ -120,19 +131,20 @@ func (status *StatusManager) SetFromPods() {
 		if dsProgressing && !isNonCritical(ds) {
 			reachedAvailableLevel = false
 
-			dsState, exists := daemonsetStates[dsName]
-			if !exists || !reflect.DeepEqual(dsState.LastSeenStatus, ds.Status) {
+			if !hadState || !reflect.DeepEqual(dsState.LastSeenStatus, ds.Status) {
 				dsState.LastChangeTime = time.Now()
 				ds.Status.DeepCopyInto(&dsState.LastSeenStatus)
-				daemonsetStates[dsName] = dsState
 			}
 
 			// Catch hung rollouts
-			if exists && (time.Since(dsState.LastChangeTime)) > ProgressTimeout {
+			if hadState && (time.Since(dsState.LastChangeTime)) > ProgressTimeout {
 				hung = append(hung, fmt.Sprintf("DaemonSet %q rollout is not making progress - last change %s", dsName.String(), dsState.LastChangeTime.Format(time.RFC3339)))
 				empty := ""
 				dsHung = &empty
 			}
+		}
+		if dsRolloutActive {
+			daemonsetStates[dsName] = dsState
 		} else {
 			delete(daemonsetStates, dsName)
 		}
@@ -143,6 +155,9 @@ func (status *StatusManager) SetFromPods() {
 
 	for _, ss := range statefulSets {
 		ssName := NewClusteredName(ss)
+		ssState, hadState := statefulsetStates[ssName]
+		ssState.RolloutActive = statefulSetRolloutActive(ss, ssState.RolloutActive, status.installComplete)
+		ssRolloutActive := ssState.RolloutActive
 
 		ssProgressing := false
 
@@ -153,13 +168,15 @@ func (status *StatusManager) SetFromPods() {
 			progressing = append(progressing, fmt.Sprintf("StatefulSet %q update is rolling out (%d out of %d updated)", ssName.String(), ss.Status.UpdatedReplicas, ss.Status.Replicas))
 			ssProgressing = true
 		} else if ss.Status.ReadyReplicas > 0 && ss.Status.ReadyReplicas < ss.Status.Replicas {
-			progressing = append(progressing, fmt.Sprintf("StatefulSet %q is not available (awaiting %d nodes)", ssName.String(), (ss.Status.Replicas-ss.Status.ReadyReplicas)))
-			ssProgressing = true
+			if ssRolloutActive {
+				progressing = append(progressing, fmt.Sprintf("StatefulSet %q is not available (awaiting %d nodes)", ssName.String(), (ss.Status.Replicas-ss.Status.ReadyReplicas)))
+				ssProgressing = true
+			}
 			// Check for any pods in CrashLoopBackOff state and mark the operator as degraded if so.
 			if !isNonCritical(ss) {
 				hung = append(hung, status.CheckCrashLoopBackOffPods(ssName, ss.Spec.Selector.MatchLabels, "StatefulSet")...)
 			}
-		} else if ss.Status.AvailableReplicas == 0 {
+		} else if ss.Status.AvailableReplicas == 0 && ssRolloutActive {
 			progressing = append(progressing, fmt.Sprintf("StatefulSet %q is not yet scheduled on any nodes", ssName.String()))
 			ssProgressing = true
 		} else if ss.Status.ObservedGeneration < ss.Generation {
@@ -176,19 +193,20 @@ func (status *StatusManager) SetFromPods() {
 		if ssProgressing && !isNonCritical(ss) {
 			reachedAvailableLevel = false
 
-			ssState, exists := statefulsetStates[ssName]
-			if !exists || !reflect.DeepEqual(ssState.LastSeenStatus, ss.Status) {
+			if !hadState || !reflect.DeepEqual(ssState.LastSeenStatus, ss.Status) {
 				ssState.LastChangeTime = time.Now()
 				ss.Status.DeepCopyInto(&ssState.LastSeenStatus)
-				statefulsetStates[ssName] = ssState
 			}
 
 			// Catch hung rollouts
-			if exists && (time.Since(ssState.LastChangeTime)) > ProgressTimeout {
+			if hadState && (time.Since(ssState.LastChangeTime)) > ProgressTimeout {
 				hung = append(hung, fmt.Sprintf("StatefulSet %q rollout is not making progress - last change %s", ssName.String(), ssState.LastChangeTime.Format(time.RFC3339)))
 				empty := ""
 				ssHung = &empty
 			}
+		}
+		if ssRolloutActive {
+			statefulsetStates[ssName] = ssState
 		} else {
 			delete(statefulsetStates, ssName)
 		}
@@ -199,6 +217,9 @@ func (status *StatusManager) SetFromPods() {
 
 	for _, dep := range deployments {
 		depName := NewClusteredName(dep)
+		depState, hadState := deploymentStates[depName]
+		depState.RolloutActive = deploymentRolloutActive(dep, depState.RolloutActive, status.installComplete)
+		depRolloutActive := depState.RolloutActive
 		depProgressing := false
 
 		if isNonCritical(dep) && dep.Status.UnavailableReplicas > 0 && !status.installComplete {
@@ -208,13 +229,15 @@ func (status *StatusManager) SetFromPods() {
 			progressing = append(progressing, fmt.Sprintf("Deployment %q update is rolling out (%d out of %d updated)", depName.String(), dep.Status.UpdatedReplicas, dep.Status.Replicas))
 			depProgressing = true
 		} else if dep.Status.UnavailableReplicas > 0 {
-			progressing = append(progressing, fmt.Sprintf("Deployment %q is not available (awaiting %d nodes)", depName.String(), dep.Status.UnavailableReplicas))
-			depProgressing = true
+			if depRolloutActive {
+				progressing = append(progressing, fmt.Sprintf("Deployment %q is not available (awaiting %d nodes)", depName.String(), dep.Status.UnavailableReplicas))
+				depProgressing = true
+			}
 			// Check for any pods in CrashLoopBackOff state and mark the operator as degraded if so.
 			if !isNonCritical(dep) {
 				hung = append(hung, status.CheckCrashLoopBackOffPods(depName, dep.Spec.Selector.MatchLabels, "Deployment")...)
 			}
-		} else if dep.Status.AvailableReplicas == 0 {
+		} else if dep.Status.AvailableReplicas == 0 && depRolloutActive {
 			progressing = append(progressing, fmt.Sprintf("Deployment %q is not yet scheduled on any nodes", depName.String()))
 			depProgressing = true
 		} else if dep.Status.ObservedGeneration < dep.Generation {
@@ -231,19 +254,20 @@ func (status *StatusManager) SetFromPods() {
 		if depProgressing && !isNonCritical(dep) {
 			reachedAvailableLevel = false
 
-			depState, exists := deploymentStates[depName]
-			if !exists || !reflect.DeepEqual(depState.LastSeenStatus, dep.Status) {
+			if !hadState || !reflect.DeepEqual(depState.LastSeenStatus, dep.Status) {
 				depState.LastChangeTime = time.Now()
 				dep.Status.DeepCopyInto(&depState.LastSeenStatus)
-				deploymentStates[depName] = depState
 			}
 
 			// Catch hung rollouts
-			if exists && (time.Since(depState.LastChangeTime)) > ProgressTimeout {
+			if hadState && (time.Since(depState.LastChangeTime)) > ProgressTimeout {
 				hung = append(hung, fmt.Sprintf("Deployment %q rollout is not making progress - last change %s", depName.String(), depState.LastChangeTime.Format(time.RFC3339)))
 				empty := ""
 				depHung = &empty
 			}
+		}
+		if depRolloutActive {
+			deploymentStates[depName] = depState
 		} else {
 			delete(deploymentStates, depName)
 		}
@@ -253,7 +277,10 @@ func (status *StatusManager) SetFromPods() {
 	}
 
 	status.setNotDegraded(PodDeployment)
-	if err := status.setLastPodState(daemonsetStates, deploymentStates, statefulsetStates); err != nil {
+	if reachedAvailableLevel && len(progressing) == 0 {
+		status.installComplete = true
+	}
+	if err := status.setLastPodState(daemonsetStates, deploymentStates, statefulsetStates, status.installComplete); err != nil {
 		log.Printf("Failed to set pod state (continuing): %+v\n", err)
 	}
 
@@ -269,21 +296,71 @@ func (status *StatusManager) SetFromPods() {
 			Status: operv1.ConditionTrue})
 	}
 
-	if reachedAvailableLevel && len(progressing) == 0 {
-		status.installComplete = true
-	}
-
 	if len(hung) > 0 {
 		status.setDegraded(RolloutHung, "RolloutHung", strings.Join(hung, "\n"))
 	} else {
 		status.setNotDegraded(RolloutHung)
 	}
 }
 
+// We only want pod unavailability to count as Progressing when we already know a
+// CNO-managed rollout is in flight. The status snapshots below distinguish:
+// - rollout started: controller still processing a spec change
+// - rollout complete: controller has observed that change and all replicas are healthy
+// If we have neither signal after install, the same "unavailable" counters are
+// treated as ordinary node reboot churn rather than a network rollout.
+func daemonSetRolloutActive(ds *appsv1.DaemonSet, rolloutActive, installComplete bool) bool {
+	rolloutStarted := ds.Generation > ds.Status.ObservedGeneration || ds.Status.UpdatedNumberScheduled < ds.Status.DesiredNumberScheduled
+	rolloutComplete := ds.Status.ObservedGeneration >= expectedGeneration(ds.Generation) &&
+		ds.Status.NumberUnavailable == 0 &&
+		(ds.Status.DesiredNumberScheduled == 0 || ds.Status.UpdatedNumberScheduled >= ds.Status.DesiredNumberScheduled) &&
+		(ds.Status.DesiredNumberScheduled == 0 || ds.Status.NumberAvailable >= ds.Status.DesiredNumberScheduled)
+
+	return updateRolloutActive(rolloutActive, installComplete, rolloutStarted, rolloutComplete)
+}
+
+func statefulSetRolloutActive(ss *appsv1.StatefulSet, rolloutActive, installComplete bool) bool {
+	rolloutStarted := ss.Generation > ss.Status.ObservedGeneration || ss.Status.UpdatedReplicas < ss.Status.Replicas
+	rolloutComplete := ss.Status.ObservedGeneration >= expectedGeneration(ss.Generation) &&
+		ss.Status.UpdatedReplicas >= ss.Status.Replicas &&
+		ss.Status.ReadyReplicas >= ss.Status.Replicas
+
+	return updateRolloutActive(rolloutActive, installComplete, rolloutStarted, rolloutComplete)
+}
+
+func deploymentRolloutActive(dep *appsv1.Deployment, rolloutActive, installComplete bool) bool {
+	rolloutStarted := dep.Generation > dep.Status.ObservedGeneration || dep.Status.UpdatedReplicas < dep.Status.Replicas
+	rolloutComplete := dep.Status.ObservedGeneration >= expectedGeneration(dep.Generation) &&
+		dep.Status.UpdatedReplicas >= dep.Status.Replicas &&
+		dep.Status.UnavailableReplicas == 0 &&
+		(dep.Status.Replicas == 0 || dep.Status.AvailableReplicas >= dep.Status.Replicas)
+
+	return updateRolloutActive(rolloutActive, installComplete, rolloutStarted, rolloutComplete)
+}
+
+// Once install is complete, only explicit rollout signals should reactivate Progressing.
+func updateRolloutActive(rolloutActive, installComplete, rolloutStarted, rolloutComplete bool) bool {
+	if !installComplete || rolloutStarted {
+		rolloutActive = true
+	}
+	if rolloutActive && rolloutComplete {
+		return false
+	}
+	return rolloutActive
+}
+
+// Real workload objects start at generation 1; tests often omit it and leave the zero value.
+func expectedGeneration(generation int64) int64 {
+	if generation > 0 {
+		return generation
+	}
+	return 1
+}
+
 // getLastPodState reads the last-seen daemonset + deployment + statefulset
 // states from the clusteroperator annotation and parses it. On error, it
 // returns an empty state, since this should not block updating operator status.
-func (status *StatusManager) getLastPodState() (map[ClusteredName]daemonsetState, map[ClusteredName]deploymentState, map[ClusteredName]statefulsetState) {
+func (status *StatusManager) getLastPodState() (map[ClusteredName]daemonsetState, map[ClusteredName]deploymentState, map[ClusteredName]statefulsetState, bool) {
 	// with maps allocated
 	daemonsetStates := map[ClusteredName]daemonsetState{}
 	deploymentStates := map[ClusteredName]deploymentState{}
@@ -294,20 +371,20 @@ func (status *StatusManager) getLastPodState() (map[ClusteredName]daemonsetState
 	err := status.client.ClientFor("").CRClient().Get(context.TODO(), types.NamespacedName{Name: status.name}, co)
 	if err != nil {
 		log.Printf("Failed to get ClusterOperator: %v", err)
-		return daemonsetStates, deploymentStates, statefulsetStates
+		return daemonsetStates, deploymentStates, statefulsetStates, false
 	}
 
 	lsbytes := co.Annotations[lastSeenAnnotation]
 	if lsbytes == "" {
-		return daemonsetStates, deploymentStates, statefulsetStates
+		return daemonsetStates, deploymentStates, statefulsetStates, false
 	}
 
 	out := podState{}
 	err = json.Unmarshal([]byte(lsbytes), &out)
 	if err != nil {
 		// No need to return error; just move on
 		log.Printf("failed to unmashal last-seen-status: %v", err)
-		return daemonsetStates, deploymentStates, statefulsetStates
+		return daemonsetStates, deploymentStates, statefulsetStates, false
 	}
 
 	for _, ds := range out.DaemonsetStates {
@@ -322,18 +399,20 @@ func (status *StatusManager) getLastPodState() (map[ClusteredName]daemonsetState
 		statefulsetStates[ss.ClusteredName] = ss
 	}
 
-	return daemonsetStates, deploymentStates, statefulsetStates
+	return daemonsetStates, deploymentStates, statefulsetStates, out.InstallComplete
 }
 
 func (status *StatusManager) setLastPodState(
 	dss map[ClusteredName]daemonsetState,
 	deps map[ClusteredName]deploymentState,
-	sss map[ClusteredName]statefulsetState) error {
+	sss map[ClusteredName]statefulsetState,
+	installComplete bool) error {
 
 	ps := podState{
 		DaemonsetStates:   make([]daemonsetState, 0, len(dss)),
 		DeploymentStates:  make([]deploymentState, 0, len(deps)),
 		StatefulsetStates: make([]statefulsetState, 0, len(sss)),
+		InstallComplete:   installComplete,
 	}
 
 	for nsn, ds := range dss {