From b6e4d4cf25ec3ec23892cce91d97b80fcd5f5a51 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 redundancies, improve speed, and reduce flakes. --- .../pipelinerun/pipelinerun_test.go | 182 ++++++++++++++++++ test/ignore_step_error_test.go | 92 ++++----- 2 files changed, 218 insertions(+), 56 deletions(-) diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index c790049184c..2a9c9dfedc2 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -1049,6 +1049,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: "beta" + 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..bea06230255 100644 --- a/test/ignore_step_error_test.go +++ b/test/ignore_step_error_test.go @@ -22,85 +22,65 @@ package test import ( "context" "fmt" - "testing" - - "github.com/tektoncd/pipeline/pkg/reconciler/pipelinerun" + "github.com/google/go-cmp/cmp" + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" + "github.com/tektoncd/pipeline/test/diff" "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" + "testing" ) -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 { - t.Fatalf("task1 should have produced a result before failing the step") - } + expectedResults := []v1beta1.TaskRunResult{{ + Name: "result1", + Type: "string", + Value: *v1beta1.NewArrayOrString("123"), + }} - for _, r := range taskrunItem.Status.TaskRunResults { - if r.Name == "result1" && r.Value.StringVal != "123" { - t.Fatalf("task1 should have initialized a result \"result1\" to \"123\"") - } + if d := cmp.Diff(expectedResults, taskRun.Status.TaskRunResults); d != "" { + t.Errorf("Got unexpected results %s", diff.PrintWantGot(d)) } }