diff --git a/config/config-feature-flags.yaml b/config/config-feature-flags.yaml index ba0bbe80caf..2abb6a8b09a 100644 --- a/config/config-feature-flags.yaml +++ b/config/config-feature-flags.yaml @@ -94,3 +94,4 @@ data: # If set to "none", then Tekton will not have non-falsifiable provenance. # This is an experimental feature and thus should still be considered an alpha feature. enforce-nonfalsifiablity: "none" + embedded-status: "minimal" diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index 62947803f6d..0d02185426b 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -1052,6 +1052,41 @@ func propagateWorkspaces(rpt *resources.ResolvedPipelineTask) (*resources.Resolv return rpt, nil } +// Check with Andrew, if not with this it will fail the final test +// updateTaskRunsStatusDirectly is used with "full" or "both" set as the value for the "embedded-status" feature flag. +// When the "full" and "both" options are removed, updateTaskRunsStatusDirectly can be removed. +func (c *Reconciler) updateTaskRunsStatusDirectly(pr *v1beta1.PipelineRun) error { + for taskRunName := range pr.Status.TaskRuns { + prtrs := pr.Status.TaskRuns[taskRunName] + tr, err := c.taskRunLister.TaskRuns(pr.Namespace).Get(taskRunName) + if err != nil { + // If the TaskRun isn't found, it just means it won't be run + if !kerrors.IsNotFound(err) { + return fmt.Errorf("error retrieving TaskRun %s: %w", taskRunName, err) + } + } else { + prtrs.Status = &tr.Status + } + } + return nil +} + +// updateRunsStatusDirectly is used with "full" or "both" set as the value for the "embedded-status" feature flag. +// When the "full" and "both" options are removed, updateRunsStatusDirectly can be removed. +func (c *Reconciler) updateRunsStatusDirectly(pr *v1beta1.PipelineRun) error { + for runName := range pr.Status.Runs { + prRunStatus := pr.Status.Runs[runName] + run, err := c.customRunLister.CustomRuns(pr.Namespace).Get(runName) + if err != nil { + if !kerrors.IsNotFound(err) { + return fmt.Errorf("error retrieving Run %s: %w", runName, err) + } + } else { + prRunStatus.Status = &run.Status + } + } + return nil +} func getTaskrunWorkspaces(ctx context.Context, pr *v1beta1.PipelineRun, rpt *resources.ResolvedPipelineTask) ([]v1beta1.WorkspaceBinding, string, error) { var workspaces []v1beta1.WorkspaceBinding var pipelinePVCWorkspaceName string diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index 9be5adae6a0..a49373a537d 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -1458,9 +1458,10 @@ func TestReconcileOnCompletedPipelineRun(t *testing.T) { // in the PipelineRun status, that the completion status is not altered, that not error is returned and // a successful event is triggered taskRunName := "test-pipeline-run-completed-hello-world" - prs := []*v1beta1.PipelineRun{parse.MustParseV1beta1PipelineRun(t, ` + pipelineRunName := "test-pipeline-run-completed" + prs := []*v1beta1.PipelineRun{parse.MustParseV1beta1PipelineRun(t, fmt.Sprintf(` metadata: - name: test-pipeline-run-completed + name: %s namespace: foo spec: pipelineRef: @@ -1473,17 +1474,21 @@ status: reason: Succeeded status: "True" type: Succeeded - taskRuns: - test-pipeline-run-completed-hello-world: + childReferences: + - name: test-pipeline-run-completed-hello-world pipelineTaskName: hello-world-1 -`)} +`, pipelineRunName))} ps := []*v1beta1.Pipeline{simpleHelloWorldPipeline} ts := []*v1beta1.Task{simpleHelloWorldTask} trs := []*v1beta1.TaskRun{createHelloWorldTaskRunWithStatus(t, taskRunName, "foo", - "test-pipeline-run-completed", "test-pipeline", "", + pipelineRunName, "test-pipeline", "", apis.Condition{ Type: apis.ConditionSucceeded, })} + expectedChildReferences := []v1beta1.ChildStatusReference{{ + Name: taskRunName, + PipelineTaskName: "hello-world-1", + }} d := test.Data{ PipelineRuns: prs, @@ -1497,7 +1502,7 @@ status: wantEvents := []string{ "Normal Succeeded All Tasks have completed executing", } - reconciledRun, clients := prt.reconcileRun("foo", "test-pipeline-run-completed", wantEvents, false) + reconciledRun, clients := prt.reconcileRun("foo", pipelineRunName, wantEvents, false) actions := clients.Pipeline.Actions() if len(actions) < 2 { @@ -1528,18 +1533,22 @@ status: t.Errorf("Expected PipelineRun status to be complete, but was %v", reconciledRun.Status.GetCondition(apis.ConditionSucceeded)) } - expectedTaskRunsStatus := make(map[string]*v1beta1.PipelineRunTaskRunStatus) - expectedTaskRunsStatus[taskRunName] = &v1beta1.PipelineRunTaskRunStatus{ - PipelineTaskName: "hello-world-1", - Status: &v1beta1.TaskRunStatus{ - Status: duckv1.Status{ - Conditions: []apis.Condition{{Type: apis.ConditionSucceeded}}, - }, - }, + if d := cmp.Diff(reconciledRun.Status.ChildReferences, expectedChildReferences); d != "" { + t.Fatalf("Expected PipelineRun status to match ChildReference(s) status, but got a mismatch %s", diff.PrintWantGot(d)) } - if d := cmp.Diff(reconciledRun.Status.TaskRuns, expectedTaskRunsStatus); d != "" { - t.Fatalf("Expected PipelineRun status to match TaskRun(s) status, but got a mismatch %s", diff.PrintWantGot(d)) + // Check the status of taskruns from ChildReferences to be Succeeded + trFromChildRef, err := clients.Pipeline.TektonV1beta1().TaskRuns("foo").Get(prt.TestAssets.Ctx, reconciledRun.Status.ChildReferences[0].Name, metav1.GetOptions{}) + if err != nil { + t.Fatalf("Failure to get TaskRun %s from", err) + } + + expectedTaskRunStatus := v1beta1.TaskRunStatus{Status: duckv1.Status{ + Conditions: []apis.Condition{{Type: apis.ConditionSucceeded}}, + }} + + if d := cmp.Diff(trFromChildRef.Status, expectedTaskRunStatus); d != "" { + t.Fatalf("Expected TaskRun Status Conditions to match ChildReferences TaskRun Status Condition, but got a mismatch %s", diff.PrintWantGot(d)) } } @@ -4722,9 +4731,8 @@ status: name: test-pipeline-run-finally-results-task-run-b pipelineTaskName: b-task `) - var expectedPr *v1beta1.PipelineRun - expectedPr = expectedPrMinimalStatus + expectedPr := expectedPrMinimalStatus 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)) @@ -4874,9 +4882,8 @@ status: name: test-pipeline-run-results-task-run-a pipelineTaskName: a-task `) - var expectedPr *v1beta1.PipelineRun - expectedPr = expectedPrMinimalStatus + expectedPr := expectedPrMinimalStatus 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)) @@ -5457,7 +5464,7 @@ spec: verifyCustomRunOrRunStatusesNames(t, pipeline.CustomRunControllerName, reconciledRun.Status, customRunName) } -// TODO +// TODOTest func TestReconcilePipeline_FinalTasks(t *testing.T) { tests := []struct { name string @@ -5466,7 +5473,7 @@ func TestReconcilePipeline_FinalTasks(t *testing.T) { ps []*v1beta1.Pipeline ts []*v1beta1.Task trs []*v1beta1.TaskRun - // TODO how should we update this + // TODOhow should we update this expectedTaskRuns map[string]*v1beta1.PipelineRunTaskRunStatus expectedChildReferences []v1beta1.ChildStatusReference pipelineRunStatusUnknown bool @@ -5747,6 +5754,12 @@ func TestReconcilePipeline_FinalTasks(t *testing.T) { Name: "task-run-dag-task-2", PipelineTaskName: "dag-task-2", }}, + + expectedTaskRuns: map[string]*v1beta1.PipelineRunTaskRunStatus{ + "task-run-dag-task-1": getTaskRunStatus("dag-task-1", corev1.ConditionFalse), + "task-run-dag-task-2": getTaskRunStatus("dag-task-2", corev1.ConditionUnknown), + }, + pipelineRunStatusUnknown: true, }, { // pipeline run should not schedule final tasks until dag tasks are done i.e. @@ -5809,6 +5822,10 @@ func TestReconcilePipeline_FinalTasks(t *testing.T) { PipelineTaskName: "dag-task-1", }}, + expectedTaskRuns: map[string]*v1beta1.PipelineRunTaskRunStatus{ + "task-run-dag-task-1": getTaskRunStatus("dag-task-1", corev1.ConditionUnknown), + }, + pipelineRunStatusUnknown: true, }} @@ -5822,10 +5839,11 @@ func TestReconcilePipeline_FinalTasks(t *testing.T) { Tasks: tt.ts, TaskRuns: tt.trs, } + namespace := "foo" prt := newPipelineRunTest(t, d) defer prt.Cancel() - reconciledRun, clients := prt.reconcileRun("foo", tt.pipelineRunName, []string{}, false) + reconciledRun, clients := prt.reconcileRun(namespace, tt.pipelineRunName, []string{}, false) actions := clients.Pipeline.Actions() if len(actions) < 2 { @@ -5860,9 +5878,9 @@ func TestReconcilePipeline_FinalTasks(t *testing.T) { t.Errorf("Expected PipelineRun status to be failed, but was %v for %s", reconciledRun.Status.GetCondition(apis.ConditionSucceeded), tt.name) } - // if d := cmp.Diff(reconciledRun.Status.TaskRuns, tt.expectedTaskRuns); d != "" { - // t.Fatalf("Expected PipelineRunTaskRun status to match TaskRun(s) status, but got a mismatch for %s: %s", tt.name, d) - // } + if d := cmp.Diff(reconciledRun.Status.TaskRuns, tt.expectedTaskRuns); d != "" { + t.Fatalf("Expected PipelineRunTaskRun status to match TaskRun(s) status, but got a mismatch for %s: %s", tt.name, d) + } } else if tt.pipelineRunStatusUnknown { // This PipelineRun should still be running and the status should reflect that if !reconciledRun.Status.GetCondition(apis.ConditionSucceeded).IsUnknown() { @@ -5872,15 +5890,48 @@ func TestReconcilePipeline_FinalTasks(t *testing.T) { if d := cmp.Diff(reconciledRun.Status.ChildReferences, tt.expectedChildReferences); d != "" { t.Fatalf("Expected PipelineRunTaskRun status to match TaskRun(s) status, but got a mismatch for %s: %s", tt.name, d) } + compareTaskRunStatusFromChildRefs(prt.TestAssets.Ctx, t, namespace, clients, reconciledRun.Status.ChildReferences, tt.expectedTaskRuns) } - - // if d := cmp.Diff(reconciledRun.Status.TaskRuns, tt.expectedChildReferences); d != "" { - // t.Fatalf("Expected PipelineRunTaskRun status to match TaskRun(s) status, but got a mismatch for %s: %s", tt.name, d) - // } }) } } +// TODO rename this +// compareTaskRunStatusFromChildRefs checks the status of taskruns from ChildReferences to be expected +// This function is introduced during the trasition of using TaskRuns/Runs to using ChildReferences while +// removing the according to the deprecation of `PipelineRunStatus` in TEP0100. +// Question: make sure the assumptions hold that childRefs len shall be the same as expectedTRs +func compareTaskRunStatusFromChildRefs(ctx context.Context, t *testing.T, namespace string, clients test.Clients, + childRefs []v1beta1.ChildStatusReference, expectedTaskRuns map[string]*v1beta1.PipelineRunTaskRunStatus) { + t.Helper() + if len(expectedTaskRuns) == 0 { + return + } + + for _, childRef := range childRefs { + if childRef.Kind != "TaskRun" { + continue + } + trName := childRef.Name + + trFromChildRef, err := clients.Pipeline.TektonV1beta1().TaskRuns(namespace).Get(ctx, trName, metav1.GetOptions{}) + if err != nil { + t.Fatalf("Failure to get TaskRun from ChildReference %s: %s", childRef.Name, err) + } + + if expectedTaskRunStatus, ok := expectedTaskRuns[trName]; !ok { + t.Fatalf("Expected not to have TaskRun status from ChildReferences %s", trName) + } else if d := cmp.Diff(*expectedTaskRunStatus.Status, trFromChildRef.Status); d != "" { + t.Fatalf("Expected TaskRun Status Conditions to match ChildReferences TaskRun Status Condition, but got a mismatch %s", diff.PrintWantGot(d)) + } + delete(expectedTaskRuns, trName) + } + + if len(expectedTaskRuns) != 0 { + t.Fatalf("Expected ChildReferences to fully match TaskRun Status, but there are TaskRuns that are not %v", expectedTaskRuns) + } +} + func getPipelineRun(pr, p string, status corev1.ConditionStatus, reason string, m string, tr map[string]string) []*v1beta1.PipelineRun { pRun := &v1beta1.PipelineRun{ ObjectMeta: baseObjectMeta(pr, "foo"), diff --git a/pkg/reconciler/pipelinerun/pipelinerun_updatestatus_test.go b/pkg/reconciler/pipelinerun/pipelinerun_updatestatus_test.go index d7c27e8d516..947074cfa4b 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_updatestatus_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_updatestatus_test.go @@ -927,7 +927,7 @@ pipelineTaskName: task-6 for _, tc := range tcs { - t.Run(fmt.Sprintf("%s", tc.prName), func(t *testing.T) { + t.Run(tc.prName, func(t *testing.T) { ctx := context.Background() cfg := config.NewStore(logtesting.TestLogger(t)) diff --git a/test/conversion_test.go b/test/conversion_test.go index f9f3fed766f..faedee5200d 100644 --- a/test/conversion_test.go +++ b/test/conversion_test.go @@ -656,30 +656,11 @@ status: - name: "fetch-and-write-secure" image: "ubuntu" script: "echo hello" - taskRuns: - %s-fetch-secure-data: + childReferences: + - name: %s-fetch-secure-data pipelineTaskName: fetch-secure-data - status: - conditions: - - reason: Succeeded - status: "True" - type: Succeeded - podName: %s-fetch-secure-data-pod - steps: - - container: step-fetch-and-write-secure - imageID: docker.io/library/ubuntu@sha256:4b1d0c4a2d2aaf63b37111f34eb9fa89fa1bf53dd6e4ca954d47caebca4005c2 - name: fetch-and-write-secure - terminated: - containerID: containerd://07b57fc6fd515e6d1d0de27149c60be9149697207aedc130dd0d284fce6df3fd - exitCode: 0 - finishedAt: "2022-12-07T15:56:32Z" - reason: Completed - startedAt: "2022-12-07T15:56:32Z" - taskSpec: - steps: - - name: "fetch-and-write-secure" - image: "ubuntu" - script: "echo hello" + apiVersion: tekton.dev/v1beta1 + kind: TaskRun ` v1PipelineRunYaml = ` @@ -755,9 +736,8 @@ status: image: "ubuntu" script: "echo hello" childReferences: - - typeMeta: + - apiVersion: tekton.dev/v1beta1 kind: TaskRun - apiVersion: tekton.dev/v1beta1 name: %s-fetch-secure-data pipelineTaskName: fetch-secure-data ` @@ -986,7 +966,7 @@ func TestPipelineRunCRDConversion(t *testing.T) { v1ToV1beta1PRName := helpers.ObjectNameForTest(t) v1PipelineRun := parse.MustParseV1PipelineRun(t, fmt.Sprintf(v1PipelineRunYaml, v1ToV1beta1PRName, namespace)) - v1beta1PipelineRunExpected := parse.MustParseV1beta1PipelineRun(t, fmt.Sprintf(v1beta1PipelineRunExpectedYaml, v1ToV1beta1PRName, namespace, v1ToV1beta1PRName, v1ToV1beta1PRName)) + v1beta1PipelineRunExpected := parse.MustParseV1beta1PipelineRun(t, fmt.Sprintf(v1beta1PipelineRunExpectedYaml, v1ToV1beta1PRName, namespace, v1ToV1beta1PRName)) if _, err := c.V1PipelineRunClient.Create(ctx, v1PipelineRun, metav1.CreateOptions{}); err != nil { t.Fatalf("Failed to create v1 PipelineRun: %s", err)