Add documentation for using Tekton with Prow 🙌 #13974

bobcatfish · 2019-08-19T21:18:20Z

This commit adds the beginning of some usage documentation about how to
get Tekton and Prow working. I think there's probably more to add but I hope this can
be a good starting point!

Please let me know if the docs or the examples should be in different locations, I basically just tried to guess and pick something but am happy to move things around!

I wouldn't have been able to do this without @JoelSpeed's guidance in
#13874 (comment)
Thanks @JoelSpeed!!! ❤️

I was able to follow along with these docs and get Tekton + Prow running
for tektoncd itself: tektoncd/plumbing#66 🎉

Fixes #13874 (and maybe tektoncd/pipeline#1187 as well?)

k8s-ci-robot · 2019-08-19T21:18:28Z

Hi @bobcatfish. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

k8s-ci-robot · 2019-08-19T21:18:44Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: bobcatfish
To complete the pull request process, please assign cjwagner
You can assign the PR to them by writing /assign @cjwagner in a comment when ready.

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

Needs approval from an approver in each of these files:

prow/OWNERS

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

bobcatfish · 2019-08-19T21:19:39Z

Keywords which can automatically close issues and at(@) mentions are not allowed in commit messages.

I understand issue closing but I am curious why @-ing is not okay :O

stevekuznetsov · 2019-08-19T21:49:21Z

Whoever is on the other side of the at-mention will get a notification every time:

you push the commit
your commit is merged into a branch, including:
a. when this PR merges
b. when the commit is cherry-picked into other branches
c. when commits move around in forks of the repo

It ends up being pretty spammy :)

For review
/assign @cjwagner @ccojocar

ccojocar

Thanks for writing this doc! I looks great! I just left a few comments. It would be nice if you could have a look at them.

ccojocar · 2019-08-20T07:34:47Z

prow/cluster/simple_pipeline_deployment.yaml

@@ -0,0 +1,33 @@
+kind: Deployment


There is already a deployment in pipeline_deployment.yaml. I am wondering wha's the reason to not reuse it.

https://github.com/kubernetes/test-infra/blob/master/prow/cluster/pipeline_deployment.yaml uses some other configuration: a secret called build-cluster so in order to use it as is, these instructions would have to explain how to set that up - and since it's optional, I don't think they should? (or maybe by default folks should use --build-cluster=/etc/build-cluster/cluster? I dont actually know what this does 😅 )

That flag is for running the jobs in a different cluster.

I think it is too misleading to include these deployment files in the prow/cluster directory. With the exception of starter.yaml (which we should probably move) all of the files under prow/cluster are deployed to the prow.k8s.io instance, but the files you've added wouldn't work for our instance.

It looks like this is a more detailed version of prow/cmd/pipeline/dev.yaml. Consider updating that file or adding this one to that directory.

I see, it can be actually pointed to the same cluster. It's true there is an additional configuration step to set up that secret.

Maybe it shouldn't fail with an error if the provided file is empty/missing. Sounds like a warnings message and use the same cluster is more appropriate in that case.

It looks like this is a more detailed version of prow/cmd/pipeline/dev.yaml. Consider updating that file or adding this one to that directory.

Ah I didn't even see dev.yaml! For now I've moved these examples into cmd/pipeline and cmd/crier

ccojocar · 2019-08-20T07:36:23Z

prow/cluster/simple_crier_deployment.yaml

@@ -0,0 +1,40 @@
+apiVersion: apps/v1
+kind: Deployment


There is a deployment for crier in crier_deployment.yaml. I am wondering what's the reason to not reuse it.

Similar to above comment about pipeline_deployment.yaml, https://github.com/kubernetes/test-infra/blob/master/prow/cluster/crier_deployment.yaml includes an optional job-config configmap

As with the pipeline controller, this change would prevent crier from working properly on out instance so including it in prow/cluster is kind of misleading.

If you want to add a set of deployment files for a Prow instance that only uses tekton, consider grouping those in a separate directory to make it clear what the purpose is and so that users know what components they actually need to deploy (e.g. sinker and at least one of hook, horologium, or the gerrit adapter will be needed to actually trigger jobs.)

Ah, I see that in the docs you have the user first follow getting_started_deploy.md so hook, horologium, etc will already be deployed.

ccojocar · 2019-08-20T07:41:41Z

prow/tekton.md

