Fix Timeouts Default in v1 PipelienRun

This commit fixes the SetDefault for v1 pipelinerun.timeouts. Previously it would reset pipelinerun.timeouts fields with only timeouts.pipeline while it should have kept timeouts.tasks and timeouts.finally as expected.
tektoncd · Apr 19, 2023 · a1c1000 · a1c1000
1 parent 048bf9a
commit a1c1000
Show file tree

Hide file tree

Showing 2 changed files with 103 additions and 5 deletions.
diff --git a/pkg/apis/pipeline/v1/pipelinerun_defaults.go b/pkg/apis/pipeline/v1/pipelinerun_defaults.go
@@ -37,7 +37,11 @@ func (pr *PipelineRun) SetDefaults(ctx context.Context) {
 func (prs *PipelineRunSpec) SetDefaults(ctx context.Context) {
  cfg := config.FromContextOrDefaults(ctx)
 
- if prs.Timeouts != nil && prs.Timeouts.Pipeline == nil {
+ if prs.Timeouts == nil {
+ prs.Timeouts = &TimeoutFields{}
+ }
+
+ if prs.Timeouts.Pipeline == nil {
  prs.Timeouts.Pipeline = &metav1.Duration{Duration: time.Duration(cfg.Defaults.DefaultTimeoutMinutes) * time.Minute}
  }
 

diff --git a/pkg/apis/pipeline/v1/pipelinerun_defaults_test.go b/pkg/apis/pipeline/v1/pipelinerun_defaults_test.go
@@ -18,7 +18,9 @@ package v1_test
 
 import (
  "context"
+ "strconv"
  "testing"
+ "time"
 
  "github.com/google/go-cmp/cmp"
  "github.com/google/go-cmp/cmp/cmpopts"
@@ -37,13 +39,77 @@ func TestPipelineRunSpec_SetDefaults(t *testing.T) {
  prs *v1.PipelineRunSpec
  want *v1.PipelineRunSpec
  }{
+ {
+ desc: "timeouts is nil",
+ prs: &v1.PipelineRunSpec{},
+ want: &v1.PipelineRunSpec{
+ TaskRunTemplate: v1.PipelineTaskRunTemplate{
+ ServiceAccountName: config.DefaultServiceAccountValue,
+ },
+ Timeouts: &v1.TimeoutFields{
+ Pipeline: &metav1.Duration{Duration: config.DefaultTimeoutMinutes * time.Minute},
+ },
+ },
+ },
+ {
+ desc: "timeouts is not nil",
+ prs: &v1.PipelineRunSpec{
+ Timeouts: &v1.TimeoutFields{},
+ },
+ want: &v1.PipelineRunSpec{
+ TaskRunTemplate: v1.PipelineTaskRunTemplate{
+ ServiceAccountName: config.DefaultServiceAccountValue,
+ },
+ Timeouts: &v1.TimeoutFields{
+ Pipeline: &metav1.Duration{Duration: config.DefaultTimeoutMinutes * time.Minute},
+ },
+ },
+ },
+ {
+ desc: "timeouts.pipeline is not nil",
+ prs: &v1.PipelineRunSpec{
+ Timeouts: &v1.TimeoutFields{
+ Pipeline: &metav1.Duration{Duration: (config.DefaultTimeoutMinutes + 1) * time.Minute},
+ },
+ },
+ want: &v1.PipelineRunSpec{
+ TaskRunTemplate: v1.PipelineTaskRunTemplate{
+ ServiceAccountName: config.DefaultServiceAccountValue,
+ },
+ Timeouts: &v1.TimeoutFields{
+ Pipeline: &metav1.Duration{Duration: (config.DefaultTimeoutMinutes + 1) * time.Minute},
+ },
+ },
+ },
+ {
+ desc: "timeouts.pipeline is nil with timeouts.tasks and timeouts.finally",
+ prs: &v1.PipelineRunSpec{
+ Timeouts: &v1.TimeoutFields{
+ Tasks: &metav1.Duration{Duration: (config.DefaultTimeoutMinutes + 1) * time.Minute},
+ Finally: &metav1.Duration{Duration: (config.DefaultTimeoutMinutes + 1) * time.Minute},
+ },
+ },
+ want: &v1.PipelineRunSpec{
+ TaskRunTemplate: v1.PipelineTaskRunTemplate{
+ ServiceAccountName: config.DefaultServiceAccountValue,
+ },
+ Timeouts: &v1.TimeoutFields{
+ Pipeline: &metav1.Duration{Duration: (config.DefaultTimeoutMinutes) * time.Minute},
+ Tasks: &metav1.Duration{Duration: (config.DefaultTimeoutMinutes + 1) * time.Minute},
+ Finally: &metav1.Duration{Duration: (config.DefaultTimeoutMinutes + 1) * time.Minute},
+ },
+ },
+ },
  {
  desc: "pod template is nil",
  prs: &v1.PipelineRunSpec{},
  want: &v1.PipelineRunSpec{
  TaskRunTemplate: v1.PipelineTaskRunTemplate{
  ServiceAccountName: config.DefaultServiceAccountValue,
  },
+ Timeouts: &v1.TimeoutFields{
+ Pipeline: &metav1.Duration{Duration: (config.DefaultTimeoutMinutes) * time.Minute},
+ },
  },
  },
  {
@@ -56,6 +122,9 @@ func TestPipelineRunSpec_SetDefaults(t *testing.T) {
  },
  },
  },
+ Timeouts: &v1.TimeoutFields{
+ Pipeline: &metav1.Duration{Duration: (config.DefaultTimeoutMinutes) * time.Minute},
+ },
  },
  want: &v1.PipelineRunSpec{
  TaskRunTemplate: v1.PipelineTaskRunTemplate{
@@ -66,6 +135,9 @@ func TestPipelineRunSpec_SetDefaults(t *testing.T) {
  },
  },
  },
+ Timeouts: &v1.TimeoutFields{
+ Pipeline: &metav1.Duration{Duration: (config.DefaultTimeoutMinutes) * time.Minute},
+ },
  },
  },
  }
