[pipeline-to-taskrun] Add custom task that runs a Pipeline as a TaskRun 🍙 #770

bobcatfish · 2021-07-09T23:58:40Z

Changes

This commit adds a custom task that allows user to run simple sequential
Pipelines as one TaskRun - which means the Pipeline can refer to
multiple Tasks but run on only one pod.

It only supports a subset of Pipeline functionality (more detail on what
and why in the README) but is enough that folks can do a lot of what
they would have previously used PipelineResources for, e.g. doing a git
clone and then doing something with the data, in the same pod, and emit
results such as the exact commit sha used.

Next steps will be to expand the functionality supported, get feedback,
and if the feedback is good, promote this to a top level Pipeline API
feature.

Experimental project proposal: tektoncd/community#447

Added @wlynch @jerop as owners also, just ping me if you've changed your minds XD

p.s. shout out to the yaml test lib @imjasonh - i had to do some truly terrible things to make it work sometimes but it was still SOOOO much better than creating go structs XD

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

Includes docs (if user facing)
Commit messages follow commit message best practices
Commit messages includes a project tag in the subject - e.g. [OCI], [hub], [results], [task-loops]

See the contribution guide
for more details.

jerop · 2021-07-12T16:48:33Z

/assign @jerop

jerop · 2021-07-12T16:48:43Z

/assign @afrittoli

jerop

thanks for the detailed documentation!!

please add pipeline-to-taskrun here to trigger its tests:

experimental/test/presubmit-tests.sh

Line 61 in 6ae0915

    
           projects="catalogs cel commit-status-tracker generators helm hub oci pipeline/cleanup pipelines-in-pipelines tekdoc task-loops notifiers/github-app cloudevents"

can't see its tests in the logs

bobcatfish · 2021-07-21T15:33:17Z

Thanks for catching that @jerop!!

bobcatfish · 2021-07-21T15:34:16Z

/me slowly watches all the tests fail

jerop

looks great! so excited to see this feature 😀

jerop · 2021-07-21T16:47:57Z

pipeline-to-taskrun/README.md

