Design and implement retries

[bobcatfish]

Expected Behavior

Users may have some Tasks which they know will sometimes fail for reasons which are not worth failing a Pipeline over. In this case the user should be able to specify a number of times to retry the Task for.

If a Task is retried, this should be obvious in the statuses of both the PipelineRun and the resulting TaskRuns.

Actual Behavior

If a Task fails once, the whole Pipeline will stop and be considered failed.

Additional Info

@abayer do you have any other requirements we should focus on here?

[abayer]

So from discussions coming out of today's meeting and further chatting with @jchesterpivotal on Slack, I think what I'd sketch out as the goal would be:

• By default, nothing retries for any reason.
• If a TaskRun or PipelineRun is manually aborted through the API/CRD changes, nothing should retry.
• If a TaskRun or PipelineRun fails (i.e., the command returns non-zero, e.g.), it should only be retried if the Task or Pipeline is configured to retry a set number of times for failure.
• If a TaskRun or PipelineRun errors out (i.e., someone deleted the TaskRun CRD via kubectl, or the node the TaskRun is on catches on fire and explodes), it should only be retried if the Task or Pipeline is configured to retry a set number of times for error.

Thoughts?

[bobcatfish]

Thanks for sketching this out in detail @abayer !

I'm going to take a slightly different stance, but I'm also completely open this being shot down.

By default, nothing retries for any reason.

This makes sense to me when I think about the CI/CD systems I'm used to using, however I wonder if the fact that this is a kubernetes based system changes the equation a bit.

If I deployed something into a k8s cluster, i would expect that if there was an underlying failure, e.g. a node goes down, that my workload would be automatically rescheduled. That would be one of the reasons that I'd be using kubernetes in the first place.

So I'm not quite sure about the decision to strip this feature out of this CI/CD system by default.

Assuming (big assumption) that we want this k8s magic for free with a k8s based CI/CD system, my alternative set of goals is:

• By default, nothing retries for any reason.
• If a TaskRun or PipelineRun is manually aborted through the API/CRD changes, nothing should retry. <-- agree!
• If a TaskRun or PipelineRun fails (i.e., the command returns non-zero, e.g.), it should only be retried if the Task or Pipeline is configured to retry a set number of times for failure. <-- agree!
• If a TaskRun or PipelineRun errors out (i.e., someone deleted the TaskRun CRD via kubectl, or the node the TaskRun is on catches on fire and explodes), it should only be retried if the Task or Pipeline is configured to retry a set number of times for error. it will be retried up to a default number of times unless the user explicitly indicated it should not be retried

I wonder if we should add:

• By default, a PipelineRun never retries for any reason (since a PipelineRun is a meta grouping of TaskRuns which have their own retry logic), but can be configured to retry (if this is important, not sure it is)

[abayer]

@bobcatfish - that sounds perfectly fine to me. Higher level tooling, like what I'm starting to envision w.r.t. Jenkins, would be able to change the "default" behavior when translating to Pipeline CRD YAML anyway, so it probably does make more sense to have the out-of-the-box behavior match Kubernetes expectations.

And yeah, PipelineRun not retrying by default is a good idea. In practice, TaskRun retry is probably going to be sufficient in 99/100 cases, so it may not be worth it to even add support for configurable retry on PipelineRuns for now.

[abayer]

I'm going to start working on this once #355 is merged, FYI.

[abayer]

Ok, thinking about implementation in the reconcilers now:

• What exactly does "retry" mean? For a TaskRun, do we just re-initialize the Status (except for a counter of retry attempts) and let the reconciler try again? Do we want to preserve the original Steps/Results/etc somewhere? And for a PipelineRun, do we want to preserve the original TaskRuns/Results/etc?
• What should the interaction between retries and timeouts (see Add Pipeline-level timeouts #355) be? If the Pipeline has a timeout, and one of its TaskRuns retries, we presumably need to reset the timeout on the TaskRun, since the original timeout's duration is based on the time from when the TaskRun first started until the Pipeline will timeout, so if we reset the StartTime for the TaskRun, the original timeout's duration would now go until after the Pipeline will timeout. Similarly for PipelineRuns, the retried execution would basically reset the timeout clock. Not sure what's right here.
• I think the way we can distinguish between a TaskRun that failed (i.e., has a ConditionSucceeded Condition with ConditionFalse Status) due to a non-zero exit code vs any other reason is to look at each TaskRun.Status.Steps item to see if its ContainerState.Terminated is not nil and has a non-zero ExitCode - if there is no such step on a failed TaskRun, then we can assume it failed for a "retry by default" reason? Or should we be more particular and look at other potential failure cases that shouldn't be retried by default? And if so, what would those be?