Cleanup: Use CustomRun instead of RunObject #6718

lbernick · 2023-05-25T17:22:53Z

v1alpha1 Runs are no longer supported by the PipelineRun reconciler. The PipelineRun reconciler currently operates on RunObjects (an interface which is implemented by v1alpha1.Run and v1beta1.CustomRun). However, this code is misleading, since retry behavior differs between v1alpha1.Run and v1beta1.CustomRun, but the controller only supports the behavior of v1beta1.CustomRuns. This commit replaces usages of RunObject with CustomRun, and renames references to runs/runObjects to customRuns for clarity. No functional changes.

/kind cleanup
Partially addresses #6628

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

n/a Has Docs if any changes are user facing, including updates to minimum requirements e.g. Kubernetes version bumps
Has Tests included if any functionality added or changed
Follows the commit message standard
Meets the Tekton contributor standards (including functionality, content, code)
Has a kind label. You can add one by adding a comment on this PR that contains /kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tep
n/a Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings). See some examples of good release notes.
n/a Release notes contains the string "action required" if the change requires additional action from users switching to the new release

Release Notes

NONE

tekton-robot · 2023-05-25T17:28:08Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	90.3%	90.4%	0.1

tekton-robot · 2023-05-25T17:30:07Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	90.3%	90.4%	0.1

tekton-robot · 2023-05-25T17:33:43Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	90.3%	90.4%	0.1

tekton-robot · 2023-05-25T17:37:27Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	90.3%	90.4%	0.1

lbernick · 2023-05-25T18:26:20Z

Unit tests are failing due to build failures, but this passes locally so I'm not sure what's going on here... 🤔

tekton-robot · 2023-05-26T13:39:45Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	90.3%	90.4%	0.1

tekton-robot · 2023-05-26T13:41:16Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	90.3%	90.4%	0.1

tekton-robot · 2023-05-26T13:50:37Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	90.3%	90.4%	0.1
pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go	97.0%	97.0%	-0.0
pkg/reconciler/pipelinerun/resources/pipelinerunstate.go	99.5%	99.5%	-0.0
pkg/reconciler/pipelinerun/resources/resultrefresolution.go	92.3%	92.2%	-0.1

tekton-robot · 2023-05-26T13:50:47Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	90.3%	90.4%	0.1
pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go	97.0%	97.0%	-0.0
pkg/reconciler/pipelinerun/resources/pipelinerunstate.go	99.5%	99.5%	-0.0
pkg/reconciler/pipelinerun/resources/resultrefresolution.go	92.3%	92.2%	-0.1

tekton-robot · 2023-05-26T15:13:51Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	90.3%	90.4%	0.1
pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go	97.0%	97.0%	-0.0
pkg/reconciler/pipelinerun/resources/pipelinerunstate.go	99.5%	99.5%	-0.0
pkg/reconciler/pipelinerun/resources/resultrefresolution.go	92.3%	92.2%	-0.1

tekton-robot · 2023-05-26T15:15:58Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	90.3%	90.4%	0.1
pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go	97.0%	97.0%	-0.0
pkg/reconciler/pipelinerun/resources/pipelinerunstate.go	99.5%	99.5%	-0.0
pkg/reconciler/pipelinerun/resources/resultrefresolution.go	92.3%	92.2%	-0.1

tekton-robot · 2023-05-30T14:03:07Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	90.4%	90.4%	0.1

tekton-robot · 2023-05-30T14:07:02Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	90.4%	90.4%	0.1

tekton-robot · 2023-05-30T16:21:07Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	90.4%	90.4%	0.1
pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go	97.0%	97.0%	-0.0
pkg/reconciler/pipelinerun/resources/pipelinerunstate.go	99.5%	99.5%	-0.0
pkg/reconciler/pipelinerun/resources/resultrefresolution.go	92.3%	92.2%	-0.1

tekton-robot · 2023-05-30T16:24:33Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	90.4%	90.4%	0.1
pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go	97.0%	97.0%	-0.0
pkg/reconciler/pipelinerun/resources/pipelinerunstate.go	99.5%	99.5%	-0.0
pkg/reconciler/pipelinerun/resources/resultrefresolution.go	92.3%	92.2%	-0.1

v1alpha1 Runs are no longer supported by the PipelineRun reconciler. The PipelineRun reconciler currently operates on RunObjects (an interface which is implemented by v1alpha1.Run and v1beta1.CustomRun). However, this code is misleading, since retry behavior differs between v1alpha1.Run and v1beta1.CustomRun, but the controller only supports the behavior of v1beta1.CustomRuns. This commit replaces usages of RunObject with CustomRun, and renames references to runs/runObjects to customRuns for clarity. No functional changes.

tekton-robot · 2023-05-30T16:45:53Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	90.4%	90.4%	0.1
pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go	97.0%	97.0%	-0.0
pkg/reconciler/pipelinerun/resources/pipelinerunstate.go	99.5%	99.5%	-0.0
pkg/reconciler/pipelinerun/resources/resultrefresolution.go	92.3%	92.2%	-0.1

tekton-robot · 2023-05-30T16:46:56Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	90.4%	90.4%	0.1
pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go	97.0%	97.0%	-0.0
pkg/reconciler/pipelinerun/resources/pipelinerunstate.go	99.5%	99.5%	-0.0
pkg/reconciler/pipelinerun/resources/resultrefresolution.go	92.3%	92.2%	-0.1

tekton-robot · 2023-05-30T16:48:38Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage-df to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/reconciler/pipelinerun/pipelinerun.go	90.4%	90.4%	0.1
pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go	97.0%	97.0%	-0.0
pkg/reconciler/pipelinerun/resources/pipelinerunstate.go	99.5%	99.5%	-0.0
pkg/reconciler/pipelinerun/resources/resultrefresolution.go	92.3%	92.2%	-0.1

