From f3f418d187af0db3060fc464a1dd5bcd074257f1 Mon Sep 17 00:00:00 2001 From: Andrea Frittoli Date: Mon, 25 May 2020 11:26:11 +0100 Subject: [PATCH] Make phase condition reasons part of the API TaskRuns and PipelineRuns use the "Reason" field to complement the value of the "Succeeded" condition. Those values are not part of the API and are even owned by the underlying resource (pod) in case of TaskRuns. This makes it difficult to rely on them to understand that the state of the resource is. In case of corev1.ConditionTrue, the reason can be used to distinguish between: - Successful - Successful, some parts were skipped (pipelinerun only) In case of corev1.ConditionFalse, the reason can be used to distinguish between: - Failed - Failed because of timeout - Failed because of cancelled by the user In case of corev1.ConditionUnknown, the reason can be used to distinguish between: - Just started reconciling - Validation done, running (or still running) - Cancellation requested This is implemented through the following changes: - Bubble-up reasons for taskrun and pipelinerun to the v1beta1 API, except for reason that are defined by the underlying resource - Enforce the start reason to be set during condition init This allows for an additional change in the eventing module: the condition before and after can be used to decide whether to send an event at all. If they are different, the after condition now contains enough information to send the event. The cloudevent module is extended with ability to send the correct event based on both status and reason. --- docs/pipelineruns.md | 66 ++++++++++ docs/taskruns.md | 21 +++- .../pipeline/v1beta1/pipelinerun_types.go | 51 ++++++-- .../v1beta1/pipelinerun_types_test.go | 19 ++- pkg/apis/pipeline/v1beta1/taskrun_types.go | 70 ++++++++--- .../pipeline/v1beta1/taskrun_types_test.go | 34 +++++ pkg/pod/status.go | 26 ++-- pkg/pod/status_test.go | 14 +-- .../cloudevent/cloud_event_controller_test.go | 22 ++++ .../events/cloudevent/cloudevent.go | 117 ++++++++++-------- .../events/cloudevent/cloudevent_test.go | 56 ++++++--- .../events/cloudevent/interface.go} | 26 ++-- pkg/reconciler/pipelinerun/pipelinerun.go | 10 +- .../pipelinerun/pipelinerun_test.go | 12 +- .../resources/pipelinerunresolution.go | 39 ++---- pkg/reconciler/reconciler_test.go | 3 +- pkg/reconciler/taskrun/taskrun.go | 6 +- pkg/reconciler/taskrun/taskrun_test.go | 14 +-- test/timeout_test.go | 5 +- test/v1alpha1/cancel_test.go | 11 +- test/v1alpha1/timeout_test.go | 5 +- 21 files changed, 416 insertions(+), 211 deletions(-) rename pkg/{apis/pipeline/v1beta1/run_interface.go => reconciler/events/cloudevent/interface.go} (52%) diff --git a/docs/pipelineruns.md b/docs/pipelineruns.md index 3405b7e1830..8075beca360 100644 --- a/docs/pipelineruns.md +++ b/docs/pipelineruns.md @@ -18,6 +18,7 @@ weight: 4 - [Specifying `Workspaces`](#specifying-workspaces) - [Specifying `LimitRange` values](#specifying-limitrange-values) - [Configuring a failure timeout](#configuring-a-failure-timeout) +- [Monitoring execution status](#monitoring-execution-status) - [Cancelling a `PipelineRun`](#cancelling-a-pipelinerun) - [Events](events.md#pipelineruns) @@ -362,6 +363,71 @@ The `timeout` value is a `duration` conforming to Go's values are `1h30m`, `1h`, `1m`, and `60s`. If you set the global timeout to 0, all `PipelineRuns` that do not have an idividual timeout set will fail immediately upon encountering an error. +## Monitoring execution status + +As your `PipelineRun` executes, its `status` field accumulates information on the execution of each `TaskRun` +as well as the `PipelineRun` as a whole. This information includes the name of the pipeline `Task` associated +to a `TaskRun`, the complete [status of the `TaskrRun`](taskruns.md#monitoring-execution-status) and details +about `Conditions` that may be associated to a `TaskRun`. + +The following example shows an extract from the `status` field of a `PipelineRun` that has executed successfully: + +```yaml +completionTime: "2020-05-04T02:19:14Z" +conditions: +- lastTransitionTime: "2020-05-04T02:19:14Z" + message: 'Tasks Completed: 4, Skipped: 0' + reason: Succeeded + status: "True" + type: Succeeded +startTime: "2020-05-04T02:00:11Z" +taskRuns: + triggers-release-nightly-frwmw-build-ng2qk: + pipelineTaskName: build + status: + completionTime: "2020-05-04T02:10:49Z" + conditions: + - lastTransitionTime: "2020-05-04T02:10:49Z" + message: All Steps have completed executing + reason: Succeeded + status: "True" + type: Succeeded + podName: triggers-release-nightly-frwmw-build-ng2qk-pod-8vj99 + resourcesResult: + - key: commit + resourceRef: + name: git-source-triggers-frwmw + value: 9ab5a1234166a89db352afa28f499d596ebb48db + startTime: "2020-05-04T02:05:07Z" + steps: + - container: step-build + imageID: docker-pullable://golang@sha256:a90f2671330831830e229c3554ce118009681ef88af659cd98bfafd13d5594f9 + name: build + terminated: + containerID: docker://6b6471f501f59dbb7849f5cdde200f4eeb64302b862a27af68821a7fb2c25860 + exitCode: 0 + finishedAt: "2020-05-04T02:10:45Z" + reason: Completed + startedAt: "2020-05-04T02:06:24Z" + ``` + +The following tables shows how to read the overall status of a `PipelineRun`: + +`status`|`reason`|`completionTime` is set|Description +:-------|:-------|:---------------------:|--------------: +Unknown|Started|No|The `PipelineRun` has just been picked up by the controller. +Unknown|Running|No|The `PipelineRun` has been validate and started to perform its work. +Unknown|PipelineRunCancelled|No|The user requested the PipelineRun to be cancelled. Cancellation has not be done yet. +True|Succeeded|Yes|The `PipelineRun` completed successfully. +True|Completed|Yes|The `PipelineRun` completed successfully, one or more Tasks were skipped. +False|Failed|Yes|The `PipelineRun` failed because one of the `TaskRuns` failed. +False|\[Error message\]|No|The `PipelineRun` encountered an non-permanent error, but it's still running and it may ultimately succeed. +False|\[Error message\]|Yes|The `PipelineRun` failed with a permanent error (usually validation). +False|PipelineRunCancelled|Yes|The `PipelineRun` was cancelled successfully. +False|PipelineRunTimeout|Yes|The `PipelineRun` timed out. + +When a `PipelineRun` changes status, [events](events.md#pipelineruns) are triggered accordingly. + ## Cancelling a `PipelineRun` To cancel a `PipelineRun` that's currently executing, update its definition diff --git a/docs/taskruns.md b/docs/taskruns.md index bdf083987e4..d54d1e50803 100644 --- a/docs/taskruns.md +++ b/docs/taskruns.md @@ -155,7 +155,7 @@ point for the `Pod` in which the container images specified in your `Task` will customize the `Pod` configuration specifically for that `TaskRun`. In the following example, the `Task` specifies a `volumeMount` (`my-cache`) object, also provided by the `TaskRun`, -using a `PersistentVolumeClaim` volume. A specific scheduler is also configured in the `SchedulerName` field. +using a `PersistentVolumeClaim` volume. A specific scheduler is also configured in the `SchedulerName` field. The `Pod` executes with regular (non-root) user permissions. ```yaml @@ -281,7 +281,7 @@ For more information, see [`ServiceAccount`](auth.md). ## Monitoring execution status As your `TaskRun` executes, its `status` field accumulates information on the execution of each `Step` -as well as the `TaskRun` as a whole. This information includes start and stop times, exit codes, the +as well as the `TaskRun` as a whole. This information includes start and stop times, exit codes, the fully-qualified name of the container image, and the corresponding digest. **Note:** If any `Pods` have been [`OOMKilled`](https://kubernetes.io/docs/tasks/administer-cluster/out-of-resource/) @@ -311,6 +311,23 @@ steps: startedAt: "2019-08-12T18:22:54Z" ``` +The following tables shows how to read the overall status of a `TaskRun`: + +`status`|`reason`|`completionTime` is set|Description +:-------|:-------|:---------------------:|--------------: +Unknown|Started|No|The TaskRun has just been picked up by the controller. +Unknown|Pending|No|The TaskRun is waiting on a Pod in status Pending. +Unknown|Running|No|The TaskRun has been validate and started to perform its work. +Unknown|TaskRunCancelled|No|The user requested the TaskRun to be cancelled. Cancellation has not be done yet. +True|Succeeded|Yes|The TaskRun completed successfully. +False|Failed|Yes|The TaskRun failed because one of the steps failed. +False|\[Error message\]|No|The TaskRun encountered a non-permanent error, and it's still running. It may ultimately succeed. +False|\[Error message\]|Yes|The TaskRun failed with a permanent error (usually validation). +False|TaskRunCancelled|Yes|The TaskRun was cancelled successfully. +False|TaskRunTimeout|Yes|The TaskRun timed out. + +When a `TaskRun` changes status, [events](events.md#taskruns) are triggered accordingly. + ### Monitoring `Steps` If multiple `Steps` are defined in the `Task` invoked by the `TaskRun`, you can monitor their execution diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_types.go b/pkg/apis/pipeline/v1beta1/pipelinerun_types.go index aec34507061..e8a102b9ca4 100644 --- a/pkg/apis/pipeline/v1beta1/pipelinerun_types.go +++ b/pkg/apis/pipeline/v1beta1/pipelinerun_types.go @@ -73,18 +73,8 @@ func (pr *PipelineRun) GetTaskRunRef() corev1.ObjectReference { } } -// GetTypeMeta returns the task run type meta -func (pr *PipelineRun) GetTypeMeta() *metav1.TypeMeta { - return &pr.TypeMeta -} - -// GetObjectMeta returns the task run type meta -func (pr *PipelineRun) GetObjectMeta() *metav1.ObjectMeta { - return &pr.ObjectMeta -} - -// GetStatus returns the task run status as a RunsToCompletionStatus -func (pr *PipelineRun) GetStatus() RunsToCompletionStatus { +// GetStatusCondition returns the task run status as a ConditionAccessor +func (pr *PipelineRun) GetStatusCondition() apis.ConditionAccessor { return &pr.Status } @@ -221,6 +211,32 @@ type PipelineRunStatus struct { PipelineRunStatusFields `json:",inline"` } +// PipelineRunReason represents a reason for the pipeline run "Succeeded" condition +type PipelineRunReason string + +const ( + // PipelineRunReasonStarted is the reason set when the PipelineRun has just started + PipelineRunReasonStarted PipelineRunReason = "Started" + // PipelineRunReasonRunning is the reason set when the PipelineRun is running + PipelineRunReasonRunning PipelineRunReason = "Running" + // PipelineRunReasonSuccessful is the reason set when the PipelineRun completed successfully + PipelineRunReasonSuccessful PipelineRunReason = "Succeeded" + // PipelineRunReasonCompleted is the reason set when the PipelineRun completed successfully with one or more skipped Tasks + PipelineRunReasonCompleted PipelineRunReason = "Completed" + // PipelineRunReasonFailed is the reason set when the PipelineRun completed with a failure + PipelineRunReasonFailed PipelineRunReason = "Failed" + // PipelineRunReasonCancelled is the reason set when the PipelineRun cancelled by the user + // This reason may be found with a corev1.ConditionFalse status, if the cancellation was processed successfully + // This reason may be found with a corev1.ConditionUnknown status, if the cancellation is being processed or failed + PipelineRunReasonCancelled PipelineRunReason = "Cancelled" + // PipelineRunReasonTimedOut is the reason set when the PipelineRun has timed out + PipelineRunReasonTimedOut PipelineRunReason = "PipelineRunTimeout" +) + +func (t PipelineRunReason) String() string { + return string(t) +} + var pipelineRunCondSet = apis.NewBatchConditionSet() // GetCondition returns the Condition matching the given type. @@ -231,13 +247,22 @@ func (pr *PipelineRunStatus) GetCondition(t apis.ConditionType) *apis.Condition // InitializeConditions will set all conditions in pipelineRunCondSet to unknown for the PipelineRun // and set the started time to the current time func (pr *PipelineRunStatus) InitializeConditions() { + started := false if pr.TaskRuns == nil { pr.TaskRuns = make(map[string]*PipelineRunTaskRunStatus) } if pr.StartTime.IsZero() { pr.StartTime = &metav1.Time{Time: time.Now()} + started = true + } + conditionManager := pipelineRunCondSet.Manage(pr) + conditionManager.InitializeConditions() + // Ensure the started reason is set for the "Succeeded" condition + if started { + initialCondition := conditionManager.GetCondition(apis.ConditionSucceeded) + initialCondition.Reason = PipelineRunReasonStarted.String() + conditionManager.SetCondition(*initialCondition) } - pipelineRunCondSet.Manage(pr).InitializeConditions() } // SetCondition sets the condition, unsetting previous conditions with the same diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_types_test.go b/pkg/apis/pipeline/v1beta1/pipelinerun_types_test.go index 69673a6c005..8388361bd22 100644 --- a/pkg/apis/pipeline/v1beta1/pipelinerun_types_test.go +++ b/pkg/apis/pipeline/v1beta1/pipelinerun_types_test.go @@ -83,7 +83,7 @@ func TestPipelineRun_TaskRunref(t *testing.T) { } } -func TestInitializeConditions(t *testing.T) { +func TestInitializePipelineRunConditions(t *testing.T) { p := &v1beta1.PipelineRun{ ObjectMeta: metav1.ObjectMeta{ Name: "test-name", @@ -100,12 +100,29 @@ func TestInitializeConditions(t *testing.T) { t.Fatalf("PipelineRun StartTime not initialized correctly") } + condition := p.Status.GetCondition(apis.ConditionSucceeded) + if condition.Reason != v1beta1.PipelineRunReasonStarted.String() { + t.Fatalf("PipelineRun initialize reason should be %s, got %s instead", v1beta1.PipelineRunReasonStarted.String(), condition.Reason) + } p.Status.TaskRuns["fooTask"] = &v1beta1.PipelineRunTaskRunStatus{} + // Change the reason before we initialize again + p.Status.SetCondition(&apis.Condition{ + Type: apis.ConditionSucceeded, + Status: corev1.ConditionUnknown, + Reason: "not just started", + Message: "hello", + }) + p.Status.InitializeConditions() if len(p.Status.TaskRuns) != 1 { t.Fatalf("PipelineRun status getting reset") } + + newCondition := p.Status.GetCondition(apis.ConditionSucceeded) + if newCondition.Reason != "not just started" { + t.Fatalf("PipelineRun initialize reset the condition reason to %s", newCondition.Reason) + } } func TestPipelineRunIsDone(t *testing.T) { diff --git a/pkg/apis/pipeline/v1beta1/taskrun_types.go b/pkg/apis/pipeline/v1beta1/taskrun_types.go index 3e52a29054d..bdf38ae3bfd 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_types.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_types.go @@ -72,10 +72,6 @@ const ( // TaskRunSpecStatusCancelled indicates that the user wants to cancel the task, // if not already cancelled or terminated TaskRunSpecStatusCancelled = "TaskRunCancelled" - - // TaskRunReasonCancelled indicates that the TaskRun has been cancelled - // because it was requested so by the user - TaskRunReasonCancelled = "TaskRunCancelled" ) // TaskRunInputs holds the input values that this task was invoked with. @@ -102,6 +98,43 @@ type TaskRunStatus struct { TaskRunStatusFields `json:",inline"` } +// TaskRunReason is an enum used to store all TaskRun reason for +// the Succeeded condition that are controlled by the TaskRun itself. Failure +// reasons that emerge from underlying resources are not included here +type TaskRunReason string + +const ( + // TaskRunReasonStarted is the reason set when the TaskRun has just started + TaskRunReasonStarted TaskRunReason = "Started" + // TaskRunReasonRunning is the reason set when the TaskRun is running + TaskRunReasonRunning TaskRunReason = "Running" + // TaskRunReasonSuccessful is the reason set when the TaskRun completed successfully + TaskRunReasonSuccessful TaskRunReason = "Succeeded" + // TaskRunReasonFailed is the reason set when the TaskRun completed with a failure + TaskRunReasonFailed TaskRunReason = "Failed" + // TaskRunReasonCancelled is the reason set when the Taskrun is cancelled by the user + TaskRunReasonCancelled TaskRunReason = "TaskRunCancelled" + // TaskRunReasonTimedOut is the reason set when the Taskrun has timed out + TaskRunReasonTimedOut TaskRunReason = "TaskRunTimeout" +) + +func (t TaskRunReason) String() string { + return string(t) +} + +// GetStartedReason returns the reason set to the "Succeeded" condition when +// InitializeConditions is invoked +func (trs *TaskRunStatus) GetStartedReason() string { + return TaskRunReasonStarted.String() +} + +// GetRunningReason returns the reason set to the "Succeeded" condition when +// the RunsToCompletion starts running. This is used indicate that the resource +// could be validated is starting to perform its job. +func (trs *TaskRunStatus) GetRunningReason() string { + return TaskRunReasonRunning.String() +} + // MarkResourceNotConvertible adds a Warning-severity condition to the resource noting // that it cannot be converted to a higher version. func (trs *TaskRunStatus) MarkResourceNotConvertible(err *CannotConvertError) { @@ -116,11 +149,11 @@ func (trs *TaskRunStatus) MarkResourceNotConvertible(err *CannotConvertError) { // MarkResourceFailed sets the ConditionSucceeded condition to ConditionFalse // based on an error that occurred and a reason -func (trs *TaskRunStatus) MarkResourceFailed(reason string, err error) { +func (trs *TaskRunStatus) MarkResourceFailed(reason TaskRunReason, err error) { taskRunCondSet.Manage(trs).SetCondition(apis.Condition{ Type: apis.ConditionSucceeded, Status: corev1.ConditionFalse, - Reason: reason, + Reason: reason.String(), Message: err.Error(), }) } @@ -180,23 +213,13 @@ type TaskRunResult struct { Value string `json:"value"` } -// GetTypeMeta returns the task run type meta -func (tr *TaskRun) GetTypeMeta() *metav1.TypeMeta { - return &tr.TypeMeta -} - -// GetObjectMeta returns the task run type meta -func (tr *TaskRun) GetObjectMeta() *metav1.ObjectMeta { - return &tr.ObjectMeta -} - // GetOwnerReference gets the task run as owner reference for any related objects func (tr *TaskRun) GetOwnerReference() metav1.OwnerReference { return *metav1.NewControllerRef(tr, taskRunGroupVersionKind) } -// GetStatus returns the task run status as a RunsToCompletionStatus -func (tr *TaskRun) GetStatus() RunsToCompletionStatus { +// GetStatusCondition returns the task run status as a ConditionAccessor +func (tr *TaskRun) GetStatusCondition() apis.ConditionAccessor { return &tr.Status } @@ -208,10 +231,19 @@ func (trs *TaskRunStatus) GetCondition(t apis.ConditionType) *apis.Condition { // InitializeConditions will set all conditions in taskRunCondSet to unknown for the TaskRun // and set the started time to the current time func (trs *TaskRunStatus) InitializeConditions() { + started := false if trs.StartTime.IsZero() { trs.StartTime = &metav1.Time{Time: time.Now()} + started = true + } + conditionManager := taskRunCondSet.Manage(trs) + conditionManager.InitializeConditions() + // Ensure the started reason is set for the "Succeeded" condition + if started { + initialCondition := conditionManager.GetCondition(apis.ConditionSucceeded) + initialCondition.Reason = TaskRunReasonStarted.String() + conditionManager.SetCondition(*initialCondition) } - taskRunCondSet.Manage(trs).InitializeConditions() } // SetCondition sets the condition, unsetting previous conditions with the same diff --git a/pkg/apis/pipeline/v1beta1/taskrun_types_test.go b/pkg/apis/pipeline/v1beta1/taskrun_types_test.go index 5bc932521a8..7b8195b9328 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_types_test.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_types_test.go @@ -340,3 +340,37 @@ func TestHasTimedOut(t *testing.T) { }) } } + +func TestInitializeTaskRunConditions(t *testing.T) { + tr := &v1beta1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-name", + Namespace: "test-ns", + }, + } + tr.Status.InitializeConditions() + + if tr.Status.StartTime.IsZero() { + t.Fatalf("TaskRun StartTime not initialized correctly") + } + + condition := tr.Status.GetCondition(apis.ConditionSucceeded) + if condition.Reason != v1beta1.TaskRunReasonStarted.String() { + t.Fatalf("TaskRun initialize reason should be %s, got %s instead", v1beta1.TaskRunReasonStarted.String(), condition.Reason) + } + + // Change the reason before we initialize again + tr.Status.SetCondition(&apis.Condition{ + Type: apis.ConditionSucceeded, + Status: corev1.ConditionUnknown, + Reason: "not just started", + Message: "hello", + }) + + tr.Status.InitializeConditions() + + newCondition := tr.Status.GetCondition(apis.ConditionSucceeded) + if newCondition.Reason != "not just started" { + t.Fatalf("PipelineRun initialize reset the condition reason to %s", newCondition.Reason) + } +} diff --git a/pkg/pod/status.go b/pkg/pod/status.go index 92dc52cf9d4..55046be8bc5 100644 --- a/pkg/pod/status.go +++ b/pkg/pod/status.go @@ -45,13 +45,6 @@ const ( // that taskrun failed runtime validation ReasonFailedValidation = "TaskRunValidationFailed" - // ReasonRunning indicates that the reason for the inprogress status is that the TaskRun - // is just starting to be reconciled - ReasonRunning = "Running" - - // ReasonTimedOut indicates that the TaskRun has taken longer than its configured timeout - ReasonTimedOut = "TaskRunTimeout" - // ReasonExceededResourceQuota indicates that the TaskRun failed to create a pod due to // a ResourceQuota in the namespace ReasonExceededResourceQuota = "ExceededResourceQuota" @@ -68,12 +61,9 @@ const ( // is that the creation of the pod backing the TaskRun failed ReasonPodCreationFailed = "PodCreationFailed" - // ReasonSucceeded indicates that the reason for the finished status is that all of the steps - // completed successfully - ReasonSucceeded = "Succeeded" - - // ReasonFailed indicates that the reason for the failure status is unknown or that one of the steps failed - ReasonFailed = "Failed" + // ReasonPending indicates that the pod is in corev1.Pending, and the reason is not + // ReasonExceededNodeResources or IsPodHitConfigError + ReasonPending = "Pending" //timeFormat is RFC3339 with millisecond timeFormat = "2006-01-02T15:04:05.000Z07:00" @@ -114,7 +104,7 @@ func MakeTaskRunStatus(logger *zap.SugaredLogger, tr v1beta1.TaskRun, pod *corev trs.SetCondition(&apis.Condition{ Type: apis.ConditionSucceeded, Status: corev1.ConditionUnknown, - Reason: ReasonRunning, + Reason: v1beta1.TaskRunReasonRunning.String(), Message: "Not all Steps in the Task have finished executing", }) } @@ -197,14 +187,14 @@ func updateCompletedTaskRun(trs *v1beta1.TaskRunStatus, pod *corev1.Pod) { trs.SetCondition(&apis.Condition{ Type: apis.ConditionSucceeded, Status: corev1.ConditionFalse, - Reason: ReasonFailed, + Reason: v1beta1.TaskRunReasonFailed.String(), Message: msg, }) } else { trs.SetCondition(&apis.Condition{ Type: apis.ConditionSucceeded, Status: corev1.ConditionTrue, - Reason: ReasonSucceeded, + Reason: v1beta1.TaskRunReasonSuccessful.String(), Message: "All Steps have completed executing", }) } @@ -219,7 +209,7 @@ func updateIncompleteTaskRun(trs *v1beta1.TaskRunStatus, pod *corev1.Pod) { trs.SetCondition(&apis.Condition{ Type: apis.ConditionSucceeded, Status: corev1.ConditionUnknown, - Reason: ReasonRunning, + Reason: v1beta1.TaskRunReasonRunning.String(), Message: "Not all Steps in the Task have finished executing", }) case corev1.PodPending: @@ -232,7 +222,7 @@ func updateIncompleteTaskRun(trs *v1beta1.TaskRunStatus, pod *corev1.Pod) { reason = ReasonCreateContainerConfigError msg = getWaitingMessage(pod) default: - reason = "Pending" + reason = ReasonPending msg = getWaitingMessage(pod) } trs.SetCondition(&apis.Condition{ diff --git a/pkg/pod/status_test.go b/pkg/pod/status_test.go index bd9cd723886..a44e932bdfa 100644 --- a/pkg/pod/status_test.go +++ b/pkg/pod/status_test.go @@ -36,7 +36,7 @@ func TestMakeTaskRunStatus(t *testing.T) { conditionRunning := apis.Condition{ Type: apis.ConditionSucceeded, Status: corev1.ConditionUnknown, - Reason: ReasonRunning, + Reason: v1beta1.TaskRunReasonRunning.String(), Message: "Not all Steps in the Task have finished executing", } for _, c := range []struct { @@ -146,7 +146,7 @@ func TestMakeTaskRunStatus(t *testing.T) { Conditions: []apis.Condition{{ Type: apis.ConditionSucceeded, Status: corev1.ConditionTrue, - Reason: ReasonSucceeded, + Reason: v1beta1.TaskRunReasonSuccessful.String(), Message: "All Steps have completed executing", }}, }, @@ -214,7 +214,7 @@ func TestMakeTaskRunStatus(t *testing.T) { Conditions: []apis.Condition{{ Type: apis.ConditionSucceeded, Status: corev1.ConditionFalse, - Reason: ReasonFailed, + Reason: v1beta1.TaskRunReasonFailed.String(), Message: "\"step-failure\" exited with code 123 (image: \"image-id\"); for logs run: kubectl -n foo logs pod -c step-failure\n", }}, }, @@ -245,7 +245,7 @@ func TestMakeTaskRunStatus(t *testing.T) { Conditions: []apis.Condition{{ Type: apis.ConditionSucceeded, Status: corev1.ConditionFalse, - Reason: ReasonFailed, + Reason: v1beta1.TaskRunReasonFailed.String(), Message: "boom", }}, }, @@ -276,7 +276,7 @@ func TestMakeTaskRunStatus(t *testing.T) { Conditions: []apis.Condition{{ Type: apis.ConditionSucceeded, Status: corev1.ConditionFalse, - Reason: ReasonFailed, + Reason: v1beta1.TaskRunReasonFailed.String(), Message: "OOMKilled", }}, }, @@ -304,7 +304,7 @@ func TestMakeTaskRunStatus(t *testing.T) { Conditions: []apis.Condition{{ Type: apis.ConditionSucceeded, Status: corev1.ConditionFalse, - Reason: ReasonFailed, + Reason: v1beta1.TaskRunReasonFailed.String(), Message: "build failed for unspecified reasons.", }}, }, @@ -652,7 +652,7 @@ func TestMakeTaskRunStatus(t *testing.T) { Conditions: []apis.Condition{{ Type: apis.ConditionSucceeded, Status: corev1.ConditionFalse, - Reason: ReasonFailed, + Reason: v1beta1.TaskRunReasonFailed.String(), Message: "\"step-non-json\" exited with code 1 (image: \"image\"); for logs run: kubectl -n foo logs pod -c step-non-json\n", }}, }, diff --git a/pkg/reconciler/events/cloudevent/cloud_event_controller_test.go b/pkg/reconciler/events/cloudevent/cloud_event_controller_test.go index f48cf96d055..669d68e43f8 100644 --- a/pkg/reconciler/events/cloudevent/cloud_event_controller_test.go +++ b/pkg/reconciler/events/cloudevent/cloud_event_controller_test.go @@ -25,6 +25,8 @@ import ( resourcev1alpha1 "github.com/tektoncd/pipeline/pkg/apis/resource/v1alpha1" "github.com/tektoncd/pipeline/pkg/logging" "github.com/tektoncd/pipeline/test/diff" + corev1 "k8s.io/api/core/v1" + "knative.dev/pkg/apis" ) func TestCloudEventDeliveryFromTargets(t *testing.T) { @@ -88,6 +90,11 @@ func TestSendCloudEvents(t *testing.T) { tb.TaskRunTaskRef("fakeTaskName"), ), tb.TaskRunStatus( + tb.StatusCondition(apis.Condition{ + Type: apis.ConditionSucceeded, + Status: corev1.ConditionUnknown, + Reason: "somethingelse", + }), tb.TaskRunCloudEvent("http//notattemptedunknown", "", 0, v1beta1.CloudEventConditionUnknown), tb.TaskRunCloudEvent("http//notattemptedfailed", "somehow", 0, v1beta1.CloudEventConditionFailed), tb.TaskRunCloudEvent("http//notattemptedsucceeded", "", 0, v1beta1.CloudEventConditionSent), @@ -102,6 +109,11 @@ func TestSendCloudEvents(t *testing.T) { tb.TaskRunTaskRef("fakeTaskName"), ), tb.TaskRunStatus( + tb.StatusCondition(apis.Condition{ + Type: apis.ConditionSucceeded, + Status: corev1.ConditionUnknown, + Reason: "somethingelse", + }), tb.TaskRunCloudEvent("http//notattemptedunknown", "", 1, v1beta1.CloudEventConditionSent), tb.TaskRunCloudEvent("http//notattemptedfailed", "somehow", 0, v1beta1.CloudEventConditionFailed), tb.TaskRunCloudEvent("http//notattemptedsucceeded", "", 0, v1beta1.CloudEventConditionSent), @@ -145,6 +157,11 @@ func TestSendCloudEventsErrors(t *testing.T) { tb.TaskRunStatus( tb.TaskRunCloudEvent("http//sink1", "", 0, v1beta1.CloudEventConditionUnknown), tb.TaskRunCloudEvent("http//sink2", "", 0, v1beta1.CloudEventConditionUnknown), + tb.StatusCondition(apis.Condition{ + Type: apis.ConditionSucceeded, + Status: corev1.ConditionUnknown, + Reason: "somethingelse", + }), ), ), wantTaskRun: tb.TaskRun("test-taskrun-multiple-cloudeventdelivery", @@ -156,6 +173,11 @@ func TestSendCloudEventsErrors(t *testing.T) { // Error message is not checked in the Diff below tb.TaskRunCloudEvent("http//sink1", "", 1, v1beta1.CloudEventConditionFailed), tb.TaskRunCloudEvent("http//sink2", "", 1, v1beta1.CloudEventConditionFailed), + tb.StatusCondition(apis.Condition{ + Type: apis.ConditionSucceeded, + Status: corev1.ConditionUnknown, + Reason: "somethingelse", + }), ), ), }} diff --git a/pkg/reconciler/events/cloudevent/cloudevent.go b/pkg/reconciler/events/cloudevent/cloudevent.go index 780fce0879b..16634741907 100644 --- a/pkg/reconciler/events/cloudevent/cloudevent.go +++ b/pkg/reconciler/events/cloudevent/cloudevent.go @@ -35,32 +35,32 @@ import ( type TektonEventType string const ( - // TektonTaskRunStartedV1 is sent for TaskRuns with "ConditionSucceeded" "Unknown" + // TaskRunStartedEventV1 is sent for TaskRuns with "ConditionSucceeded" "Unknown" // the first time they are picked up by the reconciler - TektonTaskRunStartedV1 TektonEventType = "dev.tekton.event.taskrun.started.v1" - // TektonTaskRunRunningV1 is sent for TaskRuns with "ConditionSucceeded" "Unknown" + TaskRunStartedEventV1 TektonEventType = "dev.tekton.event.taskrun.started.v1" + // TaskRunRunningEventV1 is sent for TaskRuns with "ConditionSucceeded" "Unknown" // once the TaskRun is validated and Pod created - TektonTaskRunRunningV1 TektonEventType = "dev.tekton.event.taskrun.running.v1" - // TektonTaskRunUnknownV1 is sent for TaskRuns with "ConditionSucceeded" "Unknown" + TaskRunRunningEventV1 TektonEventType = "dev.tekton.event.taskrun.running.v1" + // TaskRunUnknownEventV1 is sent for TaskRuns with "ConditionSucceeded" "Unknown" // It can be used as a confirmation that the TaskRun is still running. - TektonTaskRunUnknownV1 TektonEventType = "dev.tekton.event.taskrun.unknown.v1" - // TektonTaskRunSuccessfulV1 is sent for TaskRuns with "ConditionSucceeded" "True" - TektonTaskRunSuccessfulV1 TektonEventType = "dev.tekton.event.taskrun.successful.v1" - // TektonTaskRunFailedV1 is sent for TaskRuns with "ConditionSucceeded" "False" - TektonTaskRunFailedV1 TektonEventType = "dev.tekton.event.taskrun.failed.v1" - // TektonPipelineRunStartedV1 is sent for PipelineRuns with "ConditionSucceeded" "Unknown" + TaskRunUnknownEventV1 TektonEventType = "dev.tekton.event.taskrun.unknown.v1" + // TaskRunSuccessfulEventV1 is sent for TaskRuns with "ConditionSucceeded" "True" + TaskRunSuccessfulEventV1 TektonEventType = "dev.tekton.event.taskrun.successful.v1" + // TaskRunFailedEventV1 is sent for TaskRuns with "ConditionSucceeded" "False" + TaskRunFailedEventV1 TektonEventType = "dev.tekton.event.taskrun.failed.v1" + // PipelineRunStartedEventV1 is sent for PipelineRuns with "ConditionSucceeded" "Unknown" // the first time they are picked up by the reconciler - TektonPipelineRunStartedV1 TektonEventType = "dev.tekton.event.pipelinerun.started.v1" - // TektonPipelineRunRunningV1 is sent for PipelineRuns with "ConditionSucceeded" "Unknown" + PipelineRunStartedEventV1 TektonEventType = "dev.tekton.event.pipelinerun.started.v1" + // PipelineRunRunningEventV1 is sent for PipelineRuns with "ConditionSucceeded" "Unknown" // once the PipelineRun is validated and Pod created - TektonPipelineRunRunningV1 TektonEventType = "dev.tekton.event.pipelinerun.running.v1" - // TektonPipelineRunUnknownV1 is sent for PipelineRuns with "ConditionSucceeded" "Unknown" + PipelineRunRunningEventV1 TektonEventType = "dev.tekton.event.pipelinerun.running.v1" + // PipelineRunUnknownEventV1 is sent for PipelineRuns with "ConditionSucceeded" "Unknown" // It can be used as a confirmation that the PipelineRun is still running. - TektonPipelineRunUnknownV1 TektonEventType = "dev.tekton.event.pipelinerun.unknown.v1" - // TektonPipelineRunSuccessfulV1 is sent for PipelineRuns with "ConditionSucceeded" "True" - TektonPipelineRunSuccessfulV1 TektonEventType = "dev.tekton.event.pipelinerun.successful.v1" - // TektonPipelineRunFailedV1 is sent for PipelineRuns with "ConditionSucceeded" "False" - TektonPipelineRunFailedV1 TektonEventType = "dev.tekton.event.pipelinerun.failed.v1" + PipelineRunUnknownEventV1 TektonEventType = "dev.tekton.event.pipelinerun.unknown.v1" + // PipelineRunSuccessfulEventV1 is sent for PipelineRuns with "ConditionSucceeded" "True" + PipelineRunSuccessfulEventV1 TektonEventType = "dev.tekton.event.pipelinerun.successful.v1" + // PipelineRunFailedEventV1 is sent for PipelineRuns with "ConditionSucceeded" "False" + PipelineRunFailedEventV1 TektonEventType = "dev.tekton.event.pipelinerun.failed.v1" ) func (t TektonEventType) String() string { @@ -78,28 +78,31 @@ type TektonCloudEventData struct { } // NewTektonCloudEventData returns a new instance of NewTektonCloudEventData -func NewTektonCloudEventData(runObject v1beta1.RunsToCompletion) TektonCloudEventData { +func NewTektonCloudEventData(runObject objectWithCondition) TektonCloudEventData { tektonCloudEventData := TektonCloudEventData{} - switch runObject.(type) { + switch v := runObject.(type) { case *v1beta1.TaskRun: - tektonCloudEventData.TaskRun = runObject.(*v1beta1.TaskRun) + tektonCloudEventData.TaskRun = v case *v1beta1.PipelineRun: - tektonCloudEventData.PipelineRun = runObject.(*v1beta1.PipelineRun) + tektonCloudEventData.PipelineRun = v } return tektonCloudEventData } -// EventForRunsToCompletion creates a new event based for a RunsToCompletion, +// EventForObjectWithCondition creates a new event based for a objectWithCondition, // or return an error if not possible. -func EventForRunsToCompletion(runObject v1beta1.RunsToCompletion) (*cloudevents.Event, error) { +func EventForObjectWithCondition(runObject objectWithCondition) (*cloudevents.Event, error) { event := cloudevents.NewEvent() event.SetID(uuid.New().String()) - event.SetSubject(runObject.GetObjectMeta().Name) - event.SetSource(runObject.GetObjectMeta().SelfLink) // TODO: SelfLink is deprecated https://github.com/tektoncd/pipeline/issues/2676 + event.SetSubject(runObject.GetObjectMeta().GetName()) + event.SetSource(runObject.GetObjectMeta().GetSelfLink()) // TODO: SelfLink is deprecated https://github.com/tektoncd/pipeline/issues/2676 eventType, err := getEventType(runObject) if err != nil { return nil, err } + if eventType == nil { + return nil, errors.New("No matching event type found") + } event.SetType(eventType.String()) if err := event.SetData(cloudevents.ApplicationJSON, NewTektonCloudEventData(runObject)); err != nil { @@ -115,7 +118,7 @@ func EventForTaskRun(taskRun *v1beta1.TaskRun) (*cloudevents.Event, error) { if taskRun == nil { return nil, errors.New("Cannot send an event for an empty TaskRun") } - return EventForRunsToCompletion(taskRun) + return EventForObjectWithCondition(taskRun) } // EventForPipelineRun will create a new event based on a TaskRun, @@ -125,40 +128,50 @@ func EventForPipelineRun(pipelineRun *v1beta1.PipelineRun) (*cloudevents.Event, if pipelineRun == nil { return nil, errors.New("Cannot send an event for an empty PipelineRun") } - return EventForRunsToCompletion(pipelineRun) + return EventForObjectWithCondition(pipelineRun) } -func getEventType(runObject v1beta1.RunsToCompletion) (*TektonEventType, error) { - c := runObject.GetStatus().GetCondition(apis.ConditionSucceeded) - t := runObject.GetTypeMeta() +func getEventType(runObject objectWithCondition) (*TektonEventType, error) { + c := runObject.GetStatusCondition().GetCondition(apis.ConditionSucceeded) var eventType TektonEventType switch { case c.IsUnknown(): - // TBD We should have different event types here, e.g. started, running - // That requires having either knowledge about the previous condition or - // TaskRun and PipelineRun using dedicated "Reasons" or "Conditions" - switch t.Kind { - case "TaskRun": - eventType = TektonTaskRunUnknownV1 - case "PipelineRun": - eventType = TektonPipelineRunUnknownV1 + switch runObject.(type) { + case *v1beta1.TaskRun: + switch c.Reason { + case v1beta1.TaskRunReasonStarted.String(): + eventType = TaskRunStartedEventV1 + case v1beta1.TaskRunReasonRunning.String(): + eventType = TaskRunRunningEventV1 + default: + eventType = TaskRunUnknownEventV1 + } + case *v1beta1.PipelineRun: + switch c.Reason { + case v1beta1.PipelineRunReasonStarted.String(): + eventType = PipelineRunStartedEventV1 + case v1beta1.PipelineRunReasonRunning.String(): + eventType = PipelineRunRunningEventV1 + default: + eventType = PipelineRunUnknownEventV1 + } } case c.IsFalse(): - switch t.Kind { - case "TaskRun": - eventType = TektonTaskRunFailedV1 - case "PipelineRun": - eventType = TektonPipelineRunFailedV1 + switch runObject.(type) { + case *v1beta1.TaskRun: + eventType = TaskRunFailedEventV1 + case *v1beta1.PipelineRun: + eventType = PipelineRunFailedEventV1 } case c.IsTrue(): - switch t.Kind { - case "TaskRun": - eventType = TektonTaskRunSuccessfulV1 - case "PipelineRun": - eventType = TektonPipelineRunSuccessfulV1 + switch runObject.(type) { + case *v1beta1.TaskRun: + eventType = TaskRunSuccessfulEventV1 + case *v1beta1.PipelineRun: + eventType = PipelineRunSuccessfulEventV1 } default: - return nil, fmt.Errorf("unknown condition for in TaskRun.Status %s", c.Status) + return nil, fmt.Errorf("unknown condition for in %T.Status %s", runObject, c.Status) } return &eventType, nil } diff --git a/pkg/reconciler/events/cloudevent/cloudevent_test.go b/pkg/reconciler/events/cloudevent/cloudevent_test.go index ddeff54c82c..958e9b20469 100644 --- a/pkg/reconciler/events/cloudevent/cloudevent_test.go +++ b/pkg/reconciler/events/cloudevent/cloudevent_test.go @@ -35,7 +35,7 @@ const ( pipelineRunName = "fakepipelinerunname" ) -func getTaskRunByCondition(status corev1.ConditionStatus) *v1beta1.TaskRun { +func getTaskRunByCondition(status corev1.ConditionStatus, reason string) *v1beta1.TaskRun { return &v1beta1.TaskRun{ TypeMeta: metav1.TypeMeta{ Kind: "TaskRun", @@ -52,13 +52,14 @@ func getTaskRunByCondition(status corev1.ConditionStatus) *v1beta1.TaskRun { Conditions: []apis.Condition{{ Type: apis.ConditionSucceeded, Status: status, + Reason: reason, }}, }, }, } } -func getPipelineRunByCondition(status corev1.ConditionStatus) *v1beta1.PipelineRun { +func getPipelineRunByCondition(status corev1.ConditionStatus, reason string) *v1beta1.PipelineRun { return &v1beta1.PipelineRun{ TypeMeta: metav1.TypeMeta{ Kind: "PipelineRun", @@ -75,6 +76,7 @@ func getPipelineRunByCondition(status corev1.ConditionStatus) *v1beta1.PipelineR Conditions: []apis.Condition{{ Type: apis.ConditionSucceeded, Status: status, + Reason: reason, }}, }, }, @@ -87,17 +89,25 @@ func TestEventForTaskRun(t *testing.T) { taskRun *v1beta1.TaskRun wantEventType TektonEventType }{{ - desc: "send a cloud event with unknown status taskrun", - taskRun: getTaskRunByCondition(corev1.ConditionUnknown), - wantEventType: TektonTaskRunUnknownV1, + desc: "send a cloud event when a taskrun starts", + taskRun: getTaskRunByCondition(corev1.ConditionUnknown, v1beta1.TaskRunReasonStarted.String()), + wantEventType: TaskRunStartedEventV1, }, { - desc: "send a cloud event with successful status taskrun", - taskRun: getTaskRunByCondition(corev1.ConditionTrue), - wantEventType: TektonTaskRunSuccessfulV1, + desc: "send a cloud event when a taskrun starts running", + taskRun: getTaskRunByCondition(corev1.ConditionUnknown, v1beta1.TaskRunReasonRunning.String()), + wantEventType: TaskRunRunningEventV1, }, { desc: "send a cloud event with unknown status taskrun", - taskRun: getTaskRunByCondition(corev1.ConditionFalse), - wantEventType: TektonTaskRunFailedV1, + taskRun: getTaskRunByCondition(corev1.ConditionUnknown, "doesn't matter"), + wantEventType: TaskRunUnknownEventV1, + }, { + desc: "send a cloud event with failed status taskrun", + taskRun: getTaskRunByCondition(corev1.ConditionFalse, "meh"), + wantEventType: TaskRunFailedEventV1, + }, { + desc: "send a cloud event with successful status taskrun", + taskRun: getTaskRunByCondition(corev1.ConditionTrue, "yay"), + wantEventType: TaskRunSuccessfulEventV1, }} { t.Run(c.desc, func(t *testing.T) { names.TestingSeed() @@ -136,17 +146,25 @@ func TestEventForPipelineRun(t *testing.T) { pipelineRun *v1beta1.PipelineRun wantEventType TektonEventType }{{ - desc: "send a cloud event with unknown status taskrun", - pipelineRun: getPipelineRunByCondition(corev1.ConditionUnknown), - wantEventType: TektonPipelineRunUnknownV1, + desc: "send a cloud event with unknown status pipelinerun, just started", + pipelineRun: getPipelineRunByCondition(corev1.ConditionUnknown, v1beta1.PipelineRunReasonStarted.String()), + wantEventType: PipelineRunStartedEventV1, }, { - desc: "send a cloud event with successful status taskrun", - pipelineRun: getPipelineRunByCondition(corev1.ConditionTrue), - wantEventType: TektonPipelineRunSuccessfulV1, + desc: "send a cloud event with unknown status pipelinerun, just started running", + pipelineRun: getPipelineRunByCondition(corev1.ConditionUnknown, v1beta1.PipelineRunReasonRunning.String()), + wantEventType: PipelineRunRunningEventV1, }, { - desc: "send a cloud event with unknown status taskrun", - pipelineRun: getPipelineRunByCondition(corev1.ConditionFalse), - wantEventType: TektonPipelineRunFailedV1, + desc: "send a cloud event with unknown status pipelinerun", + pipelineRun: getPipelineRunByCondition(corev1.ConditionUnknown, "doesn't matter"), + wantEventType: PipelineRunUnknownEventV1, + }, { + desc: "send a cloud event with successful status pipelinerun", + pipelineRun: getPipelineRunByCondition(corev1.ConditionTrue, "yay"), + wantEventType: PipelineRunSuccessfulEventV1, + }, { + desc: "send a cloud event with unknown status pipelinerun", + pipelineRun: getPipelineRunByCondition(corev1.ConditionFalse, "meh"), + wantEventType: PipelineRunFailedEventV1, }} { t.Run(c.desc, func(t *testing.T) { names.TestingSeed() diff --git a/pkg/apis/pipeline/v1beta1/run_interface.go b/pkg/reconciler/events/cloudevent/interface.go similarity index 52% rename from pkg/apis/pipeline/v1beta1/run_interface.go rename to pkg/reconciler/events/cloudevent/interface.go index ddaf2ffb979..de168fd9dbb 100644 --- a/pkg/apis/pipeline/v1beta1/run_interface.go +++ b/pkg/reconciler/events/cloudevent/interface.go @@ -14,29 +14,19 @@ See the License for the specific language governing permissions and limitations under the License. */ -package v1beta1 +package cloudevent import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "knative.dev/pkg/apis" ) -// RunsToCompletionStatus is implemented by TaskRun.Status and PipelineRun.Status -type RunsToCompletionStatus interface { - GetCondition(t apis.ConditionType) *apis.Condition - InitializeConditions() - SetCondition(newCond *apis.Condition) -} +// objectWithCondition is implemented by TaskRun and PipelineRun +type objectWithCondition interface { + + // ObjectMetaAccessor requires a GetObjectMeta that returns the ObjectMeta + metav1.ObjectMetaAccessor -// RunsToCompletion is implemented by TaskRun and PipelineRun -type RunsToCompletion interface { - GetTypeMeta() *metav1.TypeMeta - GetObjectMeta() *metav1.ObjectMeta - GetOwnerReference() metav1.OwnerReference - GetStatus() RunsToCompletionStatus - IsDone() bool - HasStarted() bool - IsCancelled() bool - HasTimedOut() bool - GetRunKey() string + // GetStatusCondition returns a ConditionAccessor for the status of the RunsToCompletion + GetStatusCondition() apis.ConditionAccessor } diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index 8d0998fa6aa..963469363d0 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -94,10 +94,6 @@ const ( // pipelineRunAgentName defines logging agent name for PipelineRun Controller pipelineRunAgentName = "pipeline-controller" - - // Event reasons - eventReasonFailed = "PipelineRunFailed" - eventReasonSucceeded = "PipelineRunSucceeded" ) type configStore interface { @@ -187,7 +183,7 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, pr *v1beta1.PipelineRun) default: if err := c.tracker.Track(pr.GetTaskRunRef(), pr); err != nil { c.Logger.Errorf("Failed to create tracker for TaskRuns for PipelineRun %s: %v", pr.Name, err) - c.Recorder.Event(pr, corev1.EventTypeWarning, eventReasonFailed, "Failed to create tracker for TaskRuns for PipelineRun") + c.Recorder.Event(pr, corev1.EventTypeWarning, v1beta1.PipelineRunReasonFailed.String(), "Failed to create tracker for TaskRuns for PipelineRun") return err } @@ -212,7 +208,7 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, pr *v1beta1.PipelineRun) if !reflect.DeepEqual(original.ObjectMeta.Labels, pr.ObjectMeta.Labels) || !reflect.DeepEqual(original.ObjectMeta.Annotations, pr.ObjectMeta.Annotations) { if _, err := c.updateLabelsAndAnnotations(pr); err != nil { c.Logger.Warn("Failed to update PipelineRun labels/annotations", zap.Error(err)) - c.Recorder.Event(pr, corev1.EventTypeWarning, eventReasonFailed, "PipelineRun failed to update labels/annotations") + c.Recorder.Event(pr, corev1.EventTypeWarning, "Error", "PipelineRun failed to update labels/annotations") return multierror.Append(merr, err) } } @@ -388,7 +384,7 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun) err if pipelineState.IsDone() && pr.IsDone() { c.timeoutHandler.Release(pr) - c.Recorder.Event(pr, corev1.EventTypeNormal, eventReasonSucceeded, "PipelineRun completed successfully.") + c.Recorder.Event(pr, corev1.EventTypeNormal, v1beta1.PipelineRunReasonSuccessful.String(), "PipelineRun completed successfully.") return nil } diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index 5949f54b935..42d7f7e65c4 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -279,8 +279,8 @@ func TestReconcile(t *testing.T) { if condition == nil || condition.Status != corev1.ConditionUnknown { t.Errorf("Expected PipelineRun status to be in progress, but was %v", condition) } - if condition != nil && condition.Reason != resources.ReasonRunning { - t.Errorf("Expected reason %q but was %s", resources.ReasonRunning, condition.Reason) + if condition != nil && condition.Reason != v1beta1.PipelineRunReasonRunning.String() { + t.Errorf("Expected reason %q but was %s", v1beta1.PipelineRunReasonRunning.String(), condition.Reason) } if len(reconciledRun.Status.TaskRuns) != 2 { @@ -786,7 +786,7 @@ func TestReconcileOnCompletedPipelineRun(t *testing.T) { tb.PipelineRunStatus(tb.PipelineRunStatusCondition(apis.Condition{ Type: apis.ConditionSucceeded, Status: corev1.ConditionTrue, - Reason: resources.ReasonSucceeded, + Reason: v1beta1.PipelineRunReasonSuccessful.String(), Message: "All Tasks have completed executing", }), tb.PipelineRunTaskRunsStatus(taskRunName, &v1beta1.PipelineRunTaskRunStatus{ @@ -971,7 +971,7 @@ func TestReconcileWithTimeout(t *testing.T) { } // The PipelineRun should be timed out. - if reconciledRun.Status.GetCondition(apis.ConditionSucceeded).Reason != resources.ReasonTimedOut { + if reconciledRun.Status.GetCondition(apis.ConditionSucceeded).Reason != "PipelineRunTimeout" { t.Errorf("Expected PipelineRun to be timed out, but condition reason is %s", reconciledRun.Status.GetCondition(apis.ConditionSucceeded)) } @@ -1733,7 +1733,7 @@ func TestReconcileWithFailingConditionChecks(t *testing.T) { tb.PipelineRunStatus(tb.PipelineRunStatusCondition(apis.Condition{ Type: apis.ConditionSucceeded, Status: corev1.ConditionUnknown, - Reason: resources.ReasonRunning, + Reason: v1beta1.PipelineRunReasonRunning.String(), Message: "Not all Tasks in the Pipeline have finished executing", }), tb.PipelineRunTaskRunsStatus(pipelineRunName+"task-1", &v1beta1.PipelineRunTaskRunStatus{ PipelineTaskName: "task-1", @@ -2394,7 +2394,7 @@ func TestReconcileWithPipelineResults(t *testing.T) { tb.PipelineRunStatusCondition(apis.Condition{ Type: apis.ConditionSucceeded, Status: corev1.ConditionTrue, - Reason: resources.ReasonSucceeded, + Reason: v1beta1.PipelineRunReasonSuccessful.String(), Message: "All Tasks have completed executing", }), tb.PipelineRunTaskRunsStatus(trs[0].Name, &v1beta1.PipelineRunTaskRunStatus{ diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go index 20b0609b597..77c046f81dc 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go @@ -37,28 +37,6 @@ import ( ) const ( - // ReasonRunning indicates that the reason for the inprogress status is that the TaskRun - // is just starting to be reconciled - ReasonRunning = "Running" - - // ReasonFailed indicates that the reason for the failure status is that one of the TaskRuns failed - ReasonFailed = "Failed" - - // ReasonCancelled indicates that the reason for the cancelled status is that one of the TaskRuns cancelled - ReasonCancelled = "Cancelled" - - // ReasonSucceeded indicates that the reason for the finished status is that all of the TaskRuns - // completed successfully - ReasonSucceeded = "Succeeded" - - // ReasonCompleted indicates that the reason for the finished status is that all of the TaskRuns - // completed successfully but with some conditions checking failed - ReasonCompleted = "Completed" - - // ReasonTimedOut indicates that the PipelineRun has taken longer than its configured - // timeout - ReasonTimedOut = "PipelineRunTimeout" - // ReasonConditionCheckFailed indicates that the reason for the failure status is that the // condition check associated to the pipeline task evaluated to false ReasonConditionCheckFailed = "ConditionCheckFailed" @@ -74,6 +52,7 @@ func (e *TaskNotFoundError) Error() string { return fmt.Sprintf("Couldn't retrieve Task %q: %s", e.Name, e.Msg) } +// ConditionNotFoundError is used to track failures to the type ConditionNotFoundError struct { Name string Msg string @@ -145,7 +124,7 @@ func (t ResolvedPipelineRunTask) IsCancelled() bool { return false } - return c.IsFalse() && c.Reason == v1beta1.TaskRunSpecStatusCancelled + return c.IsFalse() && c.Reason == v1beta1.TaskRunReasonCancelled.String() } // ToMap returns a map that maps pipeline task name to the resolved pipeline run task @@ -192,7 +171,7 @@ func (state PipelineRunState) GetNextTasks(candidateTasks map[string]struct{}) [ if _, ok := candidateTasks[t.PipelineTask.Name]; ok && t.TaskRun != nil { status := t.TaskRun.Status.GetCondition(apis.ConditionSucceeded) if status != nil && status.IsFalse() { - if !(t.TaskRun.IsCancelled() || status.Reason == v1beta1.TaskRunSpecStatusCancelled || status.Reason == ReasonConditionCheckFailed) { + if !(t.TaskRun.IsCancelled() || status.Reason == v1beta1.TaskRunReasonCancelled.String() || status.Reason == ReasonConditionCheckFailed) { if len(t.TaskRun.Status.RetriesStatus) < t.PipelineTask.Retries { tasks = append(tasks, t) } @@ -417,7 +396,7 @@ func GetPipelineConditionStatus(pr *v1beta1.PipelineRun, state PipelineRunState, return &apis.Condition{ Type: apis.ConditionSucceeded, Status: corev1.ConditionFalse, - Reason: ReasonTimedOut, + Reason: v1beta1.PipelineRunReasonTimedOut.String(), Message: fmt.Sprintf("PipelineRun %q failed to finish within %q", pr.Name, pr.Spec.Timeout.Duration.String()), } } @@ -429,7 +408,7 @@ func GetPipelineConditionStatus(pr *v1beta1.PipelineRun, state PipelineRunState, return &apis.Condition{ Type: apis.ConditionSucceeded, Status: corev1.ConditionFalse, - Reason: ReasonCancelled, + Reason: v1beta1.PipelineRunReasonCancelled.String(), Message: fmt.Sprintf("TaskRun %s has cancelled", rprt.TaskRun.Name), } } @@ -439,7 +418,7 @@ func GetPipelineConditionStatus(pr *v1beta1.PipelineRun, state PipelineRunState, return &apis.Condition{ Type: apis.ConditionSucceeded, Status: corev1.ConditionFalse, - Reason: ReasonFailed, + Reason: v1beta1.PipelineRunReasonFailed.String(), Message: fmt.Sprintf("TaskRun %s has failed", rprt.TaskRun.Name), } } @@ -463,9 +442,9 @@ func GetPipelineConditionStatus(pr *v1beta1.PipelineRun, state PipelineRunState, if reflect.DeepEqual(allTasks, successOrSkipTasks) { logger.Infof("All TaskRuns have finished for PipelineRun %s so it has finished", pr.Name) - reason := ReasonSucceeded + reason := v1beta1.PipelineRunReasonSuccessful.String() if skipTasks != 0 { - reason = ReasonCompleted + reason = v1beta1.PipelineRunReasonCompleted.String() } return &apis.Condition{ @@ -481,7 +460,7 @@ func GetPipelineConditionStatus(pr *v1beta1.PipelineRun, state PipelineRunState, return &apis.Condition{ Type: apis.ConditionSucceeded, Status: corev1.ConditionUnknown, - Reason: ReasonRunning, + Reason: v1beta1.PipelineRunReasonRunning.String(), Message: fmt.Sprintf("Tasks Completed: %d, Incomplete: %d, Skipped: %d", len(successOrSkipTasks)-skipTasks, len(allTasks)-len(successOrSkipTasks), skipTasks), } } diff --git a/pkg/reconciler/reconciler_test.go b/pkg/reconciler/reconciler_test.go index 8367d577852..d5e4049e13c 100644 --- a/pkg/reconciler/reconciler_test.go +++ b/pkg/reconciler/reconciler_test.go @@ -25,7 +25,6 @@ import ( tb "github.com/tektoncd/pipeline/internal/builder/v1beta1" "github.com/tektoncd/pipeline/pkg/apis/pipeline" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" - "github.com/tektoncd/pipeline/pkg/reconciler/pipelinerun/resources" ttesting "github.com/tektoncd/pipeline/pkg/reconciler/testing" test "github.com/tektoncd/pipeline/test" "go.uber.org/zap" @@ -43,7 +42,7 @@ func TestRecorderOptions(t *testing.T) { tb.PipelineRunStatus(tb.PipelineRunStatusCondition(apis.Condition{ Type: apis.ConditionSucceeded, Status: corev1.ConditionTrue, - Reason: resources.ReasonSucceeded, + Reason: v1beta1.PipelineRunReasonSuccessful.String(), Message: "All Tasks have completed executing", })), )} diff --git a/pkg/reconciler/taskrun/taskrun.go b/pkg/reconciler/taskrun/taskrun.go index ca91020bceb..e1fa4c79472 100644 --- a/pkg/reconciler/taskrun/taskrun.go +++ b/pkg/reconciler/taskrun/taskrun.go @@ -192,7 +192,7 @@ func (c *Reconciler) Reconcile(ctx context.Context, key string) error { if tr.HasTimedOut() { before := tr.Status.GetCondition(apis.ConditionSucceeded) message := fmt.Sprintf("TaskRun %q failed to finish within %q", tr.Name, tr.GetTimeout()) - err := c.failTaskRun(tr, podconvert.ReasonTimedOut, message) + err := c.failTaskRun(tr, v1beta1.TaskRunReasonTimedOut, message) return c.finishReconcileUpdateEmitEvents(tr, original, before, err) } @@ -513,9 +513,9 @@ func (c *Reconciler) handlePodCreationError(tr *v1beta1.TaskRun, err error) { // If a pod is associated to the TaskRun, it stops it // failTaskRun function may return an error in case the pod could not be deleted // failTaskRun may update the local TaskRun status, but it won't push the updates to etcd -func (c *Reconciler) failTaskRun(tr *v1beta1.TaskRun, reason, message string) error { +func (c *Reconciler) failTaskRun(tr *v1beta1.TaskRun, reason v1beta1.TaskRunReason, message string) error { - c.Logger.Warn("stopping task run %q because of %q", tr.Name, reason) + c.Logger.Warn("stopping task run %q because of %q", tr.Name, reason.String()) tr.Status.MarkResourceFailed(reason, errors.New(message)) // update tr completed time diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go index 6319f8389c3..662758f68a1 100644 --- a/pkg/reconciler/taskrun/taskrun_test.go +++ b/pkg/reconciler/taskrun/taskrun_test.go @@ -495,8 +495,8 @@ func TestReconcile_ExplicitDefaultSA(t *testing.T) { if condition == nil || condition.Status != corev1.ConditionUnknown { t.Errorf("Expected invalid TaskRun to have in progress status, but had %v", condition) } - if condition != nil && condition.Reason != podconvert.ReasonRunning { - t.Errorf("Expected reason %q but was %s", podconvert.ReasonRunning, condition.Reason) + if condition != nil && condition.Reason != v1beta1.TaskRunReasonRunning.String() { + t.Errorf("Expected reason %q but was %s", v1beta1.TaskRunReasonRunning.String(), condition.Reason) } if tr.Status.PodName == "" { @@ -1303,8 +1303,8 @@ func TestReconcile(t *testing.T) { if condition == nil || condition.Status != corev1.ConditionUnknown { t.Errorf("Expected invalid TaskRun to have in progress status, but had %v", condition) } - if condition != nil && condition.Reason != podconvert.ReasonRunning { - t.Errorf("Expected reason %q but was %s", podconvert.ReasonRunning, condition.Reason) + if condition != nil && condition.Reason != v1beta1.TaskRunReasonRunning.String() { + t.Errorf("Expected reason %q but was %s", v1beta1.TaskRunReasonRunning.String(), condition.Reason) } if tr.Status.PodName == "" { @@ -1637,7 +1637,7 @@ func TestReconcilePodUpdateStatus(t *testing.T) { if d := cmp.Diff(&apis.Condition{ Type: apis.ConditionSucceeded, Status: corev1.ConditionUnknown, - Reason: "Running", + Reason: v1beta1.TaskRunReasonRunning.String(), Message: "Not all Steps in the Task have finished executing", }, newTr.Status.GetCondition(apis.ConditionSucceeded), ignoreLastTransitionTime); d != "" { t.Fatalf("Did not get expected condition %s", diff.PrintWantGot(d)) @@ -1666,7 +1666,7 @@ func TestReconcilePodUpdateStatus(t *testing.T) { if d := cmp.Diff(&apis.Condition{ Type: apis.ConditionSucceeded, Status: corev1.ConditionTrue, - Reason: podconvert.ReasonSucceeded, + Reason: v1beta1.TaskRunReasonSuccessful.String(), Message: "All Steps have completed executing", }, newTr.Status.GetCondition(apis.ConditionSucceeded), ignoreLastTransitionTime); d != "" { t.Errorf("Did not get expected condition %s", diff.PrintWantGot(d)) @@ -2685,7 +2685,7 @@ func TestFailTaskRun(t *testing.T) { name string taskRun *v1beta1.TaskRun pod *corev1.Pod - reason string + reason v1beta1.TaskRunReason message string expectedStatus apis.Condition }{{ diff --git a/test/timeout_test.go b/test/timeout_test.go index a9997e564fb..266f2700931 100644 --- a/test/timeout_test.go +++ b/test/timeout_test.go @@ -25,7 +25,6 @@ import ( "time" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" - "github.com/tektoncd/pipeline/pkg/reconciler/pipelinerun/resources" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "knative.dev/pkg/apis" @@ -112,7 +111,7 @@ func TestPipelineRunTimeout(t *testing.T) { } t.Logf("Waiting for PipelineRun %s in namespace %s to be timed out", pipelineRun.Name, namespace) - if err := WaitForPipelineRunState(c, pipelineRun.Name, timeout, FailedWithReason(resources.ReasonTimedOut, pipelineRun.Name), "PipelineRunTimedOut"); err != nil { + if err := WaitForPipelineRunState(c, pipelineRun.Name, timeout, FailedWithReason(v1beta1.PipelineRunReasonTimedOut.String(), pipelineRun.Name), "PipelineRunTimedOut"); err != nil { t.Errorf("Error waiting for PipelineRun %s to finish: %s", pipelineRun.Name, err) } @@ -301,7 +300,7 @@ func TestPipelineTaskTimeout(t *testing.T) { } t.Logf("Waiting for PipelineRun %s with PipelineTask timeout in namespace %s to fail", pipelineRun.Name, namespace) - if err := WaitForPipelineRunState(c, pipelineRun.Name, timeout, FailedWithReason(resources.ReasonFailed, pipelineRun.Name), "PipelineRunTimedOut"); err != nil { + if err := WaitForPipelineRunState(c, pipelineRun.Name, timeout, FailedWithReason(v1beta1.PipelineRunReasonFailed.String(), pipelineRun.Name), "PipelineRunTimedOut"); err != nil { t.Fatalf("Error waiting for PipelineRun %s to finish: %s", pipelineRun.Name, err) } diff --git a/test/v1alpha1/cancel_test.go b/test/v1alpha1/cancel_test.go index 7c5ff8edf85..3c4cddc255d 100644 --- a/test/v1alpha1/cancel_test.go +++ b/test/v1alpha1/cancel_test.go @@ -19,6 +19,7 @@ limitations under the License. package test import ( + "fmt" "sync" "testing" @@ -141,6 +142,7 @@ func TestTaskRunPipelineRunCancel(t *testing.T) { wg.Wait() matchKinds := map[string][]string{"PipelineRun": {"pear"}, "TaskRun": trName} + // Expected failure events: 1 for the pipelinerun cancel, 1 for each TaskRun expectedNumberOfEvents := 1 + len(trName) t.Logf("Making sure %d events were created from pipelinerun with kinds %v", expectedNumberOfEvents, matchKinds) events, err := collectMatchingEvents(c.KubeClient, namespace, matchKinds, "Failed") @@ -148,7 +150,14 @@ func TestTaskRunPipelineRunCancel(t *testing.T) { t.Fatalf("Failed to collect matching events: %q", err) } if len(events) != expectedNumberOfEvents { - t.Fatalf("Expected %d number of successful events from pipelinerun and taskrun but got %d; list of receieved events : %#v", expectedNumberOfEvents, len(events), events) + collectedEvents := "" + for i, event := range events { + collectedEvents += fmt.Sprintf("%#v", event) + if i < (len(events) - 1) { + collectedEvents += ", " + } + } + t.Fatalf("Expected %d number of successful events from pipelinerun and taskrun but got %d; list of received events : %#v", expectedNumberOfEvents, len(events), collectedEvents) } }) } diff --git a/test/v1alpha1/timeout_test.go b/test/v1alpha1/timeout_test.go index fd146400e78..d9ac9a15236 100644 --- a/test/v1alpha1/timeout_test.go +++ b/test/v1alpha1/timeout_test.go @@ -26,7 +26,6 @@ import ( tb "github.com/tektoncd/pipeline/internal/builder/v1alpha1" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" - "github.com/tektoncd/pipeline/pkg/reconciler/pipelinerun/resources" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "knative.dev/pkg/apis" @@ -95,7 +94,7 @@ func TestPipelineRunTimeout(t *testing.T) { } t.Logf("Waiting for PipelineRun %s in namespace %s to be timed out", pipelineRun.Name, namespace) - if err := WaitForPipelineRunState(c, pipelineRun.Name, timeout, FailedWithReason(resources.ReasonTimedOut, pipelineRun.Name), "PipelineRunTimedOut"); err != nil { + if err := WaitForPipelineRunState(c, pipelineRun.Name, timeout, FailedWithReason("PipelineRunTimeout", pipelineRun.Name), "PipelineRunTimedOut"); err != nil { t.Errorf("Error waiting for PipelineRun %s to finish: %s", pipelineRun.Name, err) } @@ -230,7 +229,7 @@ func TestPipelineTaskTimeout(t *testing.T) { } t.Logf("Waiting for PipelineRun %s with PipelineTask timeout in namespace %s to fail", pipelineRun.Name, namespace) - if err := WaitForPipelineRunState(c, pipelineRun.Name, timeout, FailedWithReason(resources.ReasonFailed, pipelineRun.Name), "PipelineRunTimedOut"); err != nil { + if err := WaitForPipelineRunState(c, pipelineRun.Name, timeout, FailedWithReason("Failed", pipelineRun.Name), "PipelineRunTimedOut"); err != nil { t.Errorf("Error waiting for PipelineRun %s to finish: %s", pipelineRun.Name, err) }