Migrate/Upgrade previous version with a default timeout…

In case of a controller upgrade, we may still store previous versions of resource that may not have been the default set. This migrates those "in memory" to make sure we do not panic in those cases. This also gives a great example of how to use a mutating webhook to migrate some resources from one version to another. Signed-off-by: Vincent Demeester <[email protected]>
tektoncd · Aug 6, 2019 · fbfbfea · fbfbfea
1 parent e3f4f4a
commit fbfbfea
Show file tree

Hide file tree

Showing 7 changed files with 217 additions and 3 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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)