Skip to content

Commit

Permalink
Remove PipelineRun cancelation of Runs when Pipeline Task timeout is …
Browse files Browse the repository at this point in the history
…reached

TestWaitCustomTask_PipelineRun/Wait_Task_Retries_on_Timeout has been
flaky for a while. This commit stops the PipelineRun reconciler from
cancelling Run when it detects that the task-level Timeout configured
for the Run has passed, which will address the flake (similar to tektoncd#5134
which addresses TestPipelineRunTimeout).
  • Loading branch information
XinruZhang committed Oct 20, 2022
1 parent 3f46fbb commit 37d3bb3
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 43 deletions.
32 changes: 0 additions & 32 deletions pkg/reconciler/pipelinerun/pipelinerun.go
Original file line number Diff line number Diff line change
Expand Up @@ -654,10 +654,6 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun, get
return err
}

if err := c.processRunTimeouts(ctx, pr, pipelineRunState); err != nil {
return err
}

// Reset the skipped status to trigger recalculation
pipelineRunFacts.ResetSkippedCache()

Expand Down Expand Up @@ -704,34 +700,6 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun, get
return nil
}

// processRunTimeouts custom tasks are requested to cancel, if they have timed out. Since there is no guarantee that a
// custom task reconciler has its own logic for timing out a Run, this needs to be handled by the PipelineRun reconciler.
// Custom tasks can do any cleanup during this step.
func (c *Reconciler) processRunTimeouts(ctx context.Context, pr *v1beta1.PipelineRun, pipelineState resources.PipelineRunState) error {
errs := []string{}
logger := logging.FromContext(ctx)
if pr.IsCancelled() {
return nil
}
for _, rpt := range pipelineState {
if rpt.IsCustomTask() {
if rpt.Run != nil && !rpt.Run.IsCancelled() && rpt.Run.HasTimedOut(c.Clock) && !rpt.Run.IsDone() {
logger.Infof("Cancelling run task: %s due to timeout.", rpt.RunName)
err := cancelRun(ctx, rpt.RunName, pr.Namespace, c.PipelineClientSet)
if err != nil {
errs = append(errs,
fmt.Errorf("failed to patch Run `%s` with cancellation: %s", rpt.RunName, err).Error())
}
}
}
}
if len(errs) > 0 {
e := strings.Join(errs, "\n")
return fmt.Errorf("error(s) from processing cancel request for timed out Run(s) of PipelineRun %s: %s", pr.Name, e)
}
return nil
}

// runNextSchedulableTask gets the next schedulable Tasks from the dag based on the current
// pipeline run state, and starts them
// after all DAG tasks are done, it's responsible for scheduling final tasks and start executing them
Expand Down
15 changes: 4 additions & 11 deletions pkg/reconciler/pipelinerun/pipelinerun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1634,17 +1634,9 @@ status:
}
}
}
want := []jsonpatch.JsonPatchOperation{{
Operation: "add",
Path: "/spec/status",
Value: "RunCancelled",
}, {
Operation: "add",
Path: "/spec/statusMessage",
Value: string(v1alpha1.RunCancelledByPipelineMsg),
}}
var want []jsonpatch.JsonPatchOperation
if d := cmp.Diff(got, want); d != "" {
t.Fatalf("Expected cancel patch operation, but got a mismatch %s", diff.PrintWantGot(d))
t.Fatalf("Expected no operation, but got %v", got)
}
}

Expand Down Expand Up @@ -1718,6 +1710,7 @@ status:
- status: Unknown
type: Succeeded
startTime: "2021-12-31T11:58:59Z"
creationTime: "2021-12-31T11:58:58Z"
`)}

cms := []*corev1.ConfigMap{withCustomTasks(newFeatureFlagsConfigMap())}
Expand Down Expand Up @@ -1772,7 +1765,7 @@ status:
}, {
Operation: "add",
Path: "/spec/statusMessage",
Value: string(v1alpha1.RunCancelledByPipelineMsg),
Value: string(v1alpha1.RunCancelledByPipelineTimeoutMsg),
}}
if d := cmp.Diff(got, want); d != "" {
t.Fatalf("Expected RunCancelled patch operation, but got a mismatch %s", diff.PrintWantGot(d))
Expand Down

0 comments on commit 37d3bb3

Please sign in to comment.