TEP-0123 - proposal to specify on-demand-retry in a pipelineTask

[pritidesai]

Adding a proposal to support pipeline authors to overcome any flakes and to allow running the same instance of the pipeline until it finishes. This PR introduces a TEP with a real world use cases.

/kind tep

[lbernick]

/assign @dibyom @XinruZhang

[tekton-robot]

@lbernick: GitHub didn't allow me to assign the following users: xinruzhang.

Note that only tektoncd members, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

In response to this:

/assign @dibyom @XinruZhang

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.

[dibyom]

Wouldn't another choice be to configure the known flaky task with a reasonable amount of retires? If that is not a valid choice for this scenario, could you explain why?

[pritidesai]

Yup, configuring the known flaky task with reasonable amount of retries will work in some scenarios. In cases where the failure is the result of a momentary bandwidth bottleneck, the reasonable amount of retries might not be a feasible choice. In case of a bandwidth issue or any other infrastructure issue, the desire is to be able to retry once the issue is resolved. Also, for a cluster with 100s or 1000s of pipelineRuns of the same CD pipeline, I would want to avoid retrying all flaky taskRuns at the same time.

[dibyom]

ok, this is a compelling use case that I think we'll run into for our dogfooding pipeline (assuming we have a single presubmit pipeline that runs build, unit test, coverage etc)

cc @abayer

[vdemeester]

I am not sure I understand this use case. A user that doesn't have access to /test … command will not have tests running when opening a PR (it "needs" the label). If the test fails due to a flakyness, "someone" (a reviewer) will look into it and run the /test. Ideally it will be relatively quick, so I am not sure I see a problem to solve there.

[dibyom]

Right - where I was coming from was the assumption that the entire presubmit is one tekton pipeline and so /test or /re-test will rerun everything. Like you said below, one alternative would be to have smaller pipelines chained together

[afrittoli]

The setup we have now is one pipeline per CI job.
The main Disadvantage of this approach we need to clone the repo for each CI job (but that is not different from what prow does today).

There are a few ways we could improve on that, one of which is to have a single pipeline, but I did not consider it really until now because of a few reasons:

• CI jobs may be provided / maintained by different teams. So the granularity at best could be one pipeline per group of jobs from the same team.
• We don't have partial pipeline execution, so a re-run would always be a re-run of the whole pipeline

[pritidesai]

I might not have done a good job of explaining this use case.

The assumption for this use case is that it's one single tekton pipeline we are talking about.

With most of the reviewers working on multiple projects, it will be beneficial to the contributors to be able to avoid such dependencies on the reviewers.

This proposal is addressing re-run from a failed task. I might have to go back and update this use case to better explain.

[afrittoli]

Thank you @pritidesai

[dibyom]

Why is on demand retry the best way to solve flakes? Why not automated retries? Are there other alternatives?

[vdemeester]

I think one approach (imo) is to create smaller Pipeline that are connected through events. That way you can easily re-run a Pipeline(Run) (that failed) and it would "continue" the flow

[pritidesai]

yup, creating a smaller pipeline and connect them through events could be an alternative.

But, the disadvantages of this alternative are very painful for the users. I have a user who has deployed one pipeline for 10 different applications. If he has to split up the pipeline for each possible flake, he will end up in a management nightmare of creating additional triggers for each application and managing pieces of pipeline (which might not even make sense if the pieces contains just one single task).

As a user of the pipeline controller, it's reasonable to expect a solution to handle such failures.

[afrittoli]

I agree that smaller pipelines can be an alternative here, but as @pritidesai highlighted, it's not necessarily applicable to all users and all cases. It's a bit like the many small repos vs. large mono-repo divide - we should support users that require large pipelines for their use cases.

[vdemeester]

I am not sure I understand the proposal here.
Is it :

• an "infinite" retry — retry until it suceed
• a new PipelineRun that is created from a previous one, but will only run the "selected" tasks ?

In the case of the later, I am not sure this is something tektoncd/pipeline has to solve. But in any case, we may want to explore how other CI/CD tools (Jenkins, github workflows, …) handle those case – if they do at all.

[vdemeester]

I think one approach (imo) is to create smaller Pipeline that are connected through events. That way you can easily re-run a Pipeline(Run) (that failed) and it would "continue" the flow

[vdemeester]

