Clean up RunsToCompletion interface

[lbernick]

Changes

The RunsToCompletion interface was introduced so that PipelineRuns and Runs
could conform to a common interface for CloudEvents and subsequently removed (both in #2677).

This commit removes the redundant function PipelineRun.IsTimedOut (redundant with PipelineRun.HasTimedOut). PipelineRun.HasTimedOut was introduced so that PipelineRun would implement RunsToCompletion.
"HasTimedOut" was kept (as opposed to "IsTimedOut") for the sake of consistency with TaskRun and Run.

It also removes stale references to RunsToCompletion. No functional changes.
This PR moves cleanup code out of #4440.

/kind cleanup

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

• Docs included if any changes are user facing
• Tests included if any functionality added or changed
• Follows the commit message standard
• Meets the Tekton contributor standards (including
  functionality, content, code)
• Release notes block below has been filled in or deleted (only if no user facing changes)

Release Notes

NONE

[lbernick]

/assign jerop
/assign afrittoli

[tekton-robot]

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/pipeline/v1beta1/pipelinerun_types.go	81.8%	81.6%	-0.2

[tekton-robot]

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/pipeline/v1beta1/pipelinerun_types.go	81.8%	81.6%	-0.2

[jerop]

Thanks for splitting out the refactor @lbernick 😸

I have some non-blocking comments (can fix in a follow up PR)

1. The function name in the TaskRun object is HasTimedOut - can we consider using HasTimedOut instead of IsTimedOut for consistency?

pipeline/pkg/apis/pipeline/v1beta1/taskrun_types.go

Lines 386 to 398 in f4c939c

	// HasTimedOut returns true if the TaskRun runtime is beyond the allowed timeout
	func (tr *TaskRun) HasTimedOut(ctx context.Context) bool {
	if tr.Status.StartTime.IsZero() {
	return false
	}
	timeout := tr.GetTimeout(ctx)
	// If timeout is set to 0 or defaulted to 0, there is no timeout.
	if timeout == apisconfig.NoTimeoutDuration {
	return false
	}
	runtime := time.Since(tr.Status.StartTime.Time)
	return runtime > timeout
	}

2. The HasTimedOut function in TaskRun has tests - can we please add some tests for the same functionality PipelineRuns as well?

pipeline/pkg/apis/pipeline/v1beta1/taskrun_types_test.go

Lines 247 to 322 in f0a2f00

	func TestHasTimedOut(t *testing.T) {
	// IsZero reports whether t represents the zero time instant, January 1, year 1, 00:00:00 UTC
	zeroTime := time.Date(1, 1, 1, 0, 0, 0, 0, time.UTC)
	testCases := []struct {
	name string
	taskRun *v1beta1.TaskRun
	expectedStatus bool
	}{{
	name: "TaskRun not started",
	taskRun: &v1beta1.TaskRun{
	Status: v1beta1.TaskRunStatus{
	Status: duckv1beta1.Status{
	Conditions: []apis.Condition{{
	Type: apis.ConditionSucceeded,
	Status: corev1.ConditionFalse,
	}},
	},
	TaskRunStatusFields: v1beta1.TaskRunStatusFields{
	StartTime: &metav1.Time{Time: zeroTime},
	},
	},
	},
	expectedStatus: false,
	}, {
	name: "TaskRun no timeout",
	taskRun: &v1beta1.TaskRun{
	Spec: v1beta1.TaskRunSpec{
	Timeout: &metav1.Duration{
	Duration: 0 * time.Minute,
	},
	},
	Status: v1beta1.TaskRunStatus{
	Status: duckv1beta1.Status{
	Conditions: []apis.Condition{{
	Type: apis.ConditionSucceeded,
	Status: corev1.ConditionFalse,
	}},
	},
	TaskRunStatusFields: v1beta1.TaskRunStatusFields{
	StartTime: &metav1.Time{Time: time.Now().Add(-15 * time.Hour)},
	},
	},
	},
	expectedStatus: false,
	}, {
	name: "TaskRun timed out",
	taskRun: &v1beta1.TaskRun{
	Spec: v1beta1.TaskRunSpec{
	Timeout: &metav1.Duration{
	Duration: 10 * time.Second,
	},
	},
	Status: v1beta1.TaskRunStatus{
	Status: duckv1beta1.Status{
	Conditions: []apis.Condition{{
	Type: apis.ConditionSucceeded,
	Status: corev1.ConditionFalse,
	}},
	},
	TaskRunStatusFields: v1beta1.TaskRunStatusFields{
	StartTime: &metav1.Time{Time: time.Now().Add(-15 * time.Second)},
	},
	},
	},
	expectedStatus: true,
	}}
	for _, tc := range testCases {
	t.Run(tc.name, func(t *testing.T) {
	result := tc.taskRun.HasTimedOut(context.Background())
	if d := cmp.Diff(result, tc.expectedStatus); d != "" {
	t.Fatalf(diff.PrintWantGot(d))
	}
	})
	}
	}

[tekton-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jerop

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [jerop]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[tekton-robot]

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/pipeline/v1beta1/pipelinerun_types.go	81.8%	81.6%	-0.2

[lbernick]

1. The function name in the TaskRun object is HasTimedOut - can we consider using HasTimedOut instead of IsTimedOut for consistency?

Done.

2. The HasTimedOut function in TaskRun has tests - can we please add some tests for the same functionality PipelineRuns as well?

I think PipelineRuns.IsTimedOut is already tested; just replaced with HasTimedOut.

[tekton-robot]

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/pipeline/v1beta1/pipelinerun_types.go	81.8%	81.6%	-0.2

[jerop]

sounds good, thanks @lbernick!

[jerop]

TestPipelineTaskTimeout is a known flake: #4427

/test pull-tekton-pipeline-integration-tests

[ghost]

/lgtm