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

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)

pkg/reconciler/pipelinerun/pipelinerun_test.go

@@ -5162,7 +5162,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))
 	}
 }
@@ -5484,7 +5484,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))
 	}
 }
@@ -8952,7 +8952,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))
 			}
 		})
@@ -9560,7 +9560,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))
 			}
 		})
@@ -10020,7 +10020,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))
 			}
 		})
@@ -10554,7 +10554,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))
 			}
 		})