@@ -91,6 +163,7 @@ func TestPipelineRunSpec_SetDefaults(t *testing.T) {
 }
 
 func TestPipelineRunDefaulting(t *testing.T) {
+ const defaultTimeoutMinutes = 5
  tests := []struct {
  name string
  in *v1.PipelineRun
@@ -104,6 +177,9 @@ func TestPipelineRunDefaulting(t *testing.T) {
  TaskRunTemplate: v1.PipelineTaskRunTemplate{
  ServiceAccountName: config.DefaultServiceAccountValue,
  },
+ Timeouts: &v1.TimeoutFields{
+ Pipeline: &metav1.Duration{Duration: (config.DefaultTimeoutMinutes) * time.Minute},
+ },
  },
  },
  }, {
@@ -128,6 +204,9 @@ func TestPipelineRunDefaulting(t *testing.T) {
  TaskRunTemplate: v1.PipelineTaskRunTemplate{
  ServiceAccountName: config.DefaultServiceAccountValue,
  },
+ Timeouts: &v1.TimeoutFields{
+ Pipeline: &metav1.Duration{Duration: (config.DefaultTimeoutMinutes) * time.Minute},
+ },
  },
  },
  }, {
@@ -143,6 +222,9 @@ func TestPipelineRunDefaulting(t *testing.T) {
  TaskRunTemplate: v1.PipelineTaskRunTemplate{
  ServiceAccountName: config.DefaultServiceAccountValue,
  },
+ Timeouts: &v1.TimeoutFields{
+ Pipeline: &metav1.Duration{Duration: (config.DefaultTimeoutMinutes) * time.Minute},
+ },
  },
  },
  wc: func(ctx context.Context) context.Context {
@@ -168,6 +250,9 @@ func TestPipelineRunDefaulting(t *testing.T) {
  TaskRunTemplate: v1.PipelineTaskRunTemplate{
  ServiceAccountName: "tekton",
  },
+ Timeouts: &v1.TimeoutFields{
+ Pipeline: &metav1.Duration{Duration: defaultTimeoutMinutes * time.Minute},
+ },
  },
  },
  wc: func(ctx context.Context) context.Context {
@@ -177,7 +262,7 @@ func TestPipelineRunDefaulting(t *testing.T) {
  Name: config.GetDefaultsConfigName(),
  },
  Data: map[string]string{
- "default-timeout-minutes": "5",
+ "default-timeout-minutes": strconv.Itoa(defaultTimeoutMinutes),
  "default-service-account": "tekton",
  },
  })
@@ -201,6 +286,9 @@ func TestPipelineRunDefaulting(t *testing.T) {
  },
  },
  },
+ Timeouts: &v1.TimeoutFields{
+ Pipeline: &metav1.Duration{Duration: defaultTimeoutMinutes * time.Minute},
+ },
  },
  },
  wc: func(ctx context.Context) context.Context {
@@ -210,7 +298,7 @@ func TestPipelineRunDefaulting(t *testing.T) {
  Name: config.GetDefaultsConfigName(),
  },
  Data: map[string]string{
- "default-timeout-minutes": "5",
+ "default-timeout-minutes": strconv.Itoa(defaultTimeoutMinutes),
  "default-service-account": "tekton",
  "default-pod-template": "nodeSelector: { 'label': 'value' }",
  },
@@ -242,6 +330,9 @@ func TestPipelineRunDefaulting(t *testing.T) {
  },
  },
  },
+ Timeouts: &v1.TimeoutFields{
+ Pipeline: &metav1.Duration{Duration: defaultTimeoutMinutes * time.Minute},
+ },
  },
  },
  wc: func(ctx context.Context) context.Context {
@@ -251,7 +342,7 @@ func TestPipelineRunDefaulting(t *testing.T) {
  Name: config.GetDefaultsConfigName(),
  },
  Data: map[string]string{
- "default-timeout-minutes": "5",
+ "default-timeout-minutes": strconv.Itoa(defaultTimeoutMinutes),
  "default-service-account": "tekton",
  "default-pod-template": "nodeSelector: { 'label': 'value' }",
  },
@@ -290,6 +381,9 @@ func TestPipelineRunDefaulting(t *testing.T) {
  HostNetwork: true,
  },
  },
+ Timeouts: &v1.TimeoutFields{
+ Pipeline: &metav1.Duration{Duration: defaultTimeoutMinutes * time.Minute},
+ },
  },
  },
  wc: func(ctx context.Context) context.Context {
@@ -299,7 +393,7 @@ func TestPipelineRunDefaulting(t *testing.T) {
  Name: config.GetDefaultsConfigName(),
  },
  Data: map[string]string{
- "default-timeout-minutes": "5",
+ "default-timeout-minutes": strconv.Itoa(defaultTimeoutMinutes),
  "default-service-account": "tekton",
  "default-pod-template": "nodeSelector: { 'label': 'value' }\nhostNetwork: true",
  },