From be9ac3bf6b27f6beb3f4b7c828ef8388f9b323c3 Mon Sep 17 00:00:00 2001 From: Jonas Pettersson Date: Sat, 23 May 2020 22:11:36 +0200 Subject: [PATCH] Propagate TaskRunSpecs to created TaskRuns TaskRunSpec set in PipelineRun is never propagated to new TaskRuns in `createTaskRun()`, they are only propagated to Condition-pods in `makeConditionCheckContainer()` This commit - Adds propagation to new TaskRuns - Also adds a unit test Fixes #2682 /kind bug --- .../{ => no-ci}/pipelinerun-taskrunspecs.yaml | 0 internal/builder/v1beta1/pipeline.go | 7 ++ internal/builder/v1beta1/task.go | 7 ++ pkg/reconciler/pipelinerun/pipelinerun.go | 9 ++- .../pipelinerun/pipelinerun_test.go | 78 +++++++++++++++++++ 5 files changed, 97 insertions(+), 4 deletions(-) rename examples/v1beta1/pipelineruns/{ => no-ci}/pipelinerun-taskrunspecs.yaml (100%) diff --git a/examples/v1beta1/pipelineruns/pipelinerun-taskrunspecs.yaml b/examples/v1beta1/pipelineruns/no-ci/pipelinerun-taskrunspecs.yaml similarity index 100% rename from examples/v1beta1/pipelineruns/pipelinerun-taskrunspecs.yaml rename to examples/v1beta1/pipelineruns/no-ci/pipelinerun-taskrunspecs.yaml diff --git a/internal/builder/v1beta1/pipeline.go b/internal/builder/v1beta1/pipeline.go index 51c05d1b471..0f1dc557f4e 100644 --- a/internal/builder/v1beta1/pipeline.go +++ b/internal/builder/v1beta1/pipeline.go @@ -428,6 +428,13 @@ func PipelineRunServiceAccountNameTask(taskName, sa string) PipelineRunSpecOp { } } +// PipelineTaskRunSpec adds customs TaskRunSpecs +func PipelineTaskRunSpecs(taskRunSpecs []v1beta1.PipelineTaskRunSpec) PipelineRunSpecOp { + return func(prs *v1beta1.PipelineRunSpec) { + prs.TaskRunSpecs = taskRunSpecs + } +} + // PipelineRunParam add a param, with specified name and value, to the PipelineRunSpec. func PipelineRunParam(name string, value string, additionalValues ...string) PipelineRunSpecOp { arrayOrString := ArrayOrString(value, additionalValues...) diff --git a/internal/builder/v1beta1/task.go b/internal/builder/v1beta1/task.go index 811c84e73e9..f5605f1259a 100644 --- a/internal/builder/v1beta1/task.go +++ b/internal/builder/v1beta1/task.go @@ -760,6 +760,13 @@ func TaskResourceBindingPaths(paths ...string) TaskResourceBindingOp { } } +// TaskRunPodTemplate add a custom PodTemplate to the TaskRun +func TaskRunPodTemplate(podTemplate *v1beta1.PodTemplate) TaskRunSpecOp { + return func(spec *v1beta1.TaskRunSpec) { + spec.PodTemplate = podTemplate + } +} + // TaskRunWorkspaceEmptyDir adds a workspace binding to an empty dir volume source. func TaskRunWorkspaceEmptyDir(name, subPath string) TaskRunSpecOp { return func(spec *v1beta1.TaskRunSpec) { diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index b44f7f4d7a0..50a409ef80e 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -745,6 +745,7 @@ func (c *Reconciler) createTaskRun(rprt *resources.ResolvedPipelineRunTask, pr * return c.PipelineClientSet.TektonV1beta1().TaskRuns(pr.Namespace).UpdateStatus(tr) } + serviceAccountName, podTemplate := pr.GetTaskRunSpecs(rprt.PipelineTask.Name) tr = &v1beta1.TaskRun{ ObjectMeta: metav1.ObjectMeta{ Name: rprt.TaskRunName, @@ -755,9 +756,9 @@ func (c *Reconciler) createTaskRun(rprt *resources.ResolvedPipelineRunTask, pr * }, Spec: v1beta1.TaskRunSpec{ Params: rprt.PipelineTask.Params, - ServiceAccountName: pr.GetServiceAccountName(rprt.PipelineTask.Name), + ServiceAccountName: serviceAccountName, Timeout: getTaskRunTimeout(pr, rprt), - PodTemplate: pr.Spec.PodTemplate, + PodTemplate: podTemplate, }} if rprt.ResolvedTaskResources.TaskName != "" { @@ -951,7 +952,7 @@ func (c *Reconciler) makeConditionCheckContainer(rprt *resources.ResolvedPipelin if err != nil { return nil, fmt.Errorf("failed to get TaskSpec from Condition: %w", err) } - serviceAccountName, podtemplate := pr.GetTaskRunSpecs(rprt.PipelineTask.Name) + serviceAccountName, podTemplate := pr.GetTaskRunSpecs(rprt.PipelineTask.Name) tr := &v1beta1.TaskRun{ ObjectMeta: metav1.ObjectMeta{ Name: rcc.ConditionCheckName, @@ -968,7 +969,7 @@ func (c *Reconciler) makeConditionCheckContainer(rprt *resources.ResolvedPipelin Inputs: rcc.ToTaskResourceBindings(), }, Timeout: getTaskRunTimeout(pr, rprt), - PodTemplate: podtemplate, + PodTemplate: podTemplate, }} cctr, err := c.PipelineClientSet.TektonV1beta1().TaskRuns(pr.Namespace).Create(tr) diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index a61fb4b09c3..c101cb1d185 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -1531,6 +1531,84 @@ func TestGetTaskRunTimeout(t *testing.T) { } } +// TestReconcileAndPropagateCustomPipelineTaskRunSpec tests that custom PipelineTaskRunSpec declared +// in PipelineRun is propagated to created TaskRuns +func TestReconcileAndPropagateCustomPipelineTaskRunSpec(t *testing.T) { + names.TestingSeed() + prName := "test-pipeline-run" + ps := []*v1beta1.Pipeline{tb.Pipeline("test-pipeline", tb.PipelineNamespace("foo"), tb.PipelineSpec( + tb.PipelineTask("hello-world-1", "hello-world"), + ))} + prs := []*v1beta1.PipelineRun{tb.PipelineRun(prName, tb.PipelineRunNamespace("foo"), + tb.PipelineRunAnnotation("PipelineRunAnnotation", "PipelineRunValue"), + tb.PipelineRunSpec("test-pipeline", + tb.PipelineRunServiceAccountName("test-sa"), + tb.PipelineTaskRunSpecs([]v1beta1.PipelineTaskRunSpec{{ + PipelineTaskName: "hello-world-1", + TaskServiceAccountName: "custom-sa", + TaskPodTemplate: &v1beta1.PodTemplate{ + NodeSelector: map[string]string{ + "workloadtype": "tekton", + }, + }, + }}), + ), + )} + ts := []*v1beta1.Task{tb.Task("hello-world", tb.TaskNamespace("foo"))} + + d := test.Data{ + PipelineRuns: prs, + Pipelines: ps, + Tasks: ts, + } + + testAssets, cancel := getPipelineRunController(t, d) + defer cancel() + c := testAssets.Controller + clients := testAssets.Clients + + err := c.Reconciler.Reconcile(context.Background(), "foo/"+prName) + if err != nil { + t.Errorf("Did not expect to see error when reconciling completed PipelineRun but saw %s", err) + } + + // Check that the PipelineRun was reconciled correctly + _, err = clients.Pipeline.TektonV1beta1().PipelineRuns("foo").Get(prName, metav1.GetOptions{}) + if err != nil { + t.Fatalf("Somehow had error getting completed reconciled run out of fake client: %s", err) + } + + // Check that the expected TaskRun was created + actual := clients.Pipeline.Actions()[1].(ktesting.CreateAction).GetObject().(*v1beta1.TaskRun) + if actual == nil { + t.Errorf("Expected a TaskRun to be created, but it wasn't.") + } + expectedTaskRun := tb.TaskRun("test-pipeline-run-hello-world-1-9l9zj", + tb.TaskRunNamespace("foo"), + tb.TaskRunOwnerReference("PipelineRun", "test-pipeline-run", + tb.OwnerReferenceAPIVersion("tekton.dev/v1beta1"), + tb.Controller, tb.BlockOwnerDeletion, + ), + tb.TaskRunLabel(pipeline.GroupName+pipeline.PipelineLabelKey, "test-pipeline"), + tb.TaskRunLabel(pipeline.GroupName+pipeline.PipelineTaskLabelKey, "hello-world-1"), + tb.TaskRunLabel(pipeline.GroupName+pipeline.PipelineRunLabelKey, "test-pipeline-run"), + tb.TaskRunAnnotation("PipelineRunAnnotation", "PipelineRunValue"), + tb.TaskRunSpec( + tb.TaskRunTaskRef("hello-world"), + tb.TaskRunServiceAccountName("custom-sa"), + tb.TaskRunPodTemplate(&v1beta1.PodTemplate{ + NodeSelector: map[string]string{ + "workloadtype": "tekton", + }, + }), + ), + ) + + if d := cmp.Diff(actual, expectedTaskRun); d != "" { + t.Errorf("expected to see propagated custom ServiceAccountName and PodTemplate in TaskRun %v created. Diff %s", expectedTaskRun, diff.PrintWantGot(d)) + } +} + func TestReconcileWithConditionChecks(t *testing.T) { names.TestingSeed() prName := "test-pipeline-run"