Enforce TaskRun timeout using entrypoint binary

[imjasonh]

Problem

Today, TaskRun timeouts are enforced by the Tekton controller scheduling a goroutine to check in the future that a TaskRun has completed -- if it hasn't, it stops execution by deleting the TaskRun's underlying Pod, and updates the TaskRun's status to indicate timeout.

This leads to Pod logs being GCed more quickly than in the case of successful or failed TaskRuns, where the Pod stays around on the cluster, potentially making debugging timeouts harder than necessary.

Proposal

Instead, we could use the entrypoint binary we inject to order steps, and have the entrypoint binary also take a flag describing the time after which execution should fail with timeout. This would be passed in by the controller, and the entrypoint binary would execute the underlying command, with a context that times out after that time (time.Until(deadline)) and reports timeout.

Considerations

This moves timeout enforcement into each executing Pod, so timeouts wouldn't be enforced until the Pod actually starts executing -- if a TaskRun's Pod is never scheduled, then its timeout wouldn't be enforced, unlike today. We could keep controller-side timeout enforcement for scheduling timeouts, maybe?

Related to #1690 which proposes using the entrypoint binary to enforce per-step timeouts.

[vdemeester]

/kind feature

[tekton-robot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
If this issue is safe to close now please do so with /close.

/lifecycle rotten

Send feedback to tektoncd/plumbing.

[tekton-robot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close.

/lifecycle stale

Send feedback to tektoncd/plumbing.

[tekton-robot]

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

/close

Send feedback to tektoncd/plumbing.

[tekton-robot]

@tekton-robot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

/close

Send feedback to tektoncd/plumbing.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[bobcatfish]

/reopen
/remove-lifecycle rotten

[tekton-robot]

@bobcatfish: Reopened this issue.

In response to this:

/reopen
/remove-lifecycle rotten

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[tekton-robot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
If this issue is safe to close now please do so with /close.

/lifecycle rotten

Send feedback to tektoncd/plumbing.

[imjasonh]

/reopen
/remove-lifecycle-rotten

I think this is still a worthwhile change, and might even make a good-first-issue.

I think it would still be worth enforcing timeouts in the reconciler, to catch cases where a pod sat unschedulable until its timeout elapsed, and we could just not create the pod in that case.

[imjasonh]

/remove-lifecycle rotten

[ywluogg]

I'm interested in taking this if no one is working on it.

[imjasonh]

I'm interested in taking this if no one is working on it.

Sounds good! There's going to be some overlap with @Peaorl 's work in #3087, which you should just be aware of, but otherwise this seems fairly straightforward.

One thing we should make sure to do is guard this behavior change with a feature flag defaulting to false, in case the change in behavior takes anybody by surprise. In general I don't expect it to cause any problems though.

[imjasonh]

/assign @ywluogg

[tekton-robot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale with a justification.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle stale

Send feedback to tektoncd/plumbing.

[tekton-robot]

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten with a justification.
Rotten issues close after an additional 30d of inactivity.
If this issue is safe to close now please do so with /close with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/lifecycle rotten

Send feedback to tektoncd/plumbing.

[tekton-robot]

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen with a justification.
Mark the issue as fresh with /remove-lifecycle rotten with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/close

Send feedback to tektoncd/plumbing.

[tekton-robot]

@tekton-robot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen with a justification.
Mark the issue as fresh with /remove-lifecycle rotten with a justification.
If this issue should be exempted, mark the issue as frozen with /lifecycle frozen with a justification.

/close

Send feedback to tektoncd/plumbing.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.