Address PR review feedback for workload condition detection

- Add "after successful rollout" to flapping Ready condition test names
- Add a young pod to the stably ready test case
- Always call PodContainersStatus when reporting UnavailablePod

Author: tchap

pkg/operator/apiserver/controller/workload/workload.go

@@ -346,17 +346,13 @@ func (c *Controller) updateOperatorStatus(ctx context.Context, previousStatus *o
 			errs = append(errs, err)
 		}
 		if hasFailing || workload.Status.AvailableReplicas == 0 {
+			containerMessages, err := deployment.PodContainersStatus(workload, c.podsLister)
+			if err != nil {
+				errs = append(errs, err)
+			}
 			var failureDescription string
-			// Only fetch container details when pods actually exist and are failing.
-			// When entering via AvailableReplicas==0 alone, no pods exist to inspect.
-			if hasFailing {
-				containerMessages, err := deployment.PodContainersStatus(workload, c.podsLister)
-				if err != nil {
-					errs = append(errs, err)
-				}
-				if len(containerMessages) > 0 {
-					failureDescription = ` (` + strings.Join(containerMessages, ", ") + `)`
-				}
+			if len(containerMessages) > 0 {
+				failureDescription = ` (` + strings.Join(containerMessages, ", ") + `)`
 			}
 
 			numUnavailable := desiredReplicas - workload.Status.AvailableReplicas

pkg/operator/apiserver/controller/workload/workload_test.go

@@ -707,6 +707,11 @@ func TestUpdateOperatorStatus(t *testing.T) {
 				},
 				Spec: appsv1.DeploymentSpec{
 					Replicas: ptr.To[int32](3),
+					Template: corev1.PodTemplateSpec{
+						ObjectMeta: metav1.ObjectMeta{
+							Labels: map[string]string{"app": "apiserver"},
+						},
+					},
 				},
 				Status: appsv1.DeploymentStatus{
 					AvailableReplicas:  0,
@@ -734,7 +739,7 @@ func TestUpdateOperatorStatus(t *testing.T) {
 						Type:    fmt.Sprintf("%sDeploymentDegraded", defaultControllerName),
 						Status:  operatorv1.ConditionTrue,
 						Reason:  "UnavailablePod",
-						Message: "3 of 3 requested instances are unavailable for apiserver.openshift-apiserver",
+						Message: `3 of 3 requested instances are unavailable for apiserver.openshift-apiserver (no pods found with labels "app=apiserver")`,
 					},
 					{
 						Type:   fmt.Sprintf("%sDeployment%s", defaultControllerName, operatorv1.OperatorStatusTypeProgressing),
@@ -861,7 +866,7 @@ func TestUpdateOperatorStatus(t *testing.T) {
 			// Combined deadline: 600+60 = 660s (11m).
 			// Pod created 15m ago → past combined deadline → check is relevant.
 			// LastTransitionTime 10s ago < MinReadySeconds (60s) → flapping → degraded.
-			name: "scenario: pod with flapping Ready condition detected as failing",
+			name: "scenario: pod with flapping Ready condition after successful rollout detected as failing",
 			workload: &appsv1.Deployment{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:       "apiserver",
@@ -925,7 +930,7 @@ func TestUpdateOperatorStatus(t *testing.T) {
 			// MinReadySeconds=300, ProgressDeadlineSeconds defaults to 600.
 			// Combined deadline: 600+300 = 900s (15m).
 			// Pod created 8m ago → still within combined deadline → check not relevant → not degraded.
-			name: "scenario: pod with flapping Ready condition within combined deadline not flagged",
+			name: "scenario: pod with flapping Ready condition after successful rollout within combined deadline not flagged",
 			workload: &appsv1.Deployment{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:       "apiserver",
@@ -986,7 +991,7 @@ func TestUpdateOperatorStatus(t *testing.T) {
 			// Combined deadline: 600+60 = 660s (11m).
 			// Pod created 15m ago → past combined deadline → check is relevant.
 			// LastTransitionTime 5m ago > MinReadySeconds (60s) → stable → not degraded.
-			name: "scenario: stably ready pod past combined deadline not flagged as flapping",
+			name: "scenario: stably ready pod after successful rollout past combined deadline not flagged as flapping",
 			workload: &appsv1.Deployment{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:       "apiserver",
@@ -1016,6 +1021,15 @@ func TestUpdateOperatorStatus(t *testing.T) {
 						},
 					},
 				},
+				{
+					ObjectMeta: metav1.ObjectMeta{Name: "apiserver-2", Namespace: "openshift-apiserver", CreationTimestamp: metav1.NewTime(time.Now().Add(-10 * time.Second))},
+					Status: corev1.PodStatus{
+						Phase: corev1.PodRunning,
+						Conditions: []corev1.PodCondition{
+							{Type: corev1.PodReady, Status: corev1.ConditionTrue, LastTransitionTime: metav1.NewTime(time.Now().Add(-10 * time.Second))},
+						},
+					},
+				},
 			},
 			validateOperatorStatus: func(actualStatus *operatorv1.OperatorStatus) error {
 				expectedConditions := []operatorv1.OperatorCondition{