Yongxuanzhang

/lgtm
Thanks for the cleanup!

Yongxuanzhang · 2023-05-30T20:38:06Z

pkg/reconciler/pipelinerun/pipelinerun.go

 			if err != nil {
-				recorder.Eventf(pr, corev1.EventTypeWarning, "RunsCreationFailed", "Failed to create Runs %q: %v", rpt.RunObjectNames, err)
-				err = fmt.Errorf("error creating Runs called %s for PipelineTask %s from PipelineRun %s: %w", rpt.RunObjectNames, rpt.PipelineTask.Name, pr.Name, err)
+				recorder.Eventf(pr, corev1.EventTypeWarning, "RunsCreationFailed", "Failed to create CustomRuns %q: %v", rpt.CustomRunNames, err)


nit: should this be "CustomRunsCreationFailed"?

TBH I am not sure whether people filter on event types-- I wasn't sure if changing this would be backwards compatible

Maybe keep it as is

tekton-robot · 2023-06-08T19:34:24Z

[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:

~~OWNERS~~ [jerop]

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

lbernick · 2023-06-08T19:34:34Z

/hold

want to experiment a bit

lbernick · 2023-06-08T20:32:06Z

/hold cancel

tekton-robot added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. release-note-none Denotes a PR that doesnt merit a release note. labels May 25, 2023

tekton-robot requested review from abayer and dibyom May 25, 2023 17:22

tekton-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label May 25, 2023

lbernick force-pushed the replace-runobject branch from 33b4240 to 4d9b96c Compare May 25, 2023 17:28

lbernick force-pushed the replace-runobject branch from 4d9b96c to db1f8f7 Compare May 26, 2023 13:34

lbernick force-pushed the replace-runobject branch from db1f8f7 to 70a79dc Compare May 26, 2023 13:44

lbernick force-pushed the replace-runobject branch from 70a79dc to 608f53d Compare May 26, 2023 15:08

tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 29, 2023

lbernick force-pushed the replace-runobject branch from 608f53d to 50131c4 Compare May 30, 2023 13:57

tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 30, 2023

lbernick force-pushed the replace-runobject branch from 50131c4 to 73928cf Compare May 30, 2023 16:15

lbernick force-pushed the replace-runobject branch from 73928cf to fdc0112 Compare May 30, 2023 16:39

lbernick force-pushed the replace-runobject branch from fdc0112 to 02ea45f Compare May 30, 2023 16:40

Yongxuanzhang reviewed May 30, 2023

View reviewed changes

tekton-robot assigned Yongxuanzhang May 30, 2023

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label May 30, 2023

jerop self-assigned this Jun 5, 2023

jerop approved these changes Jun 8, 2023

View reviewed changes

tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 8, 2023

tekton-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 8, 2023

tekton-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 8, 2023

tekton-robot merged commit ee3c22c into tektoncd:main Jun 8, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cleanup: Use CustomRun instead of RunObject #6718

Cleanup: Use CustomRun instead of RunObject #6718

lbernick commented May 25, 2023 •

edited

Loading

tekton-robot commented May 25, 2023

tekton-robot commented May 25, 2023

tekton-robot commented May 25, 2023

tekton-robot commented May 25, 2023

lbernick commented May 25, 2023

tekton-robot commented May 26, 2023

tekton-robot commented May 26, 2023

tekton-robot commented May 26, 2023

tekton-robot commented May 26, 2023

tekton-robot commented May 26, 2023

tekton-robot commented May 26, 2023

tekton-robot commented May 30, 2023

tekton-robot commented May 30, 2023

tekton-robot commented May 30, 2023

tekton-robot commented May 30, 2023

tekton-robot commented May 30, 2023

tekton-robot commented May 30, 2023

tekton-robot commented May 30, 2023

Yongxuanzhang left a comment

Yongxuanzhang May 30, 2023

lbernick May 31, 2023

Yongxuanzhang May 31, 2023

tekton-robot commented Jun 8, 2023

lbernick commented Jun 8, 2023

lbernick commented Jun 8, 2023

Cleanup: Use CustomRun instead of RunObject #6718

Cleanup: Use CustomRun instead of RunObject #6718

Conversation

lbernick commented May 25, 2023 • edited Loading

Submitter Checklist

Release Notes

tekton-robot commented May 25, 2023

tekton-robot commented May 25, 2023

tekton-robot commented May 25, 2023

tekton-robot commented May 25, 2023

lbernick commented May 25, 2023

tekton-robot commented May 26, 2023

tekton-robot commented May 26, 2023

tekton-robot commented May 26, 2023

tekton-robot commented May 26, 2023

tekton-robot commented May 26, 2023

tekton-robot commented May 26, 2023

tekton-robot commented May 30, 2023

tekton-robot commented May 30, 2023

tekton-robot commented May 30, 2023

tekton-robot commented May 30, 2023

tekton-robot commented May 30, 2023

tekton-robot commented May 30, 2023

tekton-robot commented May 30, 2023

Yongxuanzhang left a comment

Choose a reason for hiding this comment

Yongxuanzhang May 30, 2023

Choose a reason for hiding this comment

lbernick May 31, 2023

Choose a reason for hiding this comment

Yongxuanzhang May 31, 2023

Choose a reason for hiding this comment

tekton-robot commented Jun 8, 2023

lbernick commented Jun 8, 2023

lbernick commented Jun 8, 2023

lbernick commented May 25, 2023 •

edited

Loading