Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release-v0.47.x] Don't mark done PipelineRuns as timed out #6634

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/reconciler/pipelinerun/pipelinerun.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, pr *v1beta1.PipelineRun)
// reconcile. We are assuming here that if the PipelineRun has timed out for a long time, it had time to run
// before and it kept failing. One reason that can happen is exceeding etcd request size limit. Finishing it early
// makes sure the request size is manageable
if pr.HasTimedOutForALongTime(ctx, c.Clock) && !pr.IsTimeoutConditionSet() {
if !pr.IsDone() && pr.HasTimedOutForALongTime(ctx, c.Clock) && !pr.IsTimeoutConditionSet() {
if err := timeoutPipelineRun(ctx, logger, pr, c.PipelineClientSet); err != nil {
return err
}
Expand Down
67 changes: 67 additions & 0 deletions pkg/reconciler/pipelinerun/pipelinerun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10849,3 +10849,70 @@ spec:
t.Errorf("Expected PipelineRun to be create run failed, but condition reason is %s", reconciledRun.Status.GetCondition(apis.ConditionSucceeded))
}
}

func TestReconcileWithTimeoutsOfCompletedPipelineRun(t *testing.T) {
// TestReconcileWithTimeoutsOfCompletedPipelineRun runs "Reconcile" on a PipelineRun that has completed and
// which has a passed timeout.
// It verifies that reconcile is successful, and the PipelineRun is not marked as timed out.
ps := []*v1beta1.Pipeline{parse.MustParseV1beta1Pipeline(t, `
metadata:
name: test-pipeline
namespace: foo
spec:
tasks:
- name: hello-world-1
taskRef:
name: hello-world
`)}
prs := []*v1beta1.PipelineRun{parse.MustParseV1beta1PipelineRun(t, `
metadata:
name: test-pipeline-run-with-timeout
namespace: foo
spec:
pipelineRef:
name: test-pipeline
serviceAccountName: test-sa
timeouts:
pipeline: 2m
status:
conditions:
- lastTransitionTime: "2021-12-31T11:01:00Z"
message: 'Tasks Completed: 1 (Failed: 0, Cancelled 0), Skipped: 0'
reason: Succeeded
status: "True"
type: Succeeded
completionTime: "2021-12-31T11:01:00Z"
startTime: "2021-12-31T11:00:00Z"
childReferences:
- name: test-pipeline-run-with-timeout-hello-world-1
pipelineTaskName: hello-world-1
kind: TaskRun
`)}
ts := []*v1beta1.Task{simpleHelloWorldTask}

trs := []*v1beta1.TaskRun{mustParseTaskRunWithObjectMeta(t, taskRunObjectMeta("test-pipeline-run-with-timeout-hello-world-1", "foo", "test-pipeline-run-with-timeout",
"test-pipeline", "hello-world-1", false), `
spec:
serviceAccountName: test-sa
taskRef:
name: hello-world
kind: Task
`)}

d := test.Data{
PipelineRuns: prs,
Pipelines: ps,
Tasks: ts,
TaskRuns: trs,
}
prt := newPipelineRunTest(t, d)
defer prt.Cancel()

wantEvents := []string{}
reconciledRun, _ := prt.reconcileRun("foo", "test-pipeline-run-with-timeout", wantEvents, false)

// The PipelineRun should not be timed out.
if reconciledRun.Status.GetCondition(apis.ConditionSucceeded).Reason != "Succeeded" {
t.Errorf("Expected PipelineRun to still be Succeeded, but reason is %s", reconciledRun.Status.GetCondition(apis.ConditionSucceeded))
}
}