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
- Add helpers to the RunsToCompletion to distinguish the
  different conditions from a combination of Status and Reason

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.

TBD:
- Add documentation on various reason
- Cleanup reasons from the pipelineresolution.go and sync them
  with the pipelinerun_types.go
- Decide if we need the started reason, and sync code accordingly
- Remove GetStartedReason and GetRunningReason from run_interface
- Add the missing helpers to RunsToCompletion for the various
  status
- Use the new helpers where needed in existing code
- Use the new helpers in the events module to decide on the event
  types to be sent for k8s and cloudevents
- Add documentation

pkg/apis/pipeline/v1beta1/pipelinerun_types.go

@@ -220,6 +220,38 @@ type PipelineRunStatus struct {
 	PipelineRunStatusFields `json:",inline"`
 }
 
+type PipelineRunSucceededReason string
+
+const (
+	// PipelineRunSucceededStartedReason is the reason set when the PipelineRun has just started
+	PipelineRunSucceededStartedReason PipelineRunSucceededReason = "Started"
+	// PipelineRunSucceededRunningReason is the reason set when the PipelineRun is running
+	PipelineRunSucceededRunningReason PipelineRunSucceededReason = "Running"
+	// PipelineRunSucceededSuccessfulReason is the reason set when the PipelineRun completed successfully
+	PipelineRunSucceededSuccessfulReason PipelineRunSucceededReason = "Succeeded"
+	// PipelineRunSucceededFailedReason is the reason set when the PipelineRun completed with a failure
+	PipelineRunSucceededFailedReason PipelineRunSucceededReason = "Failed"
+	// PipelineRunSucceededCompletedReason is the reason set when the PipelineRun completed successfully with one or more skipped Tasks
+	PipelineRunSucceededCompletedReason PipelineRunSucceededReason = "Completed"
+)
+
+func (t PipelineRunSucceededReason) String() string {
+	return string(t)
+}
+
+// GetStartedReason returns the reason set to the "Succeeded" condition when
+// InitializeConditions is invoked
+func (pr *PipelineRunStatus) GetStartedReason() string {
+	return PipelineRunSucceededStartedReason.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 (pr *PipelineRunStatus) GetRunningReason() string {
+	return PipelineRunSucceededRunningReason.String()
+}
+
 var pipelineRunCondSet = apis.NewBatchConditionSet()
 
 // GetCondition returns the Condition matching the given type.

pkg/apis/pipeline/v1beta1/run_interface.go

@@ -23,20 +23,59 @@ import (
 
 // RunsToCompletionStatus is implemented by TaskRun.Status and PipelineRun.Status
 type RunsToCompletionStatus interface {
+
+	// GetCondition returns the Condition for the specified ConditionType
 	GetCondition(t apis.ConditionType) *apis.Condition
+
+	// InitializeConditions is used to set up the initial conditions for the
+	// RunsToCompletion  when it's initially started
 	InitializeConditions()
+
+	// SetCondition is used to set up a conditions for the specified ConditionType
 	SetCondition(newCond *apis.Condition)
+
+	// GetStartedReason returns the reason set to the "Succeeded" condition when
+	// InitializeConditions is invoked. This reason is only used combined to the
+	// value corev1.ConditionUnknown.
+	GetStartedReason() 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. This reason is only used
+	// combined to the value corev1.ConditionUnknown.
+	GetRunningReason() string
 }
 
 // RunsToCompletion is implemented by TaskRun and PipelineRun
 type RunsToCompletion interface {
+
+	// GetTypeMeta returns the TypeMeta
 	GetTypeMeta() *metav1.TypeMeta
+
+	// GetObjectMeta returns the ObjectMeta
 	GetObjectMeta() *metav1.ObjectMeta
+
+	// GetOwnerReference returns the RunsToCompletion as owner reference for any related object
 	GetOwnerReference() metav1.OwnerReference
+
+	// GetStatus returns the status as RunsToCompletionStatus
 	GetStatus() RunsToCompletionStatus
+
+	// IsDone returns true once the reconcile work on the resource is complete
+	// except for postrun actions (stop timeout timer, emit events, record metrics)
 	IsDone() bool
+
+	// HasStarted returns true after the RunsToCompletion has been reconciled for
+	// the first time. It must be true after InitializeConditions has been invoked
+	// on the associated RunsToCompletionStatus
 	HasStarted() bool
+
+	// IsCancelled returns true if the user marked the RunsToCompletion for cancellation
 	IsCancelled() bool
+
+	// HasTimedOut returns true once the RunsToCompletion has passed its maximum run time
 	HasTimedOut() bool
+
+	// GetRunKey returns the RunsToCompletion keuy which is equeued in the controller queue
 	GetRunKey() string
 }

pkg/apis/pipeline/v1beta1/taskrun_types.go

@@ -102,6 +102,39 @@ type TaskRunStatus struct {
 	TaskRunStatusFields `json:",inline"`
 }
 
+// TaskRunSucceededReason 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 TaskRunSucceededReason string
+
+const (
+	// TaskRunSucceededStartedReason is the reason set when the TaskRun has just started
+	TaskRunSucceededStartedReason TaskRunSucceededReason = "Started"
+	// TaskRunSucceededRunningReason is the reason set when the TaskRun is running
+	TaskRunSucceededRunningReason TaskRunSucceededReason = "Running"
+	// TaskRunSucceededSuccessfulReason is the reason set when the TaskRun completed successfully
+	TaskRunSucceededSuccessfulReason TaskRunSucceededReason = "Succeeded"
+	// TaskRunSucceededFailedReason is the reason set when the TaskRun completed with a failure
+	TaskRunSucceededFailedReason TaskRunSucceededReason = "Failed"
+)
+
+func (t TaskRunSucceededReason) String() string {
+	return string(t)
+}
+
+// GetStartedReason returns the reason set to the "Succeeded" condition when
+// InitializeConditions is invoked
+func (trs *TaskRunStatus) GetStartedReason() string {
+	return TaskRunSucceededStartedReason.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 TaskRunSucceededRunningReason.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) {
@@ -211,7 +244,12 @@ func (trs *TaskRunStatus) InitializeConditions() {
 	if trs.StartTime.IsZero() {
 		trs.StartTime = &metav1.Time{Time: time.Now()}
 	}
-	taskRunCondSet.Manage(trs).InitializeConditions()
+	conditionManager := taskRunCondSet.Manage(trs)
+	conditionManager.InitializeConditions()
+	// Ensure the started reason is set for the "Succeeded" condition
+	initialCondition := conditionManager.GetCondition(apis.ConditionSucceeded)
+	initialCondition.Reason = TaskRunSucceededStartedReason.String()
+	conditionManager.SetCondition(*initialCondition)
 }
 
 // SetCondition sets the condition, unsetting previous conditions with the same

pkg/pod/status.go

@@ -45,10 +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"
 
@@ -68,13 +64,6 @@ 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"
-
 	//timeFormat is RFC3339 with millisecond
 	timeFormat = "2006-01-02T15:04:05.000Z07:00"
 )
@@ -114,7 +103,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.TaskRunSucceededRunningReason.String(),
 			Message: "Not all Steps in the Task have finished executing",
 		})
 	}
