Skip to content

Commit

Permalink
temp commit: alternative that uses global timeout config as requeue w…
Browse files Browse the repository at this point in the history
…ait time when timeout disabled
  • Loading branch information
gabemontero committed Jan 24, 2024
1 parent b6664ab commit 11dea3d
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 27 deletions.
12 changes: 9 additions & 3 deletions pkg/reconciler/pipelinerun/pipelinerun.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"reflect"
"regexp"
"strings"
"time"

"github.com/hashicorp/go-multierror"
"github.com/tektoncd/pipeline/pkg/apis/config"
Expand Down Expand Up @@ -277,12 +278,17 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, pr *v1.PipelineRun) pkgr
// do not want to subtract from 0, because a negative wait time will
// result in the requeue happening essentially immediately
timeout := pr.PipelineTimeout(ctx)
taskTimeout := pr.TasksTimeout()
waitTime := timeout - elapsed
if timeout == config.NoTimeoutDuration {
timeout = taskrun.DeriveTimeoutForRequeueWaitTime(elapsed)
//timeout = taskrun.DeriveTimeoutForRequeueWaitTime(elapsed)
waitTime = time.Duration(config.FromContextOrDefaults(ctx).Defaults.DefaultTimeoutMinutes) * time.Minute
}
waitTime := timeout - elapsed
if pr.Status.FinallyStartTime == nil && pr.TasksTimeout() != nil {
if pr.Status.FinallyStartTime == nil && taskTimeout != nil {
waitTime = pr.TasksTimeout().Duration - elapsed
if taskTimeout.Duration == config.NoTimeoutDuration {
waitTime = time.Duration(config.FromContextOrDefaults(ctx).Defaults.DefaultTimeoutMinutes) * time.Minute
}
} else if pr.Status.FinallyStartTime != nil && pr.FinallyTimeout() != nil {
finallyWaitTime := pr.FinallyTimeout().Duration - c.Clock.Since(pr.Status.FinallyStartTime.Time)
if finallyWaitTime < waitTime {
Expand Down
49 changes: 25 additions & 24 deletions pkg/reconciler/taskrun/taskrun.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"context"
"errors"
"fmt"
"math"
"reflect"
"strings"
"time"
Expand Down Expand Up @@ -213,10 +212,12 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, tr *v1.TaskRun) pkgrecon
elapsed := c.Clock.Since(tr.Status.StartTime.Time)
// Snooze this resource until the timeout has elapsed.
timeout := tr.GetTimeout(ctx)
waitTime := timeout - elapsed
if timeout == config.NoTimeoutDuration {
timeout = DeriveTimeoutForRequeueWaitTime(elapsed)
//timeout = DeriveTimeoutForRequeueWaitTime(elapsed)
waitTime = time.Duration(config.FromContextOrDefaults(ctx).Defaults.DefaultTimeoutMinutes) * time.Minute
}
return controller.NewRequeueAfter(timeout - elapsed)
return controller.NewRequeueAfter(waitTime)
}
return nil
}
Expand Down Expand Up @@ -1062,24 +1063,24 @@ func retryTaskRun(tr *v1.TaskRun, message string) {
taskRunCondSet.Manage(&tr.Status).MarkUnknown(apis.ConditionSucceeded, v1.TaskRunReasonToBeRetried.String(), message)
}

func DeriveTimeoutForRequeueWaitTime(elapsed time.Duration) time.Duration {
timeout := time.Duration(math.MaxInt64)
// an explicit choice has been made to not make these demarcation times configurable
// wrt wait times when timeout has been disabled; rationale: allowing
// disablement of timeout is problematic, but the feature has already been
// exposed. We'll get it to behave (i.e. no negative wait times) but we
// will not add configuration to encourage use of the feature.
switch {
case elapsed < 30*time.Minute:
timeout = 30 * time.Minute
case elapsed < time.Hour:
timeout = time.Hour
case elapsed < 6*time.Hour:
timeout = 6 * time.Hour
case elapsed < 24*time.Hour:
timeout = 24 * time.Hour
case elapsed < 48*time.Hour:
timeout = 48 * time.Hour
}
return timeout
}
//func DeriveTimeoutForRequeueWaitTime(elapsed time.Duration) time.Duration {
// timeout := time.Duration(math.MaxInt64)
// // an explicit choice has been made to not make these demarcation times configurable
// // wrt wait times when timeout has been disabled; rationale: allowing
// // disablement of timeout is problematic, but the feature has already been
// // exposed. We'll get it to behave (i.e. no negative wait times) but we
// // will not add configuration to encourage use of the feature.
// switch {
// case elapsed < 30*time.Minute:
// timeout = 30 * time.Minute
// case elapsed < time.Hour:
// timeout = time.Hour
// case elapsed < 6*time.Hour:
// timeout = 6 * time.Hour
// case elapsed < 24*time.Hour:
// timeout = 24 * time.Hour
// case elapsed < 48*time.Hour:
// timeout = 48 * time.Hour
// }
// return timeout
//}

0 comments on commit 11dea3d

Please sign in to comment.