diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index 6f40fff9ece..b3b89f1dcc3 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -1331,20 +1331,32 @@ 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: + // Clear any ChildReferences that are present. + // This can occur if the value of "embedded-status" was modified during PipelineRun execution. + 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: + // Clear any TaskRuns and Runs present in the status. + // This can occur if the value of "embedded-status" was modified during PipelineRun execution. + pr.Status.Runs = nil + pr.Status.TaskRuns = nil + updatePipelineRunStatusFromChildRefs(logger, pr, taskRuns, runObjects) } return validateChildObjectsInPipelineRunStatus(ctx, pr.Status) } +// 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. func validateChildObjectsInPipelineRunStatus(ctx context.Context, prs v1beta1.PipelineRunStatus) error { cfg := config.FromContextOrDefaults(ctx) diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index 074ff102495..e7602c7c0e5 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/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)) } })