From 52f03a56ba1509ce250fbc3773971e5535f9b460 Mon Sep 17 00:00:00 2001 From: Lee Bernick Date: Fri, 29 Jul 2022 12:23:43 -0400 Subject: [PATCH] V1: Add conversion for Task.Resources This commit adds support for Task.Resources when converting between v1beta1 and v1 versions of Tasks. This will allow us to release v1 Task in a backwards compatible way, by ensuring that v1beta1 Tasks with Resources will have the Resources serialized into annotations on the v1 Task on conversion. --- pkg/apis/pipeline/v1beta1/task_conversion.go | 30 +++++++++- .../pipeline/v1beta1/task_conversion_test.go | 17 +++--- pkg/apis/version/conversion.go | 57 +++++++++++++++++++ pkg/apis/version/conversion_test.go | 54 ++++++++++++++++++ 4 files changed, 150 insertions(+), 8 deletions(-) create mode 100644 pkg/apis/version/conversion.go create mode 100644 pkg/apis/version/conversion_test.go diff --git a/pkg/apis/pipeline/v1beta1/task_conversion.go b/pkg/apis/pipeline/v1beta1/task_conversion.go index 7c0a65010ea..ee0e8bd7b13 100644 --- a/pkg/apis/pipeline/v1beta1/task_conversion.go +++ b/pkg/apis/pipeline/v1beta1/task_conversion.go @@ -21,9 +21,13 @@ import ( "fmt" v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" + "github.com/tektoncd/pipeline/pkg/apis/version" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "knative.dev/pkg/apis" ) +const resourcesAnnotationKey = "tekton.dev/v1beta1Resources" + var _ apis.Convertible = (*Task)(nil) // ConvertTo implements apis.Convertible @@ -34,6 +38,9 @@ func (t *Task) ConvertTo(ctx context.Context, to apis.Convertible) error { switch sink := to.(type) { case *v1.Task: sink.ObjectMeta = t.ObjectMeta + if err := serializeResources(&sink.ObjectMeta, &t.Spec); err != nil { + return err + } return t.Spec.ConvertTo(ctx, &sink.Spec) default: return fmt.Errorf("unknown version, got: %T", sink) @@ -79,7 +86,6 @@ func (ts *TaskSpec) ConvertTo(ctx context.Context, sink *v1.TaskSpec) error { sink.Params = append(sink.Params, new) } sink.Description = ts.Description - // TODO(#4546): Handle Resources return nil } @@ -91,6 +97,9 @@ func (t *Task) ConvertFrom(ctx context.Context, from apis.Convertible) error { switch source := from.(type) { case *v1.Task: t.ObjectMeta = source.ObjectMeta + if err := deserializeResources(&t.ObjectMeta, &t.Spec); err != nil { + return err + } return t.Spec.ConvertFrom(ctx, &source.Spec) default: return fmt.Errorf("unknown version, got: %T", t) @@ -138,3 +147,22 @@ func (ts *TaskSpec) ConvertFrom(ctx context.Context, source *v1.TaskSpec) error ts.Description = source.Description return nil } + +func serializeResources(meta *metav1.ObjectMeta, spec *TaskSpec) error { + if spec.Resources == nil { + return nil + } + return version.SerializeToMetadata(meta, spec.Resources, resourcesAnnotationKey) +} + +func deserializeResources(meta *metav1.ObjectMeta, spec *TaskSpec) error { + resources := &TaskResources{} + err := version.DeserializeFromMetadata(meta, resources, resourcesAnnotationKey) + if err != nil { + return err + } + if resources.Inputs != nil || resources.Outputs != nil { + spec.Resources = resources + } + return nil +} diff --git a/pkg/apis/pipeline/v1beta1/task_conversion_test.go b/pkg/apis/pipeline/v1beta1/task_conversion_test.go index 7c99582191f..d5896e6cb0e 100644 --- a/pkg/apis/pipeline/v1beta1/task_conversion_test.go +++ b/pkg/apis/pipeline/v1beta1/task_conversion_test.go @@ -187,9 +187,6 @@ func TestTaskConversion(t *testing.T) { } func TestTaskConversionFromDeprecated(t *testing.T) { - // TODO(#4546): We're just dropping Resources when converting from - // v1beta1 to v1. Before moving the stored version to v1, we should - // come up with a better strategy versions := []apis.Convertible{&v1.Task{}} tests := []struct { name string @@ -205,7 +202,7 @@ func TestTaskConversionFromDeprecated(t *testing.T) { }, Spec: v1beta1.TaskSpec{ Resources: &v1beta1.TaskResources{ - Inputs: []v1beta1.TaskResource{}, + Inputs: []v1beta1.TaskResource{{v1beta1.ResourceDeclaration{Name: "input-resource"}}}, }, }, }, @@ -215,7 +212,11 @@ func TestTaskConversionFromDeprecated(t *testing.T) { Namespace: "bar", Generation: 1, }, - Spec: v1beta1.TaskSpec{}, + Spec: v1beta1.TaskSpec{ + Resources: &v1beta1.TaskResources{ + Inputs: []v1beta1.TaskResource{{v1beta1.ResourceDeclaration{Name: "input-resource"}}}, + }, + }, }, }, { name: "output resources", @@ -227,7 +228,7 @@ func TestTaskConversionFromDeprecated(t *testing.T) { }, Spec: v1beta1.TaskSpec{ Resources: &v1beta1.TaskResources{ - Outputs: []v1beta1.TaskResource{}, + Outputs: []v1beta1.TaskResource{{v1beta1.ResourceDeclaration{Name: "output-resource"}}}, }, }, }, @@ -237,7 +238,9 @@ func TestTaskConversionFromDeprecated(t *testing.T) { Namespace: "bar", Generation: 1, }, - Spec: v1beta1.TaskSpec{}, + Spec: v1beta1.TaskSpec{Resources: &v1beta1.TaskResources{ + Outputs: []v1beta1.TaskResource{{v1beta1.ResourceDeclaration{Name: "output-resource"}}}, + }}, }, }} for _, test := range tests { diff --git a/pkg/apis/version/conversion.go b/pkg/apis/version/conversion.go new file mode 100644 index 00000000000..f32ca9a944b --- /dev/null +++ b/pkg/apis/version/conversion.go @@ -0,0 +1,57 @@ +/* +Copyright 2022 The Tekton Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package version + +import ( + "encoding/json" + "fmt" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// SerializeToMetadata serializes the input field and adds it as an annotation to +// the metadata under the input key. +func SerializeToMetadata(meta *metav1.ObjectMeta, field interface{}, key string) error { + bytes, err := json.Marshal(field) + if err != nil { + return fmt.Errorf("error serializing field: %s", err) + } + if meta.Annotations == nil { + meta.Annotations = make(map[string]string) + } + meta.Annotations[key] = string(bytes) + return nil +} + +// DeserializeFromMetadata takes the value of the input key from the metadata's annotations, +// deserializes it into "to", and removes the key from the metadata's annotations. +// Returns nil if the key is not present in the annotations. +func DeserializeFromMetadata(meta *metav1.ObjectMeta, to interface{}, key string) error { + if meta.Annotations == nil { + return nil + } + if str, ok := meta.Annotations[key]; ok { + if err := json.Unmarshal([]byte(str), to); err != nil { + return fmt.Errorf("error deserializing key %s from metadata: %s", key, err) + } + delete(meta.Annotations, key) + if len(meta.Annotations) == 0 { + meta.Annotations = nil + } + } + return nil +} diff --git a/pkg/apis/version/conversion_test.go b/pkg/apis/version/conversion_test.go new file mode 100644 index 00000000000..b2e127a5ec6 --- /dev/null +++ b/pkg/apis/version/conversion_test.go @@ -0,0 +1,54 @@ +/* +Copyright 2022 The Tekton Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package version_test + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/tektoncd/pipeline/pkg/apis/version" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +type testStruct struct { + Field string +} + +func TestSerializationRoundTrip(t *testing.T) { + meta := metav1.ObjectMeta{} + source := testStruct{Field: "foo"} + key := "my-key" + err := version.SerializeToMetadata(&meta, source, key) + if err != nil { + t.Fatalf("Serialization error: %s", err) + } + + sink := testStruct{} + err = version.DeserializeFromMetadata(&meta, &sink, key) + if err != nil { + t.Fatalf("Deserialization error: %s", err) + } + + _, ok := meta.Annotations[key] + if ok { + t.Errorf("Expected key %s not to be present in annotations but it was", key) + } + + if d := cmp.Diff(source, sink); d != "" { + t.Errorf("Unexpected diff after serialization/deserialization round trip: %s", d) + } +}