Skip to content

Commit

Permalink
Keeps Deprecated Fields in Step and StepTemplate When Switching Versions
Browse files Browse the repository at this point in the history 
Prior to this commit, we have information loss when converting the
step and steptemplate from v1beta1 to v1. This commit preserves
those information by serializing steps and steptemplate, saving
them in object annoations when converting from v1beta1 to v1, then
deserializing when converting back.
  • Loading branch information
@XinruZhang
XinruZhang committed May 4, 2023
1 parent 9b82287 commit 378609b
Show file tree
Hide file tree
Showing 9 changed files with 507 additions and 234 deletions.
33 changes: 17 additions & 16 deletions pkg/apis/pipeline/v1beta1/pipeline_conversion.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package v1beta1
import (
"context"
"fmt"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
"knative.dev/pkg/apis"
Expand All @@ -34,20 +35,20 @@ func (p *Pipeline) ConvertTo(ctx context.Context, to apis.Convertible) error {
switch sink := to.(type) {
case *v1.Pipeline:
sink.ObjectMeta = p.ObjectMeta
return p.Spec.ConvertTo(ctx, &sink.Spec)
return p.Spec.ConvertTo(ctx, &sink.Spec, &sink.ObjectMeta)
default:
return fmt.Errorf("unknown version, got: %T", sink)
}
}

// ConvertTo implements apis.Convertible
func (ps *PipelineSpec) ConvertTo(ctx context.Context, sink *v1.PipelineSpec) error {
func (ps *PipelineSpec) ConvertTo(ctx context.Context, sink *v1.PipelineSpec, meta *metav1.ObjectMeta) error {
sink.DisplayName = ps.DisplayName
sink.Description = ps.Description
sink.Tasks = nil
for _, t := range ps.Tasks {
new := v1.PipelineTask{}
err := t.convertTo(ctx, &new)
err := t.convertTo(ctx, &new, meta)
if err != nil {
return err
}
Expand All @@ -74,7 +75,7 @@ func (ps *PipelineSpec) ConvertTo(ctx context.Context, sink *v1.PipelineSpec) er
sink.Finally = nil
for _, f := range ps.Finally {
new := v1.PipelineTask{}
err := f.convertTo(ctx, &new)
err := f.convertTo(ctx, &new, meta)
if err != nil {
return err
}
Expand All @@ -88,20 +89,20 @@ func (p *Pipeline) ConvertFrom(ctx context.Context, from apis.Convertible) error
switch source := from.(type) {
case *v1.Pipeline:
p.ObjectMeta = source.ObjectMeta
return p.Spec.ConvertFrom(ctx, &source.Spec)
return p.Spec.ConvertFrom(ctx, &source.Spec, &p.ObjectMeta)
default:
return fmt.Errorf("unknown version, got: %T", p)
}
}

// ConvertFrom implements apis.Convertible
func (ps *PipelineSpec) ConvertFrom(ctx context.Context, source *v1.PipelineSpec) error {
func (ps *PipelineSpec) ConvertFrom(ctx context.Context, source *v1.PipelineSpec, meta *metav1.ObjectMeta) error {
ps.DisplayName = source.DisplayName
ps.Description = source.Description
ps.Tasks = nil
for _, t := range source.Tasks {
new := PipelineTask{}
err := new.convertFrom(ctx, t)
err := new.convertFrom(ctx, t, meta)
if err != nil {
return err
}
Expand All @@ -128,7 +129,7 @@ func (ps *PipelineSpec) ConvertFrom(ctx context.Context, source *v1.PipelineSpec
ps.Finally = nil
for _, f := range source.Finally {
new := PipelineTask{}
err := new.convertFrom(ctx, f)
err := new.convertFrom(ctx, f, meta)
if err != nil {
return err
}
Expand All @@ -137,7 +138,7 @@ func (ps *PipelineSpec) ConvertFrom(ctx context.Context, source *v1.PipelineSpec
return nil
}

func (pt PipelineTask) convertTo(ctx context.Context, sink *v1.PipelineTask) error {
func (pt PipelineTask) convertTo(ctx context.Context, sink *v1.PipelineTask, meta *metav1.ObjectMeta) error {
sink.Name = pt.Name
sink.DisplayName = pt.DisplayName
sink.Description = pt.Description
Expand All @@ -147,7 +148,7 @@ func (pt PipelineTask) convertTo(ctx context.Context, sink *v1.PipelineTask) err
}
if pt.TaskSpec != nil {
sink.TaskSpec = &v1.EmbeddedTask{}
err := pt.TaskSpec.convertTo(ctx, sink.TaskSpec)
err := pt.TaskSpec.convertTo(ctx, sink.TaskSpec, meta, pt.Name)
if err != nil {
return err
}
Expand Down Expand Up @@ -183,7 +184,7 @@ func (pt PipelineTask) convertTo(ctx context.Context, sink *v1.PipelineTask) err
return nil
}

func (pt *PipelineTask) convertFrom(ctx context.Context, source v1.PipelineTask) error {
func (pt *PipelineTask) convertFrom(ctx context.Context, source v1.PipelineTask, meta *metav1.ObjectMeta) error {
pt.Name = source.Name
pt.DisplayName = source.DisplayName
pt.Description = source.Description
Expand All @@ -194,7 +195,7 @@ func (pt *PipelineTask) convertFrom(ctx context.Context, source v1.PipelineTask)
}
if source.TaskSpec != nil {
newTaskSpec := EmbeddedTask{}
err := newTaskSpec.convertFrom(ctx, *source.TaskSpec)
err := newTaskSpec.convertFrom(ctx, *source.TaskSpec, meta, pt.Name)
pt.TaskSpec = &newTaskSpec
if err != nil {
return err
Expand Down Expand Up @@ -231,20 +232,20 @@ func (pt *PipelineTask) convertFrom(ctx context.Context, source v1.PipelineTask)
return nil
}

func (et EmbeddedTask) convertTo(ctx context.Context, sink *v1.EmbeddedTask) error {
func (et EmbeddedTask) convertTo(ctx context.Context, sink *v1.EmbeddedTask, meta *metav1.ObjectMeta, taskName string) error {
sink.TypeMeta = et.TypeMeta
sink.Spec = et.Spec
sink.Metadata = v1.PipelineTaskMetadata(et.Metadata)
sink.TaskSpec = v1.TaskSpec{}
return et.TaskSpec.ConvertTo(ctx, &sink.TaskSpec)
return et.TaskSpec.ConvertTo(ctx, &sink.TaskSpec, meta, taskName)
}

func (et *EmbeddedTask) convertFrom(ctx context.Context, source v1.EmbeddedTask) error {
func (et *EmbeddedTask) convertFrom(ctx context.Context, source v1.EmbeddedTask, meta *metav1.ObjectMeta, taskName string) error {
et.TypeMeta = source.TypeMeta
et.Spec = source.Spec
et.Metadata = PipelineTaskMetadata(source.Metadata)
et.TaskSpec = TaskSpec{}
return et.TaskSpec.ConvertFrom(ctx, &source.TaskSpec)
return et.TaskSpec.ConvertFrom(ctx, &source.TaskSpec, meta, taskName)
}

func (we WhenExpression) convertTo(ctx context.Context, sink *v1.WhenExpression) {
Expand Down
58 changes: 58 additions & 0 deletions pkg/apis/pipeline/v1beta1/pipeline_conversion_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package v1beta1_test

import (
"context"
corev1 "k8s.io/api/core/v1"
"testing"
"time"

Expand Down Expand Up @@ -67,6 +68,63 @@ func TestPipelineConversion(t *testing.T) {
}},
},
},
}, {
name: "pipeline with deprecated fields in step and stepTemplate",
in: &v1beta1.Pipeline{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Namespace: "bar",
},
Spec: v1beta1.PipelineSpec{
Tasks: []v1beta1.PipelineTask{{
Name: "task-1",
TaskSpec: &v1beta1.EmbeddedTask{
TaskSpec: v1beta1.TaskSpec{
Steps: []v1beta1.Step{{
DeprecatedLivenessProbe: &corev1.Probe{InitialDelaySeconds: 1},
DeprecatedReadinessProbe: &corev1.Probe{InitialDelaySeconds: 2},
DeprecatedPorts: []corev1.ContainerPort{{Name: "port"}},
DeprecatedStartupProbe: &corev1.Probe{InitialDelaySeconds: 3},
DeprecatedLifecycle: &corev1.Lifecycle{PostStart: &corev1.LifecycleHandler{Exec: &corev1.ExecAction{
Command: []string{"lifecycle command"},
}}},
DeprecatedTerminationMessagePath: "path",
DeprecatedTerminationMessagePolicy: corev1.TerminationMessagePolicy("policy"),
DeprecatedStdin: true,
DeprecatedStdinOnce: true,
DeprecatedTTY: true,
}},
StepTemplate: &v1beta1.StepTemplate{
DeprecatedLivenessProbe: &corev1.Probe{InitialDelaySeconds: 1},
DeprecatedReadinessProbe: &corev1.Probe{InitialDelaySeconds: 2},
DeprecatedPorts: []corev1.ContainerPort{{Name: "port"}},
DeprecatedStartupProbe: &corev1.Probe{InitialDelaySeconds: 3},
DeprecatedLifecycle: &corev1.Lifecycle{PostStart: &corev1.LifecycleHandler{Exec: &corev1.ExecAction{
Command: []string{"lifecycle command"},
}}},
DeprecatedTerminationMessagePath: "path",
DeprecatedTerminationMessagePolicy: corev1.TerminationMessagePolicy("policy"),
DeprecatedStdin: true,
DeprecatedStdinOnce: true,
DeprecatedTTY: true,
},
},
},
}, {
Name: "task-2",
TaskSpec: &v1beta1.EmbeddedTask{
TaskSpec: v1beta1.TaskSpec{
Steps: []v1beta1.Step{{
DeprecatedLivenessProbe: &corev1.Probe{InitialDelaySeconds: 1},
}},
StepTemplate: &v1beta1.StepTemplate{
DeprecatedLivenessProbe: &corev1.Probe{InitialDelaySeconds: 1},
},
},
},
}},
},
},
}, {
name: "pipeline conversion all non deprecated fields",
in: &v1beta1.Pipeline{
Expand Down
25 changes: 13 additions & 12 deletions pkg/apis/pipeline/v1beta1/pipelinerun_conversion.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package v1beta1
import (
"context"
"fmt"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
"knative.dev/pkg/apis"
Expand All @@ -34,24 +35,24 @@ func (pr *PipelineRun) ConvertTo(ctx context.Context, to apis.Convertible) error
switch sink := to.(type) {
case *v1.PipelineRun:
sink.ObjectMeta = pr.ObjectMeta
if err := pr.Status.convertTo(ctx, &sink.Status); err != nil {
if err := pr.Status.convertTo(ctx, &sink.Status, &sink.ObjectMeta); err != nil {
return err
}
return pr.Spec.ConvertTo(ctx, &sink.Spec)
return pr.Spec.ConvertTo(ctx, &sink.Spec, &sink.ObjectMeta)
default:
return fmt.Errorf("unknown version, got: %T", sink)
}
}

// ConvertTo implements apis.Convertible
func (prs PipelineRunSpec) ConvertTo(ctx context.Context, sink *v1.PipelineRunSpec) error {
func (prs PipelineRunSpec) ConvertTo(ctx context.Context, sink *v1.PipelineRunSpec, meta *metav1.ObjectMeta) error {
if prs.PipelineRef != nil {
sink.PipelineRef = &v1.PipelineRef{}
prs.PipelineRef.convertTo(ctx, sink.PipelineRef)
}
if prs.PipelineSpec != nil {
sink.PipelineSpec = &v1.PipelineSpec{}
err := prs.PipelineSpec.ConvertTo(ctx, sink.PipelineSpec)
err := prs.PipelineSpec.ConvertTo(ctx, sink.PipelineSpec, meta)
if err != nil {
return err
}
Expand Down Expand Up @@ -94,25 +95,25 @@ func (pr *PipelineRun) ConvertFrom(ctx context.Context, from apis.Convertible) e
switch source := from.(type) {
case *v1.PipelineRun:
pr.ObjectMeta = source.ObjectMeta
if err := pr.Status.convertFrom(ctx, &source.Status); err != nil {
if err := pr.Status.convertFrom(ctx, &source.Status, &pr.ObjectMeta); err != nil {
return err
}
return pr.Spec.ConvertFrom(ctx, &source.Spec)
return pr.Spec.ConvertFrom(ctx, &source.Spec, &pr.ObjectMeta)
default:
return fmt.Errorf("unknown version, got: %T", pr)
}
}

// ConvertFrom implements apis.Convertible
func (prs *PipelineRunSpec) ConvertFrom(ctx context.Context, source *v1.PipelineRunSpec) error {
func (prs *PipelineRunSpec) ConvertFrom(ctx context.Context, source *v1.PipelineRunSpec, meta *metav1.ObjectMeta) error {
if source.PipelineRef != nil {
newPipelineRef := PipelineRef{}
newPipelineRef.convertFrom(ctx, *source.PipelineRef)
prs.PipelineRef = &newPipelineRef
}
if source.PipelineSpec != nil {
newPipelineSpec := PipelineSpec{}
err := newPipelineSpec.ConvertFrom(ctx, source.PipelineSpec)
err := newPipelineSpec.ConvertFrom(ctx, source.PipelineSpec, meta)
if err != nil {
return err
}
Expand Down Expand Up @@ -206,7 +207,7 @@ func (ptrs *PipelineTaskRunSpec) convertFrom(ctx context.Context, source v1.Pipe
ptrs.ComputeResources = source.ComputeResources
}

func (prs *PipelineRunStatus) convertTo(ctx context.Context, sink *v1.PipelineRunStatus) error {
func (prs *PipelineRunStatus) convertTo(ctx context.Context, sink *v1.PipelineRunStatus, meta *metav1.ObjectMeta) error {
sink.Status = prs.Status
sink.StartTime = prs.StartTime
sink.CompletionTime = prs.CompletionTime
Expand All @@ -218,7 +219,7 @@ func (prs *PipelineRunStatus) convertTo(ctx context.Context, sink *v1.PipelineRu
}
if prs.PipelineSpec != nil {
sink.PipelineSpec = &v1.PipelineSpec{}
err := prs.PipelineSpec.ConvertTo(ctx, sink.PipelineSpec)
err := prs.PipelineSpec.ConvertTo(ctx, sink.PipelineSpec, meta)
if err != nil {
return err
}
Expand All @@ -244,7 +245,7 @@ func (prs *PipelineRunStatus) convertTo(ctx context.Context, sink *v1.PipelineRu
return nil
}

func (prs *PipelineRunStatus) convertFrom(ctx context.Context, source *v1.PipelineRunStatus) error {
func (prs *PipelineRunStatus) convertFrom(ctx context.Context, source *v1.PipelineRunStatus, meta *metav1.ObjectMeta) error {
prs.Status = source.Status
prs.StartTime = source.StartTime
prs.CompletionTime = source.CompletionTime
Expand All @@ -256,7 +257,7 @@ func (prs *PipelineRunStatus) convertFrom(ctx context.Context, source *v1.Pipeli
}
if source.PipelineSpec != nil {
newPipelineSpec := PipelineSpec{}
err := newPipelineSpec.ConvertFrom(ctx, source.PipelineSpec)
err := newPipelineSpec.ConvertFrom(ctx, source.PipelineSpec, meta)
if err != nil {
return err
}
Expand Down
62 changes: 61 additions & 1 deletion pkg/apis/pipeline/v1beta1/pipelinerun_conversion_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,67 @@ func TestPipelineRunConversion(t *testing.T) {
Spec: v1beta1.PipelineRunSpec{
PipelineRef: &v1beta1.PipelineRef{Name: "pipeline-1"},
},
}}, {
},
}, {
name: "pipelinerun with with deprecated fields in step and stepTemplate",
in: &v1beta1.PipelineRun{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Namespace: "bar",
},
Spec: v1beta1.PipelineRunSpec{
PipelineSpec: &v1beta1.PipelineSpec{
Tasks: []v1beta1.PipelineTask{{
Name: "task-1",
TaskSpec: &v1beta1.EmbeddedTask{
TaskSpec: v1beta1.TaskSpec{
Steps: []v1beta1.Step{{
DeprecatedLivenessProbe: &corev1.Probe{InitialDelaySeconds: 1},
DeprecatedReadinessProbe: &corev1.Probe{InitialDelaySeconds: 2},
DeprecatedPorts: []corev1.ContainerPort{{Name: "port"}},
DeprecatedStartupProbe: &corev1.Probe{InitialDelaySeconds: 3},
DeprecatedLifecycle: &corev1.Lifecycle{PostStart: &corev1.LifecycleHandler{Exec: &corev1.ExecAction{
Command: []string{"lifecycle command"},
}}},
DeprecatedTerminationMessagePath: "path",
DeprecatedTerminationMessagePolicy: corev1.TerminationMessagePolicy("policy"),
DeprecatedStdin: true,
DeprecatedStdinOnce: true,
DeprecatedTTY: true,
}},
StepTemplate: &v1beta1.StepTemplate{
DeprecatedLivenessProbe: &corev1.Probe{InitialDelaySeconds: 1},
DeprecatedReadinessProbe: &corev1.Probe{InitialDelaySeconds: 2},
DeprecatedPorts: []corev1.ContainerPort{{Name: "port"}},
DeprecatedStartupProbe: &corev1.Probe{InitialDelaySeconds: 3},
DeprecatedLifecycle: &corev1.Lifecycle{PostStart: &corev1.LifecycleHandler{Exec: &corev1.ExecAction{
Command: []string{"lifecycle command"},
}}},
DeprecatedTerminationMessagePath: "path",
DeprecatedTerminationMessagePolicy: corev1.TerminationMessagePolicy("policy"),
DeprecatedStdin: true,
DeprecatedStdinOnce: true,
DeprecatedTTY: true,
},
},
},
}, {
Name: "task-2",
TaskSpec: &v1beta1.EmbeddedTask{
TaskSpec: v1beta1.TaskSpec{
Steps: []v1beta1.Step{{
DeprecatedLivenessProbe: &corev1.Probe{InitialDelaySeconds: 1},
}},
StepTemplate: &v1beta1.StepTemplate{
DeprecatedLivenessProbe: &corev1.Probe{InitialDelaySeconds: 1},
},
},
},
}},
},
},
},
}, {
name: "pipelinerun conversion all non deprecated fields",
in: &v1beta1.PipelineRun{
ObjectMeta: metav1.ObjectMeta{
Expand Down
Loading

0 comments on commit 378609b

Please sign in to comment.