Skip to content

Commit

Permalink
Fix: do not fail TaskRun for concurrent modification errors
Browse files Browse the repository at this point in the history
This commit fixes the behaviour that a concurrent modification error
when stopping sidecar will fail the TaskRun, which could cause
successful Tasks to fail even though it could succeed after retrying.

/kind bug
  • Loading branch information
JeromeJu committed Dec 7, 2023
1 parent b6d27a8 commit 1759704
Showing 1 changed file with 27 additions and 0 deletions.
27 changes: 27 additions & 0 deletions pkg/reconciler/taskrun/taskrun.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,11 @@ func (c *Reconciler) stopSidecars(ctx context.Context, tr *v1.TaskRun) error {
// it has probably evicted. We can return the error, but we consider it a permanent one.
return controller.NewPermanentError(err)
} else if err != nil {
// It is admissible for Pods to fail with concurrentModification errors
// when stopping sideCars. The reconciler can retry on this.
if isConcurrentModificationError(err) {
return controller.NewRequeueAfter(time.Second)
}
logger.Errorf("Error stopping sidecars for TaskRun %q: %v", tr.Name, err)
tr.Status.MarkResourceFailed(v1.TaskRunReasonStopSidecarFailed, err)
}
Expand Down Expand Up @@ -1014,6 +1019,28 @@ func isResourceQuotaConflictError(err error) bool {
return k8ErrStatus.Details != nil && k8ErrStatus.Details.Kind == "resourcequotas"
}

const (
// TODO(#7466) Currently this appears as a local constant due to upstream dependencies bump blocker.
// This shall reference to k8s.io/apiserver/pkg/registry/generic/registry.OptimisticLockErrorMsg
// once #7464 is unblocked.
optimisticLockErrorMsg = "the object has been modified; please apply your changes to the latest version and try again"
)

// isConcurrentModificationError determines whether it is a concurrent
// modification error depending on its error type and error message.
func isConcurrentModificationError(err error) bool {
if !k8serrors.IsConflict(err) {
return false
}

se, ok := err.(*k8serrors.StatusError)
if !ok {
return false
}

return strings.Contains(se.Error(), optimisticLockErrorMsg)
}

// retryTaskRun archives taskRun.Status to taskRun.Status.RetriesStatus, and set
// taskRun status to Unknown with Reason v1.TaskRunReasonToBeRetried.
func retryTaskRun(tr *v1.TaskRun, message string) {
Expand Down

0 comments on commit 1759704

Please sign in to comment.