I am not sure I understand this use case. A user that doesn't have access to /test … command will not have tests running when opening a PR (it "needs" the label). If the test fails due to a flakyness, "someone" (a reviewer) will look into it and run the /test. Ideally it will be relatively quick, so I am not sure I see a problem to solve there.

[afrittoli]

One could argue that submit-evidence should be executed regardless to track the deployment and failed tests.
Having a retry on the test task would allow submitting evidence that does not include the flake, or that at least includes both runs of the tests.

[pritidesai]

I am not sure I understand the proposal here. Is it :

• an "infinite" retry — retry until it suceed
• a new PipelineRun that is created from a previous one, but will only run the "selected" tasks ?

In the case of the later, I am not sure this is something tektoncd/pipeline has to solve. But in any case, we may want to explore how other CI/CD tools (Jenkins, github workflows, …) handle those case – if they do at all.

thanks @vdemeester I will look into how other CI/CD systems are solving this issue.

This proposal is a retry pipelineTask until it succeeds or cancel the pipelineRun (original cancellation implementation takes higher precedence).

There is no new pipelineRun created with the "selected task". This is somewhat similar to pending pipelines but extending it to failed tasks. Its the same pipelineRun being utilized.

[pritidesai]

thank you @dibyom, @vdemeester, and @afrittoli for the reviews 🙏 I have addressed some of the comments, will continue addressing the rest of the comments in a day or two. Thanks!

[pritidesai]

But in any case, we may want to explore how other CI/CD tools (Jenkins, github workflows, …) handle those case – if they do at all.

Hey @vdemeester I have added a small section on how other CI/CD systems such as Argo Workflow, GitHub actions, and Jenkins handle this use case.

[pritidesai]

Hey @afrittoli, @vdemeester, and @dibyom, please take another look at the changes, thanks! 🙏

[afrittoli]

/assign @afrittoli

[dibyom]

Thanks @pritidesai I'd like to understand better the motivation behind restarting the same run vs creating a new run with the same specs.
One way to experiment I can think of is with a custom run that takes a failed pipelinerun and the task to be restarted and starts a new pipelinerun

[dibyom]

Not sure what this means...couldn't we keep the pod running but signal the entrypoint to stop executing steps?

[afrittoli]

Yeah, we probably don't need to specify why "Pause/resume" is not a goal.
There may be ways to achieve that functionality, but it's not what the TEP is about.

[dibyom]

