diff --git a/pkg/apis/pipeline/v1alpha1/contexts.go b/pkg/apis/pipeline/v1alpha1/contexts.go index 7edc042d5de..1b7daa482b3 100644 --- a/pkg/apis/pipeline/v1alpha1/contexts.go +++ b/pkg/apis/pipeline/v1alpha1/contexts.go @@ -28,3 +28,27 @@ type hdcnKey struct{} func WithDefaultConfigurationName(ctx context.Context) context.Context { return context.WithValue(ctx, hdcnKey{}, struct{}{}) } + +// HasDefaultConfigurationName checks to see whether the given context has +// been marked as having a default configurationName. +func HasDefaultConfigurationName(ctx context.Context) bool { + return ctx.Value(hdcnKey{}) != nil +} + +// lemonadeKey is used as the key for associating information +// with a context.Context. This variable doesn't really matter, so it's +// a total random name (for history purpose, used lemonade as it was written +// in an hot summer day). +type lemonadeKey struct{} + +// WithUpgradeViaDefaulting notes on the context that we want defaulting to rewrite +// from v1alpha1 pre-defaults to v1alpha1 post-defaults. +func WithUpgradeViaDefaulting(ctx context.Context) context.Context { + return context.WithValue(ctx, lemonadeKey{}, struct{}{}) +} + +// IsUpgradeViaDefaulting checks whether we should be "defaulting" from v1alpha1 pre-defaults to +// the v1alpha1 post-defaults subset. +func IsUpgradeViaDefaulting(ctx context.Context) bool { + return ctx.Value(lemonadeKey{}) != nil +} diff --git a/pkg/apis/pipeline/v1alpha1/pipelinerun_defaults.go b/pkg/apis/pipeline/v1alpha1/pipelinerun_defaults.go index b3b05ebd975..b24760aebbf 100644 --- a/pkg/apis/pipeline/v1alpha1/pipelinerun_defaults.go +++ b/pkg/apis/pipeline/v1alpha1/pipelinerun_defaults.go @@ -31,6 +31,15 @@ func (pr *PipelineRun) SetDefaults(ctx context.Context) { func (prs *PipelineRunSpec) SetDefaults(ctx context.Context) { cfg := config.FromContextOrDefaults(ctx) if prs.Timeout == nil { - prs.Timeout = &metav1.Duration{Duration: time.Duration(cfg.Defaults.DefaultTimeoutMinutes) * time.Minute} + var timeout *metav1.Duration + if IsUpgradeViaDefaulting(ctx) { + // This case is for preexisting `TaskRun` before 0.5.0, so let's + // add the old default timeout. + // Most likely those TaskRun passing here are already done and/or already running + timeout = &metav1.Duration{Duration: config.DefaultTimeoutMinutes * time.Minute} + } else { + timeout = &metav1.Duration{Duration: time.Duration(cfg.Defaults.DefaultTimeoutMinutes) * time.Minute} + } + prs.Timeout = timeout } } diff --git a/pkg/apis/pipeline/v1alpha1/pipelinerun_defaults_test.go b/pkg/apis/pipeline/v1alpha1/pipelinerun_defaults_test.go index d8337125deb..38545291206 100644 --- a/pkg/apis/pipeline/v1alpha1/pipelinerun_defaults_test.go +++ b/pkg/apis/pipeline/v1alpha1/pipelinerun_defaults_test.go @@ -22,9 +22,16 @@ import ( "time" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + logtesting "knative.dev/pkg/logging/testing" +) + +var ( + ignoreUnexportedResources = cmpopts.IgnoreUnexported() ) func TestPipelineRunSpec_SetDefaults(t *testing.T) { @@ -60,4 +67,75 @@ func TestPipelineRunSpec_SetDefaults(t *testing.T) { } }) } + +} + +func TestPipelineRunDefaulting(t *testing.T) { + tests := []struct { + name string + in *v1alpha1.PipelineRun + want *v1alpha1.PipelineRun + wc func(context.Context) context.Context + }{{ + name: "empty no context", + in: &v1alpha1.PipelineRun{}, + want: &v1alpha1.PipelineRun{ + Spec: v1alpha1.PipelineRunSpec{ + Timeout: &metav1.Duration{Duration: config.DefaultTimeoutMinutes * time.Minute}, + }, + }, + }, { + name: "PipelineRef upgrade context", + in: &v1alpha1.PipelineRun{ + Spec: v1alpha1.PipelineRunSpec{ + PipelineRef: v1alpha1.PipelineRef{Name: "foo"}, + }, + }, + want: &v1alpha1.PipelineRun{ + Spec: v1alpha1.PipelineRunSpec{ + PipelineRef: v1alpha1.PipelineRef{Name: "foo"}, + Timeout: &metav1.Duration{Duration: config.DefaultTimeoutMinutes * time.Minute}, + }, + }, + wc: v1alpha1.WithUpgradeViaDefaulting, + }, { + name: "PipelineRef default config context", + in: &v1alpha1.PipelineRun{ + Spec: v1alpha1.PipelineRunSpec{ + PipelineRef: v1alpha1.PipelineRef{Name: "foo"}, + }, + }, + want: &v1alpha1.PipelineRun{ + Spec: v1alpha1.PipelineRunSpec{ + PipelineRef: v1alpha1.PipelineRef{Name: "foo"}, + Timeout: &metav1.Duration{Duration: 5 * time.Minute}, + }, + }, + wc: func(ctx context.Context) context.Context { + s := config.NewStore(logtesting.TestLogger(t)) + s.OnConfigChanged(&corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: config.DefaultsConfigName, + }, + Data: map[string]string{ + "default-timeout-minutes": "5", + }, + }) + return s.ToContext(ctx) + }, + }} + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := tc.in + ctx := context.Background() + if tc.wc != nil { + ctx = tc.wc(ctx) + } + got.SetDefaults(ctx) + if !cmp.Equal(got, tc.want, ignoreUnexportedResources) { + t.Errorf("SetDefaults (-want, +got) = %v", + cmp.Diff(got, tc.want, ignoreUnexportedResources)) + } + }) + } } diff --git a/pkg/apis/pipeline/v1alpha1/taskrun_defaults.go b/pkg/apis/pipeline/v1alpha1/taskrun_defaults.go index e130966da78..ee92487efb1 100644 --- a/pkg/apis/pipeline/v1alpha1/taskrun_defaults.go +++ b/pkg/apis/pipeline/v1alpha1/taskrun_defaults.go @@ -29,11 +29,21 @@ func (tr *TaskRun) SetDefaults(ctx context.Context) { } func (trs *TaskRunSpec) SetDefaults(ctx context.Context) { + cfg := config.FromContextOrDefaults(ctx) if trs.TaskRef != nil && trs.TaskRef.Kind == "" { trs.TaskRef.Kind = NamespacedTaskKind } - cfg := config.FromContextOrDefaults(ctx) + if trs.Timeout == nil { - trs.Timeout = &metav1.Duration{Duration: time.Duration(cfg.Defaults.DefaultTimeoutMinutes) * time.Minute} + var timeout *metav1.Duration + if IsUpgradeViaDefaulting(ctx) { + // This case is for preexisting `TaskRun` before 0.5.0, so let's + // add the old default timeout. + // Most likely those TaskRun passing here are already done and/or already running + timeout = &metav1.Duration{Duration: config.DefaultTimeoutMinutes * time.Minute} + } else { + timeout = &metav1.Duration{Duration: time.Duration(cfg.Defaults.DefaultTimeoutMinutes) * time.Minute} + } + trs.Timeout = timeout } } diff --git a/pkg/apis/pipeline/v1alpha1/taskrun_defaults_test.go b/pkg/apis/pipeline/v1alpha1/taskrun_defaults_test.go index 6ef455ac78f..4f82997244a 100644 --- a/pkg/apis/pipeline/v1alpha1/taskrun_defaults_test.go +++ b/pkg/apis/pipeline/v1alpha1/taskrun_defaults_test.go @@ -24,7 +24,9 @@ import ( "github.com/google/go-cmp/cmp" "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + logtesting "knative.dev/pkg/logging/testing" ) func TestTaskRunSpec_SetDefaults(t *testing.T) { @@ -77,3 +79,86 @@ func TestTaskRunSpec_SetDefaults(t *testing.T) { }) } } + +func TestTaskRunDefaulting(t *testing.T) { + tests := []struct { + name string + in *v1alpha1.TaskRun + want *v1alpha1.TaskRun + wc func(context.Context) context.Context + }{{ + name: "empty no context", + in: &v1alpha1.TaskRun{}, + want: &v1alpha1.TaskRun{ + Spec: v1alpha1.TaskRunSpec{ + Timeout: &metav1.Duration{Duration: config.DefaultTimeoutMinutes * time.Minute}, + }, + }, + }, { + name: "TaskRef default to namespace kind", + in: &v1alpha1.TaskRun{ + Spec: v1alpha1.TaskRunSpec{ + TaskRef: &v1alpha1.TaskRef{Name: "foo"}, + }, + }, + want: &v1alpha1.TaskRun{ + Spec: v1alpha1.TaskRunSpec{ + TaskRef: &v1alpha1.TaskRef{Name: "foo", Kind: v1alpha1.NamespacedTaskKind}, + Timeout: &metav1.Duration{Duration: config.DefaultTimeoutMinutes * time.Minute}, + }, + }, + }, { + name: "TaskRef upgrade context", + in: &v1alpha1.TaskRun{ + Spec: v1alpha1.TaskRunSpec{ + TaskRef: &v1alpha1.TaskRef{Name: "foo"}, + }, + }, + want: &v1alpha1.TaskRun{ + Spec: v1alpha1.TaskRunSpec{ + TaskRef: &v1alpha1.TaskRef{Name: "foo", Kind: v1alpha1.NamespacedTaskKind}, + Timeout: &metav1.Duration{Duration: config.DefaultTimeoutMinutes * time.Minute}, + }, + }, + wc: v1alpha1.WithUpgradeViaDefaulting, + }, { + name: "TaskRef default config context", + in: &v1alpha1.TaskRun{ + Spec: v1alpha1.TaskRunSpec{ + TaskRef: &v1alpha1.TaskRef{Name: "foo"}, + }, + }, + want: &v1alpha1.TaskRun{ + Spec: v1alpha1.TaskRunSpec{ + TaskRef: &v1alpha1.TaskRef{Name: "foo", Kind: v1alpha1.NamespacedTaskKind}, + Timeout: &metav1.Duration{Duration: 5 * time.Minute}, + }, + }, + wc: func(ctx context.Context) context.Context { + s := config.NewStore(logtesting.TestLogger(t)) + s.OnConfigChanged(&corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: config.DefaultsConfigName, + }, + Data: map[string]string{ + "default-timeout-minutes": "5", + }, + }) + return s.ToContext(ctx) + }, + }} + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := tc.in + ctx := context.Background() + if tc.wc != nil { + ctx = tc.wc(ctx) + } + got.SetDefaults(ctx) + if !cmp.Equal(got, tc.want, ignoreUnexportedResources) { + t.Errorf("SetDefaults (-want, +got) = %v", + cmp.Diff(got, tc.want, ignoreUnexportedResources)) + } + }) + } +} diff --git a/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go b/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go index 37ee58dac84..cfa42cc10fb 100644 --- a/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go @@ -195,6 +195,10 @@ func (c *Reconciler) Reconcile(ctx context.Context, key string) error { } func (c *Reconciler) reconcile(ctx context.Context, pr *v1alpha1.PipelineRun) error { + // We may be reading a version of the object that was stored at an older version + // and may not have had all of the assumed default specified. + pr.SetDefaults(v1alpha1.WithUpgradeViaDefaulting(ctx)) + p, err := c.pipelineLister.Pipelines(pr.Namespace).Get(pr.Spec.PipelineRef.Name) if err != nil { // This Run has failed, so we need to mark it as failed and stop reconciling it diff --git a/pkg/reconciler/v1alpha1/taskrun/taskrun.go b/pkg/reconciler/v1alpha1/taskrun/taskrun.go index 7cee5d53505..e43f09ee721 100644 --- a/pkg/reconciler/v1alpha1/taskrun/taskrun.go +++ b/pkg/reconciler/v1alpha1/taskrun/taskrun.go @@ -173,6 +173,10 @@ func (c *Reconciler) getTaskFunc(tr *v1alpha1.TaskRun) (resources.GetTask, v1alp } func (c *Reconciler) reconcile(ctx context.Context, tr *v1alpha1.TaskRun) error { + // We may be reading a version of the object that was stored at an older version + // and may not have had all of the assumed default specified. + tr.SetDefaults(v1alpha1.WithUpgradeViaDefaulting(ctx)) + // If the taskrun is cancelled, kill resources and update status if tr.IsCancelled() { before := tr.Status.GetCondition(apis.ConditionSucceeded)