From 59557e41a4b6e0eaac2cbf76762977d0f3436d07 Mon Sep 17 00:00:00 2001 From: Emma Munley <46881325+EmmaMunley@users.noreply.github.com> Date: Mon, 5 Jun 2023 11:46:00 -0400 Subject: [PATCH] Add Unit Tests for TestMissingResultWhenStepErrorIsIgnored and update e2e test: TestFailingStepOnContinue This commit adds test coverage for a pipelinerun that has 2 tasks where the first task produces 2 results that are consumed in the 2nd task. However, the first task contains a failing step so only 1 of the 2 results are produced which results in a failing pipeline run. Prior to this commit, this was an E2E integration test, but missing unit test coverage. This increases the test coverage and also modifies the e2e test to only test the behavior of the TaskRun, since the PipelineRun behavior is now being tested in unit tests to reduce redunancies, improve speed, andreduce flakes. --- .../pipelinerun/pipelinerun_test.go | 182 ++++++++++++++++++ test/ignore_step_error_test.go | 78 +++----- 2 files changed, 210 insertions(+), 50 deletions(-) diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index edbf15bf241..444ef14e60a 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -1044,6 +1044,188 @@ spec: } } +func TestMissingResultWhenStepErrorIsIgnored(t *testing.T) { + prs := []*v1beta1.PipelineRun{parse.MustParseV1beta1PipelineRun(t, ` +metadata: + name: test-pipeline-missing-results + namespace: foo +spec: + serviceAccountName: test-sa-0 + pipelineSpec: + tasks: + - name: task1 + taskSpec: + results: + - name: result1 + type: string + - name: result2 + type: string + steps: + - name: failing-step + onError: continue + image: busybox + script: 'echo -n 123 | tee $(results.result1.path); exit 1; echo -n 456 | tee $(results.result2.path)' + - name: task2 + runAfter: [ task1 ] + params: + - name: param1 + value: $(tasks.task1.results.result1) + - name: param2 + value: $(tasks.task1.results.result2) + taskSpec: + params: + - name: param1 + type: string + - name: param2 + type: string + steps: + - name: foo + image: busybox + script: 'exit 0' + conditions: + - type: Succeeded + status: "False" + reason: "InvalidTaskResultReference" + message: "Could not find result with name result2 for task task1" + childReferences: + - apiVersion: tekton.dev/v1beta1 + kind: TaskRun + name: pr-test-pipeline-missing-results-0 + pipelineTaskName: test-pipeline-missing-results-task1`)} + + trs := []*v1beta1.TaskRun{mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("test-pipeline-missing-results-task1", "foo", + "test-pipeline-missing-results", "test-pipeline", "task1", true), + ` +spec: + serviceAccountName: test-sa + timeout: 1h0m0s +status: + conditions: + - status: "True" + type: Succeeded + taskResults: + - name: result1 + value: 123 +`)} + + expectedPipelineRun := + parse.MustParseV1beta1PipelineRun(t, ` +metadata: + name: test-pipeline-missing-results + namespace: foo + annotations: {} + labels: + tekton.dev/pipeline: test-pipeline-missing-results +spec: + serviceAccountName: test-sa-0 + pipelineSpec: + tasks: + - name: task1 + taskSpec: + results: + - name: result1 + type: string + - name: result2 + type: string + steps: + - name: failing-step + onError: continue + image: busybox + script: 'echo -n 123 | tee $(results.result1.path); exit 1; echo -n 456 | tee $(results.result2.path)' + - name: task2 + runAfter: [ task1 ] + params: + - name: param1 + value: $(tasks.task1.results.result1) + - name: param2 + value: $(tasks.task1.results.result2) + taskSpec: + params: + - name: param1 + type: string + - name: param2 + type: string + steps: + - name: foo + image: busybox + script: 'exit 0' +status: + pipelineSpec: + tasks: + - name: task1 + taskSpec: + results: + - name: result1 + type: string + - name: result2 + type: string + steps: + - name: failing-step + onError: continue + image: busybox + script: 'echo -n 123 | tee $(results.result1.path); exit 1; echo -n 456 | tee $(results.result2.path)' + - name: task2 + runAfter: [ task1 ] + params: + - name: param1 + value: $(tasks.task1.results.result1) + - name: param2 + value: $(tasks.task1.results.result2) + taskSpec: + params: + - name: param1 + type: string + - name: param2 + type: string + steps: + - name: foo + image: busybox + script: 'exit 0' + conditions: + - message: "Could not find result with name result2 for task task1" + reason: InvalidTaskResultReference + status: "False" + type: Succeeded + childReferences: + - apiVersion: tekton.dev/v1beta1 + kind: TaskRun + name: test-pipeline-missing-results-task1 + pipelineTaskName: task1 + provenance: + featureFlags: + RunningInEnvWithInjectedSidecars: true + EnableAPIFields: "stable" + AwaitSidecarReadiness: true + VerificationNoMatchPolicy: "ignore" + EnableProvenanceInStatus: true + ResultExtractionMethod: "termination-message" + MaxResultSize: 4096 +`) + d := test.Data{ + PipelineRuns: prs, + TaskRuns: trs, + } + prt := newPipelineRunTest(t, d) + defer prt.Cancel() + + reconciledRun, clients := prt.reconcileRun("foo", "test-pipeline-missing-results", []string{}, true) + if reconciledRun.Status.CompletionTime == nil { + t.Errorf("Expected a CompletionTime on invalid PipelineRun but was nil") + } + + // The PipelineRun should be marked as failed due to InvalidTaskResultReference. + if d := cmp.Diff(reconciledRun, expectedPipelineRun, ignoreResourceVersion, ignoreLastTransitionTime, ignoreTypeMeta, ignoreStartTime, ignoreCompletionTime); d != "" { + t.Errorf("Expected to see PipelineRun run marked as failed with the reason: InvalidTaskResultReference. Diff %s", diff.PrintWantGot(d)) + } + + // Check that the expected TaskRun was created + taskRuns := getTaskRunsForPipelineRun(prt.TestAssets.Ctx, t, clients, "foo", "test-pipeline-missing-results") + + // We expect only 1 TaskRun to be created, since the PipelineRun should fail before creating the 2nd TaskRun due to the InvalidTaskResultReference + validateTaskRunsCount(t, taskRuns, 1) +} + func TestReconcile_InvalidPipelineRunNames(t *testing.T) { // TestReconcile_InvalidPipelineRunNames runs "Reconcile" on several PipelineRuns that have invalid names. // It verifies that reconcile fails, how it fails and which events are triggered. diff --git a/test/ignore_step_error_test.go b/test/ignore_step_error_test.go index f9b0664c087..5ad55f1d27c 100644 --- a/test/ignore_step_error_test.go +++ b/test/ignore_step_error_test.go @@ -24,82 +24,60 @@ import ( "fmt" "testing" - "github.com/tektoncd/pipeline/pkg/reconciler/pipelinerun" "github.com/tektoncd/pipeline/test/parse" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "knative.dev/pkg/apis" knativetest "knative.dev/pkg/test" - "knative.dev/pkg/test/helpers" ) -func TestMissingResultWhenStepErrorIsIgnored(t *testing.T) { +func TestFailingStepOnContinue(t *testing.T) { ctx := context.Background() ctx, cancel := context.WithCancel(ctx) defer cancel() c, namespace := setup(ctx, t) knativetest.CleanupOnInterrupt(func() { tearDown(ctx, t, c, namespace) }, t.Logf) defer tearDown(ctx, t, c, namespace) - - pipelineRun := parse.MustParseV1beta1PipelineRun(t, fmt.Sprintf(` + taskRunName := "mytaskrun" + tr := parse.MustParseV1beta1TaskRun(t, fmt.Sprintf(` metadata: name: %s + namespace: %s spec: - pipelineSpec: - tasks: - - name: task1 - taskSpec: - results: - - name: result1 - - name: result2 - steps: - - name: failing-step - onError: continue - image: busybox - script: 'echo -n 123 | tee $(results.result1.path); exit 1; echo -n 456 | tee $(results.result2.path)' - - name: task2 - runAfter: [ task1 ] - params: - - name: param1 - value: $(tasks.task1.results.result1) - - name: param2 - value: $(tasks.task1.results.result2) - taskSpec: - params: - - name: param1 - - name: param2 - steps: - - name: foo - image: busybox - script: 'exit 0'`, helpers.ObjectNameForTest(t))) - - if _, err := c.V1beta1PipelineRunClient.Create(ctx, pipelineRun, metav1.CreateOptions{}); err != nil { - t.Fatalf("Failed to create PipelineRun `%s`: %s", pipelineRun.Name, err) + taskSpec: + results: + - name: result1 + - name: result2 + steps: + - name: failing-step + onError: continue + image: busybox + script: 'echo -n 123 | tee $(results.result1.path); exit 1; echo -n 456 | tee $(results.result2.path)' +`, taskRunName, namespace)) + + if _, err := c.V1beta1TaskRunClient.Create(ctx, tr, metav1.CreateOptions{}); err != nil { + t.Fatalf("Failed to create TaskRun: %s", err) } - t.Logf("Waiting for PipelineRun in namespace %s to fail", namespace) - if err := WaitForPipelineRunState(ctx, c, pipelineRun.Name, timeout, FailedWithReason(pipelinerun.ReasonInvalidTaskResultReference, pipelineRun.Name), "InvalidTaskResultReference", v1beta1Version); err != nil { - t.Errorf("Error waiting for PipelineRun to fail: %s", err) + t.Logf("Waiting for TaskRun in namespace %s to finish", namespace) + if err := WaitForTaskRunState(ctx, c, taskRunName, TaskRunSucceed(taskRunName), "TaskRunSucceeded", v1beta1Version); err != nil { + t.Errorf("Error waiting for TaskRun to finish: %s", err) } - taskrunList, err := c.V1beta1TaskRunClient.List(ctx, metav1.ListOptions{LabelSelector: "tekton.dev/pipelineRun=" + pipelineRun.Name}) + taskRun, err := c.V1beta1TaskRunClient.Get(ctx, taskRunName, metav1.GetOptions{}) if err != nil { - t.Fatalf("Error listing TaskRuns for PipelineRun %s: %s", pipelineRun.Name, err) - } - - if len(taskrunList.Items) != 1 { - t.Fatalf("The pipelineRun should have exactly 1 taskRun for the first task \"task1\"") + t.Fatalf("Couldn't get expected TaskRun %s: %s", taskRunName, err) } - taskrunItem := taskrunList.Items[0] - if taskrunItem.Labels["tekton.dev/pipelineTask"] != "task1" { - t.Fatalf("TaskRun was not found for the task \"task1\"") + if !taskRun.Status.GetCondition(apis.ConditionSucceeded).IsTrue() { + t.Fatalf("Expected TaskRun %s to have succeeded but Status is %v", taskRunName, taskRun.Status) } - if len(taskrunItem.Status.TaskRunResults) != 1 { + if len(taskRun.Status.TaskRunResults) != 1 { t.Fatalf("task1 should have produced a result before failing the step") } - for _, r := range taskrunItem.Status.TaskRunResults { - if r.Name == "result1" && r.Value.StringVal != "123" { + for _, r := range taskRun.Status.TaskRunResults { + if r.Name == "taskRunesult1" && r.Value.StringVal != "123" { t.Fatalf("task1 should have initialized a result \"result1\" to \"123\"") } }