@@ -197,14 +186,14 @@ func updateCompletedTaskRun(trs *v1beta1.TaskRunStatus, pod *corev1.Pod) {
 		trs.SetCondition(&apis.Condition{
 			Type:    apis.ConditionSucceeded,
 			Status:  corev1.ConditionFalse,
-			Reason:  ReasonFailed,
+			Reason:  v1beta1.TaskRunSucceededFailedReason.String(),
 			Message: msg,
 		})
 	} else {
 		trs.SetCondition(&apis.Condition{
 			Type:    apis.ConditionSucceeded,
 			Status:  corev1.ConditionTrue,
-			Reason:  ReasonSucceeded,
+			Reason:  v1beta1.TaskRunSucceededSuccessfulReason.String(),
 			Message: "All Steps have completed executing",
 		})
 	}

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 {
@@ -203,7 +199,7 @@ func (c *Reconciler) Reconcile(ctx context.Context, key string) error {
 	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, v1beta.PipelineRunSucceededFailedReason.String(), "Failed to create tracker for TaskRuns for PipelineRun")
 			return err
 		}
 
@@ -227,7 +223,7 @@ func (c *Reconciler) Reconcile(ctx context.Context, key string) error {
 	if !equality.Semantic.DeepEqual(original.Status, pr.Status) {
 		if _, err := c.updateStatus(pr); err != nil {
 			c.Logger.Warn("Failed to update PipelineRun status", zap.Error(err))
-			c.Recorder.Event(pr, corev1.EventTypeWarning, eventReasonFailed, "PipelineRun failed to update")
+			c.Recorder.Event(pr, corev1.EventTypeWarning, v1beta1.PipelineRunSucceededFailedReason.String(), "PipelineRun failed to update")
 			return multierror.Append(merr, err)
 		}
 		updated = true
@@ -240,7 +236,7 @@ func (c *Reconciler) Reconcile(ctx context.Context, key string) error {
 	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, v1beta1.PipelineRunSucceededFailedReason.String(), "PipelineRun failed to update labels/annotations")
 			return multierror.Append(merr, err)
 		}
 		updated = true
@@ -533,7 +529,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.PipelineRunSucceededSuccessfulReason.String(), "PipelineRun completed successfully.")
 		return nil
 	}