+
+```
+
+### Install Tekton Pipelines


I am not sure but I think this step needs to be before installing the controller because the controller expects the PipelineRun CRD to be already installed in order to be able to start watching it.

kk i can try to make that more clear

ccojocar · 2019-08-20T07:54:59Z

prow/tekton.md

+      trigger: # Required by versions 0.2.0 - v0.3.1 of Tekton Pipelines
+        type: manual
+      pipelineRef:
+        name: special-pipeline # Use the Tekton Pipeline called `special-pipeline`


I think the serviceAccount needs also to be defined. It should be the Tekton's service account with the GitHub token linked to it. I am not sure that service account is used by default by the Tekton controller.

It should be the Tekton's service account with the GitHub token linked to it.

Maybe you can explain more about what this service account would be doing? You can optionally provide a serviceAccount to a PipelineRun but that just makes it available to executing Tasks if they need it (e.g. to push to an image registry).

Re the GitHub token, if the PipelineRun was trying to push something to GitHub for example, if you wanted to use the PullRequest resource and push to GitHub from the Pipeline itself, you'd need to provide a secret in the PipelineResource

TL;DR it depends on what the PipelineRun is doing - I can add a link to the PipelineRun docs and try to make that more clear here.

I am not sure that service account is used by default by the Tekton controller.

The controller uses the tekton-pipelines-controller service account by default (https://github.com/tektoncd/pipeline/blob/master/config/controller.yaml#L28) - but pods that execute as part of a PipelineRun would use the default service account of the namespace by default, unless serviceAccount or serviceAccounts are specified

I've added a link to the PipelineRun docs right below this and mentioned ServiceAccounts!

I mean this service account https://github.com/tektoncd/pipeline/blob/master/docs/auth.md#basic-authentication-git, but pointing to the doc is enough if someone needs to configure it. Thanks for adding that!

bobcatfish · 2019-08-20T19:02:53Z

I made some changes, lemme know what you think @ccojocar ! Also let me know what you want to do about the differences between those example yaml files :S

cjwagner

Thanks Christie!

cjwagner · 2019-08-20T22:29:55Z

prow/cluster/simple_pipeline_deployment.yaml

@@ -0,0 +1,33 @@
+kind: Deployment


That flag is for running the jobs in a different cluster.

I think it is too misleading to include these deployment files in the prow/cluster directory. With the exception of starter.yaml (which we should probably move) all of the files under prow/cluster are deployed to the prow.k8s.io instance, but the files you've added wouldn't work for our instance.

It looks like this is a more detailed version of prow/cmd/pipeline/dev.yaml. Consider updating that file or adding this one to that directory.

cjwagner · 2019-08-20T22:38:55Z

prow/cluster/simple_crier_deployment.yaml

@@ -0,0 +1,40 @@
+apiVersion: apps/v1
+kind: Deployment


As with the pipeline controller, this change would prevent crier from working properly on out instance so including it in prow/cluster is kind of misleading.

If you want to add a set of deployment files for a Prow instance that only uses tekton, consider grouping those in a separate directory to make it clear what the purpose is and so that users know what components they actually need to deploy (e.g. sinker and at least one of hook, horologium, or the gerrit adapter will be needed to actually trigger jobs.)

cjwagner · 2019-08-20T22:43:45Z

prow/tekton.md

+**Warning**
+
+Prow users may expect to see logs from their `PipelineRuns` be visible in
+Gubernator, however this functionality is not supported yet, and logs must be


Gubernator is deprecated in favor of Spyglass. It might be more accurate to note that pipeline prowjobs cannot be decorated with the pod utilities so job logs, metadata, and artifacts are not uploaded to GCS.

kk updated!

cjwagner · 2019-08-20T22:45:53Z

prow/cluster/simple_crier_deployment.yaml

@@ -0,0 +1,40 @@
+apiVersion: apps/v1
+kind: Deployment


Ah, I see that in the docs you have the user first follow getting_started_deploy.md so hook, horologium, etc will already be deployed.

cjwagner · 2019-08-20T22:50:58Z

prow/tekton.md

+    skip_report: false
+    pipeline_run_spec: # Template for creation of the `PipelineRun`
+      trigger: # Required by versions 0.2.0 - v0.3.1 of Tekton Pipelines
+        type: manual


If this is required should we have the pipeline prow controller add it if it isn't present?

that would work! even better tho would be to update the version of Pipelines that Prow is using tho - this field is deprecated

cjwagner · 2019-08-20T22:50:59Z

prow/tekton.md

+    rerun_command: "/run do-the-pipeline"
+    trigger: "(?m)^/run do-the-pipeline,?(\\s+|$)"
+    always_run: true
+    skip_report: false


nit: I'd omit the skip_report, rerun_command, and trigger fields. They aren't necessary if you are ok with using /test do-the-pipeline to trigger the job.

cjwagner · 2019-08-20T22:54:37Z

prow/tekton.md

+* `pipelineRef` - Refers to a
+  [`Pipeline`](#create-and-apply-pipelines-and-tasks) that must exist for the
+  `PipelineRun` to be able to execute
+* [`PROW_IMPLICIT_GIT_REF`] - Using this special


It is probably worth noting that we also support PROW_EXTRA_GIT_REF_0, PROW_EXTRA_GIT_REF_1, etc. if the ProwJob specifies extraRefs e.g.

test-infra/config/jobs/kubernetes/test-infra/test-infra-trusted.yaml

Lines 677 to 680 in 6eb5baa

extra_refs:

- org: kubernetes

repo: org

base_ref: master

thanks for mentioning this, this saved my life when I went to use this for periodic jobs XD

cjwagner · 2019-08-20T22:58:01Z

prow/tekton.md

+1. [Setting up Pipelines with Prow](#setting-up-pipelines-with-prow)
+2. [Configuring a `tekton-pipeline` job](#configuring-a-tekton-pipeline-job)
+
+**Warning**


The other caveat is that git pipelineresources are currently not garbage collected. (I think we can fix this by updating the pipelineresources with owner references after the pipelinerun is created.)

I implemented this initially in the controller and it worked well, but I was asked to remove it due to the multi-cluster build support. Referencing the ProwJob is problematic when the builds are executed in a different cluster but I don't see an issue with a reference to the PipelineRun.

Added a warning about this! Btw tektoncd/pipeline#544 is about this and it looks like this will be addressed in tektoncd/pipeline#872 (which was your request originally @cjwagner!) - you'd have to update Prow to be able to embed PipelineResources tho :)

bobcatfish · 2019-09-03T15:02:43Z

Quick update: thanks for the detailed review @cjwagner + @ccojocar - I'm planning to come back and update the PR with your suggestions just haven't gotten a chance yet 🙏

This commit adds the beginning of some usage documentation about how to get Tekton and Prow working. I wouldn't have been able to do this without JoelSpeed's guidance in kubernetes#13874 (comment) Thanks JoelSpeed!!! ❤️ I was able to follow along with these docs and get Tekton + Prow running for `tektoncd` itself: tektoncd/plumbing#66 🎉 This is being done for kubernetes#13874

bobcatfish · 2019-09-19T20:37:01Z

Okay @cjwagner @ccojocar I finally came back to this (sorry for the delay! 🙏 ) and I think I've addressed all your feedback so far (I also added a periodic example), PTAL :D

matthyx · 2019-09-20T05:47:12Z

/ok-to-test

cjwagner

Looks good, I just have some minor suggestions.

cjwagner · 2019-09-23T20:18:39Z

prow/cmd/crier/simple_crier_deployment.yaml

+        - --github-workers=1
+        - --config-path=/etc/config/config.yaml
+        - --github-endpoint=http://ghproxy
+        - --github-endpoint=https://api.github.com


The instructions you provided don't include setting up ghproxy so the --github-endpoint flags will make every request fail on the first attempt. I'd suggest removing them both.

cjwagner · 2019-09-23T20:19:12Z

prow/cmd/crier/simple_crier_deployment.yaml

+      labels:
+        app: crier
+    spec:
+      # Using the service account defined in prow/cluster/pipeline_rbac.yaml


Suggested change

# Using the service account defined in prow/cluster/pipeline_rbac.yaml

# Using the service account defined in prow/cluster/crier_rbac.yaml

cjwagner · 2019-09-23T20:24:06Z

prow/tekton.md

+## Setting up Pipelines with Prow
+
+1. [Setup and configure Prow itself](getting_started_deploy.md)
+2. [Install Tekton Pipelines](#install-tekton)


Suggested change

2. [Install Tekton Pipelines](#install-tekton)

2. [Install Tekton Pipelines](#install-tekton-pipelines)

matthyx · 2019-11-14T13:19:50Z

@bobcatfish will you have time to address these nits? It's too bad to delay merging for small stuff...

zzq889 · 2019-11-21T03:10:33Z

prow/tekton.md

+can start, and in order to be able to execute [Tekton Pipelines](https://github.com/tektoncd/pipeline,
+you also need to install and setup Tekton Pipelines itself.
+
+[Prow is currently compatible with versions 0.2.0 - 0.3.1 of Tekton Pipelines](https://github.com/kubernetes/test-infra/issues/13948)


0.8.0 are supported now

bobcatfish · 2020-02-07T17:30:47Z

Just in case I ever get back to this, prow supports tekton pipelines 0.10.1 now #16099

(sorry @matthyx !!! im gonna try to get this in asap 😭 totally happy if someone else is able to take this over tho!)

bobcatfish · 2020-03-10T15:42:06Z

At this rate I don't think I'm getting back to this anytime soon :( Hopefully what I wrote can still be helpful if someone else can take this on later!

noahkawasakigoogle · 2022-04-04T14:57:56Z

Did any documentation on Prow + Tekton ever happen? I couldnt find anything

k8s-ci-robot added do-not-merge/invalid-commit-message Indicates that a PR should not merge because it has an invalid commit message. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Aug 19, 2019

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Aug 19, 2019

k8s-ci-robot requested review from alvaroaleman and matthyx August 19, 2019 21:18

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. area/prow Issues or PRs related to prow area/prow/bump Updates to the k8s prow cluster sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Aug 19, 2019

bobcatfish force-pushed the pipeline_docs branch from 04bd227 to a8a9b96 Compare August 19, 2019 21:18

bobcatfish force-pushed the pipeline_docs branch from a8a9b96 to a6c702b Compare August 19, 2019 21:20

k8s-ci-robot removed the do-not-merge/invalid-commit-message Indicates that a PR should not merge because it has an invalid commit message. label Aug 19, 2019

k8s-ci-robot assigned ccojocar and cjwagner Aug 19, 2019

ccojocar reviewed Aug 20, 2019

View reviewed changes

bobcatfish force-pushed the pipeline_docs branch from a6c702b to 5a26cc2 Compare August 20, 2019 19:01

cjwagner reviewed Aug 20, 2019

View reviewed changes

bobcatfish force-pushed the pipeline_docs branch from 5a26cc2 to 48be47e Compare September 19, 2019 20:36

k8s-ci-robot added the area/prow/crier Issues or PRs related to prow's crier component label Sep 19, 2019

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Sep 20, 2019

cjwagner reviewed Sep 23, 2019

View reviewed changes

zzq889 reviewed Nov 21, 2019

View reviewed changes

ruzickap mentioned this pull request Feb 12, 2020

Pipeline (Tekton) extension for Prow is not working - pipeline pod is dying #16120

Closed

bobcatfish closed this Mar 10, 2020

bobcatfish mentioned this pull request Apr 8, 2020

Documentations for using tekton as agent in prowjob tektoncd/pipeline#1187

Closed

	# Using the service account defined in prow/cluster/pipeline_rbac.yaml
	# Using the service account defined in prow/cluster/crier_rbac.yaml

	2. [Install Tekton Pipelines](#install-tekton)
	2. [Install Tekton Pipelines](#install-tekton-pipelines)

Add documentation for using Tekton with Prow 🙌 #13974

Add documentation for using Tekton with Prow 🙌 #13974

Conversation

bobcatfish commented Aug 19, 2019

k8s-ci-robot commented Aug 19, 2019

k8s-ci-robot commented Aug 19, 2019

bobcatfish commented Aug 19, 2019

stevekuznetsov commented Aug 19, 2019

ccojocar 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ccojocar Aug 20, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ccojocar Aug 20, 2019 • edited Loading

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

bobcatfish commented Aug 20, 2019

cjwagner 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

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

bobcatfish commented Sep 3, 2019

bobcatfish commented Sep 19, 2019

matthyx commented Sep 20, 2019

cjwagner 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

matthyx commented Nov 14, 2019

Choose a reason for hiding this comment

bobcatfish commented Feb 7, 2020

bobcatfish commented Mar 10, 2020

noahkawasakigoogle commented Apr 4, 2022

ccojocar Aug 20, 2019 •

edited

Loading

ccojocar Aug 20, 2019 •

edited

Loading