Skip to content

Commit 284fbf4

Browse files
XinruZhangXinruZhang
committed
Keeps Deprecated Fields in Step and StepTemplate When Switching Versions
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.
1 parent d6c0f69 commit 284fbf4

12 files changed

+706
-231
lines changed

‎pkg/apis/pipeline/v1beta1/container_conversion.go

-4
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,6 @@ func (s Step) convertTo(ctx context.Context, sink *v1.Step) {
4747
sink.OnError = (v1.OnErrorType)(s.OnError)
4848
sink.StdoutConfig = (*v1.StepOutputConfig)(s.StdoutConfig)
4949
sink.StderrConfig = (*v1.StepOutputConfig)(s.StderrConfig)
50-
51-
// TODO(#4546): Handle deprecated fields
52-
// Ports, LivenessProbe, ReadinessProbe, StartupProbe, Lifecycle, TerminationMessagePath
53-
// TerminationMessagePolicy, Stdin, StdinOnce, TTY
5450
}
5551

5652
func (s *Step) convertFrom(ctx context.Context, source v1.Step) {

‎pkg/apis/pipeline/v1beta1/pipeline_conversion.go

+18-16
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ import (
2020
"context"
2121
"fmt"
2222

23+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
24+
2325
v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
2426
"knative.dev/pkg/apis"
2527
)
@@ -34,20 +36,20 @@ func (p *Pipeline) ConvertTo(ctx context.Context, to apis.Convertible) error {
3436
switch sink := to.(type) {
3537
case *v1.Pipeline:
3638
sink.ObjectMeta = p.ObjectMeta
37-
return p.Spec.ConvertTo(ctx, &sink.Spec)
39+
return p.Spec.ConvertTo(ctx, &sink.Spec, &sink.ObjectMeta)
3840
default:
3941
return fmt.Errorf("unknown version, got: %T", sink)
4042
}
4143
}
4244

4345
// ConvertTo implements apis.Convertible
44-
func (ps *PipelineSpec) ConvertTo(ctx context.Context, sink *v1.PipelineSpec) error {
46+
func (ps *PipelineSpec) ConvertTo(ctx context.Context, sink *v1.PipelineSpec, meta *metav1.ObjectMeta) error {
4547
sink.DisplayName = ps.DisplayName
4648
sink.Description = ps.Description
4749
sink.Tasks = nil
4850
for _, t := range ps.Tasks {
4951
new := v1.PipelineTask{}
50-
err := t.convertTo(ctx, &new)
52+
err := t.convertTo(ctx, &new, meta)
5153
if err != nil {
5254
return err
5355
}
@@ -74,7 +76,7 @@ func (ps *PipelineSpec) ConvertTo(ctx context.Context, sink *v1.PipelineSpec) er
7476
sink.Finally = nil
7577
for _, f := range ps.Finally {
7678
new := v1.PipelineTask{}
77-
err := f.convertTo(ctx, &new)
79+
err := f.convertTo(ctx, &new, meta)
7880
if err != nil {
7981
return err
8082
}
@@ -88,20 +90,20 @@ func (p *Pipeline) ConvertFrom(ctx context.Context, from apis.Convertible) error
8890
switch source := from.(type) {
8991
case *v1.Pipeline:
9092
p.ObjectMeta = source.ObjectMeta
91-
return p.Spec.ConvertFrom(ctx, &source.Spec)
93+
return p.Spec.ConvertFrom(ctx, &source.Spec, &p.ObjectMeta)
9294
default:
9395
return fmt.Errorf("unknown version, got: %T", p)
9496
}
9597
}
9698

9799
// ConvertFrom implements apis.Convertible
98-
func (ps *PipelineSpec) ConvertFrom(ctx context.Context, source *v1.PipelineSpec) error {
100+
func (ps *PipelineSpec) ConvertFrom(ctx context.Context, source *v1.PipelineSpec, meta *metav1.ObjectMeta) error {
99101
ps.DisplayName = source.DisplayName
100102
ps.Description = source.Description
101103
ps.Tasks = nil
102104
for _, t := range source.Tasks {
103105
new := PipelineTask{}
104-
err := new.convertFrom(ctx, t)
106+
err := new.convertFrom(ctx, t, meta)
105107
if err != nil {
106108
return err
107109
}
@@ -128,7 +130,7 @@ func (ps *PipelineSpec) ConvertFrom(ctx context.Context, source *v1.PipelineSpec
128130
ps.Finally = nil
129131
for _, f := range source.Finally {
130132
new := PipelineTask{}
131-
err := new.convertFrom(ctx, f)
133+
err := new.convertFrom(ctx, f, meta)
132134
if err != nil {
133135
return err
134136
}
@@ -137,7 +139,7 @@ func (ps *PipelineSpec) ConvertFrom(ctx context.Context, source *v1.PipelineSpec
137139
return nil
138140
}
139141

140-
func (pt PipelineTask) convertTo(ctx context.Context, sink *v1.PipelineTask) error {
142+
func (pt PipelineTask) convertTo(ctx context.Context, sink *v1.PipelineTask, meta *metav1.ObjectMeta) error {
141143
sink.Name = pt.Name
142144
sink.DisplayName = pt.DisplayName
143145
sink.Description = pt.Description
@@ -147,7 +149,7 @@ func (pt PipelineTask) convertTo(ctx context.Context, sink *v1.PipelineTask) err
147149
}
148150
if pt.TaskSpec != nil {
149151
sink.TaskSpec = &v1.EmbeddedTask{}
150-
err := pt.TaskSpec.convertTo(ctx, sink.TaskSpec)
152+
err := pt.TaskSpec.convertTo(ctx, sink.TaskSpec, meta, pt.Name)
151153
if err != nil {
152154
return err
153155
}
@@ -183,7 +185,7 @@ func (pt PipelineTask) convertTo(ctx context.Context, sink *v1.PipelineTask) err
183185
return nil
184186
}
185187

186-
func (pt *PipelineTask) convertFrom(ctx context.Context, source v1.PipelineTask) error {
188+
func (pt *PipelineTask) convertFrom(ctx context.Context, source v1.PipelineTask, meta *metav1.ObjectMeta) error {
187189
pt.Name = source.Name
188190
pt.DisplayName = source.DisplayName
189191
pt.Description = source.Description
@@ -194,7 +196,7 @@ func (pt *PipelineTask) convertFrom(ctx context.Context, source v1.PipelineTask)
194196
}
195197
if source.TaskSpec != nil {
196198
newTaskSpec := EmbeddedTask{}
197-
err := newTaskSpec.convertFrom(ctx, *source.TaskSpec)
199+
err := newTaskSpec.convertFrom(ctx, *source.TaskSpec, meta, pt.Name)
198200
pt.TaskSpec = &newTaskSpec
199201
if err != nil {
200202
return err
@@ -231,20 +233,20 @@ func (pt *PipelineTask) convertFrom(ctx context.Context, source v1.PipelineTask)
231233
return nil
232234
}
233235

234-
func (et EmbeddedTask) convertTo(ctx context.Context, sink *v1.EmbeddedTask) error {
236+
func (et EmbeddedTask) convertTo(ctx context.Context, sink *v1.EmbeddedTask, meta *metav1.ObjectMeta, taskName string) error {
235237
sink.TypeMeta = et.TypeMeta
236238
sink.Spec = et.Spec
237239
sink.Metadata = v1.PipelineTaskMetadata(et.Metadata)
238240
sink.TaskSpec = v1.TaskSpec{}
239-
return et.TaskSpec.ConvertTo(ctx, &sink.TaskSpec)
241+
return et.TaskSpec.ConvertTo(ctx, &sink.TaskSpec, meta, taskName)
240242
}
241243

242-
func (et *EmbeddedTask) convertFrom(ctx context.Context, source v1.EmbeddedTask) error {
244+
func (et *EmbeddedTask) convertFrom(ctx context.Context, source v1.EmbeddedTask, meta *metav1.ObjectMeta, taskName string) error {
243245
et.TypeMeta = source.TypeMeta
244246
et.Spec = source.Spec
245247
et.Metadata = PipelineTaskMetadata(source.Metadata)
246248
et.TaskSpec = TaskSpec{}
247-
return et.TaskSpec.ConvertFrom(ctx, &source.TaskSpec)
249+
return et.TaskSpec.ConvertFrom(ctx, &source.TaskSpec, meta, taskName)
248250
}
249251

250252
func (we WhenExpression) convertTo(ctx context.Context, sink *v1.WhenExpression) {

‎pkg/apis/pipeline/v1beta1/pipeline_conversion_test.go

+83-23
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ import (
2121
"testing"
2222
"time"
2323

24+
corev1 "k8s.io/api/core/v1"
25+
2426
"github.com/google/go-cmp/cmp"
2527
v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
2628
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
@@ -43,7 +45,7 @@ func TestPipelineConversionBadType(t *testing.T) {
4345
}
4446

4547
func TestPipelineConversion(t *testing.T) {
46-
tests := []struct {
48+
for _, test := range []struct {
4749
name string
4850
in *v1beta1.Pipeline
4951
}{{
@@ -67,6 +69,64 @@ func TestPipelineConversion(t *testing.T) {
6769
}},
6870
},
6971
},
72+
}, {
73+
name: "pipeline with deprecated fields in step and stepTemplate",
74+
in: &v1beta1.Pipeline{
75+
ObjectMeta: metav1.ObjectMeta{
76+
Name: "foo",
77+
Namespace: "bar",
78+
},
79+
Spec: v1beta1.PipelineSpec{
80+
Tasks: []v1beta1.PipelineTask{{
81+
Name: "task-1",
82+
TaskSpec: &v1beta1.EmbeddedTask{
83+
TaskSpec: v1beta1.TaskSpec{
84+
Steps: []v1beta1.Step{{
85+
DeprecatedLivenessProbe: &corev1.Probe{InitialDelaySeconds: 1},
86+
DeprecatedReadinessProbe: &corev1.Probe{InitialDelaySeconds: 2},
87+
DeprecatedPorts: []corev1.ContainerPort{{Name: "port"}},
88+
DeprecatedStartupProbe: &corev1.Probe{InitialDelaySeconds: 3},
89+
DeprecatedLifecycle: &corev1.Lifecycle{PostStart: &corev1.LifecycleHandler{Exec: &corev1.ExecAction{
90+
Command: []string{"lifecycle command"},
91+
}}},
92+
DeprecatedTerminationMessagePath: "path",
93+
DeprecatedTerminationMessagePolicy: corev1.TerminationMessagePolicy("policy"),
94+
DeprecatedStdin: true,
95+
DeprecatedStdinOnce: true,
96+
DeprecatedTTY: true,
97+
}},
98+
StepTemplate: &v1beta1.StepTemplate{
99+
DeprecatedName: "deprecated-step-template",
100+
DeprecatedLivenessProbe: &corev1.Probe{InitialDelaySeconds: 1},
101+
DeprecatedReadinessProbe: &corev1.Probe{InitialDelaySeconds: 2},
102+
DeprecatedPorts: []corev1.ContainerPort{{Name: "port"}},
103+
DeprecatedStartupProbe: &corev1.Probe{InitialDelaySeconds: 3},
104+
DeprecatedLifecycle: &corev1.Lifecycle{PostStart: &corev1.LifecycleHandler{Exec: &corev1.ExecAction{
105+
Command: []string{"lifecycle command"},
106+
}}},
107+
DeprecatedTerminationMessagePath: "path",
108+
DeprecatedTerminationMessagePolicy: corev1.TerminationMessagePolicy("policy"),
109+
DeprecatedStdin: true,
110+
DeprecatedStdinOnce: true,
111+
DeprecatedTTY: true,
112+
},
113+
},
114+
},
115+
}, {
116+
Name: "task-2",
117+
TaskSpec: &v1beta1.EmbeddedTask{
118+
TaskSpec: v1beta1.TaskSpec{
119+
Steps: []v1beta1.Step{{
120+
DeprecatedLivenessProbe: &corev1.Probe{InitialDelaySeconds: 1},
121+
}},
122+
StepTemplate: &v1beta1.StepTemplate{
123+
DeprecatedLivenessProbe: &corev1.Probe{InitialDelaySeconds: 1},
124+
},
125+
},
126+
},
127+
}},
128+
},
129+
},
70130
}, {
71131
name: "pipeline conversion all non deprecated fields",
72132
in: &v1beta1.Pipeline{
@@ -160,27 +220,27 @@ func TestPipelineConversion(t *testing.T) {
160220
}},
161221
},
162222
},
163-
}}
164-
165-
for _, test := range tests {
166-
versions := []apis.Convertible{&v1.Pipeline{}}
167-
for _, version := range versions {
168-
t.Run(test.name, func(t *testing.T) {
169-
ver := version
170-
if err := test.in.ConvertTo(context.Background(), ver); err != nil {
171-
t.Errorf("ConvertTo() = %v", err)
172-
return
173-
}
174-
t.Logf("ConvertTo() = %#v", ver)
175-
got := &v1beta1.Pipeline{}
176-
if err := got.ConvertFrom(context.Background(), ver); err != nil {
177-
t.Errorf("ConvertFrom() = %v", err)
178-
}
179-
t.Logf("ConvertFrom() = %#v", got)
180-
if d := cmp.Diff(test.in, got); d != "" {
181-
t.Errorf("roundtrip %s", diff.PrintWantGot(d))
182-
}
183-
})
184-
}
223+
}} {
224+
t.Run(test.name, func(t *testing.T) {
225+
versions := []apis.Convertible{&v1.Pipeline{}}
226+
for _, version := range versions {
227+
t.Run(test.name, func(t *testing.T) {
228+
ver := version
229+
if err := test.in.ConvertTo(context.Background(), ver); err != nil {
230+
t.Errorf("ConvertTo() = %v", err)
231+
return
232+
}
233+
t.Logf("ConvertTo() = %#v", ver)
234+
got := &v1beta1.Pipeline{}
235+
if err := got.ConvertFrom(context.Background(), ver); err != nil {
236+
t.Errorf("ConvertFrom() = %v", err)
237+
}
238+
t.Logf("ConvertFrom() = %#v", got)
239+
if d := cmp.Diff(test.in, got); d != "" {
240+
t.Errorf("roundtrip %s", diff.PrintWantGot(d))
241+
}
242+
})
243+
}
244+
})
185245
}
186246
}

‎pkg/apis/pipeline/v1beta1/pipelinerun_conversion.go

+14-12
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ import (
2020
"context"
2121
"fmt"
2222

23+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
24+
2325
v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
2426
"knative.dev/pkg/apis"
2527
)
@@ -34,24 +36,24 @@ func (pr *PipelineRun) ConvertTo(ctx context.Context, to apis.Convertible) error
3436
switch sink := to.(type) {
3537
case *v1.PipelineRun:
3638
sink.ObjectMeta = pr.ObjectMeta
37-
if err := pr.Status.convertTo(ctx, &sink.Status); err != nil {
39+
if err := pr.Status.convertTo(ctx, &sink.Status, &sink.ObjectMeta); err != nil {
3840
return err
3941
}
40-
return pr.Spec.ConvertTo(ctx, &sink.Spec)
42+
return pr.Spec.ConvertTo(ctx, &sink.Spec, &sink.ObjectMeta)
4143
default:
4244
return fmt.Errorf("unknown version, got: %T", sink)
4345
}
4446
}
4547

4648
// ConvertTo implements apis.Convertible
47-
func (prs PipelineRunSpec) ConvertTo(ctx context.Context, sink *v1.PipelineRunSpec) error {
49+
func (prs PipelineRunSpec) ConvertTo(ctx context.Context, sink *v1.PipelineRunSpec, meta *metav1.ObjectMeta) error {
4850
if prs.PipelineRef != nil {
4951
sink.PipelineRef = &v1.PipelineRef{}
5052
prs.PipelineRef.convertTo(ctx, sink.PipelineRef)
5153
}
5254
if prs.PipelineSpec != nil {
5355
sink.PipelineSpec = &v1.PipelineSpec{}
54-
err := prs.PipelineSpec.ConvertTo(ctx, sink.PipelineSpec)
56+
err := prs.PipelineSpec.ConvertTo(ctx, sink.PipelineSpec, meta)
5557
if err != nil {
5658
return err
5759
}
@@ -94,25 +96,25 @@ func (pr *PipelineRun) ConvertFrom(ctx context.Context, from apis.Convertible) e
9496
switch source := from.(type) {
9597
case *v1.PipelineRun:
9698
pr.ObjectMeta = source.ObjectMeta
97-
if err := pr.Status.convertFrom(ctx, &source.Status); err != nil {
99+
if err := pr.Status.convertFrom(ctx, &source.Status, &pr.ObjectMeta); err != nil {
98100
return err
99101
}
100-
return pr.Spec.ConvertFrom(ctx, &source.Spec)
102+
return pr.Spec.ConvertFrom(ctx, &source.Spec, &pr.ObjectMeta)
101103
default:
102104
return fmt.Errorf("unknown version, got: %T", pr)
103105
}
104106
}
105107

106108
// ConvertFrom implements apis.Convertible
107-
func (prs *PipelineRunSpec) ConvertFrom(ctx context.Context, source *v1.PipelineRunSpec) error {
109+
func (prs *PipelineRunSpec) ConvertFrom(ctx context.Context, source *v1.PipelineRunSpec, meta *metav1.ObjectMeta) error {
108110
if source.PipelineRef != nil {
109111
newPipelineRef := PipelineRef{}
110112
newPipelineRef.convertFrom(ctx, *source.PipelineRef)
111113
prs.PipelineRef = &newPipelineRef
112114
}
113115
if source.PipelineSpec != nil {
114116
newPipelineSpec := PipelineSpec{}
115-
err := newPipelineSpec.ConvertFrom(ctx, source.PipelineSpec)
117+
err := newPipelineSpec.ConvertFrom(ctx, source.PipelineSpec, meta)
116118
if err != nil {
117119
return err
118120
}
@@ -206,7 +208,7 @@ func (ptrs *PipelineTaskRunSpec) convertFrom(ctx context.Context, source v1.Pipe
206208
ptrs.ComputeResources = source.ComputeResources
207209
}
208210

209-
func (prs *PipelineRunStatus) convertTo(ctx context.Context, sink *v1.PipelineRunStatus) error {
211+
func (prs *PipelineRunStatus) convertTo(ctx context.Context, sink *v1.PipelineRunStatus, meta *metav1.ObjectMeta) error {
210212
sink.Status = prs.Status
211213
sink.StartTime = prs.StartTime
212214
sink.CompletionTime = prs.CompletionTime
@@ -218,7 +220,7 @@ func (prs *PipelineRunStatus) convertTo(ctx context.Context, sink *v1.PipelineRu
218220
}
219221
if prs.PipelineSpec != nil {
220222
sink.PipelineSpec = &v1.PipelineSpec{}
221-
err := prs.PipelineSpec.ConvertTo(ctx, sink.PipelineSpec)
223+
err := prs.PipelineSpec.ConvertTo(ctx, sink.PipelineSpec, meta)
222224
if err != nil {
223225
return err
224226
}
@@ -244,7 +246,7 @@ func (prs *PipelineRunStatus) convertTo(ctx context.Context, sink *v1.PipelineRu
244246
return nil
245247
}
246248

247-
func (prs *PipelineRunStatus) convertFrom(ctx context.Context, source *v1.PipelineRunStatus) error {
249+
func (prs *PipelineRunStatus) convertFrom(ctx context.Context, source *v1.PipelineRunStatus, meta *metav1.ObjectMeta) error {
248250
prs.Status = source.Status
249251
prs.StartTime = source.StartTime
250252
prs.CompletionTime = source.CompletionTime
@@ -256,7 +258,7 @@ func (prs *PipelineRunStatus) convertFrom(ctx context.Context, source *v1.Pipeli
256258
}
257259
if source.PipelineSpec != nil {
258260
newPipelineSpec := PipelineSpec{}
259-
err := newPipelineSpec.ConvertFrom(ctx, source.PipelineSpec)
261+
err := newPipelineSpec.ConvertFrom(ctx, source.PipelineSpec, meta)
260262
if err != nil {
261263
return err
262264
}

0 commit comments

Comments
 (0)