Skip to content

Commit

Permalink
adding finally type at the pipeline level
Browse files Browse the repository at this point in the history
these changes are adding finally type and its validation, does not
implement this new functionality.
  • Loading branch information
pritidesai committed May 29, 2020
1 parent 3554a30 commit a692b8b
Show file tree
Hide file tree
Showing 9 changed files with 877 additions and 224 deletions.
4 changes: 4 additions & 0 deletions pkg/apis/pipeline/v1alpha1/conversion_error.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ const (
// resources when they cannot be converted to warn of a forthcoming
// breakage.
ConditionTypeConvertible apis.ConditionType = v1beta1.ConditionTypeConvertible
// Conversion Error Field for finally
ConversionErrorFinally = "Finally"
// Conversion Error descriptive message for finally
ConversionErrorFinallyMsg = ConversionErrorFinally + " is not available in v1alpha1"
)

// CannotConvertError is returned when a field cannot be converted.
Expand Down
5 changes: 5 additions & 0 deletions pkg/apis/pipeline/v1alpha1/pipeline_conversion.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ func (source *PipelineSpec) ConvertTo(ctx context.Context, sink *v1beta1.Pipelin
}
}
}
sink.Finally = nil
return nil
}

Expand Down Expand Up @@ -97,6 +98,10 @@ func (sink *PipelineSpec) ConvertFrom(ctx context.Context, source v1beta1.Pipeli
}
}
}
// finally clause was introduced in v1beta1 and not available in v1alpha1
if len(source.Finally) > 0 {
return ConvertErrorf(ConversionErrorFinally, ConversionErrorFinallyMsg)
}
return nil
}

Expand Down
119 changes: 98 additions & 21 deletions pkg/apis/pipeline/v1alpha1/pipeline_conversion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,12 @@ func TestPipelineConversionBadType(t *testing.T) {
}
}

func TestPipelineConversion(t *testing.T) {
func TestPipelineConversion_Success(t *testing.T) {
versions := []apis.Convertible{&v1beta1.Pipeline{}}

tests := []struct {
name string
in *Pipeline
wantErr bool
name string
in *Pipeline
}{{
name: "simple conversion",
in: &Pipeline{
Expand Down Expand Up @@ -114,7 +113,38 @@ func TestPipelineConversion(t *testing.T) {
}},
},
},
}, {
}}

for _, test := range tests {
for _, version := range versions {
t.Run(test.name, func(t *testing.T) {
ver := version
// convert v1alpha1 Pipeline to v1beta1 Pipeline
if err := test.in.ConvertTo(context.Background(), ver); err != nil {
t.Errorf("ConvertTo() = %v", err)
}
got := &Pipeline{}
// converting it back to v1alpha1 pipeline and storing it in got variable to compare with original input
if err := got.ConvertFrom(context.Background(), ver); err != nil {
t.Errorf("ConvertFrom() = %v", err)
}
// compare origin input and roundtrip Pipeline i.e. v1alpha1 pipeline converted to v1beta1 and then converted back to v1alpha1
// this check is making sure that we do not end up with different object than what we started with
if d := cmp.Diff(test.in, got); d != "" {
t.Errorf("roundtrip %s", diff.PrintWantGot(d))
}
})
}
}
}

func TestPipelineConversion_Failure(t *testing.T) {
versions := []apis.Convertible{&v1beta1.Pipeline{}}

tests := []struct {
name string
in *Pipeline
}{{
name: "simple conversion with task spec error",
in: &Pipeline{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -152,29 +182,76 @@ func TestPipelineConversion(t *testing.T) {
}},
},
},
wantErr: true,
}}

for _, test := range tests {
for _, version := range versions {
t.Run(test.name, func(t *testing.T) {
ver := version
if err := test.in.ConvertTo(context.Background(), ver); err != nil {
if !test.wantErr {
t.Errorf("ConvertTo() = %v", err)
}
return
}
t.Logf("ConvertTo() = %#v", ver)
got := &Pipeline{}
if err := got.ConvertFrom(context.Background(), ver); err != nil {
t.Errorf("ConvertFrom() = %v", err)
}
t.Logf("ConvertFrom() = %#v", got)
if d := cmp.Diff(test.in, got); d != "" {
t.Errorf("roundtrip %s", diff.PrintWantGot(d))
if err := test.in.ConvertTo(context.Background(), ver); err == nil {
t.Errorf("Expected ConvertTo to fail but did not produce any error")
}
return
})
}
}
}