It doesn't have to be here but I think its worth specifying why Pause/Resume isn't a valid alternative (or at least why that isn't the chosen path for the proposal)

[pritidesai]

This proposal is about restarting a failed pipelineTask. Is “Pause/resume” limited to just failure? Some use case wouldn’t require pausing a pipeline even without any failures?

[dibyom]

Interesting...but it seems like the retry logic is still not on demand though...Jenkins is still responsible for retrying it it thinks there is a connection issue to the agent

[pritidesai]

Right, retry logic is not on demand. Jenkins is responsible for retrying in case of a connection issue.

But, Jenkins does support catching failures and further processing somehow:

Retry the block (up to N times) if any exception happens during its body execution. If an exception happens on the final attempt then it will lead to aborting the build (unless it is caught and processed somehow). User aborts of the build are not caught.

I have not looked into it further though.

Also, Jenkins support conditions which is very advanced in terms of specifying what condition triggers the retry. Also, Jenkins retry logic understands the difference between infrastructure failure (connection to agent itself is broken) v/s build failure and allows pipeline author to choose to retry in case of an infrastructure failure. Jenkins retry logic can also distinguishes between pod termination v/s pod deletion.

conditions (optional)
Conditions under which the block should be retried. If none match, the block will fail. If there are no specified conditions, the block will always be retried except in case of user aborts.
Array / List of Nested Choice of Objects
agent
Detects that a node block, or certain steps inside it such as sh, failed for reasons which are likely due to infrastructure rather than the behavior of the build. If the connection to an agent is broken or the agent is removed from the list of executors while in use (typically in response to the disappearance of underlying cloud resources), this condition will allow retry to allocate a fresh agent and try the whole block again.
kubernetesAgent
Similar to agent() (Agent errors) but tailored to agents provisioned from a Kubernetes cloud. Unlike the generic agent error condition, this will ignore certain pod termination reasons which are likely to be under the control of the Pipeline author (e.g., OOMKilled) while still allowing retry to recover after common cases of pod deletion.
handleNonKubernetes : boolean (optional)
Behave like the generic agent() (Agent errors) when applied to a non-Kubernetes agent. Useful in cases where it is hard to predict in a job definition whether a Kubernetes or other sort of agent will be used.
nonresumable
The Jenkins controller was restarted while the build was running a step which cannot be resumed. Some steps like sh or input are written to survive a Jenkins restart and simply pick up where they left off when the build resumes. Others like checkout or junit normally complete promptly but cannot tolerate a restart. In case one of these steps happened to be in progress when Jenkins shut down, the resumed build will throw an error; using this condition with retry allows the step (or the whole enclosing node block) to be rerun.

What we are trying build in this PR is the limitation in Jenkins that the users are experiencing: https://issues.jenkins.io/browse/JENKINS-54735 And something on their roadmap.

[dibyom]

I found some docs here: https://www.jenkins.io/doc/book/pipeline/running-pipelines/#restart-from-a-stage

Some takeaways for me:

You can restart any completed Declarative Pipeline from any top-level stage which ran in that Pipeline. This allows you to rerun a Pipeline from a stage which failed due to transient or environmental considerations, for example. All inputs to the Pipeline will be the same. This includes SCM information, build parameters, and the contents of any stash step calls in the original Pipeline, if specified.

I think this bit about the stash is important. We don't have anything similar. Also later in the docs:

Normally, when you run the stash step in your Pipeline, the resulting stash of artifacts is cleared when the Pipeline completes, regardless of the result of the Pipeline. Since stash artifacts aren’t accessible outside of the Pipeline run that created them, this has not created any limitations on usage. But with Declarative stage restarting, you may want to be able to unstash artifacts from a stage which ran before the stage you’re restarting from.

So, it seems like Jenkins preserves the artifacts that may have been created in a previous stage and makes it optionally accessible to another build/pipeline.

Once you choose a stage to restart from and click submit, a new build, with a new build number, will be started. All stages before the selected stage will be skipped, and the Pipeline will start executing at the selected stage. From that point on, the Pipeline will run as normal.

[afrittoli]

This TEP does not consider the partial execution of a pipeline that reached completion, whether successful or not. The key assumption for the retries in this TEP is that the pipeline is still running.

If a PipelineTask marked for "on-demand-retry" fails, the execution of the Pipeline continues until either of the following happens:

• the PipelineTask is successful, and the associated branch of execution is resumed. This may happen if a retry is requested and is successful.
• timeout is reached
• the PipelineRun is cancelled

[dibyom]

Reading through this section again it seems like other CI systems provide a way to configure a delay before configuring other retries. Jenkins allows users to specify conditional logic before retrying. Jenkins Declarative pipelines does support restarting stages on demand but like you pointed out that does not keep the current pipeline running.
So, I think the current proposal is a bit unique in that respect. I don't think this by itself should mean should should not pursue on demand retries just that we should consider the alternative solutions for the use cases as well before implementing this

[pritidesai]

The clear distinction here is You can restart any completed Declarative Pipeline, this proposal is not applicable if the pipeline is completed/failed/cancelled.

[pritidesai]

Like @afrittoli explained this proposal is only applicable until one of those three criteria is met.

[pritidesai]

thanks @dibyom, please elaborate on this

other CI systems provide a way to configure a delay before configuring other retries.

Delay is applicable in terms of retries, if a stage fails and asked to retry, a delay can be introduced to schedule a stage for the execution after that interval.

Jenkins allows users to specify conditional logic before retrying.

Conditional logic is part of retry. Jenkins support try-catch in addition to retry which is much more robust when used along with retry.

https://www.jenkins.io/doc/pipeline/steps/workflow-basic-steps/#catcherror-catch-error-and-set-build-result-to-failure

I don't think this by itself should mean should should not pursue on demand retries just that we should consider the alternative solutions for the use cases as well before implementing this

What are the alternative solutions here apart from custom task? This is a very common use case and if we decide to support it, it must live as a first class solution rather than a custom task.

[dibyom]

Why restart a taskRun and not just recreate a new taskRun?

[afrittoli]

It seems to me that this belongs more to the design details than to the goals.
The goal is to allow for on-demand retries of a PipelineTask:

• if a task fails the PipelineRun execution does not end
• execution of PipelineTasks that do not depend on the failed one continues normally
• one or more retries of the failed PipelineTask may be requested, until either
  • the PipelineTask is successful
  • the PipelineRun times out
  • the PipelineRun is cancelled

[dibyom]

Why is this a non-goal? It seems like it would be an alternative at least?

[afrittoli]

The scope of this TEP is to retry a task in a running pipeline run.
We could consider partial re-execution of a finished pipeline run, possibly with different inputs, but that would be a different TEP. Partial execution after completion is a whole different story as we would need to make sure we are able to recreate the required inputs at a certain point of execution.

[dibyom]

I don't think close/re-open are the only alternatives. Pushing a new commit or a /retest like chat command can both work here.

[afrittoli]

I agree that closing/re-opening is not the only option here.

I think @pritidesai's point was not about the specific way the pipeline is triggered, but rather that a completely new PipelineRun is required. On-demand retries of a task would change things so that the PipelineRun would not terminate right away upon failure, giving the engineer an option to fix the underlying condition and retry the specific task without having to repeat the execution of the rest of the pipeline.

[dibyom]

Wouldn't this mean that if a user does not detect that the unit test failed and triggered a on-demand retry, the entire pipeline will be kept running and so if you were relying on notifications at the end of a pipelinerun, you wouldn't get it?

[afrittoli]

Yes, it would keep running until timeout.

[afrittoli]

One could configure a high-enough number of retries - so that the user could still go and fix the underlying cause while the retries are running. This solution would be less efficient and user-friendly though - as it is likely to run many more retries compared to the on-demand one, and also it forces the users to wait for the next failure and retry to verify their fix.

[afrittoli]

Thanks for the updates @pritidesai
I left some comments but nothing blocking for the proposed state.
One thing that we might want to give users is the option to just say "continue" and let the pipeline fail, in cases where no solution is available, but that's something we could add at a later stage too.

/approve

[dibyom]

@pritidesai @afrittoli I'm ok merging this as proposed but I think we should consider retry strategies such as allowing users to specify a backoff (or even some kind of a condition) as a more general purpose solution to the use cases here.

/approve

[tekton-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: afrittoli, dibyom

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:

• teps/OWNERS [afrittoli,dibyom]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[pritidesai]

thanks a bunch @dibyom and @afrittoli for the approval 🎉 we can certainly add continue kind of an option and allow backoff along with a condition in addition to on-demand-retry. I will update the TEP in next PR. Thanks!

[jerop]

Merging at API WG

/lgtm

[afrittoli]

@pritidesai In light of what we agreed upon in TEP-0121, retries for TaskRuns and CustomRuns will be managed by the respective controllers.
On-demand retries of a pipelineTask are, by nature, a pipeline feature, so this kind of retry will require implementing a pipeline controller driven retry, which MUST then create new TaskRuns when running.
The same retry mechanism could be used to implement pipeline driven retries like described in my tep-0121 comment, and we'll have to decide which of the two to implement first.

[pritidesai]

@pritidesai In light of what we agreed upon in TEP-0121, retries for TaskRuns and CustomRuns will be managed by the respective controllers.

thanks @afrittoli 👍 My understanding is the retries for a pipelineTask in a pipeline will be managed by the respective controllers but pipelineRun controller will continue watching for taskRuns created for a pipelineTask. Right?

On-demand retries of a pipelineTask are, by nature, a pipeline feature, so this kind of retry will require implementing a pipeline controller driven retry, which MUST then create new TaskRuns when running.

Any kind of retries for a pipelineTask for example, number of attempts, on-demand-retry, delay-retry, etc is still a pipeline feature, since pipelineTask is part of pipeline, the implementers could be any controller but in the end pipelineRun controller is the one checking if pipelineTask is done executing and has the control of scheduling next pipelineTask in the pipeline. Yup, I agree, its the respective controller that implements this feature and we will limit this feature to the first class entity which is taskRun since this feature can be implemented by customRun controller by any custom controller provider if needed.

The same retry mechanism could be used to implement pipeline driven retries like described in my #879 (comment), and we'll have to decide which of the two to implement first.

Yup, let's discuss this further since I think, all retries for a pipelineTask are pipeline driven but could be implemented by respective controllers.