Skip to content

Commit

Permalink
Fixing nil pointer in case of no timeout ⏲
Browse files Browse the repository at this point in the history
If `pr.Spec.Timeout` is nil (aka no timeout applied), dereferencing
it (`*pr.Spec.Timout`) will panic.

Signed-off-by: Vincent Demeester <vdemeest@redhat.com>
(cherry picked from commit d3bf694)
  • Loading branch information
vdemeester authored and tekton-robot committed Sep 15, 2020
1 parent 4e23d50 commit 61e4167
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 8 deletions.
24 changes: 17 additions & 7 deletions pkg/reconciler/pipelinerun/pipelinerun.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,9 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, pr *v1beta1.PipelineRun)
logger.Warnf("PipelineRun %s createTimestamp %s is after the pipelineRun started %s", pr.GetNamespacedName().String(), pr.CreationTimestamp, pr.Status.StartTime)
pr.Status.StartTime = &pr.CreationTimestamp
}

// start goroutine to track pipelinerun timeout only startTime is not set
go c.timeoutHandler.Wait(pr.GetNamespacedName(), *pr.Status.StartTime, *pr.Spec.Timeout)
go c.timeoutHandler.Wait(pr.GetNamespacedName(), *pr.Status.StartTime, getPipelineRunTimeout(ctx, pr))
// Emit events. During the first reconcile the status of the PipelineRun may change twice
// from not Started to Started and then to Running, so we need to sent the event here
// and at the end of 'Reconcile' again.
Expand Down Expand Up @@ -534,7 +535,7 @@ func (c *Reconciler) runNextSchedulableTask(ctx context.Context, pr *v1beta1.Pip
}
} else if !rprt.ResolvedConditionChecks.HasStarted() {
for _, rcc := range rprt.ResolvedConditionChecks {
rcc.ConditionCheck, err = c.makeConditionCheckContainer(rprt, rcc, pr)
rcc.ConditionCheck, err = c.makeConditionCheckContainer(ctx, rprt, rcc, pr)
if err != nil {
recorder.Eventf(pr, corev1.EventTypeWarning, "ConditionCheckCreationFailed", "Failed to create TaskRun %q: %v", rcc.ConditionCheckName, err)
return fmt.Errorf("error creating ConditionCheck container called %s for PipelineTask %s from PipelineRun %s: %w", rcc.ConditionCheckName, rprt.PipelineTask.Name, pr.Name, err)
Expand Down Expand Up @@ -669,7 +670,7 @@ func (c *Reconciler) createTaskRun(ctx context.Context, rprt *resources.Resolved
Spec: v1beta1.TaskRunSpec{
Params: rprt.PipelineTask.Params,
ServiceAccountName: serviceAccountName,
Timeout: getTaskRunTimeout(pr, rprt),
Timeout: getTaskRunTimeout(ctx, pr, rprt),
PodTemplate: podTemplate,
}}

Expand Down Expand Up @@ -818,12 +819,21 @@ func combineTaskRunAndTaskSpecAnnotations(pr *v1beta1.PipelineRun, pipelineTask
return annotations
}

func getTaskRunTimeout(pr *v1beta1.PipelineRun, rprt *resources.ResolvedPipelineRunTask) *metav1.Duration {
func getPipelineRunTimeout(ctx context.Context, pr *v1beta1.PipelineRun) metav1.Duration {
if pr.Spec.Timeout == nil {
defaultTimeout := time.Duration(config.FromContextOrDefaults(ctx).Defaults.DefaultTimeoutMinutes)
return metav1.Duration{Duration: defaultTimeout * time.Minute}
}
return *pr.Spec.Timeout
}

func getTaskRunTimeout(ctx context.Context, pr *v1beta1.PipelineRun, rprt *resources.ResolvedPipelineRunTask) *metav1.Duration {
var taskRunTimeout = &metav1.Duration{Duration: apisconfig.NoTimeoutDuration}

var timeout time.Duration
if pr.Spec.Timeout == nil {
timeout = config.DefaultTimeoutMinutes * time.Minute
defaultTimeout := time.Duration(config.FromContextOrDefaults(ctx).Defaults.DefaultTimeoutMinutes)
timeout = defaultTimeout * time.Minute
} else {
timeout = pr.Spec.Timeout.Duration
}
Expand Down Expand Up @@ -874,7 +884,7 @@ func (c *Reconciler) updateLabelsAndAnnotations(pr *v1beta1.PipelineRun) (*v1bet
return newPr, nil
}

func (c *Reconciler) makeConditionCheckContainer(rprt *resources.ResolvedPipelineRunTask, rcc *resources.ResolvedConditionCheck, pr *v1beta1.PipelineRun) (*v1beta1.ConditionCheck, error) {
func (c *Reconciler) makeConditionCheckContainer(ctx context.Context, rprt *resources.ResolvedPipelineRunTask, rcc *resources.ResolvedConditionCheck, pr *v1beta1.PipelineRun) (*v1beta1.ConditionCheck, error) {
labels := getTaskrunLabels(pr, rprt.PipelineTask.Name)
labels[pipeline.GroupName+pipeline.ConditionCheckKey] = rcc.ConditionCheckName
labels[pipeline.GroupName+pipeline.ConditionNameKey] = rcc.Condition.Name
Expand Down Expand Up @@ -910,7 +920,7 @@ func (c *Reconciler) makeConditionCheckContainer(rprt *resources.ResolvedPipelin
Resources: &v1beta1.TaskRunResources{
Inputs: rcc.ToTaskResourceBindings(),
},
Timeout: getTaskRunTimeout(pr, rprt),
Timeout: getTaskRunTimeout(ctx, pr, rprt),
PodTemplate: podTemplate,
}}

Expand Down
2 changes: 1 addition & 1 deletion pkg/reconciler/pipelinerun/pipelinerun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1677,7 +1677,7 @@ func TestGetTaskRunTimeout(t *testing.T) {

for _, tc := range tcs {
t.Run(tc.name, func(t *testing.T) {
if d := cmp.Diff(getTaskRunTimeout(tc.pr, tc.rprt), tc.expected); d != "" {
if d := cmp.Diff(getTaskRunTimeout(context.TODO(), tc.pr, tc.rprt), tc.expected); d != "" {
t.Errorf("Unexpected task run timeout. Diff %s", diff.PrintWantGot(d))
}
})
Expand Down

0 comments on commit 61e4167

Please sign in to comment.