fix: Prevent excessive reconciliation when timeout disabled

pkg/reconciler/pipelinerun/pipelinerun.go

@@ -25,7 +25,6 @@ import (
 	"reflect"
 	"regexp"
 	"strings"
-	"time"
 
 	"k8s.io/apimachinery/pkg/util/wait"
 
@@ -275,31 +274,50 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, pr *v1.PipelineRun) pkgr
 	}
 
 	if pr.Status.StartTime != nil {
-		// Compute the time since the task started.
+		// Compute the time since the pipeline started.
 		elapsed := c.Clock.Since(pr.Status.StartTime.Time)
 		// Snooze this resource until the appropriate timeout has elapsed.
-		// but if the timeout has been disabled by setting timeout to 0, we
-		// do not want to subtract from 0, because a negative wait time will
-		// result in the requeue happening essentially immediately
 		timeout := pr.PipelineTimeout(ctx)
 		taskTimeout := pr.TasksTimeout()
-		waitTime := timeout - elapsed
-		if timeout == config.NoTimeoutDuration {
-			waitTime = time.Duration(config.FromContextOrDefaults(ctx).Defaults.DefaultTimeoutMinutes) * time.Minute
-		}
-		if pr.Status.FinallyStartTime == nil && taskTimeout != nil {
-			waitTime = pr.TasksTimeout().Duration - elapsed
-			if taskTimeout.Duration == config.NoTimeoutDuration {
-				waitTime = time.Duration(config.FromContextOrDefaults(ctx).Defaults.DefaultTimeoutMinutes) * time.Minute
+
+		// If the main pipeline timeout is NoTimeoutDuration (0), it means no timeout is configured.
+		// This can happen in two ways:
+		// 1. User explicitly set pr.Spec.Timeouts.Pipeline to 0 (wants no timeout)
+		// 2. User didn't set pr.Spec.Timeouts.Pipeline (nil) AND default-timeout-minutes config is "0"
+		// In these cases, check if there are specific task or finally timeouts to enforce.
+		// If not, don't requeue - the reconciler will be triggered by watch events.
+		// Check which phase we're in and handle timeout accordingly
+		if pr.Status.FinallyStartTime != nil {
+			// We're in finally phase - check for finally-specific timeout
+			if pr.FinallyTimeout() != nil && pr.FinallyTimeout().Duration != config.NoTimeoutDuration {
+				finallyWaitTime := pr.FinallyTimeout().Duration - c.Clock.Since(pr.Status.FinallyStartTime.Time)
+				// If pipeline timeout is also set, use the most restrictive timeout
+				if timeout != config.NoTimeoutDuration {
+					waitTime := timeout - elapsed
+					if finallyWaitTime < waitTime {
+						waitTime = finallyWaitTime
+					}
+					return controller.NewRequeueAfter(waitTime)
+				}
+				return controller.NewRequeueAfter(finallyWaitTime)
 			}
-		} else if pr.Status.FinallyStartTime != nil && pr.FinallyTimeout() != nil &&
-			pr.FinallyTimeout().Duration != config.NoTimeoutDuration {
-			finallyWaitTime := pr.FinallyTimeout().Duration - c.Clock.Since(pr.Status.FinallyStartTime.Time)
-			if finallyWaitTime < waitTime {
-				waitTime = finallyWaitTime
+			// No finally timeout, use pipeline timeout if set
+			if timeout != config.NoTimeoutDuration {
+				return controller.NewRequeueAfter(timeout - elapsed)
 			}
+			return nil
 		}
-		return controller.NewRequeueAfter(waitTime)
+
+		// We're in tasks phase - check for task-specific timeout
+		if taskTimeout != nil && taskTimeout.Duration != config.NoTimeoutDuration {
+			return controller.NewRequeueAfter(taskTimeout.Duration - elapsed)
+		}
+		// No task timeout, use pipeline timeout if set
+		if timeout != config.NoTimeoutDuration {
+			return controller.NewRequeueAfter(timeout - elapsed)
+		}
+		return nil
+
 	}
 	return nil
 }

pkg/reconciler/pipelinerun/pipelinerun_test.go

@@ -2599,13 +2599,26 @@ spec:
 			c := prt.TestAssets.Controller
 			clients := prt.TestAssets.Clients
 			reconcileError := c.Reconciler.Reconcile(prt.TestAssets.Ctx, fmt.Sprintf("%s/%s", "foo", tc.prs[0].Name))
-			if reconcileError == nil {
-				t.Errorf("expected error, but got nil")
-			}
-			if isRequeueError, requeueDuration := controller.IsRequeueKey(reconcileError); !isRequeueError {
-				t.Errorf("Expected requeue error, but got: %s", reconcileError.Error())
-			} else if requeueDuration < 0 {
-				t.Errorf("Expected a positive requeue duration but got %s", requeueDuration.String())
+
+			// When timeout is explicitly disabled (set to 0), we should NOT requeue
+			// This prevents excessive reconciliation (issue #8495)
+			// Check if the pipeline has timeout disabled
+			pipelineTimeout := tc.prs[0].PipelineTimeout(prt.TestAssets.Ctx)
+			if pipelineTimeout == config.NoTimeoutDuration {
+				// Timeout is disabled - should not requeue
+				if reconcileError != nil {
+					t.Errorf("expected no error when timeout is disabled, but got: %v", reconcileError)
+				}
+			} else {
+				// Timeout is enabled - should requeue
+				if reconcileError == nil {
+					t.Errorf("expected error when timeout is enabled, but got nil")
+				}
+				if isRequeueError, requeueDuration := controller.IsRequeueKey(reconcileError); !isRequeueError {
+					t.Errorf("Expected requeue error, but got: %v", reconcileError)
+				} else if requeueDuration < 0 {
+					t.Errorf("Expected a positive requeue duration but got %s", requeueDuration.String())
+				}
 			}
 			prt.Test.Logf("Getting reconciled run")
 			reconciledRun, err := clients.Pipeline.TektonV1().PipelineRuns("foo").Get(prt.TestAssets.Ctx, tc.prs[0].Name, metav1.GetOptions{})

pkg/reconciler/taskrun/taskrun.go

@@ -237,10 +237,16 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, tr *v1.TaskRun) pkgrecon
 		elapsed := c.Clock.Since(tr.Status.StartTime.Time)
 		// Snooze this resource until the timeout has elapsed.
 		timeout := tr.GetTimeout(ctx)
-		waitTime := timeout - elapsed
+		// If timeout is NoTimeoutDuration (0), it means no timeout is configured.
+		// This can happen in two ways:
+		// 1. User explicitly set tr.Spec.Timeout.Duration to 0 (wants no timeout)
+		// 2. User didn't set tr.Spec.Timeout (nil) AND default-timeout-minutes config is "0"
+		// In both cases, we should not requeue based on timeout. The reconciler will
+		// still be triggered appropriately by pod watch events when the TaskRun changes.
 		if timeout == config.NoTimeoutDuration {
-			waitTime = time.Duration(config.FromContextOrDefaults(ctx).Defaults.DefaultTimeoutMinutes) * time.Minute
+			return nil
 		}
+		waitTime := timeout - elapsed
 		return controller.NewRequeueAfter(waitTime)
 	}
 	return nil

pkg/reconciler/taskrun/taskrun_test.go

@@ -2932,13 +2932,25 @@ status:
 			clients := testAssets.Clients
 
 			err = c.Reconciler.Reconcile(testAssets.Ctx, getRunName(tc.taskRun))
-			if err == nil {
-				t.Errorf("expected error when reconciling completed TaskRun : %v", err)
-			}
-			if isRequeueError, requeueDuration := controller.IsRequeueKey(err); !isRequeueError {
-				t.Errorf("Expected requeue error, but got: %s", err.Error())
-			} else if requeueDuration < 0 {
-				t.Errorf("Expected a positive requeue duration but got %s", requeueDuration.String())
+
+			// When timeout is explicitly disabled (set to 0), we should NOT requeue
+			// This prevents excessive reconciliation (issue #8495)
+			taskTimeout := tc.taskRun.GetTimeout(testAssets.Ctx)
+			if taskTimeout == config.NoTimeoutDuration {
+				// Timeout is disabled - should not requeue
+				if err != nil {
+					t.Errorf("expected no error when timeout is disabled, but got: %v", err)
+				}
+			} else {
+				// Timeout is enabled - should requeue
+				if err == nil {
+					t.Errorf("expected error when timeout is enabled, but got nil")
+				}
+				if isRequeueError, requeueDuration := controller.IsRequeueKey(err); !isRequeueError {
+					t.Errorf("Expected requeue error, but got: %v", err)
+				} else if requeueDuration < 0 {
+					t.Errorf("Expected a positive requeue duration but got %s", requeueDuration.String())
+				}
 			}
 			_, err = clients.Pipeline.TektonV1().TaskRuns(tc.taskRun.Namespace).Get(testAssets.Ctx, tc.taskRun.Name, metav1.GetOptions{})
 			if err != nil {