diff --git a/pkg/reconciler/v1alpha1/pipelinerun/cancel.go b/pkg/reconciler/v1alpha1/pipelinerun/cancel.go index b76ccb4f0c0..a0ec59d5051 100644 --- a/pkg/reconciler/v1alpha1/pipelinerun/cancel.go +++ b/pkg/reconciler/v1alpha1/pipelinerun/cancel.go @@ -47,7 +47,7 @@ func cancelPipelineRun(pr *v1alpha1.PipelineRun, pipelineState []*resources.Reso continue } rprt.TaskRun.Spec.Status = v1alpha1.TaskRunSpecStatusCancelled - if _, err := clientSet.PipelineV1alpha1().TaskRuns(pr.Namespace).UpdateStatus(rprt.TaskRun); err != nil { + if _, err := clientSet.PipelineV1alpha1().TaskRuns(pr.Namespace).Update(rprt.TaskRun); err != nil { errs = append(errs, err.Error()) continue } diff --git a/test/cancel_test.go b/test/cancel_test.go index 8607987b34b..d017c6767af 100644 --- a/test/cancel_test.go +++ b/test/cancel_test.go @@ -52,12 +52,13 @@ func TestTaskRunPipelineRunCancel(t *testing.T) { pipeline := tb.Pipeline("tomatoes", namespace, tb.PipelineSpec(tb.PipelineTask("foo", "banana")), ) - - logger.Infof("Creating PipelineRun in namespace %s", namespace) - pipelineRun := tb.PipelineRun("pear", namespace, tb.PipelineRunSpec(pipeline.Name)) if _, err := c.PipelineClient.Create(pipeline); err != nil { t.Fatalf("Failed to create Pipeline `%s`: %s", "tomatoes", err) } + + pipelineRun := tb.PipelineRun("pear", namespace, tb.PipelineRunSpec(pipeline.Name)) + + logger.Infof("Creating PipelineRun in namespace %s", namespace) if _, err := c.PipelineRunClient.Create(pipelineRun); err != nil { t.Fatalf("Failed to create PipelineRun `%s`: %s", "pear", err) } @@ -87,25 +88,24 @@ func TestTaskRunPipelineRunCancel(t *testing.T) { defer close(errChan) for _, taskrunItem := range taskrunList.Items { - go func() { + go func(chan error) { err := WaitForTaskRunState(c, taskrunItem.Name, func(tr *v1alpha1.TaskRun) (bool, error) { - c := tr.Status.GetCondition(duckv1alpha1.ConditionSucceeded) - if c != nil { - if c.Status == corev1.ConditionTrue || c.Status == corev1.ConditionFalse { + if c := tr.Status.GetCondition(duckv1alpha1.ConditionSucceeded); c != nil { + if c.IsTrue() || c.IsFalse() { return true, fmt.Errorf("taskRun %s already finished!", taskrunItem.Name) - } else if c.Status == corev1.ConditionUnknown && (c.Reason == "Running" || c.Reason == "Pending") { + } else if c.IsUnknown() && (c.Reason == "Running" || c.Reason == "Pending") { return true, nil } } return false, nil }, "TaskRunRunning") errChan <- err - }() - + }(errChan) } + for i := 1; i <= len(taskrunList.Items); i++ { if <-errChan != nil { - t.Errorf("Error waiting for TaskRun %s to be running: %s", taskrunList.Items[i-1].Name, err) + t.Errorf("Error waiting for TaskRun %s to be running: %v", taskrunList.Items[i-1].Name, err) } } @@ -121,49 +121,46 @@ func TestTaskRunPipelineRunCancel(t *testing.T) { logger.Infof("Waiting for PipelineRun %s in namespace %s to be cancelled", "pear", namespace) if err := WaitForPipelineRunState(c, "pear", pipelineRunTimeout, func(pr *v1alpha1.PipelineRun) (bool, error) { - c := pr.Status.GetCondition(duckv1alpha1.ConditionSucceeded) - if c != nil { - if c.Status == corev1.ConditionFalse { + if c := pr.Status.GetCondition(duckv1alpha1.ConditionSucceeded); c != nil { + if c.IsFalse() { if c.Reason == "PipelineRunCancelled" { return true, nil } return true, fmt.Errorf("pipelineRun %s completed with the wrong reason: %s", "pear", c.Reason) - } else if c.Status == corev1.ConditionTrue { + } else if c.IsTrue() { return true, fmt.Errorf("pipelineRun %s completed successfully, should have been cancelled", "pear") } } return false, nil }, "PipelineRunCancelled"); err != nil { - t.Errorf("Error waiting for PipelineRun `pear` to finish: %s", err) + t.Errorf("Error waiting for PipelineRun `pear` to finished: %s", err) } logger.Infof("Waiting for TaskRuns in PipelineRun %s in namespace %s to be cancelled", "pear", namespace) errChan2 := make(chan error, len(taskrunList.Items)) defer close(errChan2) for _, taskrunItem := range taskrunList.Items { - go func() { + go func(errChan2 chan error) { err := WaitForTaskRunState(c, taskrunItem.Name, func(tr *v1alpha1.TaskRun) (bool, error) { - c := tr.Status.GetCondition(duckv1alpha1.ConditionSucceeded) - if c != nil { - if c.Status == corev1.ConditionFalse { + if c := tr.Status.GetCondition(duckv1alpha1.ConditionSucceeded); c != nil { + if c.IsFalse() { if c.Reason == "TaskRunCancelled" { return true, nil } return true, fmt.Errorf("taskRun %s completed with the wrong reason: %s", taskrunItem.Name, c.Reason) - } else if c.Status == corev1.ConditionTrue { + } else if c.IsTrue() { return true, fmt.Errorf("taskRun %s completed successfully, should have been cancelled", taskrunItem.Name) } } return false, nil }, "TaskRunCancelled") errChan2 <- err - }() + }(errChan2) } for i := 1; i <= len(taskrunList.Items); i++ { if <-errChan2 != nil { - t.Errorf("Error waiting for TaskRun %s to be finish: %s", taskrunList.Items[i-1].Name, err) + t.Errorf("Error waiting for TaskRun %s to be finished: %v", taskrunList.Items[i-1].Name, err) } } - } diff --git a/test/e2e-common.sh b/test/e2e-common.sh index ed5bb984c71..9d108f461a1 100755 --- a/test/e2e-common.sh +++ b/test/e2e-common.sh @@ -63,8 +63,6 @@ function dump_extra_cluster_state() { } function validate_run() { - # Wait for tests to finish. - echo ">> Waiting for tests to finish" local tests_finished=0 for i in {1..60}; do local finished="$(kubectl get $1.pipeline.knative.dev --output=jsonpath='{.items[*].status.conditions[*].status}')" @@ -74,14 +72,12 @@ function validate_run() { fi sleep 10 done - if (( ! tests_finished )); then - echo "ERROR: tests timed out" - return 1 - fi - # Check that tests passed. + return ${tests_finished} +} + +function check_results() { local failed=0 - echo ">> Checking test results" results="$(kubectl get $1.pipeline.knative.dev --output=jsonpath='{range .items[*]}{.metadata.name}={.status.conditions[*].type}{.status.conditions[*].status}{" "}{end}')" for result in ${results}; do if [[ ! "${result,,}" == *"=succeededtrue" ]]; then @@ -106,10 +102,22 @@ function run_yaml_tests() { perl -p -e 's/gcr.io\/christiewilson-catfactory/$ENV{KO_DOCKER_REPO}/g' ${file} | ko apply -f - || return 1 done - if validate_run $1; then - echo ">> All YAML tests passed" - return 0 - fi + # Wait for tests to finish. + echo ">> Waiting for tests to finish" + for test in taskrun pipelinerun; do + if validate_run ${test}; then + echo "ERROR: tests timed out" + fi + done + + # Check that tests passed. + echo ">> Checking test results" + for test in taskrun pipelinerun; do + if check_results ${test}; then + echo ">> All YAML tests passed" + return 0 + fi + done return 1 } diff --git a/test/wait.go b/test/wait.go index a7246e86426..666c7e95eea 100644 --- a/test/wait.go +++ b/test/wait.go @@ -58,7 +58,7 @@ import ( const ( interval = 1 * time.Second - timeout = 5 * time.Minute + timeout = 10 * time.Minute ) // TaskRunStateFn is a condition function on TaskRun used polling functions