TEP of adding pipelinerun.spec.taskruntemplate #783

yuzp1996 · 2022-08-15T12:21:06Z

No description provided.

yuzp1996 · 2022-08-15T12:25:01Z

Related Issue: tektoncd/pipeline#5302

PR is not ready for review now! 😂

lbernick · 2022-08-15T14:09:15Z

/kind tep

dibyom · 2022-08-15T16:09:13Z

/assign @lbernick

lbernick

thanks @yuzp1996! would you mind removing the formatting changes from this PR?

lbernick · 2022-08-16T20:27:21Z

teps/0119-add-taskrunspec-template-in-pipelinerun.md

+## Proposal
+Add filed taskRunSpecTemplate to PipelineRun.Spec then we can specify the common configuration in taskRunSpecTemplate, and the configuration will apply to all the TasRun.
+
+```yaml


It would be good to have an example of how the new field would be used to meet one of these use cases. For example, for use case number 1 (service accounts), the user might have a pipeline that clones and then runs unit tests, integration tests, and linters. They might want to use the same service account for all the tests, but a different service account for the clone. It would be helpful to include the pipeline definition as well. In this example, the taskRunTemplate has the same configuration as taskRunSpecs, which a user is unlikely to do.

lbernick · 2022-08-16T20:27:40Z

teps/0119-add-taskrunspec-template-in-pipelinerun.md

+          vault.hashicorp.com/agent-inject-secret-foo: "/path/to/foo"
+          vault.hashicorp.com/role: role-name
+// Add the following filed that same as taskRunSpecs
+ taskRunSpecTemplate:


I think we should call this taskRunTemplate instead of taskRunSpecTemplate.

lbernick · 2022-08-16T20:30:25Z

teps/0119-add-taskrunspec-template-in-pipelinerun.md

+Expand pipelinerun.Spec and add taskRunSpecTemplate
+
+
+```go


no need to include all this code here-- that can be discussed in a pull request. Instead, I'd recommend including an example of a user-configured PipelineRun before this change, and the same PipelineRun after this change.

(A good example is in the matrix TEP: https://github.com/tektoncd/community/blob/main/teps/0090-matrix.md#substituting-string-parameters-in-the-tasks. There's an example of what a PipelineRun would look like with this feature, and the same PipelineRun without this feature.)

lbernick · 2022-08-17T18:09:19Z

teps/0119-add-taskrun-template-in-pipelinerun.md

+I want to specify compute resources that each TaskRun in my PipelineRun should run with, and don't want to have to specify them individually for each TaskRun.
+
+## Proposal
+Add filed taskRunTemplate to PipelineRun.Spec then users can specify common configuration in taskRunTemplate and the configuration will apply to all the TaskRun. 


Suggested change

Add filed taskRunTemplate to PipelineRun.Spec then users can specify common configuration in taskRunTemplate and the configuration will apply to all the TaskRun.

Add field taskRunTemplate to PipelineRun.Spec so that users can specify common configuration in taskRunTemplate that will apply to all the TaskRuns.

lbernick · 2022-08-17T18:09:42Z

teps/0119-add-taskrun-template-in-pipelinerun.md

@@ -0,0 +1,179 @@
+---
+status: proposed


this can be "implementable"

lbernick · 2022-08-17T18:10:05Z

teps/0119-add-taskrun-template-in-pipelinerun.md

+  ComputeResources *corev1.ResourceRequirements `json:"computeResources,omitempty"`
+}
+
+// Reference PipelineTaskRunTemplate in PipelineRunSpec as filed TaskRunSpecTemplate


Suggested change

// Reference PipelineTaskRunTemplate in PipelineRunSpec as filed TaskRunSpecTemplate

// Reference PipelineTaskRunTemplate in PipelineRunSpec via the field TaskRunSpecTemplate

lbernick · 2022-08-17T18:10:45Z

teps/0119-add-taskrun-template-in-pipelinerun.md

