Skip to content

Commit

Permalink
avoid setting activeDeadlineSeconds for notimeouts
Browse files Browse the repository at this point in the history
PR tektoncd#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.
  • Loading branch information
pritidesai committed Jan 5, 2022
1 parent 02d4f7e commit fc8786b
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 0 deletions.
10 changes: 10 additions & 0 deletions pkg/pod/pod.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package pod
import (
"context"
"fmt"
"math"
"path/filepath"
"strconv"

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down
49 changes: 49 additions & 0 deletions pkg/pod/pod_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down

0 comments on commit fc8786b

Please sign in to comment.