feat: use Tekton Resolvers for a standard on cluster build #1843

zroubalik · 2023-07-04T07:45:15Z

Changes

🎁 Standard Tekton On-cluster builds (non PAC) are using resolvers to get referenced tasks - users don't have to install them
🧹 PAC related templates are stored in .tekton directory in files pipeline-pac.yaml and pipeline-run-pac.yaml (added the -pac suffix to distingush them from standard ones
🐛 PAC templates contain knative/func related labels

I have changed the way how Pipeline and PipelineRun resources are created - not using go client anymore, but using Manifestival to apply YAML resources - this simplifies templating and follows the pattern used for PAC builds.
Another benefit is, that users don't have preinstall Tasks anymore, they should be resolved by Tekton.

The workflow is following:

user runs func deploy --remote
if there are files pipeline.yaml and/or pipeline-run.yaml in the .tekton directory in the function project, we use the resources defined there to create the Pipeline and/or PipelineRun
if these files are not present there, new resources are created on the fly and applied to the cluster

Ability to save the pipelines to the .tekton directory is not implemented yet, it will be in the follow up PR.

Relates #620

knative-prow · 2023-07-04T07:45:17Z

@zroubalik: The label(s) kind/<kind> cannot be applied, because the repository doesn't have them.

In response to this:

Changes

/kind

Fixes #

Release Note
Docs

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.

knative-prow · 2023-07-04T07:45:17Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

knative-prow · 2023-07-04T07:45:19Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: zroubalik

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:

~~OWNERS~~ [zroubalik]

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

codecov · 2023-07-04T10:33:16Z

Codecov Report

Patch coverage: 69.65% and project coverage change: -2.38 ⚠️

Comparison is base (b38d19b) 62.50% compared to head (b795fbe) 60.13%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1843      +/-   ##
==========================================
- Coverage   62.50%   60.13%   -2.38%     
==========================================
  Files         107      107              
  Lines       13800    13710      -90     
==========================================
- Hits         8626     8244     -382     
- Misses       4349     4628     +279     
- Partials      825      838      +13

Flag	Coverage Δ
e2e-test	`?`
e2e-test-oncluster	`30.63% <46.15%> (-0.60%)`	⬇️
e2e-test-oncluster-runtime	`25.67% <52.99%> (?)`
integration-tests	`51.40% <64.95%> (+3.24%)`	⬆️
unit-tests-macos-latest	`49.33% <52.56%> (-0.31%)`	⬇️
unit-tests-ubuntu-latest	`50.12% <52.56%> (-0.27%)`	⬇️
unit-tests-windows-latest	`49.33% <52.56%> (-0.31%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pkg/builders/builders.go	`85.00% <0.00%> (-4.48%)`	⬇️
pkg/pipelines/tekton/pipelines_pac_provider.go	`42.85% <43.75%> (+38.28%)`	⬆️
pkg/k8s/manifestival.go	`50.00% <50.00%> (ø)`
pkg/pipelines/tekton/pipelines_provider.go	`57.54% <62.22%> (+2.96%)`	⬆️
pkg/pipelines/tekton/templates.go	`73.01% <75.46%> (+0.69%)`	⬆️
pkg/pipelines/tekton/resources.go	`81.81% <100.00%> (-8.14%)`	⬇️

... and 14 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Signed-off-by: Zbynek Roubalik <[email protected]>

zroubalik · 2023-07-04T13:04:56Z

I have no idea why does Code Style / style / suggester / shell (pull_request) fail.

lance · 2023-07-04T16:46:15Z

/override "style / suggester / shell"

knative-prow · 2023-07-04T16:46:19Z

@lance: Overrode contexts on behalf of lance: style / suggester / shell

In response to this:

/override "style / suggester / shell"

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.

lance

Looks good @zroubalik! I'd like to understand how we might use this in the openshift dev console as well. It's great that the tasks don't need to be explicitly installed now which will help scenarios like developer sandbox (where users don't have permission to install tasks). Currently dev sandbox does explicitly need to install both the tasks, AND the pipeline (here: https://github.com/openshift-knative/kn-plugin-func/blob/serverless-1.29.0/pkg/pipelines/resources/tekton/pipeline/dev-console/0.1/nodejs-pipeline.yaml). It would be great to eliminate the need for all of that to be installed, which it seems this PR does. We need to communicate with the dev console folks to ensure that we're all on the same page though. Thanks!

zroubalik · 2023-07-07T10:13:56Z

/override "style / suggester / shell"

knative-prow · 2023-07-07T10:13:59Z

@zroubalik: /override requires failed status contexts, check run or a prowjob name to operate on.
The following unknown contexts/checkruns were given:

style / suggester / shell

Only the following failed contexts/checkruns were expected:

EasyCLA
style / suggester / github_actions
style / suggester / yaml
tide
unit-tests_func_main

If you are trying to override a checkrun that has a space in it, you must put a double quote on the context.

In response to this:

/override "style / suggester / shell"

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.

zroubalik · 2023-07-07T10:19:38Z

Looks good @zroubalik! I'd like to understand how we might use this in the openshift dev console as well. It's great that the tasks don't need to be explicitly installed now which will help scenarios like developer sandbox (where users don't have permission to install tasks). Currently dev sandbox does explicitly need to install both the tasks, AND the pipeline (here: https://github.com/openshift-knative/kn-plugin-func/blob/serverless-1.29.0/pkg/pipelines/resources/tekton/pipeline/dev-console/0.1/nodejs-pipeline.yaml). It would be great to eliminate the need for all of that to be installed, which it seems this PR does. We need to communicate with the dev console folks to ensure that we're all on the same page though. Thanks!

Yeah, that's correct. You don't need to install Tasks, Pipeline etc manually.

Does Dev Console solution run func deploy? If so, then all should be set there as well.

The only thing I don't like about this solution is that Tasks need to be fetched from git (and checking out https://github.com/knative/func.git can take some time). I think that we can workaround this by moving tasks to a separate repo (thus a smaller one) or to Tekton Hub, which I prefer. Btw Tekton PAC resolvers (a different technology) can fetch tasks directly from url, which is faster than pulling whole repo...

func/pkg/pipelines/tekton/templates.go

Lines 33 to 72 in b795fbe

    
           	// Following section contains references for Tasks to be used in Pipeline templates, 
        
           	// there is a difference if we use PAC approach or standard Tekton approach. 
        
           	// 
        
           	// This can be simplified once we start consuming tasks from Tekton Hub 
        
           	taskFuncBuildpacksTaskRef = `taskRef: 
        
                   resolver: git 
        
                   params: 
        
                     - name: url 
        
                       value: https://github.com/%s.git 
        
                     - name: revision 
        
                       value: %s 
        
                     - name: pathInRepo 
        
                       value: pkg/pipelines/resources/tekton/task/func-buildpacks/0.1/func-buildpacks.yaml` 
        
           	taskFuncBuildpacksPACTaskRef = `taskRef: 
        
                   kind: Task 
        
                   name: func-buildpacks` 
        
           	taskFuncS2iTaskRef = `taskRef: 
        
                   resolver: git 
        
                   params: 
        
                     - name: url 
        
                       value: https://github.com/%s.git 
        
                     - name: revision 
        
                       value: %s 
        
                     - name: pathInRepo 
        
                       value: pkg/pipelines/resources/tekton/task/func-s2i/0.1/func-s2i.yaml` 
        
           	taskFuncS2iPACTaskRef = `taskRef: 
        
                   kind: Task 
        
                   name: func-s2i` 
        
           	taskFuncDeployTaskRef = `taskRef: 
        
                   resolver: git 
        
                   params: 
        
                     - name: url 
        
                       value: https://github.com/%s.git 
        
                     - name: revision 
        
                       value: %s 
        
                     - name: pathInRepo 
        
                       value: pkg/pipelines/resources/tekton/task/func-deploy/0.1/func-deploy.yaml` 
        
           	taskFuncDeployPACTaskRef = `taskRef: 
        
                   kind: Task 
        
                   name: func-deploy`

lance · 2023-07-11T13:34:10Z

/override "style / suggester / shell"

knative-prow · 2023-07-11T13:34:14Z

@lance: /override requires failed status contexts, check run or a prowjob name to operate on.
The following unknown contexts/checkruns were given:

style / suggester / shell

Only the following failed contexts/checkruns were expected:

EasyCLA
style / suggester / github_actions
style / suggester / yaml
tide
unit-tests_func_main

If you are trying to override a checkrun that has a space in it, you must put a double quote on the context.

In response to this:

/override "style / suggester / shell"

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.

lance · 2023-07-11T15:46:43Z

/override "style / suggester / github_actions"
/override "style / suggester / yaml"

knative-prow · 2023-07-11T15:46:47Z

@lance: Overrode contexts on behalf of lance: style / suggester / github_actions, style / suggester / yaml

In response to this:

/override "style / suggester / github_actions"
/override "style / suggester / yaml"

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.

lance · 2023-07-11T17:03:17Z

/override unit-tests_func_main

knative-prow · 2023-07-11T17:03:21Z

@lance: Overrode contexts on behalf of lance: unit-tests_func_main

In response to this:

/override unit-tests_func_main

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.

lance · 2023-07-11T17:40:40Z

/lgtm

lance · 2023-07-11T17:41:08Z

LOL I was trying to override everything, thinking prow was still balking on stuff - but the problem was simply that it didn't have the lgtm

matejvasek · 2023-07-11T18:25:08Z

@zroubalik does this work on Red Hat Sandbox?

knative-prow bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 4, 2023

knative-prow bot requested review from jrangelramos and vyasgun July 4, 2023 07:45

knative-prow bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jul 4, 2023

zroubalik force-pushed the ocb branch from 6207838 to 94757bb Compare July 4, 2023 10:26

zroubalik force-pushed the ocb branch 6 times, most recently from 58ec87b to 9df018f Compare July 4, 2023 12:31

use Tekton Resolvers for a standard on cluster build

b795fbe

Signed-off-by: Zbynek Roubalik <[email protected]>

zroubalik force-pushed the ocb branch from 9df018f to b795fbe Compare July 4, 2023 12:58

zroubalik marked this pull request as ready for review July 4, 2023 12:58

zroubalik changed the title ~~[WIP] use Tekton Resolvers for a standard on cluster build~~ feat: use Tekton Resolvers for a standard on cluster build Jul 4, 2023

knative-prow bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 4, 2023

knative-prow bot requested a review from nainaz July 4, 2023 12:58

zroubalik requested review from lance, matejvasek and lkingland July 4, 2023 13:01

lance reviewed Jul 4, 2023

View reviewed changes

knative deleted a comment from knative-prow bot Jul 7, 2023

knative-prow bot assigned lance Jul 11, 2023

knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Jul 11, 2023

knative-prow bot merged commit 79c36ee into knative:main Jul 11, 2023
40 of 41 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: use Tekton Resolvers for a standard on cluster build #1843

feat: use Tekton Resolvers for a standard on cluster build #1843

zroubalik commented Jul 4, 2023 •

edited

Loading

knative-prow bot commented Jul 4, 2023

Changes

knative-prow bot commented Jul 4, 2023

knative-prow bot commented Jul 4, 2023

codecov bot commented Jul 4, 2023 •

edited

Loading

zroubalik commented Jul 4, 2023

lance commented Jul 4, 2023

knative-prow bot commented Jul 4, 2023

lance left a comment

zroubalik commented Jul 7, 2023

knative-prow bot commented Jul 7, 2023

zroubalik commented Jul 7, 2023 •

edited

Loading

lance commented Jul 11, 2023

knative-prow bot commented Jul 11, 2023

lance commented Jul 11, 2023

knative-prow bot commented Jul 11, 2023

lance commented Jul 11, 2023

knative-prow bot commented Jul 11, 2023

lance commented Jul 11, 2023

lance commented Jul 11, 2023

matejvasek commented Jul 11, 2023

feat: use Tekton Resolvers for a standard on cluster build #1843

feat: use Tekton Resolvers for a standard on cluster build #1843

Conversation

zroubalik commented Jul 4, 2023 • edited Loading

Changes

knative-prow bot commented Jul 4, 2023

Changes

knative-prow bot commented Jul 4, 2023

knative-prow bot commented Jul 4, 2023

codecov bot commented Jul 4, 2023 • edited Loading

Codecov Report

zroubalik commented Jul 4, 2023

lance commented Jul 4, 2023

knative-prow bot commented Jul 4, 2023

lance left a comment

Choose a reason for hiding this comment

zroubalik commented Jul 7, 2023

knative-prow bot commented Jul 7, 2023

zroubalik commented Jul 7, 2023 • edited Loading

lance commented Jul 11, 2023

knative-prow bot commented Jul 11, 2023

lance commented Jul 11, 2023

knative-prow bot commented Jul 11, 2023

lance commented Jul 11, 2023

knative-prow bot commented Jul 11, 2023

lance commented Jul 11, 2023

lance commented Jul 11, 2023

matejvasek commented Jul 11, 2023

zroubalik commented Jul 4, 2023 •

edited

Loading

codecov bot commented Jul 4, 2023 •

edited

Loading

zroubalik commented Jul 7, 2023 •

edited

Loading