From fc8786b3cb149ad7e59a091fdaa05d485e28ca70 Mon Sep 17 00:00:00 2001
From: pritidesai <pdesai@us.ibm.com>
Date: Tue, 4 Jan 2022 14:51:21 -0800
Subject: [PATCH] avoid setting activeDeadlineSeconds for notimeouts

PR #4217 introduced better handling of the resource quota by adding
support for activeDeadlineSeconds. activeDeadlineSeconds is calculated
based on this formula:

int64(taskRun.GetTimeout(ctx).Seconds() * 1.5)

In case when a timeout on a task is set to 0s i.e. no timeout, the taskrun
fails with ambiguous message "Invalid value: 0: must be between 1 and
2147483647, inclusive." This is happening because activeDeadlineSeconds is
set to 0 in case of a 0s timeout but in this case activeDeadlineSeconds
is assigned a value out of the permitted range (1 to maxint32).

This commit is changing the way activeDeadlineSeconds is set such that it is
set to MaxInt32 for a task with 0s timeout.
---
 pkg/pod/pod.go      | 10 +++++++++
 pkg/pod/pod_test.go | 49 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+)

diff --git a/pkg/pod/pod.go b/pkg/pod/pod.go
index 402c9d0ae1f..7b4e1924bed 100644
--- a/pkg/pod/pod.go
+++ b/pkg/pod/pod.go
@@ -19,6 +19,7 @@ package pod
 import (
 	"context"
 	"fmt"
+	"math"
 	"path/filepath"
 	"strconv"
 
@@ -88,6 +89,9 @@ var (
 		Name:         "tekton-internal-steps",
 		VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}},
 	}}
+
+	// MaxActiveDeadlineSeconds is a maximum permitted value to be used for a task with no timeout
+	MaxActiveDeadlineSeconds = int64(math.MaxInt32)
 )
 
 // Builder exposes options to configure Pod construction from TaskSpecs/Runs.
@@ -300,7 +304,13 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1beta1.TaskRun, taskSpec
 	if shouldAddReadyAnnotationOnPodCreate(ctx, taskSpec.Sidecars) {
 		podAnnotations[readyAnnotation] = readyAnnotationValue
 	}
+
+	// calculate the activeDeadlineSeconds based on the specified timeout (uses default timeout if it's not specified)
 	activeDeadlineSeconds := int64(taskRun.GetTimeout(ctx).Seconds() * deadlineFactor)
+	// set activeDeadlineSeconds to the max. allowed value i.e. max int32 when timeout is explicitly set to 0
+	if taskRun.GetTimeout(ctx) == config.NoTimeoutDuration {
+		activeDeadlineSeconds = MaxActiveDeadlineSeconds
+	}
 
 	podNameSuffix := "-pod"
 	if taskRunRetries := len(taskRun.Status.RetriesStatus); taskRunRetries > 0 {
diff --git a/pkg/pod/pod_test.go b/pkg/pod/pod_test.go
index d3346562922..3b418e37999 100644
--- a/pkg/pod/pod_test.go
+++ b/pkg/pod/pod_test.go
@@ -1254,6 +1254,55 @@ _EOF_
 				}),
 				ActiveDeadlineSeconds: &defaultActiveDeadlineSeconds,
 			},
+		}, {
+			desc: "step-with-no-timeout-equivalent-to-0-second-timeout",
+			ts: v1beta1.TaskSpec{
+				Steps: []v1beta1.Step{{Container: corev1.Container{
+					Name:    "name",
+					Image:   "image",
+					Command: []string{"cmd"}, // avoid entrypoint lookup.
+				},
+					Timeout: &metav1.Duration{Duration: 0 * time.Second},
+				}},
+			},
+			trs: v1beta1.TaskRunSpec{
+				Timeout: &metav1.Duration{Duration: 0 * time.Second},
+			},
+			want: &corev1.PodSpec{
+				RestartPolicy:  corev1.RestartPolicyNever,
+				InitContainers: []corev1.Container{placeToolsInit, tektonDirInit(images.EntrypointImage, []v1beta1.Step{{Container: corev1.Container{Name: "name"}}})},
+				Containers: []corev1.Container{{
+					Name:    "step-name",
+					Image:   "image",
+					Command: []string{"/tekton/bin/entrypoint"},
+					Args: []string{
+						"-wait_file",
+						"/tekton/downward/ready",
+						"-wait_file_content",
+						"-post_file",
+						"/tekton/run/0/out",
+						"-termination_path",
+						"/tekton/termination",
+						"-step_metadata_dir",
+						"/tekton/run/0/status",
+						"-timeout",
+						"0s",
+						"-entrypoint",
+						"cmd",
+						"--",
+					},
+					VolumeMounts: append([]corev1.VolumeMount{binROMount, runMount(0, false), downwardMount, {
+						Name:      "tekton-creds-init-home-0",
+						MountPath: "/tekton/creds",
+					}}, implicitVolumeMounts...),
+					TerminationMessagePath: "/tekton/termination",
+				}},
+				Volumes: append(implicitVolumes, binVolume, runVolume(0), downwardVolume, corev1.Volume{
+					Name:         "tekton-creds-init-home-0",
+					VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{Medium: corev1.StorageMediumMemory}},
+				}),
+				ActiveDeadlineSeconds: &MaxActiveDeadlineSeconds,
+			},
 		}, {
 			desc: "task-with-creds-init-disabled",
 			featureFlags: map[string]string{