Fix PipelineRunStatus Reconciler Updates and Tests for EmbeddedStatus Switch

This commit fixes the updates for reconciling PipelineRunStatus when switching
the EmbeddedStatus feature flag. It resets the status.runs and status.taskruns
to nil with minimal EmbeddedStatus and status.childReferences to nil with
full EmbeddedStatus.

Prior to this change, the childReferences runs and taskruns in pipelineRunStatus
would not have been reset to nil and could have led to validation error when the feature
flag is being switched. The following issue could help prevent such bugs in the future:
Integration tests for changing feature flags #5999

Author: JeromeJu

pkg/reconciler/pipelinerun/pipelinerun.go

@@ -1331,15 +1331,20 @@ func (c *Reconciler) updatePipelineRunStatusFromInformer(ctx context.Context, pr
 
 func updatePipelineRunStatusFromChildObjects(ctx context.Context, logger *zap.SugaredLogger, pr *v1beta1.PipelineRun, taskRuns []*v1beta1.TaskRun, runObjects []v1beta1.RunObject) error {
 	cfg := config.FromContextOrDefaults(ctx)
-	fullEmbedded := cfg.FeatureFlags.EmbeddedStatus == config.FullEmbeddedStatus || cfg.FeatureFlags.EmbeddedStatus == config.BothEmbeddedStatus
-	minimalEmbedded := cfg.FeatureFlags.EmbeddedStatus == config.MinimalEmbeddedStatus || cfg.FeatureFlags.EmbeddedStatus == config.BothEmbeddedStatus
 
-	if minimalEmbedded {
-		updatePipelineRunStatusFromChildRefs(logger, pr, taskRuns, runObjects)
-	}
-	if fullEmbedded {
+	switch cfg.FeatureFlags.EmbeddedStatus {
+	case config.FullEmbeddedStatus:
+		pr.Status.ChildReferences = nil
+		updatePipelineRunStatusFromTaskRuns(logger, pr, taskRuns)
+		updatePipelineRunStatusFromCustomRunsOrRuns(logger, pr, runObjects)
+	case config.BothEmbeddedStatus:
 		updatePipelineRunStatusFromTaskRuns(logger, pr, taskRuns)
 		updatePipelineRunStatusFromCustomRunsOrRuns(logger, pr, runObjects)
+		updatePipelineRunStatusFromChildRefs(logger, pr, taskRuns, runObjects)
+	default:
+		pr.Status.Runs = nil
+		pr.Status.TaskRuns = nil
+		updatePipelineRunStatusFromChildRefs(logger, pr, taskRuns, runObjects)
 	}
 
 	return validateChildObjectsInPipelineRunStatus(ctx, pr.Status)
@@ -1350,6 +1355,9 @@ func validateChildObjectsInPipelineRunStatus(ctx context.Context, prs v1beta1.Pi
 
 	var err error
 
+	// TODO: https://github.com/tektoncd/pipeline/issues/5999 integration tests for changing
+	// feature flags could help prevent validation errors when the feature-flag is being switched.
+	// In such case, the error could keep the pipelineRun run indefinitely.
 	// Verify that we don't populate child references for "full"
 	if cfg.FeatureFlags.EmbeddedStatus == config.FullEmbeddedStatus && len(prs.ChildReferences) > 0 {
 		return fmt.Errorf("expected no ChildReferences with embedded-status=full, but found %d", len(prs.ChildReferences))

pkg/reconciler/pipelinerun/pipelinerun_test.go

@@ -5161,7 +5161,7 @@ status:
 		expectedPr = expectedPrMinimalStatus
 	}
 
-	if d := cmp.Diff(expectedPr, reconciledRun, ignoreResourceVersion, ignoreLastTransitionTime, ignoreCompletionTime, ignoreStartTime); d != "" {
+	if d := cmp.Diff(expectedPr, reconciledRun, ignoreResourceVersion, ignoreLastTransitionTime, ignoreCompletionTime, ignoreStartTime, cmpopts.EquateEmpty()); d != "" {
 		t.Errorf("expected to see pipeline run results created. Diff %s", diff.PrintWantGot(d))
 	}
 }
@@ -5482,7 +5482,7 @@ status:
 		expectedPr = expectedPrMinimalStatus
 	}
 
-	if d := cmp.Diff(expectedPr, reconciledRun, ignoreResourceVersion, ignoreLastTransitionTime, ignoreCompletionTime, ignoreStartTime); d != "" {
+	if d := cmp.Diff(expectedPr, reconciledRun, ignoreResourceVersion, ignoreLastTransitionTime, ignoreCompletionTime, ignoreStartTime, cmpopts.EquateEmpty()); d != "" {
 		t.Errorf("expected to see pipeline run results created. Diff %s", diff.PrintWantGot(d))
 	}
 }
@@ -8947,7 +8947,7 @@ spec:
 			if err != nil {
 				t.Fatalf("Got an error getting reconciled run out of fake client: %s", err)
 			}
-			if d := cmp.Diff(tt.expectedPipelineRun, pipelineRun, ignoreResourceVersion, ignoreTypeMeta, ignoreLastTransitionTime, ignoreStartTime, ignoreFinallyStartTime); d != "" {
+			if d := cmp.Diff(tt.expectedPipelineRun, pipelineRun, ignoreResourceVersion, ignoreTypeMeta, ignoreLastTransitionTime, ignoreStartTime, ignoreFinallyStartTime, cmpopts.EquateEmpty()); d != "" {
 				t.Errorf("expected PipelineRun was not created. Diff %s", diff.PrintWantGot(d))
 			}
 		})
@@ -9555,7 +9555,7 @@ spec:
 			if err != nil {
 				t.Fatalf("Got an error getting reconciled run out of fake client: %s", err)
 			}
-			if d := cmp.Diff(tt.expectedPipelineRun, pipelineRun, ignoreResourceVersion, ignoreTypeMeta, ignoreLastTransitionTime, ignoreStartTime, ignoreFinallyStartTime); d != "" {
+			if d := cmp.Diff(tt.expectedPipelineRun, pipelineRun, ignoreResourceVersion, ignoreTypeMeta, ignoreLastTransitionTime, ignoreStartTime, ignoreFinallyStartTime, cmpopts.EquateEmpty()); d != "" {
 				t.Errorf("expected PipelineRun was not created. Diff %s", diff.PrintWantGot(d))
 			}
 		})
@@ -10015,7 +10015,7 @@ status:
 			if err != nil {
 				t.Fatalf("Got an error getting reconciled run out of fake client: %s", err)
 			}
-			if d := cmp.Diff(tt.expectedPipelineRun, pipelineRun, ignoreResourceVersion, ignoreTypeMeta, ignoreLastTransitionTime, ignoreStartTime, cmpopts.SortSlices(lessChildReferences)); d != "" {
+			if d := cmp.Diff(tt.expectedPipelineRun, pipelineRun, ignoreResourceVersion, ignoreTypeMeta, ignoreLastTransitionTime, ignoreStartTime, cmpopts.SortSlices(lessChildReferences), cmpopts.EquateEmpty()); d != "" {
 				t.Errorf("expected PipelineRun was not created. Diff %s", diff.PrintWantGot(d))
 			}
 		})
@@ -10549,7 +10549,7 @@ spec:
 			if err != nil {
 				t.Fatalf("Got an error getting reconciled run out of fake client: %s", err)
 			}
-			if d := cmp.Diff(tt.expectedPipelineRun, pipelineRun, ignoreResourceVersion, ignoreTypeMeta, ignoreLastTransitionTime, ignoreStartTime, ignoreFinallyStartTime); d != "" {
+			if d := cmp.Diff(tt.expectedPipelineRun, pipelineRun, ignoreResourceVersion, ignoreTypeMeta, ignoreLastTransitionTime, ignoreStartTime, ignoreFinallyStartTime, cmpopts.EquateEmpty()); d != "" {
 				t.Errorf("expected PipelineRun was not created. Diff %s", diff.PrintWantGot(d))
 			}
 		})