From 9ed844d4c8b35d7b291445c67c18e7f13486f169 Mon Sep 17 00:00:00 2001 From: Priti Desai Date: Fri, 12 Jun 2020 15:28:36 -0700 Subject: [PATCH] pipelinetask metadata MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adding metadata to TaskSpec in PipelineTask to allow specifying metadata. This metadata will be propogated to taskRun and then to the pods. ``` apiVersion: tekton.dev/v1beta1 kind: PipelineRun metadata: name: pipelinerun-with-taskspec-to-echo-greetings spec: pipelineSpec: tasks: - name: echo-greetings taskSpec: metadata: labels: [ …] steps: ... ``` Metadata is already supported as part of Tasks and Pipelines while respective CRDs are created. But was not possible to specify with embedded resources. --- docs/pipelineruns.md | 22 ++++ ...inerun-with-pipelinespec-and-taskspec.yaml | 3 + internal/builder/v1beta1/pipeline.go | 15 ++- internal/builder/v1beta1/pipeline_test.go | 40 +++++-- .../pipeline/v1alpha1/pipeline_conversion.go | 6 +- .../v1beta1/pipeline_defaults_test.go | 44 +++++--- pkg/apis/pipeline/v1beta1/pipeline_types.go | 23 +++- .../v1beta1/pipeline_validation_test.go | 60 ++++------ .../pipeline/v1beta1/zz_generated.deepcopy.go | 54 ++++++++- pkg/reconciler/pipelinerun/pipelinerun.go | 48 +++++++- .../pipelinerun/pipelinerun_test.go | 103 ++++++++++++++++++ .../resources/pipelinerunresolution.go | 2 +- test/retry_test.go | 4 +- 13 files changed, 347 insertions(+), 77 deletions(-) diff --git a/docs/pipelineruns.md b/docs/pipelineruns.md index 0625dd3ec73..46d14d602c0 100644 --- a/docs/pipelineruns.md +++ b/docs/pipelineruns.md @@ -119,6 +119,28 @@ spec: In the [`taskSpec` in `pipelineSpec` example](../examples/v1beta1/pipelineruns/pipelinerun-with-pipelinespec-and-taskspec.yaml) it's `Tasks` all the way down! +You can also specify labels and annotations with `taskSpec` which are propagated to each `taskRun` and then to the +respective pods. These labels can be used to identify and filter pods for further actions (such as collecting pod metrics, +and cleaning up completed pod with certain labels, etc) even being part of one single Pipeline. + +```yaml +spec: + pipelineSpec: + tasks: + - name: task1 + metadata: + labels: + pipeline-sdk-type: kfp + taskSpec: + ... + - name: task2 + metadata: + labels: + pipeline-sdk-type: tfx + taskSpec: + ... +``` + ## Specifying `Resources` A `Pipeline` requires [`PipelineResources`](resources.md) to provide inputs and store outputs diff --git a/examples/v1beta1/pipelineruns/pipelinerun-with-pipelinespec-and-taskspec.yaml b/examples/v1beta1/pipelineruns/pipelinerun-with-pipelinespec-and-taskspec.yaml index 652c18db13b..d935c418c7f 100644 --- a/examples/v1beta1/pipelineruns/pipelinerun-with-pipelinespec-and-taskspec.yaml +++ b/examples/v1beta1/pipelineruns/pipelinerun-with-pipelinespec-and-taskspec.yaml @@ -7,6 +7,9 @@ spec: tasks: - name: echo-good-morning taskSpec: + metadata: + labels: + app: "example" steps: - name: echo image: ubuntu diff --git a/internal/builder/v1beta1/pipeline.go b/internal/builder/v1beta1/pipeline.go index 54f0a1ca50c..68a764a120c 100644 --- a/internal/builder/v1beta1/pipeline.go +++ b/internal/builder/v1beta1/pipeline.go @@ -198,7 +198,20 @@ func PipelineRunResult(name, value string) PipelineRunStatusOp { // PipelineTaskSpec sets the TaskSpec on a PipelineTask. func PipelineTaskSpec(spec *v1beta1.TaskSpec) PipelineTaskOp { return func(pt *v1beta1.PipelineTask) { - pt.TaskSpec = spec + if pt.TaskSpec == nil { + pt.TaskSpec = &v1beta1.EmbeddedTask{} + } + pt.TaskSpec.TaskSpec = spec + } +} + +// TaskSpecMetadata sets the Metadata on a TaskSpec within PipelineTask. +func TaskSpecMetadata(metadata v1beta1.PipelineTaskMetadata) PipelineTaskOp { + return func(pt *v1beta1.PipelineTask) { + if pt.TaskSpec == nil { + pt.TaskSpec = &v1beta1.EmbeddedTask{} + } + pt.TaskSpec.Metadata = metadata } } diff --git a/internal/builder/v1beta1/pipeline_test.go b/internal/builder/v1beta1/pipeline_test.go index c2637f247bf..cc40e365b5b 100644 --- a/internal/builder/v1beta1/pipeline_test.go +++ b/internal/builder/v1beta1/pipeline_test.go @@ -57,12 +57,14 @@ func TestPipeline(t *testing.T) { tb.RunAfter("foo"), tb.PipelineTaskTimeout(5*time.Second), ), - tb.PipelineTask("foo", "", tb.PipelineTaskSpec(&v1beta1.TaskSpec{ - Steps: []v1beta1.Step{{Container: corev1.Container{ - Name: "step", - Image: "myimage", - }}}, - })), + tb.PipelineTask("foo", "", tb.PipelineTaskSpec(getTaskSpec())), + tb.PipelineTask("task-with-taskSpec", "", + tb.TaskSpecMetadata(v1beta1.PipelineTaskMetadata{ + Labels: map[string]string{"label": "labelvalue"}, + Annotations: map[string]string{"annotation": "annotationvalue"}}, + ), + tb.PipelineTaskSpec(getTaskSpec()), + ), tb.PipelineWorkspaceDeclaration("workspace1"), ), tb.PipelineCreationTimestamp(creationTime), @@ -137,13 +139,19 @@ func TestPipeline(t *testing.T) { Timeout: &metav1.Duration{Duration: 5 * time.Second}, }, { Name: "foo", - TaskSpec: &v1beta1.TaskSpec{ - Steps: []v1beta1.Step{{Container: corev1.Container{ - Name: "step", - Image: "myimage", - }}}, + TaskSpec: &v1beta1.EmbeddedTask{ + TaskSpec: getTaskSpec()}, + }, { + Name: "task-with-taskSpec", + TaskSpec: &v1beta1.EmbeddedTask{ + Metadata: v1beta1.PipelineTaskMetadata{ + Labels: map[string]string{"label": "labelvalue"}, + Annotations: map[string]string{"annotation": "annotationvalue"}, + }, + TaskSpec: getTaskSpec(), }, - }}, + }, + }, Workspaces: []v1beta1.PipelineWorkspaceDeclaration{{ Name: "workspace1", }}, @@ -426,5 +434,13 @@ func TestPipelineRunWithFinalTask(t *testing.T) { if diff := cmp.Diff(expectedPipelineRun, pipelineRun); diff != "" { t.Fatalf("PipelineRun diff -want, +got: %s", diff) } +} +func getTaskSpec() *v1beta1.TaskSpec { + return &v1beta1.TaskSpec{ + Steps: []v1beta1.Step{{Container: corev1.Container{ + Name: "step", + Image: "myimage", + }}}, + } } diff --git a/pkg/apis/pipeline/v1alpha1/pipeline_conversion.go b/pkg/apis/pipeline/v1alpha1/pipeline_conversion.go index 787fd3a7827..65edb9d8128 100644 --- a/pkg/apis/pipeline/v1alpha1/pipeline_conversion.go +++ b/pkg/apis/pipeline/v1alpha1/pipeline_conversion.go @@ -61,8 +61,8 @@ func (source *PipelineTask) ConvertTo(ctx context.Context, sink *v1beta1.Pipelin sink.Name = source.Name sink.TaskRef = source.TaskRef if source.TaskSpec != nil { - sink.TaskSpec = &v1beta1.TaskSpec{} - if err := source.TaskSpec.ConvertTo(ctx, sink.TaskSpec); err != nil { + sink.TaskSpec = &v1beta1.EmbeddedTask{TaskSpec: &v1beta1.TaskSpec{}} + if err := source.TaskSpec.ConvertTo(ctx, sink.TaskSpec.TaskSpec); err != nil { return err } } @@ -112,7 +112,7 @@ func (sink *PipelineTask) ConvertFrom(ctx context.Context, source v1beta1.Pipeli sink.TaskRef = source.TaskRef if source.TaskSpec != nil { sink.TaskSpec = &TaskSpec{} - if err := sink.TaskSpec.ConvertFrom(ctx, source.TaskSpec); err != nil { + if err := sink.TaskSpec.ConvertFrom(ctx, source.TaskSpec.TaskSpec); err != nil { return err } } diff --git a/pkg/apis/pipeline/v1beta1/pipeline_defaults_test.go b/pkg/apis/pipeline/v1beta1/pipeline_defaults_test.go index 85057d115ba..2f2dce64f89 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_defaults_test.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_defaults_test.go @@ -85,20 +85,24 @@ func TestPipelineSpec_SetDefaults(t *testing.T) { 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", - }}, + Name: "foo", TaskSpec: &v1beta1.EmbeddedTask{ + 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, - }}, + Name: "foo", TaskSpec: &v1beta1.EmbeddedTask{ + TaskSpec: &v1beta1.TaskSpec{ + Params: []v1beta1.ParamSpec{{ + Name: "string-param", + Type: v1beta1.ParamTypeString, + }}, + }, }, }}, }, @@ -106,20 +110,24 @@ func TestPipelineSpec_SetDefaults(t *testing.T) { 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", - }}, + Name: "foo", TaskSpec: &v1beta1.EmbeddedTask{ + 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, - }}, + Name: "foo", TaskSpec: &v1beta1.EmbeddedTask{ + TaskSpec: &v1beta1.TaskSpec{ + Params: []v1beta1.ParamSpec{{ + Name: "string-param", + Type: v1beta1.ParamTypeString, + }}, + }, }, }}, }, diff --git a/pkg/apis/pipeline/v1beta1/pipeline_types.go b/pkg/apis/pipeline/v1beta1/pipeline_types.go index 74cfe3dc5fe..002e8b45896 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_types.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_types.go @@ -90,6 +90,23 @@ type PipelineResult struct { Value string `json:"value"` } +type PipelineTaskMetadata struct { + // +optional + Labels map[string]string `json:"labels,omitempty"` + + // +optional + Annotations map[string]string `json:"annotations,omitempty"` +} + +type EmbeddedTask struct { + // +optional + Metadata PipelineTaskMetadata `json:"metadata,omitempty"` + + // TaskSpec is a specification of a task + // +optional + *TaskSpec `json:",inline,omitempty"` +} + // PipelineTask defines a task in a Pipeline, passing inputs from both // Params and from the output of previous tasks. type PipelineTask struct { @@ -104,7 +121,7 @@ type PipelineTask struct { // TaskSpec is a specification of a task // +optional - TaskSpec *TaskSpec `json:"taskSpec,omitempty"` + TaskSpec *EmbeddedTask `json:"taskSpec,inline,omitempty"` // Conditions is a list of conditions that need to be true for the task to run // +optional @@ -139,6 +156,10 @@ type PipelineTask struct { Timeout *metav1.Duration `json:"timeout,omitempty"` } +func (pt *PipelineTask) TaskSpecMetadata() PipelineTaskMetadata { + return pt.TaskSpec.Metadata +} + func (pt PipelineTask) HashKey() string { return pt.Name } diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go index c8b4c92f1df..9615a56a07a 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go @@ -152,13 +152,9 @@ func TestPipelineSpec_Validate_Failure(t *testing.T) { Name: "valid-pipeline-task", TaskRef: &TaskRef{Name: "foo-task"}, }, { - Name: "invalid-pipeline-task", - TaskRef: &TaskRef{Name: "foo-task"}, - TaskSpec: &TaskSpec{ - Steps: []Step{{ - Container: corev1.Container{Name: "foo", Image: "bar"}, - }}, - }, + Name: "invalid-pipeline-task", + TaskRef: &TaskRef{Name: "foo-task"}, + TaskSpec: &EmbeddedTask{TaskSpec: getTaskSpec()}, }}, }, }, { @@ -250,12 +246,8 @@ func TestValidatePipelineTasks_Success(t *testing.T) { }, { name: "pipeline task with valid taskspec", tasks: []PipelineTask{{ - Name: "foo", - TaskSpec: &TaskSpec{ - Steps: []Step{{ - Container: corev1.Container{Name: "foo", Image: "bar"}, - }}, - }, + Name: "foo", + TaskSpec: &EmbeddedTask{TaskSpec: getTaskSpec()}, }}, }} for _, tt := range tests { @@ -280,19 +272,15 @@ func TestValidatePipelineTasks_Failure(t *testing.T) { }, { name: "pipeline task with both taskref and taskspec", tasks: []PipelineTask{{ - Name: "foo", - TaskRef: &TaskRef{Name: "foo-task"}, - TaskSpec: &TaskSpec{ - Steps: []Step{{ - Container: corev1.Container{Name: "foo", Image: "bar"}, - }}, - }, + Name: "foo", + TaskRef: &TaskRef{Name: "foo-task"}, + TaskSpec: &EmbeddedTask{TaskSpec: getTaskSpec()}, }}, }, { name: "pipeline task with invalid taskspec", tasks: []PipelineTask{{ Name: "foo", - TaskSpec: &TaskSpec{}, + TaskSpec: &EmbeddedTask{TaskSpec: &TaskSpec{}}, }}, }, { name: "pipeline tasks invalid (duplicate tasks)", @@ -660,14 +648,14 @@ func TestValidateGraph_Failure(t *testing.T) { func TestValidateParamResults_Success(t *testing.T) { desc := "valid pipeline task referencing task result along with parameter variable" tasks := []PipelineTask{{ - TaskSpec: &TaskSpec{ + TaskSpec: &EmbeddedTask{TaskSpec: &TaskSpec{ Results: []TaskResult{{ Name: "output", }}, Steps: []Step{{ Container: corev1.Container{Name: "foo", Image: "bar"}, }}, - }, + }}, Name: "a-task", }, { Name: "foo", @@ -1041,12 +1029,8 @@ func TestValidatePipelineWithFinalTasks_Success(t *testing.T) { Name: "final-task-1", TaskRef: &TaskRef{Name: "final-task"}, }, { - Name: "final-task-2", - TaskSpec: &TaskSpec{ - Steps: []Step{{ - Container: corev1.Container{Name: "foo", Image: "bar"}, - }}, - }, + Name: "final-task-2", + TaskSpec: &EmbeddedTask{TaskSpec: getTaskSpec()}, }}, }, }, @@ -1200,13 +1184,9 @@ func TestValidatePipelineWithFinalTasks_Failure(t *testing.T) { TaskRef: &TaskRef{Name: "non-final-task"}, }}, Finally: []PipelineTask{{ - Name: "final-task", - TaskRef: &TaskRef{Name: "non-final-task"}, - TaskSpec: &TaskSpec{ - Steps: []Step{{ - Container: corev1.Container{Name: "foo", Image: "bar"}, - }}, - }, + Name: "final-task", + TaskRef: &TaskRef{Name: "non-final-task"}, + TaskSpec: &EmbeddedTask{TaskSpec: getTaskSpec()}, }}, }, }, @@ -1491,3 +1471,11 @@ func TestContextInvalid(t *testing.T) { }) } } + +func getTaskSpec() *TaskSpec { + return &TaskSpec{ + Steps: []Step{{ + Container: corev1.Container{Name: "foo", Image: "bar"}, + }}, + } +} diff --git a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go index 0017ae23937..2d34044b58a 100644 --- a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go @@ -225,6 +225,28 @@ func (in *ConditionCheckStatusFields) DeepCopy() *ConditionCheckStatusFields { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *EmbeddedTask) DeepCopyInto(out *EmbeddedTask) { + *out = *in + in.Metadata.DeepCopyInto(&out.Metadata) + if in.TaskSpec != nil { + in, out := &in.TaskSpec, &out.TaskSpec + *out = new(TaskSpec) + (*in).DeepCopyInto(*out) + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new EmbeddedTask. +func (in *EmbeddedTask) DeepCopy() *EmbeddedTask { + if in == nil { + return nil + } + out := new(EmbeddedTask) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *InternalTaskModifier) DeepCopyInto(out *InternalTaskModifier) { *out = *in @@ -815,7 +837,7 @@ func (in *PipelineTask) DeepCopyInto(out *PipelineTask) { } if in.TaskSpec != nil { in, out := &in.TaskSpec, &out.TaskSpec - *out = new(TaskSpec) + *out = new(EmbeddedTask) (*in).DeepCopyInto(*out) } if in.Conditions != nil { @@ -938,6 +960,36 @@ func (in PipelineTaskList) DeepCopy() PipelineTaskList { return *out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *PipelineTaskMetadata) DeepCopyInto(out *PipelineTaskMetadata) { + *out = *in + if in.Labels != nil { + in, out := &in.Labels, &out.Labels + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } + if in.Annotations != nil { + in, out := &in.Annotations, &out.Annotations + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PipelineTaskMetadata. +func (in *PipelineTaskMetadata) DeepCopy() *PipelineTaskMetadata { + if in == nil { + return nil + } + out := new(PipelineTaskMetadata) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PipelineTaskOutputResource) DeepCopyInto(out *PipelineTaskOutputResource) { *out = *in diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index 3fa02f0d4b9..47d14c68273 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -651,8 +651,8 @@ func (c *Reconciler) createTaskRun(ctx context.Context, rprt *resources.Resolved Name: rprt.TaskRunName, Namespace: pr.Namespace, OwnerReferences: []metav1.OwnerReference{pr.GetOwnerReference()}, - Labels: getTaskrunLabels(pr, rprt.PipelineTask.Name), - Annotations: getTaskrunAnnotations(pr), + Labels: combineTaskRunAndTaskSpecLabels(pr, rprt.PipelineTask), + Annotations: combineTaskRunAndTaskSpecAnnotations(pr, rprt.PipelineTask), }, Spec: v1beta1.TaskRunSpec{ Params: rprt.PipelineTask.Params, @@ -762,6 +762,50 @@ func getTaskrunLabels(pr *v1beta1.PipelineRun, pipelineTaskName string) map[stri return labels } +func combineTaskRunAndTaskSpecLabels(pr *v1beta1.PipelineRun, pipelineTask *v1beta1.PipelineTask) map[string]string { + var tsLabels map[string]string + trLabels := getTaskrunLabels(pr, pipelineTask.Name) + + if pipelineTask.TaskSpec != nil { + tsLabels = pipelineTask.TaskSpecMetadata().Labels + } + + labels := make(map[string]string, len(trLabels)+len(tsLabels)) + + // labels from TaskRun takes higher precedence over the ones specified in Pipeline through TaskSpec + // initialize labels with TaskRun labels + labels = trLabels + for key, value := range tsLabels { + // add labels from TaskSpec if the label does not exist + if _, ok := labels[key]; !ok { + labels[key] = value + } + } + return labels +} + +func combineTaskRunAndTaskSpecAnnotations(pr *v1beta1.PipelineRun, pipelineTask *v1beta1.PipelineTask) map[string]string { + var tsAnnotations map[string]string + trAnnotations := getTaskrunAnnotations(pr) + + if pipelineTask.TaskSpec != nil { + tsAnnotations = pipelineTask.TaskSpecMetadata().Annotations + } + + annotations := make(map[string]string, len(trAnnotations)+len(tsAnnotations)) + + // annotations from TaskRun takes higher precedence over the ones specified in Pipeline through TaskSpec + // initialize annotations with TaskRun annotations + annotations = trAnnotations + for key, value := range tsAnnotations { + // add annotations from TaskSpec if the annotation does not exist + if _, ok := annotations[key]; !ok { + annotations[key] = value + } + } + return annotations +} + func getTaskRunTimeout(pr *v1beta1.PipelineRun, rprt *resources.ResolvedPipelineRunTask) *metav1.Duration { var taskRunTimeout = &metav1.Duration{Duration: apisconfig.NoTimeoutDuration} diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index 46df130bf2b..4f9246cbd4d 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -3646,6 +3646,95 @@ func TestReconcile_CloudEvents(t *testing.T) { } } +// this test validates taskSpec metadata is embedded into task run +func TestReconcilePipeline_TaskSpecMetadata(t *testing.T) { + names.TestingSeed() + + prs := []*v1beta1.PipelineRun{ + tb.PipelineRun("test-pipeline-run-success", + tb.PipelineRunNamespace("foo"), + tb.PipelineRunSpec("test-pipeline"), + ), + } + + ts := v1beta1.TaskSpec{ + Steps: []v1beta1.Step{{Container: corev1.Container{ + Name: "mystep", + Image: "myimage"}}}, + } + + labels := map[string]string{"label1": "labelvalue1", "label2": "labelvalue2"} + annotations := map[string]string{"annotation1": "value1", "annotation2": "value2"} + + ps := []*v1beta1.Pipeline{ + tb.Pipeline("test-pipeline", + tb.PipelineNamespace("foo"), + tb.PipelineSpec( + tb.PipelineTask("task-without-metadata", "", tb.PipelineTaskSpec(&ts)), + tb.PipelineTask("task-with-metadata", "", tb.PipelineTaskSpec(&ts), + tb.TaskSpecMetadata(v1beta1.PipelineTaskMetadata{ + Labels: labels, + Annotations: annotations}), + ), + ), + ), + } + + d := test.Data{ + PipelineRuns: prs, + Pipelines: ps, + } + prt := NewPipelineRunTest(d, t) + defer prt.Cancel() + + reconciledRun, clients := prt.reconcileRun("foo", "test-pipeline-run-success", []string{}, false) + + actions := clients.Pipeline.Actions() + if len(actions) == 0 { + t.Fatalf("Expected client to have been used to create a TaskRun but it wasn't") + } + + actualTaskRun := make(map[string]*v1beta1.TaskRun) + for _, a := range actions { + if a.GetResource().Resource == "taskruns" { + t := a.(ktesting.CreateAction).GetObject().(*v1beta1.TaskRun) + actualTaskRun[t.Name] = t + } + } + + // Check that the expected TaskRun was created + if len(actualTaskRun) != 2 { + t.Errorf("Expected two TaskRuns to be created, but found %d TaskRuns.", len(actualTaskRun)) + } + + expectedTaskRun := make(map[string]*v1beta1.TaskRun) + expectedTaskRun["test-pipeline-run-success-task-with-metadata-mz4c7"] = getTaskRunWithTaskSpec( + "test-pipeline-run-success-task-with-metadata-mz4c7", + "test-pipeline-run-success", + "test-pipeline", + "task-with-metadata", + labels, + annotations, + ) + + expectedTaskRun["test-pipeline-run-success-task-without-metadata-9l9zj"] = getTaskRunWithTaskSpec( + "test-pipeline-run-success-task-without-metadata-9l9zj", + "test-pipeline-run-success", + "test-pipeline", + "task-without-metadata", + map[string]string{}, + map[string]string{}, + ) + + if d := cmp.Diff(actualTaskRun, expectedTaskRun); d != "" { + t.Fatalf("Expected TaskRuns to match, but got a mismatch: %s", d) + } + + if len(reconciledRun.Status.TaskRuns) != 2 { + t.Errorf("Expected PipelineRun status to include both TaskRun status items that can run immediately: %v", reconciledRun.Status.TaskRuns) + } +} + // NewPipelineRunTest returns PipelineRunTest with a new PipelineRun controller created with specified state through data // This PipelineRunTest can be reused for multiple PipelineRuns by calling reconcileRun for each pipelineRun func NewPipelineRunTest(data test.Data, t *testing.T) *PipelineRunTest { @@ -3692,3 +3781,17 @@ func (prt PipelineRunTest) reconcileRun(namespace, pipelineRunName string, wantE return reconciledRun, clients } + +func getTaskRunWithTaskSpec(tr, pr, p, t string, labels, annotations map[string]string) *v1beta1.TaskRun { + return tb.TaskRun(tr, + tb.TaskRunNamespace("foo"), + tb.TaskRunOwnerReference("PipelineRun", pr, + tb.OwnerReferenceAPIVersion("tekton.dev/v1beta1"), + tb.Controller, tb.BlockOwnerDeletion), + tb.TaskRunLabel(pipeline.GroupName+pipeline.PipelineLabelKey, p), + tb.TaskRunLabel(pipeline.GroupName+pipeline.PipelineRunLabelKey, pr), + tb.TaskRunLabel(pipeline.GroupName+pipeline.PipelineTaskLabelKey, t), + tb.TaskRunLabels(labels), + tb.TaskRunAnnotations(annotations), + tb.TaskRunSpec(tb.TaskRunTaskSpec(tb.Step("myimage", tb.StepName("mystep"))))) +} diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go index 4c377879703..f528d05103e 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go @@ -453,7 +453,7 @@ func ResolvePipelineRun( taskName = t.TaskMetadata().Name kind = pt.TaskRef.Kind } else { - spec = *pt.TaskSpec + spec = *pt.TaskSpec.TaskSpec } spec.SetDefaults(contexts.WithUpgradeViaDefaulting(ctx)) rtr, err := ResolvePipelineTaskResources(pt, &spec, taskName, kind, providedResources) diff --git a/test/retry_test.go b/test/retry_test.go index cce0874b85b..8a51d305a2a 100644 --- a/test/retry_test.go +++ b/test/retry_test.go @@ -47,12 +47,12 @@ func TestTaskRunRetry(t *testing.T) { PipelineSpec: &v1beta1.PipelineSpec{ Tasks: []v1beta1.PipelineTask{{ Name: "retry-me", - TaskSpec: &v1beta1.TaskSpec{ + TaskSpec: &v1beta1.EmbeddedTask{TaskSpec: &v1beta1.TaskSpec{ Steps: []v1beta1.Step{{ Container: corev1.Container{Image: "busybox"}, Script: "exit 1", }}, - }, + }}, Retries: numRetries, }}, },