diff --git a/tekton-catalog/pipeline-loops/cmd/cli/main.go b/tekton-catalog/pipeline-loops/cmd/cli/main.go index 8341fbf0df..79f8ba4ab0 100644 --- a/tekton-catalog/pipeline-loops/cmd/cli/main.go +++ b/tekton-catalog/pipeline-loops/cmd/cli/main.go @@ -25,6 +25,7 @@ import ( "os" "strings" + "github.com/kubeflow/kfp-tekton/tekton-catalog/pipeline-loops/pkg/apis/pipelineloop" pipelineloopv1alpha1 "github.com/kubeflow/kfp-tekton/tekton-catalog/pipeline-loops/pkg/apis/pipelineloop/v1alpha1" "github.com/kubeflow/kfp-tekton/tekton-catalog/pipeline-loops/pkg/reconciler/pipelinelooprun" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" @@ -90,7 +91,7 @@ func main() { err = validateRun(marshalledBytes) case "Pipeline": err = validatePipeline(marshalledBytes) - case "PipelineLoop": + case pipelineloop.PipelineLoopControllerName: err = validatePipelineLoop(marshalledBytes) case "PipelineRun": err = validatePipelineRun(marshalledBytes) @@ -126,7 +127,7 @@ func validatePipelineSpec(p *v1beta1.PipelineSpec, name string) error { // Here we only need to validate those embedded spec, whose kind is pipelineLoop. if p.Tasks != nil { for _, task := range p.Tasks { - if task.TaskSpec != nil && task.TaskSpec.Kind == "PipelineLoop" { + if task.TaskSpec != nil && task.TaskSpec.Kind == pipelineloop.PipelineLoopControllerName { err := validatePipelineLoopEmbedded(task.TaskSpec.Spec.Raw) if err != nil { errs = append(errs, err.Error()) @@ -150,7 +151,7 @@ func validateRun(bytes []byte) error { // We do not need to validate Run because it is validated by tekton admission webhook // And r.Spec.Ref is also validated by tekton. // Here we only need to validate the embedded spec. i.e. r.Spec.Spec - if r.Spec.Spec != nil && r.Spec.Spec.Kind == "PipelineLoop" { + if r.Spec.Spec != nil && r.Spec.Spec.Kind == pipelineloop.PipelineLoopControllerName { if err := validatePipelineLoopEmbedded(r.Spec.Spec.Spec.Raw); err != nil { return fmt.Errorf("Found validation errors in Run: %s \n %s", r.Name, err.Error()) } @@ -173,8 +174,8 @@ func validatePipelineLoopEmbedded(bytes []byte) error { return err } r1 := map[string]interface{}{ - "kind": "PipelineLoop", - "apiVersion": "custom.tekton.dev/v1alpha1", + "kind": pipelineloop.PipelineLoopControllerName, + "apiVersion": pipelineloopv1alpha1.SchemeGroupVersion.String(), "metadata": metav1.ObjectMeta{Name: "embedded"}, "spec": embeddedSpec, } @@ -205,7 +206,7 @@ func validatePipelineLoop(bytes []byte) error { func validateNestedPipelineLoop(pl pipelineloopv1alpha1.PipelineLoop) (error, string) { for _, task := range pl.Spec.PipelineSpec.Tasks { - if task.TaskSpec != nil && task.TaskSpec.Kind == "PipelineLoop" { + if task.TaskSpec != nil && task.TaskSpec.Kind == pipelineloop.PipelineLoopControllerName { err := validatePipelineLoopEmbedded(task.TaskSpec.Spec.Raw) if err != nil { return err, task.Name diff --git a/tekton-catalog/pipeline-loops/pkg/reconciler/pipelinelooprun/pipelinelooprun_test.go b/tekton-catalog/pipeline-loops/pkg/reconciler/pipelinelooprun/pipelinelooprun_test.go index 5b3d42de83..65171d478b 100644 --- a/tekton-catalog/pipeline-loops/pkg/reconciler/pipelinelooprun/pipelinelooprun_test.go +++ b/tekton-catalog/pipeline-loops/pkg/reconciler/pipelinelooprun/pipelinelooprun_test.go @@ -18,6 +18,7 @@ package pipelinelooprun import ( "context" + "encoding/json" "fmt" "strings" "testing" @@ -37,6 +38,7 @@ import ( "github.com/tektoncd/pipeline/test/names" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" ktesting "k8s.io/client-go/testing" "k8s.io/client-go/tools/record" @@ -322,6 +324,64 @@ var paraPipeline = &v1beta1.Pipeline{ }, } +func getInnerLoopByte(pipeline *v1beta1.Pipeline) []byte { + innerLoop, err := json.Marshal(pipeline) + if err != nil { + fmt.Println(fmt.Errorf("error while marshalling pipelineLoop %s", err.Error()).Error()) + } + return innerLoop +} + +var nestedPipeline = &v1beta1.Pipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "nestedPipeline", Namespace: "foo"}, + Spec: v1beta1.PipelineSpec{ + Params: []v1beta1.ParamSpec{{ + Name: "iteration", + Type: v1beta1.ParamTypeString, + }, { + Name: "additional-parameter", + Type: v1beta1.ParamTypeString, + }}, + Tasks: []v1beta1.PipelineTask{{ + Name: "mytask", + TaskSpec: &v1beta1.EmbeddedTask{ + TypeMeta: runtime.TypeMeta{ + APIVersion: pipelineloopv1alpha1.SchemeGroupVersion.String(), + Kind: pipelineloop.PipelineLoopControllerName, + }, + Spec: runtime.RawExtension{ + Raw: getInnerLoopByte(paraPipeline), + }, + }, + }}, + }, +} + +var nestedPipeline2 = &v1beta1.Pipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "nestedPipeline2", Namespace: "foo"}, + Spec: v1beta1.PipelineSpec{ + Params: []v1beta1.ParamSpec{{ + Name: "iteration", + Type: v1beta1.ParamTypeString, + }, { + Name: "additional-parameter", + Type: v1beta1.ParamTypeString, + }}, + Tasks: []v1beta1.PipelineTask{{ + Name: "mytask", + TaskSpec: &v1beta1.EmbeddedTask{ + TypeMeta: runtime.TypeMeta{ + APIVersion: pipelineloopv1alpha1.SchemeGroupVersion.String(), + Kind: pipelineloop.PipelineLoopControllerName, + }, + Spec: runtime.RawExtension{ + Raw: getInnerLoopByte(nestedPipeline), + }, + }, + }}, + }, +} + var paraPipelineLoop = &pipelineloopv1alpha1.PipelineLoop{ ObjectMeta: metav1.ObjectMeta{Name: "para-pipelineloop", Namespace: "foo"}, Spec: pipelineloopv1alpha1.PipelineLoopSpec{ @@ -339,6 +399,14 @@ var nPipelineLoop = &pipelineloopv1alpha1.PipelineLoop{ }, } +var nestedPipelineLoop = &pipelineloopv1alpha1.PipelineLoop{ + ObjectMeta: metav1.ObjectMeta{Name: "nested-pipelineloop", Namespace: "foo"}, + Spec: pipelineloopv1alpha1.PipelineLoopSpec{ + PipelineSpec: &nestedPipeline2.Spec, + IterateNumeric: "iteration", + }, +} + var aPipelineLoopWithInlineTask = &pipelineloopv1alpha1.PipelineLoop{ ObjectMeta: metav1.ObjectMeta{Name: "a-pipelineloop-with-inline-task", Namespace: "foo"}, Spec: pipelineloopv1alpha1.PipelineLoopSpec{ @@ -397,6 +465,37 @@ var runPipelineLoop = &v1alpha1.Run{ }, } +var nestedRunPipelineLoop = &v1alpha1.Run{ + ObjectMeta: metav1.ObjectMeta{ + Name: "run-pipelineloop", + Namespace: "foo", + Labels: map[string]string{ + "myTestLabel": "myTestLabelValue", + }, + Annotations: map[string]string{ + "myTestAnnotation": "myTestAnnotationValue", + }, + }, + Spec: v1alpha1.RunSpec{ + Params: []v1beta1.Param{{ + Name: "current-item", + Value: v1beta1.ArrayOrString{Type: v1beta1.ParamTypeArray, ArrayVal: []string{"item1", "item2"}}, + }, { + Name: "additional-parameter", + Value: v1beta1.ArrayOrString{Type: v1beta1.ParamTypeString, StringVal: "stuff"}, + }}, + Spec: &v1alpha1.EmbeddedRunSpec{ + TypeMeta: runtime.TypeMeta{ + APIVersion: pipelineloopv1alpha1.SchemeGroupVersion.String(), + Kind: pipelineloop.PipelineLoopControllerName, + }, + Spec: runtime.RawExtension{ + Raw: getInnerLoopByte(nestedPipeline2), + }, + }, + }, +} + var paraRunPipelineLoop = &v1alpha1.Run{ ObjectMeta: metav1.ObjectMeta{ Name: "run-pipelineloop", @@ -936,6 +1035,39 @@ var expectedPipelineRunWithInlineTaskIteration1 = &v1beta1.PipelineRun{ Timeout: &metav1.Duration{Duration: 5 * time.Minute}, }, } +var expectedNestedPipelineRun = &v1beta1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "run-pipelineloop-00001-9l9zj", + Namespace: "foo", + OwnerReferences: []metav1.OwnerReference{{ + APIVersion: "tekton.dev/v1alpha1", + Kind: "Run", + Name: "run-pipelineloop", + Controller: &trueB, + BlockOwnerDeletion: &trueB, + }}, + Labels: map[string]string{ + "custom.tekton.dev/parentPipelineRun": "", + "custom.tekton.dev/pipelineLoop": "nested-pipelineloop", + "tekton.dev/run": "run-pipelineloop", + "custom.tekton.dev/pipelineLoopIteration": "1", + "myTestLabel": "myTestLabelValue", + }, + Annotations: map[string]string{ + "myTestAnnotation": "myTestAnnotationValue", + }, + }, + Spec: v1beta1.PipelineRunSpec{ + PipelineSpec: &nestedPipeline2.Spec, + Params: []v1beta1.Param{{ + Name: "current-item", + Value: v1beta1.ArrayOrString{Type: v1beta1.ParamTypeString, StringVal: "item1"}, + }, { + Name: "additional-parameter", + Value: v1beta1.ArrayOrString{Type: v1beta1.ParamTypeString, StringVal: "stuff"}, + }}, + }, +} func TestReconcilePipelineLoopRun(t *testing.T) { @@ -1078,6 +1210,16 @@ func TestReconcilePipelineLoopRun(t *testing.T) { expectedReason: pipelineloopv1alpha1.PipelineLoopRunReasonRunning, expectedPipelineruns: []*v1beta1.PipelineRun{expectedParaPipelineRun, expectedParaPipelineRun1}, expectedEvents: []string{"Normal Started", "Normal Running Iterations completed: 0"}, + }, { + name: "Reconcile a new run with a nested pipelineloop", + pipeline: nestedPipeline2, + pipelineloop: nestedPipelineLoop, + run: nestedRunPipelineLoop, + pipelineruns: []*v1beta1.PipelineRun{}, + expectedStatus: corev1.ConditionUnknown, + expectedReason: pipelineloopv1alpha1.PipelineLoopRunReasonRunning, + expectedPipelineruns: []*v1beta1.PipelineRun{expectedNestedPipelineRun}, + expectedEvents: []string{"Normal Started", "Normal Running Iterations completed: 0"}, }} for _, tc := range testcases {