+    computeResources:
+      requests:
+        cpu: 2
+    stepOverrides:


would you mind removing stepOverrides from this example, as it can't be used along with computeResources?

Thanks for your review @lbernick! I have synced the latest changes!

But I don't know why stepOverrides can not be used along with computeResources. Could you please help tell me the reason? Thanks! If so does this mean sidecarOverrides can only not be used along with computeResources either? 🙏

It's explained in TEP-0104; basically it's two different ways of configuring compute resources that aren't really compatible with each other. We are probably going to deprecate stepOverrides (see #786)

jerop · 2022-08-22T16:19:56Z

@tektoncd/core-maintainers need a non-googler assignee, please take a look

lbernick · 2022-08-30T13:45:37Z

@yuzp1996 would you mind updating this TEP to address moving podTemplate and serviceAccountName under taskRunTemplate, and clarifying that the changes will be made in v1 only? Thanks :)

yuzp1996 · 2022-08-31T01:48:25Z

I don't mind at all 😁. I'll do it when I'm fine.

yuzp1996 · 2022-09-01T23:43:21Z

@vdemeester @lbernick Changes are synced, please correct me if I don't understand correctly.

lbernick · 2022-09-02T14:17:03Z

this looks good to me! I'll let vincent take another look

We want to provide a convenient way to specify taskrun configuration Now there is ServiceAccountName and PodTemplate can do the same thing so we merge these two field to PipelineTaskRunTemplate and add metadata sidecarOverrides annd computeResources to PipelineTaskRunTemplate. Signed-off-by: yuzhipeng <[email protected]>

tekton-robot · 2022-09-06T08:17:53Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lbernick, vdemeester

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~teps/OWNERS~~ [lbernick,vdemeester]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

lbernick · 2022-09-06T16:09:28Z

/lgtm

lbernick · 2022-09-07T13:31:32Z

FYI @jerop @abayer I realized this may affect pipelines in pipelines since the TEP states that pipelinerun.spec.serviceaccountname can be passed to child PipelineRuns. With serviceaccountname moved under taskRunTemplate, this might make less sense. Maybe when we explore creating pipelineRun.spec.pipelineRunSpecs we can also explore pipelineRun.spec.pipelineRunTemplate?

jerop · 2022-09-07T14:19:38Z

sure, let's gather initial feedback on taskRunTemplate and then explore adding customRunTemplate and pipelineRunTemplate

also noticed the title calls it taskrunspectemplate but the tep itself is taskruntemplate -- which one is it? (i prefer the latter)

lbernick · 2022-09-07T14:29:09Z

also noticed the title calls it taskrunspectemplate but the tep itself is taskruntemplate -- which one is it? (i prefer the latter)

it's the latter

tekton-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 15, 2022

tekton-robot requested review from houshengbo and pradeepitm12 August 15, 2022 12:21

tekton-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Aug 15, 2022

yuzp1996 changed the title ~~WIP TEP of adding pipelinerun.spec.taskrunspectemplate~~ [WIP] TEP of adding pipelinerun.spec.taskrunspectemplate Aug 15, 2022

tekton-robot added the kind/tep Categorizes issue or PR as related to a TEP (or needs a TEP). label Aug 15, 2022

tekton-robot assigned lbernick Aug 15, 2022

yuzp1996 force-pushed the main branch from bd52f7e to 70749a1 Compare August 16, 2022 14:56

lbernick reviewed Aug 16, 2022

View reviewed changes

yuzp1996 force-pushed the main branch from 70749a1 to 611b882 Compare August 16, 2022 22:38

yuzp1996 force-pushed the main branch from 8bef729 to cf41632 Compare August 17, 2022 12:34

tekton-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Aug 17, 2022

yuzp1996 force-pushed the main branch from cf41632 to e676801 Compare August 17, 2022 12:37

yuzp1996 changed the title ~~[WIP] TEP of adding pipelinerun.spec.taskrunspectemplate~~ TEP of adding pipelinerun.spec.taskrunspectemplate Aug 17, 2022

