Support activeDeadlineSeconds for Tekton pods 🦌 #4217
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tekton-robot
added
release-note
tekton-robot
requested review from
bobcatfish,
jerop,
mattmoor,
pritidesai and
a user
tekton-robot
added
the
size/M
|
The following is the coverage report on the affected files.
|
Kubernetes (and OpenShift) mark `Pod` as either Terminating — has a "relatively" short life and will terminate at some point — and NonTerminating — is supposed to run for ever. Kubernetes does the difference between the two using the `ActiveDeadlineSeconds` field. A `Pod` with `activeDeadlineSeconds` set will be considered as Terminating. For example, `Job`'s `Pod` have this field set and are considered as Terminated. Currently the pods created by tekton fall under the NonTerminating quota limits of Kubernetes and OpenShift. This can create issues as generally builds should fall under the separate terminating quota limits. This sets the `activeDeadlineSeconds` field or TaskRun's `Pod` so that they are considered Terminating. It also sets the value of this field to a higher value than the specified (or default) Timeout set on the TaskRun so that Kubernetes won't try to terminate the `Pod` before Pipeline does. Signed-off-by: Vincent Demeester <[email protected]>
vdemeester
force-pushed
the
activeDeadlineSeconds
branch
from
ffa410d to
8c2a6ac
Compare
|
The following is the coverage report on the affected files.
|
|
Nice. This also has the effect that pods won't run forever if the controller happens to be unavailable or late to process timeouts. |
Indeed 😛 |
|
/lgtm |
|
/test pull-tekton-integration-tests |
|
I imagine the active deadline seconds are counted from when the pod is scheduled? It might be nice to add a note in the docs about this, but it's ok as a follow-up pr if you prefer. /approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: afrittoli The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
tekton-robot
added
the
approved
pritidesai
added a commit
to pritidesai/pipeline
that referenced
this pull request
Jan 4, 2022
PR tektoncd#4217 introduced better handline 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 getting set to 0 in case of a 0s timeout but in this case activeDeadlineSeconds is getting set to a value out of the permitted range. This commit is changing the way activeDeadlineSeconds is set such that its not set at all for a task with 0s timeout.
5 tasks
pritidesai
added a commit
to pritidesai/pipeline
that referenced
this pull request
Jan 5, 2022
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.
tekton-robot
pushed a commit
that referenced
this pull request
Jan 13, 2022
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.
khrm
pushed a commit
to openshift/tektoncd-pipeline
that referenced
this pull request
May 19, 2022
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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
Kubernetes (and OpenShift) mark
Podas either Terminating — has a"relatively" short life and will terminate at some point — and
NonTerminating — is supposed to run for ever. Kubernetes does the
difference between the two using the
ActiveDeadlineSecondsfield. APodwithactiveDeadlineSecondsset will be considered asTerminating. For example,
Job'sPodhave this field set and areconsidered as Terminated.
Currently the pods created by tekton fall under the NonTerminating
quota limits of Kubernetes and OpenShift. This can create issues as
generally builds should fall under the separate terminating quota
limits.
This sets the
activeDeadlineSecondsfield or TaskRun'sPodso thatthey are considered Terminating. It also sets the value of this field
to a higher value than the specified (or default) Timeout set on the
TaskRun so that Kubernetes won't try to terminate the
PodbeforePipeline does.
Signed-off-by: Vincent Demeester [email protected]
/kind feature
/cc @sbwsg @bobcatfish @mattmoor @pritidesai @jerop
Tentatively adding the 0.28 milestone 🙃
Submitter Checklist
As the author of this PR, please check off the items in this checklist:
functionality, content, code)
Release Notes