From c7a60b8a77c05d8ddbc0d5a931e3f66486d512ef Mon Sep 17 00:00:00 2001 From: Andrea Frittoli Date: Mon, 29 Jun 2020 16:54:42 +0100 Subject: [PATCH] Finish setting up events for pipeline runs Add the start events for pipelineruns. Cleanup a bit of dead code where an event was generated but it's not used anymore. --- docs/events.md | 12 +- pkg/reconciler/pipelinerun/pipelinerun.go | 17 +- .../pipelinerun/pipelinerun_test.go | 267 ++++++++++++++++-- 3 files changed, 270 insertions(+), 26 deletions(-) diff --git a/docs/events.md b/docs/events.md index a0d9302d272..dcdb46748ee 100644 --- a/docs/events.md +++ b/docs/events.md @@ -28,14 +28,22 @@ No events are emitted for `Conditions` today (https://github.com/tektoncd/pipeli successfully, including post-steps injected by Tekton. - `Failed`: this is triggered if the `TaskRun` is completed, but not successfully. Causes of failure may be: one the steps failed, the `TaskRun` was cancelled or - the `TaskRun` timed out. + the `TaskRun` timed out. `Failed` events are also triggered in case the `TaskRun` + cannot be executed at all because of validation issues. ## PipelineRuns `PipelineRun` events are generated for the following `Reasons`: +- `Started`: this is triggered the first time the `PipelineRun` is picked by the + reconciler from its work queue, so it only happens if web-hook validation was + successful. Note that this event does not imply that a step started executing, + as pipeline, task and bound resource validation must be successful first. +- `Running`: this is triggered when the `PipelineRun` passes validation and + actually starts running. - `Succeeded`: this is triggered once all `Tasks` reachable via the DAG are executed successfully. - `Failed`: this is triggered if the `PipelineRun` is completed, but not successfully. Causes of failure may be: one the `Tasks` failed or the - `PipelineRun` was cancelled. + `PipelineRun` was cancelled or timed out. `Failed` events are also triggered + in case the `PipelineRun` cannot be executed at all because of validation issues. diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index 2b7e3277821..1730afbe7d0 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -144,6 +144,16 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, pr *v1beta1.PipelineRun) } // start goroutine to track pipelinerun timeout only startTime is not set go c.timeoutHandler.WaitPipelineRun(pr, pr.Status.StartTime) + // Emit events. During the first reconcile the status of the PipelineRun may change twice + // from not Started to Started and then to Running, so we need to sent the event here + // and at the end of 'Reconcile' again. + // We also want to send the "Started" event as soon as possible for anyone who may be waiting + // on the event to perform user facing initialisations, such has reset a CI check status + afterCondition := pr.Status.GetCondition(apis.ConditionSucceeded) + events.Emit(controller.GetEventRecorder(ctx), nil, afterCondition, pr) + + // We already sent an event for start, so update `before` with the current status + before = pr.Status.GetCondition(apis.ConditionSucceeded) } if pr.IsDone() { @@ -243,7 +253,6 @@ func (c *Reconciler) updatePipelineResults(ctx context.Context, pr *v1beta1.Pipe func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun) error { logger := logging.FromContext(ctx) - recorder := controller.GetEventRecorder(ctx) // We may be reading a version of the object that was stored at an older version // and may not have had all of the assumed default specified. pr.SetDefaults(contexts.WithUpgradeViaDefaulting(ctx)) @@ -407,12 +416,6 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun) err return controller.NewPermanentError(err) } - if pipelineState.IsDone() && pr.IsDone() { - c.timeoutHandler.Release(pr) - recorder.Event(pr, corev1.EventTypeNormal, v1beta1.PipelineRunReasonSuccessful.String(), "PipelineRun completed successfully.") - return nil - } - for _, rprt := range pipelineState { err := taskrun.ValidateResolvedTaskResources(rprt.PipelineTask.Params, rprt.ResolvedTaskResources) if err != nil { diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index 15d6ac8ef96..8ebe3c1c110 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -102,7 +102,40 @@ func conditionCheckFromTaskRun(tr *v1beta1.TaskRun) *v1beta1.ConditionCheck { return &cc } +func checkEvents(fr *record.FakeRecorder, testName string, wantEvents []string) error { + // The fake recorder runs in a go routine, so the timeout is here to avoid waiting + // on the channel forever if fewer than expected events are received. + // We only hit the timeout in case of failure of the test, so the actual value + // of the timeout is not so relevant, it's only used when tests are going to fail. + // on the channel forever if fewer than expected events are received + timer := time.NewTimer(1 * time.Second) + foundEvents := []string{} + for ii := 0; ii < len(wantEvents)+1; ii++ { + // We loop over all the events that we expect. Once they are all received + // we exit the loop. If we never receive enough events, the timeout takes us + // out of the loop. + select { + case event := <-fr.Events: + foundEvents = append(foundEvents, event) + if ii > len(wantEvents)-1 { + return fmt.Errorf("Received event \"%s\" for %s but not more expected", event, testName) + } + wantEvent := wantEvents[ii] + if !(strings.HasPrefix(event, wantEvent)) { + return fmt.Errorf("Expected event \"%s\" but got \"%s\" instead for %s", wantEvent, event, testName) + } + case <-timer.C: + if len(foundEvents) > len(wantEvents) { + return fmt.Errorf("Received %d events for %s but %d expected. Found events: %#v", len(foundEvents), testName, len(wantEvents), foundEvents) + } + } + } + return nil +} + func TestReconcile(t *testing.T) { + // TestReconcile runs "Reconcile" on a PipelineRun with one Task that has not been started yet. + // It verifies that the TaskRun is created, it checks the resulting API actions, status and events. names.TestingSeed() prs := []*v1beta1.PipelineRun{ @@ -301,9 +334,20 @@ func TestReconcile(t *testing.T) { // A PVC should have been created to deal with output -> input linking ensurePVCCreated(t, clients, expectedTaskRun.GetPipelineRunPVCName(), "foo") + + wantEvents := []string{ + "Normal Started", + "Normal Running Tasks Completed: 0", + } + err = checkEvents(testAssets.Recorder, "test-pipeline-run-success", wantEvents) + if !(err == nil) { + t.Errorf(err.Error()) + } } func TestReconcile_PipelineSpecTaskSpec(t *testing.T) { + // TestReconcile_PipelineSpecTaskSpec runs "Reconcile" on a PipelineRun that has an embedded PipelineSpec that has an embedded TaskSpec. + // It verifies that a TaskRun is created, it checks the resulting API actions, status and events. names.TestingSeed() prs := []*v1beta1.PipelineRun{ @@ -385,11 +429,22 @@ func TestReconcile_PipelineSpecTaskSpec(t *testing.T) { if _, exists := reconciledRun.Status.TaskRuns["test-pipeline-run-success-unit-test-task-spec-9l9zj"]; !exists { t.Errorf("Expected PipelineRun status to include TaskRun status but was %v", reconciledRun.Status.TaskRuns) } + + wantEvents := []string{ + "Normal Started", + "Normal Running Tasks Completed: 0", + } + err = checkEvents(testAssets.Recorder, "test-pipeline-run-success", wantEvents) + if !(err == nil) { + t.Errorf(err.Error()) + } } // TestReconcile_InvalidPipelineRuns runs "Reconcile" on several PipelineRuns that are invalid in different ways. // It verifies that reconcile fails, how it fails and which events are triggered. func TestReconcile_InvalidPipelineRuns(t *testing.T) { + // TestReconcile_InvalidPipelineRuns runs "Reconcile" on several PipelineRuns that are invalid in different ways. + // It verifies that reconcile fails, how it fails and which events are triggered. ts := []*v1beta1.Task{ tb.Task("a-task-that-exists", tb.TaskNamespace("foo")), tb.Task("a-task-that-needs-params", tb.TaskSpec( @@ -466,6 +521,7 @@ func TestReconcile_InvalidPipelineRuns(t *testing.T) { reason string hasNoDefaultLabels bool permanentError bool + wantEvents []string }{ { name: "invalid-pipeline-shd-be-stop-reconciling", @@ -473,71 +529,127 @@ func TestReconcile_InvalidPipelineRuns(t *testing.T) { reason: ReasonCouldntGetPipeline, hasNoDefaultLabels: true, permanentError: true, + wantEvents: []string{ + "Normal Started", + "Warning Failed Error retrieving pipeline for pipelinerun", + }, }, { name: "invalid-pipeline-run-missing-tasks-shd-stop-reconciling", pipelineRun: prs[1], reason: ReasonCouldntGetTask, permanentError: true, + wantEvents: []string{ + "Normal Started", + "Warning Failed Pipeline foo/pipeline-missing-tasks can't be Run", + }, }, { name: "invalid-pipeline-run-params-dont-exist-shd-stop-reconciling", pipelineRun: prs[2], reason: ReasonFailedValidation, permanentError: true, + wantEvents: []string{ + "Normal Started", + "Warning Failed invalid input params: missing values", + }, }, { name: "invalid-pipeline-run-resources-not-bound-shd-stop-reconciling", pipelineRun: prs[3], reason: ReasonInvalidBindings, permanentError: true, + wantEvents: []string{ + "Normal Started", + "Warning Failed PipelineRun foo/pipeline-resources-not-bound doesn't bind Pipeline", + }, }, { name: "invalid-pipeline-run-missing-resource-shd-stop-reconciling", pipelineRun: prs[4], reason: ReasonCouldntGetResource, permanentError: true, + wantEvents: []string{ + "Normal Started", + "Warning Failed PipelineRun foo/pipeline-resources-dont-exist can't be Run; it tries to bind Resources", + }, }, { name: "invalid-pipeline-missing-declared-resource-shd-stop-reconciling", pipelineRun: prs[5], reason: ReasonFailedValidation, permanentError: true, + wantEvents: []string{ + "Normal Started", + "Warning Failed Pipeline foo/a-pipeline-that-should-be-caught-by-admission-control can't be Run; it has an invalid spec", + }, }, { name: "invalid-pipeline-mismatching-parameter-types", pipelineRun: prs[6], reason: ReasonParameterTypeMismatch, permanentError: true, + wantEvents: []string{ + "Normal Started", + "Warning Failed PipelineRun foo/pipeline-mismatching-param-type parameters have mismatching types", + }, }, { name: "invalid-pipeline-missing-conditions-shd-stop-reconciling", pipelineRun: prs[7], reason: ReasonCouldntGetCondition, permanentError: true, + wantEvents: []string{ + "Normal Started", + "Warning Failed PipelineRun foo/pipeline-conditions-missing can't be Run; it contains Conditions", + }, }, { name: "invalid-embedded-pipeline-resources-bot-bound-shd-stop-reconciling", pipelineRun: prs[8], reason: ReasonInvalidBindings, permanentError: true, + wantEvents: []string{ + "Normal Started", + "Warning Failed PipelineRun foo/embedded-pipeline-resources-not-bound doesn't bind Pipeline", + }, }, { name: "invalid-embedded-pipeline-bad-name-shd-stop-reconciling", pipelineRun: prs[9], reason: ReasonFailedValidation, permanentError: true, + wantEvents: []string{ + "Normal Started", + "Warning Failed Pipeline foo/embedded-pipeline-invalid can't be Run; it has an invalid spec: invalid value \"bad-t@$k\"", + }, }, { name: "invalid-embedded-pipeline-mismatching-parameter-types", pipelineRun: prs[10], reason: ReasonParameterTypeMismatch, permanentError: true, + wantEvents: []string{ + "Normal Started", + "Warning Failed PipelineRun foo/embedded-pipeline-mismatching-param-type parameters have mismatching types", + }, }, { name: "invalid-pipeline-run-missing-params-shd-stop-reconciling", pipelineRun: prs[11], reason: ReasonParameterMissing, permanentError: true, + wantEvents: []string{ + "Normal Started", + "Warning Failed PipelineRun foo parameters is missing some parameters required by Pipeline pipelinerun-missing-params", + }, }, { name: "invalid-pipeline-with-invalid-dag-graph", pipelineRun: prs[12], reason: ReasonInvalidGraph, permanentError: true, + wantEvents: []string{ + "Normal Started", + "Warning Failed PipelineRun foo/pipeline-invalid-dag-graph's Pipeline DAG is invalid", + }, }, { name: "invalid-pipeline-with-invalid-final-tasks-graph", pipelineRun: prs[13], reason: ReasonInvalidGraph, permanentError: true, + wantEvents: []string{ + "Normal Started", + "Warning Failed PipelineRun foo's Pipeline DAG is invalid for finally clause", + }, }, } @@ -594,11 +706,20 @@ func TestReconcile_InvalidPipelineRuns(t *testing.T) { } } } + // Check generated events match what's expected + wantEvents := append(tc.wantEvents, "Warning InternalError 1 error occurred") + err = checkEvents(testAssets.Recorder, tc.pipelineRun.Name, wantEvents) + if !(err == nil) { + t.Errorf(err.Error()) + } }) } } 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. + // Note that the code tested here is part of the genreconciler. invalidNames := []string{ "foo/test-pipeline-run-doesnot-exist", "test/invalidformat/t", @@ -632,6 +753,8 @@ func TestReconcile_InvalidPipelineRunNames(t *testing.T) { } func TestUpdateTaskRunsState(t *testing.T) { + // TestUpdateTaskRunsState runs "getTaskRunsStatus" and verifies how it updates a PipelineRun status + // from a TaskRun associated to the PipelineRun pr := tb.PipelineRun("test-pipeline-run", tb.PipelineRunNamespace("foo"), tb.PipelineRunSpec("test-pipeline")) pipelineTask := v1beta1.PipelineTask{ Name: "unit-test-1", @@ -688,6 +811,8 @@ func TestUpdateTaskRunsState(t *testing.T) { } func TestUpdateTaskRunStateWithConditionChecks(t *testing.T) { + // TestUpdateTaskRunsState runs "getTaskRunsStatus" and verifies how it updates a PipelineRun status + // from several different TaskRun with Conditions associated to the PipelineRun taskrunName := "task-run" successConditionCheckName := "success-condition" failingConditionCheckName := "fail-condition" @@ -829,6 +954,10 @@ func TestUpdateTaskRunStateWithConditionChecks(t *testing.T) { } func TestReconcileOnCompletedPipelineRun(t *testing.T) { + // TestReconcileOnCompletedPipelineRun runs "Reconcile" on a PipelineRun that already reached completion + // and that does not have the latest status from TaskRuns yet. It checks that the TaskRun status is updated + // in the PipelineRun status, that the completion status is not altered, that not error is returned and + // a succesful event is triggered taskRunName := "test-pipeline-run-completed-hello-world" prs := []*v1beta1.PipelineRun{tb.PipelineRun("test-pipeline-run-completed", tb.PipelineRunNamespace("foo"), @@ -878,25 +1007,29 @@ func TestReconcileOnCompletedPipelineRun(t *testing.T) { t.Fatalf("Error reconciling: %s", err) } - if len(clients.Pipeline.Actions()) != 2 { + actions := clients.Pipeline.Actions() + if len(actions) != 2 { + t.Errorf("# Actions: %d, Actions: %#v", len(actions), actions) t.Fatalf("Expected client to have updated the TaskRun status for a completed PipelineRun, but it did not") } - _, ok := clients.Pipeline.Actions()[1].(ktesting.UpdateAction).GetObject().(*v1beta1.PipelineRun) - if !ok { - t.Errorf("Expected a PipelineRun to be updated, but it wasn't.") - } - t.Log(clients.Pipeline.Actions()) - actions := clients.Pipeline.Actions() + pipelineUpdates := 0 for _, action := range actions { if action != nil { - resource := action.GetResource().Resource - if resource == "taskruns" { - t.Fatalf("Expected client to not have created a TaskRun for the completed PipelineRun, but it did") + switch { + case action.Matches("create", "taskruns"): + t.Errorf("Expected client to not have created a TaskRun, but it did") + case action.Matches("update", "pipelineruns"): + pipelineUpdates++ } } } + if pipelineUpdates != 1 { + // If only the pipelinerun status changed, we expect one update + t.Fatalf("Expected client to have updated the pipelinerun twice, but it did %d times", pipelineUpdates) + } + // Check that the PipelineRun was reconciled correctly reconciledRun, err := clients.Pipeline.TektonV1beta1().PipelineRuns("foo").Get("test-pipeline-run-completed", metav1.GetOptions{}) if err != nil { @@ -921,14 +1054,25 @@ func TestReconcileOnCompletedPipelineRun(t *testing.T) { 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)) } + + wantEvents := []string{ + "Normal Succeeded All Tasks have completed executing", + } + err = checkEvents(testAssets.Recorder, "test-pipeline-run-completed", wantEvents) + if !(err == nil) { + t.Errorf(err.Error()) + } } func TestReconcileOnCancelledPipelineRun(t *testing.T) { + // TestReconcileOnCancelledPipelineRun runs "Reconcile" on a PipelineRun that has been cancelled. + // It verifies that reconcile is successful, the pipeline status updated and events generated. prs := []*v1beta1.PipelineRun{tb.PipelineRun("test-pipeline-run-cancelled", tb.PipelineRunNamespace("foo"), tb.PipelineRunSpec("test-pipeline", tb.PipelineRunServiceAccountName("test-sa"), tb.PipelineRunCancelled, ), + tb.PipelineRunStatus(tb.PipelineRunStartTime(time.Now())), )} ps := []*v1beta1.Pipeline{tb.Pipeline("test-pipeline", tb.PipelineNamespace("foo"), tb.PipelineSpec( tb.PipelineTask("hello-world-1", "hello-world"), @@ -977,9 +1121,19 @@ func TestReconcileOnCancelledPipelineRun(t *testing.T) { if !reconciledRun.Status.GetCondition(apis.ConditionSucceeded).IsFalse() { t.Errorf("Expected PipelineRun status to be complete and false, but was %v", reconciledRun.Status.GetCondition(apis.ConditionSucceeded)) } + + wantEvents := []string{ + "Warning Failed PipelineRun \"test-pipeline-run-cancelled\" was cancelled", + } + err = checkEvents(testAssets.Recorder, "test-pipeline-run-cancelled", wantEvents) + if !(err == nil) { + t.Errorf(err.Error()) + } } func TestReconcileWithTimeout(t *testing.T) { + // TestReconcileWithTimeout runs "Reconcile" on a PipelineRun that has timed out. + // It verifies that reconcile is successful, the pipeline status updated and events generated. ps := []*v1beta1.Pipeline{tb.Pipeline("test-pipeline", tb.PipelineNamespace("foo"), tb.PipelineSpec( tb.PipelineTask("hello-world-1", "hello-world"), ))} @@ -1035,9 +1189,18 @@ func TestReconcileWithTimeout(t *testing.T) { if actual.Spec.Timeout.Duration > prs[0].Spec.Timeout.Duration { t.Errorf("TaskRun timeout %s should be less than or equal to PipelineRun timeout %s", actual.Spec.Timeout.Duration.String(), prs[0].Spec.Timeout.Duration.String()) } + wantEvents := []string{ + "Warning Failed PipelineRun \"test-pipeline-run-with-timeout\" failed to finish within \"12h0m0s\"", + } + err = checkEvents(testAssets.Recorder, "test-pipeline-run-with-timeout", wantEvents) + if !(err == nil) { + t.Errorf(err.Error()) + } } func TestReconcileWithoutPVC(t *testing.T) { + // TestReconcileWithoutPVC runs "Reconcile" on a PipelineRun that has two unrelated tasks. + // It verifies that reconcile is successful and that no PVC is created ps := []*v1beta1.Pipeline{tb.Pipeline("test-pipeline", tb.PipelineNamespace("foo"), tb.PipelineSpec( tb.PipelineTask("hello-world-1", "hello-world"), tb.PipelineTask("hello-world-2", "hello-world"), @@ -1086,9 +1249,12 @@ func TestReconcileWithoutPVC(t *testing.T) { } func TestReconcileCancelledFailsTaskRunCancellation(t *testing.T) { + // TestReconcileCancelledFailsTaskRunCancellation runs "Reconcile" on a PipelineRun with a single TaskRun. + // The TaskRun cannot be cancelled. Check that the pipelinerun cancel fails, that reconcile fails and + // an event is generated names.TestingSeed() ptName := "hello-world-1" - prName := "test-pipeline-run-with-timeout" + prName := "test-pipeline-fails-to-cancel" prs := []*v1beta1.PipelineRun{tb.PipelineRun(prName, tb.PipelineRunNamespace("foo"), tb.PipelineRunSpec("test-pipeline", tb.PipelineRunCancelled, @@ -1096,10 +1262,17 @@ func TestReconcileCancelledFailsTaskRunCancellation(t *testing.T) { // The reconciler uses the presence of this TaskRun in the status to determine that a TaskRun // is already running. The TaskRun will not be retrieved at all so we do not need to seed one. tb.PipelineRunStatus( + tb.PipelineRunStatusCondition(apis.Condition{ + Type: apis.ConditionSucceeded, + Status: corev1.ConditionUnknown, + Reason: v1beta1.PipelineRunReasonRunning.String(), + Message: "running...", + }), tb.PipelineRunTaskRunsStatus(prName+ptName, &v1alpha1.PipelineRunTaskRunStatus{ PipelineTaskName: ptName, Status: &v1alpha1.TaskRunStatus{}, }), + tb.PipelineRunStartTime(time.Now()), ), )} @@ -1117,13 +1290,13 @@ func TestReconcileCancelledFailsTaskRunCancellation(t *testing.T) { return true, nil, fmt.Errorf("i'm sorry Dave, i'm afraid i can't do that") }) - err := c.Reconciler.Reconcile(context.Background(), "foo/test-pipeline-run-with-timeout") + err := c.Reconciler.Reconcile(context.Background(), "foo/test-pipeline-fails-to-cancel") if err == nil { t.Errorf("Expected to see error returned from reconcile after failing to cancel TaskRun but saw none!") } // Check that the PipelineRun is still running with correct error message - reconciledRun, err := clients.Pipeline.TektonV1beta1().PipelineRuns("foo").Get("test-pipeline-run-with-timeout", metav1.GetOptions{}) + reconciledRun, err := clients.Pipeline.TektonV1beta1().PipelineRuns("foo").Get("test-pipeline-fails-to-cancel", metav1.GetOptions{}) if err != nil { t.Fatalf("Somehow had error getting reconciled run out of fake client: %s", err) } @@ -1136,16 +1309,31 @@ func TestReconcileCancelledFailsTaskRunCancellation(t *testing.T) { if condition.Reason != ReasonCouldntCancel { t.Errorf("Expected PipelineRun condition to indicate the cancellation failed but reason was %s", condition.Reason) } + // The event here is "Normal" because in case we fail to cancel we leave the condition to unknown + // Further reconcile might converge then the status of the pipeline. + // See https://github.com/tektoncd/pipeline/issues/2647 for further details. + wantEvents := []string{ + "Normal PipelineRunCouldntCancel PipelineRun \"test-pipeline-fails-to-cancel\" was cancelled but had errors trying to cancel TaskRuns", + "Warning InternalError 1 error occurred", + } + err = checkEvents(testAssets.Recorder, prName, wantEvents) + if !(err == nil) { + t.Errorf(err.Error()) + } } func TestReconcileCancelledPipelineRun(t *testing.T) { + // TestReconcileCancelledPipelineRun runs "Reconcile" on a PipelineRun that has been cancelled. + // The PipelineRun had no TaskRun associated yet, and no TaskRun should have been created. + // It verifies that reconcile is successful, the pipeline status updated and events generated. ps := []*v1beta1.Pipeline{tb.Pipeline("test-pipeline", tb.PipelineNamespace("foo"), tb.PipelineSpec( tb.PipelineTask("hello-world-1", "hello-world", tb.Retries(1)), ))} - prs := []*v1beta1.PipelineRun{tb.PipelineRun("test-pipeline-run-with-timeout", tb.PipelineRunNamespace("foo"), + prs := []*v1beta1.PipelineRun{tb.PipelineRun("test-pipeline-run-cancelled", tb.PipelineRunNamespace("foo"), tb.PipelineRunSpec("test-pipeline", tb.PipelineRunCancelled, ), + tb.PipelineRunStatus(tb.PipelineRunStartTime(time.Now())), )} ts := []*v1beta1.Task{tb.Task("hello-world", tb.TaskNamespace("foo"))} @@ -1160,13 +1348,13 @@ func TestReconcileCancelledPipelineRun(t *testing.T) { c := testAssets.Controller clients := testAssets.Clients - err := c.Reconciler.Reconcile(context.Background(), "foo/test-pipeline-run-with-timeout") + err := c.Reconciler.Reconcile(context.Background(), "foo/test-pipeline-run-cancelled") if err != nil { t.Errorf("Did not expect to see error when reconciling completed PipelineRun but saw %s", err) } // Check that the PipelineRun was reconciled correctly - reconciledRun, err := clients.Pipeline.TektonV1beta1().PipelineRuns("foo").Get("test-pipeline-run-with-timeout", metav1.GetOptions{}) + reconciledRun, err := clients.Pipeline.TektonV1beta1().PipelineRuns("foo").Get("test-pipeline-run-cancelled", metav1.GetOptions{}) if err != nil { t.Fatalf("Somehow had error getting completed reconciled run out of fake client: %s", err) } @@ -1184,6 +1372,14 @@ func TestReconcileCancelledPipelineRun(t *testing.T) { t.Errorf("Expected a TaskRun to be get/updated, but it was %s", actionType) } } + + wantEvents := []string{ + "Warning Failed PipelineRun \"test-pipeline-run-cancelled\" was cancelled", + } + err = checkEvents(testAssets.Recorder, "test-pipeline-run-cancelled", wantEvents) + if !(err == nil) { + t.Errorf(err.Error()) + } } func TestReconcilePropagateLabels(t *testing.T) { @@ -1337,21 +1533,31 @@ func TestReconcileWithDifferentServiceAccounts(t *testing.T) { } func TestReconcileWithTimeoutAndRetry(t *testing.T) { + // TestReconcileWithTimeoutAndRetry runs "Reconcile" against pipelines with retries and timeout settings, + // and status that represents different number of retries already performed. + // It verifies the reconciled status and events generated tcs := []struct { name string retries int conditionSucceeded corev1.ConditionStatus + wantEvents []string }{ { name: "One try has to be done", retries: 1, conditionSucceeded: corev1.ConditionFalse, + wantEvents: []string{ + "Warning Failed PipelineRun \"test-pipeline-retry-run-with-timeout\" failed to finish within", + }, }, { name: "No more retries are needed", retries: 2, conditionSucceeded: corev1.ConditionUnknown, + wantEvents: []string{ + "Warning Failed PipelineRun \"test-pipeline-retry-run-with-timeout\" failed to finish within", + }, }, } @@ -1430,7 +1636,10 @@ func TestReconcileWithTimeoutAndRetry(t *testing.T) { if status := reconciledRun.Status.TaskRuns["hello-world-1"].Status.GetCondition(apis.ConditionSucceeded).Status; status != tc.conditionSucceeded { t.Fatalf("Succeeded expected to be %s but is %s", tc.conditionSucceeded, status) } - + err = checkEvents(testAssets.Recorder, prs[0].Name, tc.wantEvents) + if !(err == nil) { + t.Errorf(err.Error()) + } }) } } @@ -1668,6 +1877,9 @@ func TestReconcileAndPropagateCustomPipelineTaskRunSpec(t *testing.T) { } func TestReconcileWithConditionChecks(t *testing.T) { + // TestReconcileWithConditionChecks runs "Reconcile" on a PipelineRun that has a task with + // multiple conditions. It verifies that reconcile is successful, taskruns are created and + // the status is updated. It checks that the correct events are sent. names.TestingSeed() prName := "test-pipeline-run" conditions := []*v1alpha1.Condition{ @@ -1752,9 +1964,21 @@ func TestReconcileWithConditionChecks(t *testing.T) { if d := cmp.Diff(actual, expectedConditionChecks); d != "" { t.Errorf("expected to see 2 ConditionCheck TaskRuns created. Diff %s", diff.PrintWantGot(d)) } + + wantEvents := []string{ + "Normal Started", + "Normal Running Tasks Completed: 0 (Failed: 0, Cancelled 0), Incomplete: 1, Skipped: 0", + } + err = checkEvents(testAssets.Recorder, "test-pipeline-run-completed", wantEvents) + if !(err == nil) { + t.Errorf(err.Error()) + } } func TestReconcileWithFailingConditionChecks(t *testing.T) { + // TestReconcileWithFailingConditionChecks runs "Reconcile" on a PipelineRun that has a task with + // multiple conditions, some that fails. It verifies that reconcile is successful, taskruns are + // created and the status is updated. It checks that the correct events are sent. names.TestingSeed() conditions := []*v1alpha1.Condition{ tbv1alpha1.Condition("always-false", tbv1alpha1.ConditionNamespace("foo"), tbv1alpha1.ConditionSpec( @@ -1872,6 +2096,15 @@ func TestReconcileWithFailingConditionChecks(t *testing.T) { if d := cmp.Diff(actual, expectedTaskRun); d != "" { t.Errorf("expected to see ConditionCheck TaskRun %v created. Diff %s", expectedTaskRun, diff.PrintWantGot(d)) } + + wantEvents := []string{ + "Normal Started", + "Normal Running Tasks Completed: 1 (Failed: 0, Cancelled 0), Incomplete: 1, Skipped: 1", + } + err = checkEvents(testAssets.Recorder, "test-pipeline-run-completed", wantEvents) + if !(err == nil) { + t.Errorf(err.Error()) + } } func makeExpectedTr(condName, ccName string, labels, annotations map[string]string) *v1beta1.TaskRun {