tekton-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 17, 2022

lbernick approved these changes Aug 17, 2022

View reviewed changes

tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 17, 2022

yuzp1996 force-pushed the main branch from e676801 to cef0680 Compare August 18, 2022 02:38

yuzp1996 requested review from lbernick and removed request for houshengbo and pradeepitm12 August 18, 2022 02:44

yuzp1996 closed this Sep 1, 2022

yuzp1996 force-pushed the main branch from cef0680 to a735d2e Compare September 1, 2022 14:22

tekton-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 1, 2022

yuzp1996 reopened this Sep 1, 2022

yuzp1996 force-pushed the main branch 2 times, most recently from 479b78d to 231c446 Compare September 1, 2022 23:40

yuzp1996 requested review from vdemeester and removed request for lbernick September 1, 2022 23:41

yuzp1996 force-pushed the main branch from 231c446 to c8d63cf Compare September 1, 2022 23:44

yuzp1996 force-pushed the main branch from 8e6e580 to 81dc3e6 Compare September 5, 2022 02:40

vdemeester approved these changes Sep 6, 2022

View reviewed changes

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 6, 2022

tekton-robot merged commit 9bf2ded into tektoncd:main Sep 6, 2022

yuzp1996 changed the title ~~TEP of adding pipelinerun.spec.taskrunspectemplate~~ TEP of adding pipelinerun.spec.taskruntemplate Sep 7, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TEP of adding pipelinerun.spec.taskruntemplate #783

TEP of adding pipelinerun.spec.taskruntemplate #783

yuzp1996 commented Aug 15, 2022

yuzp1996 commented Aug 15, 2022

lbernick commented Aug 15, 2022

dibyom commented Aug 15, 2022

lbernick left a comment

lbernick Aug 16, 2022

lbernick Aug 16, 2022

lbernick Aug 16, 2022

lbernick Aug 17, 2022

lbernick Aug 17, 2022

lbernick Aug 17, 2022

lbernick Aug 17, 2022

yuzp1996 Aug 18, 2022 •

edited

Loading

lbernick Aug 18, 2022

jerop commented Aug 22, 2022

lbernick commented Aug 30, 2022

yuzp1996 commented Aug 31, 2022

yuzp1996 commented Sep 1, 2022

lbernick commented Sep 2, 2022

tekton-robot commented Sep 6, 2022

lbernick commented Sep 6, 2022

lbernick commented Sep 7, 2022

jerop commented Sep 7, 2022

lbernick commented Sep 7, 2022

	Add filed taskRunTemplate to PipelineRun.Spec then users can specify common configuration in taskRunTemplate and the configuration will apply to all the TaskRun.
	Add field taskRunTemplate to PipelineRun.Spec so that users can specify common configuration in taskRunTemplate that will apply to all the TaskRuns.

	// Reference PipelineTaskRunTemplate in PipelineRunSpec as filed TaskRunSpecTemplate
	// Reference PipelineTaskRunTemplate in PipelineRunSpec via the field TaskRunSpecTemplate

TEP of adding pipelinerun.spec.taskruntemplate #783

TEP of adding pipelinerun.spec.taskruntemplate #783

Conversation

yuzp1996 commented Aug 15, 2022

yuzp1996 commented Aug 15, 2022

lbernick commented Aug 15, 2022

dibyom commented Aug 15, 2022

lbernick left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuzp1996 Aug 18, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jerop commented Aug 22, 2022

lbernick commented Aug 30, 2022

yuzp1996 commented Aug 31, 2022

yuzp1996 commented Sep 1, 2022

lbernick commented Sep 2, 2022

tekton-robot commented Sep 6, 2022

lbernick commented Sep 6, 2022

lbernick commented Sep 7, 2022

jerop commented Sep 7, 2022

lbernick commented Sep 7, 2022

yuzp1996 Aug 18, 2022 •

edited

Loading