Fix reason of PipelineRun status for PipelineRun having tasks timeout

This patch fixes to set the reason of PipelineRun status to PipelineRunTimeout if the tasks of a PipelineRun timeout because of the timeout.tasks parameter Signed-off-by: Shiv Verma <[email protected]>
tektoncd · Nov 21, 2024 · 53888a9 · 53888a9
1 parent 0d39e02
commit 53888a9
Show file tree

Hide file tree

Showing 4 changed files with 54 additions and 11 deletions.
diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go
@@ -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 {
@@ -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"
@@ -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"
@@ -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"
@@ -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"

diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunstate.go b/pkg/reconciler/pipelinerun/resources/pipelinerunstate.go
@@ -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()

diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunstate_test.go b/pkg/reconciler/pipelinerun/resources/pipelinerunstate_test.go
@@ -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{

diff --git a/test/timeout_test.go b/test/timeout_test.go
@@ -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)
 	}