+This is a controller that enables an experimental [custom task](https://github.com/tektoncd/pipeline/blob/main/docs/runs.md)
+that will allow you to execute a Pipeline (with [limited features](#supported-pipeline-features)) via a TaskRun, enabling you to
+run a Pipeline in a pod ([TEP-0044](https://github.com/tektoncd/community/blob/main/teps/0044-decouple-task-composition-from-scheduling.md)).


it would be useful to add some details here on why a user may want to do this, maybe some use cases and benefits (with a link to the TEP for further information)

jerop · 2021-07-21T16:50:08Z

pipeline-to-taskrun/README.md

+Currently supported features:
+
+* Sequential tasks (specified using [`runAfter`](https://github.com/tektoncd/pipeline/blob/main/docs/pipelines.md#using-the-runafter-parameter))
+* [Params](https://github.com/tektoncd/pipeline/blob/main/docs/pipelines.md#specifying-parameters)


it'd be helpful to specify that string params only are supported, in case a user doesn't get to the part below that says array params are not yet supported

jerop · 2021-07-21T16:58:38Z

pipeline-to-taskrun/config/500-controller.yaml

+      app.kubernetes.io/name: pipeline-to-taskrun-controller
+      app.kubernetes.io/component: pipeline-to-taskrun-controller
+      app.kubernetes.io/instance: default
+      app.kubernetes.io/part-of: pip


Suggested change

app.kubernetes.io/part-of: pip

app.kubernetes.io/part-of: tekton-pipeline-to-taskrun

jerop · 2021-07-21T16:58:50Z

pipeline-to-taskrun/config/500-controller.yaml

+        app.kubernetes.io/component: pipeline-to-taskrun-controller
+        app.kubernetes.io/instance: default
+        app.kubernetes.io/version: devel
+        app.kubernetes.io/part-of: pip


Suggested change

app.kubernetes.io/part-of: pip

app.kubernetes.io/part-of: tekton-pipeline-to-taskrun

jerop · 2021-07-21T17:47:47Z

pipeline-to-taskrun/pkg/reconciler/pipelinetotaskrun/pipelinetotaskrun.go

+	tr, err = getMergedTaskRun(run, pSpec, taskSpecs)
+	if err != nil {
+		run.Status.MarkRunFailed(ReasonRunFailedValidation,
+			"Could not create TaskRun for the pipeline - %v", err)
+		return fmt.Errorf("couldn't create taskrun for pipeline %s: %v", run.Spec.Ref.Name, err)
+	}
+
+	logger.Infof("Creating a new TaskRun object %s", tr.Name)
+	if _, err := r.pipelineClientSet.TektonV1beta1().TaskRuns(run.Namespace).Create(ctx, tr, metav1.CreateOptions{}); err != nil {
+		logger.Errorf("Run %s/%s got an error creating TaskRun - %v", run.Namespace, run.Name, err)
+		run.Status.MarkRunFailed(ReasonRunFailedCreatingPipelineRun,
+			"Run got an error creating pipelineRun - %v", err)
+		return err
+	}


following this section is a bit confusing i.e. it's not clear when taskrun is created

e.g. we have an error "couldn't create taskrun for pipeline", then later we're "creating a new taskrun object"

may help if the first part used merge and second part part used create

or you could add comments like in the other sections above, they were really helpful - thanks!

both great suggestions! ive made a couple updates but lemme know if it's not enough of a change and i could still make it more clear

bobcatfish · 2021-08-13T22:15:19Z

Should be ready for another look @jerop ! 🙏

jerop

very exciting! thank you @bobcatfish 😁

/lgtm

jerop · 2021-08-16T17:50:08Z

@afrittoli looks like I'm not allowed to approve because the owners file is not yet in, the top level owners have to approve

cc @tektoncd/experimental-maintainers

@jerop

I had tried to add the custom task and OWNERS file all at once in tektoncd#770 but @jerop pointed out it's probably a better approach to just add the OWNERS file first, which would allow getting the initial approval needed to kick off the project and from then on the project OWNERS can take care of reviews (and she documented this in tektoncd#782 !) So this commit adds just the OWNERS (more welcome if anyone else is interested!) for the pipeline to taskrun custom task, and there will be a follow-up PR with the initial controller logic. Project proposal: tektoncd/community#447

@jerop

…ustom task 🎁 I had tried to add the custom task and OWNERS file all at once in tektoncd#770 but @jerop pointed out it's probably a better approach to just add the OWNERS file first, which would allow getting the initial approval needed to kick off the project and from then on the project OWNERS can take care of reviews (and she documented this in tektoncd#782 !) So this commit adds just the OWNERS (more welcome if anyone else is interested!) for the pipeline to taskrun custom task, and there will be a follow-up PR with the initial controller logic. Project proposal: tektoncd/community#447

@jerop

…ustom task 🎁 I had tried to add the custom task and OWNERS file all at once in #770 but @jerop pointed out it's probably a better approach to just add the OWNERS file first, which would allow getting the initial approval needed to kick off the project and from then on the project OWNERS can take care of reviews (and she documented this in #782 !) So this commit adds just the OWNERS (more welcome if anyone else is interested!) for the pipeline to taskrun custom task, and there will be a follow-up PR with the initial controller logic. Project proposal: tektoncd/community#447

jerop

thanks @bobcatfish 😁

/lgtm

jerop · 2021-08-31T16:17:50Z

/approve

@jerop

In tektoncd#770 I tried to add the custom task and OWNERS file - with @jerop's help I realized it would be easier to add the OWNERS file first so that OWNERS can review the custom task itself - but even after that I was still changing a file in the top level test dir to add the custom task to the list of tests kicked off which needs top level OWNERS approval - so this commit is doing that separately. Looking at the script it should be safe to add a directory that doesn't exist because it should just get ignored

@jerop

In #770 I tried to add the custom task and OWNERS file - with @jerop's help I realized it would be easier to add the OWNERS file first so that OWNERS can review the custom task itself - but even after that I was still changing a file in the top level test dir to add the custom task to the list of tests kicked off which needs top level OWNERS approval - so this commit is doing that separately. Looking at the script it should be safe to add a directory that doesn't exist because it should just get ignored

This commit adds a custom task that allows user to run simple sequential Pipelines as one TaskRun - which means the Pipeline can refer to multiple Tasks but run on only one pod. It only supports a subset of Pipeline functionality (more detail on what and why in the README) but is enough that folks can do a lot of what they would have previously used PipelineResources for, e.g. doing a git clone and then doing something with the data, in the same pod, and emit results such as the exact commit sha used. Next steps will be to expand the functionality supported, get feedback, and if the feedback is good, promote this to a top level Pipeline API feature. Experimental project proposal: tektoncd/community#447

Building the custom task on my mac was working just fine but once we started to build it with CI it failed with `no required module provides package github.com/googleapis/gnostic/OpenAPIv2` Turns out this is a known issue: kubernetes/client-go#741 Kinda at a loss as to why other custom tasks aren't running into this and my understanding of go mod is so shallow that I feel like I'm writing with a crayon to fix this (nothing against crayons, they make cool marks) - but anyway I tried to copy what the pipeline go.mod had done since I think that's where the original version came from anyway https://github.com/tektoncd/pipeline/blob/main/go.mod

tekton-robot · 2021-09-02T14:55:23Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jerop

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:

~~pipeline-to-taskrun/OWNERS~~ [jerop]

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

jerop · 2021-09-02T14:57:20Z

/lgtm

tekton-robot requested review from dlorenc and imjasonh July 9, 2021 23:58

tekton-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jul 9, 2021

tekton-robot assigned jerop Jul 12, 2021

tekton-robot assigned afrittoli Jul 12, 2021

bobcatfish mentioned this pull request Jul 19, 2021

Update policy to bump Task/Pipeline versions whenever a change is made tektoncd/catalog#784

Open

jerop reviewed Jul 21, 2021

View reviewed changes

bobcatfish force-pushed the pipeline-to-taskrun branch from 98279da to 8acaeca Compare July 21, 2021 15:33

tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 21, 2021

bobcatfish force-pushed the pipeline-to-taskrun branch from 8acaeca to cf3ba06 Compare July 21, 2021 15:44

tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 21, 2021

jerop reviewed Jul 21, 2021

View reviewed changes

bobcatfish force-pushed the pipeline-to-taskrun branch from 34eeddd to b50f6bc Compare August 13, 2021 22:15

jerop approved these changes Aug 16, 2021

View reviewed changes

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 16, 2021

jerop mentioned this pull request Aug 16, 2021

Add details about adding Owners for new projects #782

Merged

2 tasks

bobcatfish mentioned this pull request Aug 24, 2021

[pipeline-to-taskrun] Add initial directory for Pipeline to TaskRun custom task 🎁 #783

Merged

3 tasks

tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 26, 2021

bobcatfish force-pushed the pipeline-to-taskrun branch from b50f6bc to 8536c44 Compare August 27, 2021 14:02

tekton-robot removed lgtm Indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Aug 27, 2021

jerop approved these changes Aug 31, 2021

View reviewed changes

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 31, 2021

bobcatfish mentioned this pull request Sep 1, 2021

[pipeline-to-taskrun] Get ready to run presubmit tests 🏁 #790

Merged

2 tasks

bobcatfish added 2 commits September 2, 2021 07:55

bobcatfish force-pushed the pipeline-to-taskrun branch from 8536c44 to 388450e Compare September 2, 2021 14:55

tekton-robot removed the lgtm Indicates that a PR is ready to be merged. label Sep 2, 2021

tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 2, 2021

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

tekton-robot merged commit 126b840 into tektoncd:main Sep 2, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[pipeline-to-taskrun] Add custom task that runs a Pipeline as a TaskRun 🍙 #770

[pipeline-to-taskrun] Add custom task that runs a Pipeline as a TaskRun 🍙 #770

bobcatfish commented Jul 9, 2021

jerop commented Jul 12, 2021

jerop commented Jul 12, 2021

jerop left a comment •

edited

Loading

bobcatfish commented Jul 21, 2021

bobcatfish commented Jul 21, 2021

jerop left a comment

jerop Jul 21, 2021

jerop Jul 21, 2021

jerop Jul 21, 2021

jerop Jul 21, 2021

jerop Jul 21, 2021

jerop Jul 21, 2021

bobcatfish Aug 13, 2021

bobcatfish commented Aug 13, 2021

jerop left a comment

jerop commented Aug 16, 2021

jerop left a comment

jerop commented Aug 31, 2021

tekton-robot commented Sep 2, 2021

jerop commented Sep 2, 2021

	app.kubernetes.io/part-of: pip
	app.kubernetes.io/part-of: tekton-pipeline-to-taskrun

[pipeline-to-taskrun] Add custom task that runs a Pipeline as a TaskRun 🍙 #770

[pipeline-to-taskrun] Add custom task that runs a Pipeline as a TaskRun 🍙 #770

Conversation

bobcatfish commented Jul 9, 2021

Changes

Submitter Checklist

jerop commented Jul 12, 2021

jerop commented Jul 12, 2021

jerop left a comment • edited Loading

Choose a reason for hiding this comment

bobcatfish commented Jul 21, 2021

bobcatfish commented Jul 21, 2021

jerop left a comment

Choose a reason for hiding this comment

jerop Jul 21, 2021

Choose a reason for hiding this comment

jerop Jul 21, 2021

Choose a reason for hiding this comment

jerop Jul 21, 2021

Choose a reason for hiding this comment

jerop Jul 21, 2021

Choose a reason for hiding this comment

jerop Jul 21, 2021

Choose a reason for hiding this comment

jerop Jul 21, 2021

Choose a reason for hiding this comment

bobcatfish Aug 13, 2021

Choose a reason for hiding this comment

bobcatfish commented Aug 13, 2021

jerop left a comment

Choose a reason for hiding this comment

jerop commented Aug 16, 2021

jerop left a comment

Choose a reason for hiding this comment

jerop commented Aug 31, 2021

tekton-robot commented Sep 2, 2021

jerop commented Sep 2, 2021

jerop left a comment •

edited

Loading