Skip to content

Commit fce91f0

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 fce91f0

14 files changed

+737
-222
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/openapi_generated.go

+41
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎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
}

0 commit comments

Comments
 (0)