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

Fix reason of PipelineRun status for PipelineRun having tasks timeout #7744

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
21 changes: 11 additions & 10 deletions pkg/reconciler/pipelinerun/pipelinerun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2901,20 +2901,21 @@ status:
defer prt.Cancel()

wantEvents := []string{
"Normal Started",
"Warning Failed PipelineRun",
}
reconciledRun, clients := prt.reconcileRun("foo", "test-pipeline-run-with-timeout", wantEvents, false)

if reconciledRun.Status.CompletionTime != nil {
t.Errorf("Expected nil CompletionTime on PipelineRun but was %s", reconciledRun.Status.CompletionTime)
if reconciledRun.Status.CompletionTime == nil {
t.Errorf("Expected CompletionTime on PipelineRun but was %s", reconciledRun.Status.CompletionTime)
}

// The PipelineRun should be running.
if reconciledRun.Status.GetCondition(apis.ConditionSucceeded).Reason != v1.PipelineRunReasonRunning.String() {
t.Errorf("Expected PipelineRun to be running, but condition reason is %s", reconciledRun.Status.GetCondition(apis.ConditionSucceeded).Reason)
// The PipelineRun should be timeout.
if reconciledRun.Status.GetCondition(apis.ConditionSucceeded).Reason != v1.PipelineRunReasonTimedOut.String() {
t.Errorf("Expected PipelineRun to be timeout, but condition reason is %s", reconciledRun.Status.GetCondition(apis.ConditionSucceeded).Reason)
}

// Check that there is a skipped task for the expected reason

if len(reconciledRun.Status.SkippedTasks) != 1 {
t.Errorf("expected one skipped task, found %d", len(reconciledRun.Status.SkippedTasks))
} else if reconciledRun.Status.SkippedTasks[0].Reason != v1.TasksTimedOutSkip {
Expand Down Expand Up @@ -3041,7 +3042,7 @@ spec:
pipelineRef:
name: test-pipeline-with-finally
timeouts:
tasks: 5m
tasks: 25m
pipeline: 20m
status:
finallyStartTime: "2021-12-31T23:44:59Z"
Expand Down Expand Up @@ -3269,7 +3270,7 @@ spec:
pipelineRef:
name: test-pipeline-with-finally
timeouts:
tasks: 5m
tasks: 25m
pipeline: 20m
status:
startTime: "2021-12-31T23:40:00Z"
Expand Down Expand Up @@ -3318,7 +3319,7 @@ spec:
pipelineRef:
name: test-pipeline-with-finally
timeouts:
tasks: 5m
tasks: 25m
pipeline: 20m
status:
startTime: "2021-12-31T23:40:00Z"
Expand Down Expand Up @@ -3358,7 +3359,7 @@ spec:
pipelineRef:
name: test-pipeline-with-finally
timeouts:
tasks: 5m
tasks: 25m
pipeline: 20m
status:
startTime: "2021-12-31T23:40:00Z"
Expand Down
9 changes: 9 additions & 0 deletions pkg/reconciler/pipelinerun/resources/pipelinerunstate.go
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,15 @@ func (facts *PipelineRunFacts) GetPipelineConditionStatus(ctx context.Context, p
}
}

if pr.HaveTasksTimedOut(ctx, c) {
return &apis.Condition{
Type: apis.ConditionSucceeded,
Status: corev1.ConditionFalse,
Reason: v1.PipelineRunReasonTimedOut.String(),
Message: fmt.Sprintf("PipelineRun %q failed due to tasks failed to finish within %q", pr.Name, pr.TasksTimeout().Duration.String()),
}
}

// report the count in PipelineRun Status
// get the count of successful tasks, failed tasks, cancelled tasks, skipped task, and incomplete tasks
s := facts.getPipelineTasksCount()
Expand Down
33 changes: 33 additions & 0 deletions pkg/reconciler/pipelinerun/resources/pipelinerunstate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2141,6 +2141,39 @@ func TestGetPipelineConditionStatus_PipelineTimeouts(t *testing.T) {
}
}

// pipeline should result in timeout if its runtime exceeds its spec.Timeouts.Tasks based on its status.Timeout
func TestGetPipelineConditionStatus_PipelineTasksTimeouts(t *testing.T) {
d, err := dagFromState(oneFinishedState)
if err != nil {
t.Fatalf("Unexpected error while building DAG for state %v: %v", oneFinishedState, err)
}
pr := &v1.PipelineRun{
ObjectMeta: metav1.ObjectMeta{Name: "pipelinerun-no-tasks-started"},
Spec: v1.PipelineRunSpec{
Timeouts: &v1.TimeoutFields{
Tasks: &metav1.Duration{Duration: 1 * time.Minute},
},
},
Status: v1.PipelineRunStatus{
PipelineRunStatusFields: v1.PipelineRunStatusFields{
StartTime: &metav1.Time{Time: now.Add(-2 * time.Minute)},
},
},
}
facts := PipelineRunFacts{
State: oneFinishedState,
TasksGraph: d,
FinalTasksGraph: &dag.Graph{},
TimeoutsState: PipelineRunTimeoutsState{
Clock: testClock,
},
}
c := facts.GetPipelineConditionStatus(context.Background(), pr, zap.NewNop().Sugar(), testClock)
if c.Status != corev1.ConditionFalse && c.Reason != v1.PipelineRunReasonTimedOut.String() {
t.Fatalf("Expected to get status %s but got %s for state %v", corev1.ConditionFalse, c.Status, oneFinishedState)
}
}

func TestGetPipelineConditionStatus_OnError(t *testing.T) {
var oneFailedStateOnError = PipelineRunState{{
PipelineTask: &v1.PipelineTask{
Expand Down
2 changes: 1 addition & 1 deletion test/timeout_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,7 @@ spec:
}

t.Logf("Waiting for PipelineRun %s in namespace %s to be failed", pipelineRun.Name, namespace)
if err := WaitForPipelineRunState(ctx, c, pipelineRun.Name, timeout, FailedWithReason(v1.PipelineRunReasonFailed.String(), pipelineRun.Name), "PipelineRunFailed", v1Version); err != nil {
if err := WaitForPipelineRunState(ctx, c, pipelineRun.Name, timeout, FailedWithReason(v1.PipelineRunReasonTimedOut.String(), pipelineRun.Name), "PipelineRunFailed", v1Version); err != nil {
t.Errorf("Error waiting for PipelineRun %s to finish: %s", pipelineRun.Name, err)
}

Expand Down
Loading