Allow user to specify only tasks or finally timeout

This commit updates PipelineRun validation to allow a user to specify only a tasks timeout or only a finally timeout. Prior to this commit, if a user wanted to specify no Pipeline timeout (i.e. timeouts.pipeline = 0), they would not be able to specify values for the other timeouts fields.
tektoncd · Sep 10, 2022 · 4abedf0 · 4abedf0
1 parent 6a13505
commit 4abedf0
Show file tree

Hide file tree

Showing 5 changed files with 89 additions and 5 deletions.
diff --git a/docs/pipelineruns.md b/docs/pipelineruns.md
@@ -866,7 +866,7 @@ timeouts:
 All three sub-fields are optional, and will be automatically processed according to the following constraint:
 * `timeouts.pipeline >= timeouts.tasks + timeouts.finally`
 
-You may combine the timeouts as follow:
+Example timeouts usages are as follows:
 
 Combination 1: Set the timeout for the entire `pipeline` and reserve a portion of it for `tasks`.
 
@@ -888,6 +888,26 @@ spec:
     finally: "0h3m0s"
 ```
 
+Combination 3: Set only a `tasks` timeout, with no timeout for the entire `pipeline`.
+
+```yaml
+kind: PipelineRun
+spec:
+  timeouts:
+    pipeline: "0"  # No timeout
+    tasks: "0h3m0s"
+```
+
+Combination : Set only a `finally` timeout, with no timeout for the entire `pipeline`.
+
+```yaml
+kind: PipelineRun
+spec:
+  timeouts:
+    pipeline: "0"  # No timeout
+    finally: "0h3m0s"
+```
+
 You can also use the *Deprecated* `timeout` field to set the `PipelineRun's` desired timeout value in minutes.
 If you do not specify this value in the `PipelineRun`, the global default timeout value applies.
 If you set the timeout to 0, the `PipelineRun` fails immediately upon encountering an error.

diff --git a/pkg/apis/pipeline/v1/pipelinerun_validation.go b/pkg/apis/pipeline/v1/pipelinerun_validation.go
@@ -238,7 +238,7 @@ func (ps *PipelineRunSpec) validatePipelineTimeout(timeout time.Duration, errorM
 	if ps.Timeouts.Tasks != nil {
 		tasksTimeoutErr := false
 		tasksTimeoutStr := ps.Timeouts.Tasks.Duration.String()
-		if ps.Timeouts.Tasks.Duration > timeout {
+		if ps.Timeouts.Tasks.Duration > timeout && timeout != config.NoTimeoutDuration {
 			tasksTimeoutErr = true
 		}
 		if ps.Timeouts.Tasks.Duration == config.NoTimeoutDuration && timeout != config.NoTimeoutDuration {
@@ -253,7 +253,7 @@ func (ps *PipelineRunSpec) validatePipelineTimeout(timeout time.Duration, errorM
 	if ps.Timeouts.Finally != nil {
 		finallyTimeoutErr := false
 		finallyTimeoutStr := ps.Timeouts.Finally.Duration.String()
-		if ps.Timeouts.Finally.Duration > timeout {
+		if ps.Timeouts.Finally.Duration > timeout && timeout != config.NoTimeoutDuration {
 			finallyTimeoutErr = true
 		}
 		if ps.Timeouts.Finally.Duration == config.NoTimeoutDuration && timeout != config.NoTimeoutDuration {

diff --git a/pkg/apis/pipeline/v1/pipelinerun_validation_test.go b/pkg/apis/pipeline/v1/pipelinerun_validation_test.go
@@ -793,6 +793,38 @@ func TestPipelineRunWithTimeout_Validate(t *testing.T) {
 				},
 			},
 		},
+	}, {
+		name: "timeouts.tasks only",
+		pr: v1.PipelineRun{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: "pipelinelinename",
+			},
+			Spec: v1.PipelineRunSpec{
+				PipelineRef: &v1.PipelineRef{
+					Name: "prname",
+				},
+				Timeouts: &v1.TimeoutFields{
+					Pipeline: &metav1.Duration{Duration: 0 * time.Hour},
+					Tasks:    &metav1.Duration{Duration: 30 * time.Minute},
+				},
+			},
+		},
+	}, {
+		name: "timeouts.finally only",
+		pr: v1.PipelineRun{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: "pipelinelinename",
+			},
+			Spec: v1.PipelineRunSpec{
+				PipelineRef: &v1.PipelineRef{
+					Name: "prname",
+				},
+				Timeouts: &v1.TimeoutFields{
+					Pipeline: &metav1.Duration{Duration: 0 * time.Hour},
+					Finally:  &metav1.Duration{Duration: 30 * time.Minute},
+				},
+			},
+		},
 	}}
 
 	for _, ts := range tests {

diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_validation.go b/pkg/apis/pipeline/v1beta1/pipelinerun_validation.go
@@ -250,7 +250,7 @@ func (ps *PipelineRunSpec) validatePipelineTimeout(timeout time.Duration, errorM
 	if ps.Timeouts.Tasks != nil {
 		tasksTimeoutErr := false
 		tasksTimeoutStr := ps.Timeouts.Tasks.Duration.String()
-		if ps.Timeouts.Tasks.Duration > timeout {
+		if ps.Timeouts.Tasks.Duration > timeout && timeout != config.NoTimeoutDuration {
 			tasksTimeoutErr = true
 		}
 		if ps.Timeouts.Tasks.Duration == config.NoTimeoutDuration && timeout != config.NoTimeoutDuration {
@@ -265,7 +265,7 @@ func (ps *PipelineRunSpec) validatePipelineTimeout(timeout time.Duration, errorM
 	if ps.Timeouts.Finally != nil {
 		finallyTimeoutErr := false
 		finallyTimeoutStr := ps.Timeouts.Finally.Duration.String()
-		if ps.Timeouts.Finally.Duration > timeout {
+		if ps.Timeouts.Finally.Duration > timeout && timeout != config.NoTimeoutDuration {
 			finallyTimeoutErr = true
 		}
 		if ps.Timeouts.Finally.Duration == config.NoTimeoutDuration && timeout != config.NoTimeoutDuration {

diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go b/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go
@@ -1054,6 +1054,38 @@ func TestPipelineRunWithTimeout_Validate(t *testing.T) {
 				},
 			},
 		},
+	}, {
+		name: "timeouts.tasks only",
+		pr: v1beta1.PipelineRun{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: "pipelinelinename",
+			},
+			Spec: v1beta1.PipelineRunSpec{
+				PipelineRef: &v1beta1.PipelineRef{
+					Name: "prname",
+				},
+				Timeouts: &v1beta1.TimeoutFields{
+					Pipeline: &metav1.Duration{Duration: 0 * time.Hour},
+					Tasks:    &metav1.Duration{Duration: 30 * time.Minute},
+				},
+			},
+		},
+	}, {
+		name: "timeouts.finally only",
+		pr: v1beta1.PipelineRun{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: "pipelinelinename",
+			},
+			Spec: v1beta1.PipelineRunSpec{
+				PipelineRef: &v1beta1.PipelineRef{
+					Name: "prname",
+				},
+				Timeouts: &v1beta1.TimeoutFields{
+					Pipeline: &metav1.Duration{Duration: 0 * time.Hour},
+					Finally:  &metav1.Duration{Duration: 30 * time.Minute},
+				},
+			},
+		},
 	}}
 
 	for _, ts := range tests {