func TestPipelineConversionFromWithFinally(t *testing.T) {
versions := []apis.Convertible{&v1beta1.Pipeline{}}
p := &Pipeline{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Namespace: "bar",
Generation: 1,
},
Spec: PipelineSpec{
Tasks: []PipelineTask{{Name: "mytask", TaskRef: &TaskRef{Name: "task"}}},
},
}
for _, version := range versions {
t.Run("finally not available in v1alpha1", func(t *testing.T) {
ver := version
// convert v1alpha1 to v1beta1
if err := p.ConvertTo(context.Background(), ver); err != nil {
t.Errorf("ConvertTo() = %v", err)
}
// modify ver to introduce new field which causes failure to convert v1beta1 to v1alpha1
source := ver
source.(*v1beta1.Pipeline).Spec.Finally = []v1beta1.PipelineTask{{Name: "finaltask", TaskRef: &TaskRef{Name: "task"}}}
got := &Pipeline{}
if err := got.ConvertFrom(context.Background(), source); err != nil {
cce, ok := err.(*CannotConvertError)
// conversion error contains the field name which resulted in the failure and should be equal to "Finally" here
if ok && cce.Field == ConversionErrorFinally {
return
}
t.Errorf("ConvertFrom() should have failed")
}
})
}
}

func TestPipelineConversionFromBetaToAlphaWithFinally_Failure(t *testing.T) {
p := &v1beta1.Pipeline{
ObjectMeta: metav1.ObjectMeta{
Name: "foo",
Namespace: "bar",
Generation: 1,
},
Spec: v1beta1.PipelineSpec{
Tasks: []v1beta1.PipelineTask{{Name: "mytask", TaskRef: &TaskRef{Name: "task"}}},
Finally: []v1beta1.PipelineTask{{Name: "mytask", TaskRef: &TaskRef{Name: "task"}}},
},
}
t.Run("finally not available in v1alpha1", func(t *testing.T) {
got := &Pipeline{}
if err := got.ConvertFrom(context.Background(), p); err != nil {
cce, ok := err.(*CannotConvertError)
// conversion error contains the field name which resulted in the failure and should be equal to "Finally" here
if ok && cce.Field == ConversionErrorFinally {
return
}
t.Errorf("ConvertFrom() should have failed")
}
})
}
10 changes: 10 additions & 0 deletions pkg/apis/pipeline/v1beta1/pipeline_defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,14 @@ func (ps *PipelineSpec) SetDefaults(ctx context.Context) {
for i := range ps.Params {
ps.Params[i].SetDefaults(ctx)
}
for _, ft := range ps.Finally {
if ft.TaskRef != nil {
if ft.TaskRef.Kind == "" {
ft.TaskRef.Kind = NamespacedTaskKind
}
}
if ft.TaskSpec != nil {
ft.TaskSpec.SetDefaults(ctx)
}
}
}
138 changes: 138 additions & 0 deletions pkg/apis/pipeline/v1beta1/pipeline_defaults_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
/*
Copyright 2020 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 v1beta1_test

import (
"context"
"testing"

"github.com/tektoncd/pipeline/test/diff"

"github.com/google/go-cmp/cmp"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
)

func TestPipeline_SetDefaults(t *testing.T) {
p := &v1beta1.Pipeline{}
want := &v1beta1.Pipeline{}
t.Run("set defaults for empty pipeline", func(t *testing.T) {
ctx := context.Background()
p.SetDefaults(ctx)
if d := cmp.Diff(want, p); d != "" {
t.Errorf("Mismatch of Pipeline: empty pipeline must not change after setting defaults: %s", diff.PrintWantGot(d))
}
})
}

func TestPipelineSpec_SetDefaults(t *testing.T) {
cases := []struct {
desc string
ps *v1beta1.PipelineSpec
want *v1beta1.PipelineSpec
}{{
desc: "empty pipelineSpec must not change after setting defaults",
ps: &v1beta1.PipelineSpec{},
want: &v1beta1.PipelineSpec{},
}, {
desc: "pipeline task - default task kind must be " + string(v1beta1.NamespacedTaskKind),
ps: &v1beta1.PipelineSpec{
Tasks: []v1beta1.PipelineTask{{
Name: "foo", TaskRef: &v1beta1.TaskRef{Name: "foo-task"},
}},
},
want: &v1beta1.PipelineSpec{
Tasks: []v1beta1.PipelineTask{{
Name: "foo", TaskRef: &v1beta1.TaskRef{Name: "foo-task", Kind: v1beta1.NamespacedTaskKind},
}},
},
}, {
desc: "final pipeline task - default task kind must be " + string(v1beta1.NamespacedTaskKind),
ps: &v1beta1.PipelineSpec{
Finally: []v1beta1.PipelineTask{{
Name: "final-task", TaskRef: &v1beta1.TaskRef{Name: "foo-task"},
}},
},
want: &v1beta1.PipelineSpec{
Finally: []v1beta1.PipelineTask{{
Name: "final-task", TaskRef: &v1beta1.TaskRef{Name: "foo-task", Kind: v1beta1.NamespacedTaskKind},
}},
},
}, {
desc: "param type - default param type must be " + string(v1beta1.ParamTypeString),
ps: &v1beta1.PipelineSpec{
Params: []v1beta1.ParamSpec{{
Name: "string-param",
}},
},
want: &v1beta1.PipelineSpec{
Params: []v1beta1.ParamSpec{{
Name: "string-param", Type: v1beta1.ParamTypeString,
}},
},
}, {
desc: "pipeline task with taskSpec - default param type must be " + string(v1beta1.ParamTypeString),
ps: &v1beta1.PipelineSpec{
Tasks: []v1beta1.PipelineTask{{
Name: "foo", TaskSpec: &v1beta1.TaskSpec{
Params: []v1beta1.ParamSpec{{
Name: "string-param",
}},
},
}},
},
want: &v1beta1.PipelineSpec{
Tasks: []v1beta1.PipelineTask{{
Name: "foo", TaskSpec: &v1beta1.TaskSpec{
Params: []v1beta1.ParamSpec{{
Name: "string-param",
Type: v1beta1.ParamTypeString,
}},
},
}},
},
}, {
desc: "final pipeline task with taskSpec - default param type must be " + string(v1beta1.ParamTypeString),
ps: &v1beta1.PipelineSpec{
Finally: []v1beta1.PipelineTask{{
Name: "foo", TaskSpec: &v1beta1.TaskSpec{
Params: []v1beta1.ParamSpec{{
Name: "string-param",
}},
},
}},
},
want: &v1beta1.PipelineSpec{
Finally: []v1beta1.PipelineTask{{
Name: "foo", TaskSpec: &v1beta1.TaskSpec{
Params: []v1beta1.ParamSpec{{
Name: "string-param",
Type: v1beta1.ParamTypeString,
}},
},
}},
},
}}
for _, tc := range cases {
t.Run(tc.desc, func(t *testing.T) {
ctx := context.Background()
tc.ps.SetDefaults(ctx)
if d := cmp.Diff(tc.want, tc.ps); d != "" {
t.Errorf("Mismatch of pipelineSpec after setting defaults: %s: %s", tc.desc, diff.PrintWantGot(d))
}
})
}
}
4 changes: 4 additions & 0 deletions pkg/apis/pipeline/v1beta1/pipeline_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@ type PipelineSpec struct {
// Results are values that this pipeline can output once run
// +optional
Results []PipelineResult `json:"results,omitempty"`
// Finally declares the list of Tasks that execute just before leaving the Pipeline
// i.e. either after all Tasks are finished executing successfully
// or after a failure which would result in ending the Pipeline
Finally []PipelineTask `json:"finally,omitempty"`
}

// PipelineResult used to describe the results of a pipeline
Expand Down
Loading

0 comments on commit a692b8b

Please sign in to comment.