TEP-0142: Introduce StepResults in Steps, StepActions #7382

chitrangpatel · 2023-11-17T02:30:10Z

This PR introduces StepResult in Steps. This will allow us to hold StepAction's result declarations and use it later for replacements and other logic that happens later down the chain. It will be populated by the controller when it fetches StepActions in a followup PR.

Mostly this is auto-generated and refactored code.

Changes

Refactoring:

PR TEP-0142: Introduce Params and Results into StepActions CRD #7317 introduced StepActionResults.
Since StepResults in Step has the same struct, I moved all the supporting code from v1alpha1/ to v1/ instead of duplicating it multiple times.
In StepActions, we update the result's type to Result: []v1.StepResult.

Submitter Checklist

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

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
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.
Release notes contains the string "action required" if the change requires additional action from users switching to the new release

Release Notes

[WIP] Introduce StepResults in Steps for StepAction's Result storage.

/kind feature

chitrangpatel · 2023-11-17T02:34:44Z

/test check-pr-has-kind-label

tekton-robot · 2023-11-17T02:34:47Z

@chitrangpatel: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test pull-tekton-pipeline-alpha-integration-tests
/test pull-tekton-pipeline-beta-integration-tests
/test pull-tekton-pipeline-build-tests
/test pull-tekton-pipeline-integration-tests
/test pull-tekton-pipeline-unit-tests

The following commands are available to trigger optional jobs:

/test pull-tekton-pipeline-go-coverage

Use /test all to run all jobs.

In response to this:

/test check-pr-has-kind-label

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.

tekton-robot · 2023-11-17T02:36:29Z

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/apis/pipeline/v1/result_validation.go	100.0%	98.1%	-1.9
pkg/apis/pipeline/v1/task_validation.go	98.2%	98.3%	0.1
pkg/apis/pipeline/v1alpha1/stepaction_validation.go	99.0%	98.9%	-0.1
pkg/apis/pipeline/v1beta1/result_validation.go	100.0%	97.9%	-2.1
pkg/apis/pipeline/v1beta1/task_validation.go	98.2%	98.3%	0.1

tekton-robot · 2023-11-17T02:41:48Z

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/apis/pipeline/v1/result_validation.go	100.0%	98.1%	-1.9
pkg/apis/pipeline/v1/task_validation.go	98.2%	98.3%	0.1
pkg/apis/pipeline/v1alpha1/stepaction_validation.go	99.0%	98.9%	-0.1
pkg/apis/pipeline/v1beta1/result_validation.go	100.0%	97.9%	-2.1
pkg/apis/pipeline/v1beta1/task_validation.go	98.2%	98.3%	0.1

tekton-robot · 2023-11-17T03:04:33Z

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/apis/pipeline/v1/task_validation.go	98.2%	98.3%	0.1
pkg/apis/pipeline/v1alpha1/stepaction_validation.go	99.0%	98.9%	-0.1
pkg/apis/pipeline/v1beta1/task_validation.go	98.2%	98.3%	0.1

tekton-robot · 2023-11-17T03:06:03Z

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/apis/pipeline/v1/task_validation.go	98.2%	98.3%	0.1
pkg/apis/pipeline/v1alpha1/stepaction_validation.go	99.0%	98.9%	-0.1
pkg/apis/pipeline/v1beta1/task_validation.go	98.2%	98.3%	0.1

chitrangpatel · 2023-11-17T04:05:18Z

/retest

chitrangpatel · 2023-11-17T04:05:46Z

/kind feature

pkg/apis/pipeline/v1/task_validation.go

pkg/apis/pipeline/v1/task_validation_test.go

pkg/apis/pipeline/v1/container_types.go

tekton-robot · 2023-11-17T16:33:34Z

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/apis/pipeline/v1/task_validation.go	98.2%	98.3%	0.1
pkg/apis/pipeline/v1alpha1/stepaction_validation.go	99.0%	98.9%	-0.1
pkg/apis/pipeline/v1beta1/task_validation.go	98.2%	98.3%	0.1

tekton-robot · 2023-11-17T16:36:43Z

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/apis/pipeline/v1/task_validation.go	98.2%	98.3%	0.1
pkg/apis/pipeline/v1alpha1/stepaction_validation.go	99.0%	98.9%	-0.1
pkg/apis/pipeline/v1beta1/task_validation.go	98.2%	98.3%	0.1

tekton-robot · 2023-11-17T20:03:46Z

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/apis/pipeline/v1/task_validation.go	98.2%	98.3%	0.1
pkg/apis/pipeline/v1alpha1/stepaction_validation.go	99.0%	98.9%	-0.1
pkg/apis/pipeline/v1beta1/task_validation.go	98.2%	98.3%	0.1

tekton-robot · 2023-11-17T20:04:19Z

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/apis/pipeline/v1/task_validation.go	98.2%	98.3%	0.1
pkg/apis/pipeline/v1alpha1/stepaction_validation.go	99.0%	98.9%	-0.1
pkg/apis/pipeline/v1beta1/task_validation.go	98.2%	98.3%	0.1

pkg/apis/pipeline/v1/container_types.go

pkg/apis/pipeline/v1/result_validation.go

wlynch · 2023-11-17T21:52:15Z

pkg/apis/pipeline/v1/task_validation.go

 	} else {
 		if len(s.Params) > 0 {
 			errs = errs.Also(&apis.FieldError{
 				Message: "params cannot be used without Ref",
 				Paths:   []string{"params"},
 			})
 		}
+		if len(s.Results) > 0 {
+			if !config.FromContextOrDefaults(ctx).FeatureFlags.EnableStepActions {
+				return apis.ErrGeneric("feature flag %s should be set to true in order to use Results in Steps.", config.EnableStepActions)


You only want to fail if it's a Create, or an Update where the field was originally empty. Otherwise you can cause an object to get stuck because reconcilers can't update the object to remove finalizers if the flag gets disabled mid-execution.

Knative has some helper funcs here to help you extract this data - i.e. https://pkg.go.dev/knative.dev/[email protected]/apis#GetBaseline

So the original field can be non-empty.
i.e. if Users choose to use results for inlined steps.

They just need to enable-step-actions for this, no?

done. I made use of IsInCreate and IsInUpdate for this.

The controller context has these both set to false. But for web hook events, these are set appropriately. This means that during web hook validation, we can enforce this. In the controller logic, this does not really need to be validated since it made it past the web hook.

Mostly sgtm, but apis.IsInUpdate(ctx) isn't sufficient, since it would still reject updates to unrelated fields / finalizers if the flag was disabled. :(

We also need to check if the field we want to check has changed - you can grab the old object via GetBaseline

Got it. I implemented the fix. If an update event, we compare with the baseline and check if the step results have diverged. Only then we check if the feature flag is enabled.

pkg/apis/pipeline/v1/result_validation.go

pkg/apis/pipeline/v1beta1/result_types.go

pkg/apis/pipeline/v1/container_types.go

tekton-robot · 2023-11-21T14:59:14Z

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/apis/pipeline/v1/task_validation.go	98.2%	98.3%	0.1
pkg/apis/pipeline/v1alpha1/stepaction_validation.go	99.0%	98.9%	-0.1
pkg/apis/pipeline/v1beta1/task_validation.go	98.2%	98.3%	0.1

tekton-robot · 2023-11-21T15:00:00Z

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/apis/pipeline/v1/task_validation.go	98.2%	98.3%	0.1
pkg/apis/pipeline/v1alpha1/stepaction_validation.go	99.0%	98.9%	-0.1
pkg/apis/pipeline/v1beta1/task_validation.go	98.2%	98.3%	0.1

pkg/apis/pipeline/v1/result_types.go

pkg/apis/pipeline/v1/container_types.go

chitrangpatel · 2023-11-21T22:03:05Z

/hold cancel

tekton-robot · 2023-11-22T15:09:06Z

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/apis/pipeline/v1/task_validation.go	98.2%	98.4%	0.2
pkg/apis/pipeline/v1alpha1/stepaction_validation.go	99.0%	98.9%	-0.1
pkg/apis/pipeline/v1beta1/task_validation.go	98.2%	98.4%	0.2

tekton-robot · 2023-11-22T15:10:20Z

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/apis/pipeline/v1/task_validation.go	98.2%	98.4%	0.2
pkg/apis/pipeline/v1alpha1/stepaction_validation.go	99.0%	98.9%	-0.1
pkg/apis/pipeline/v1beta1/task_validation.go	98.2%	98.4%	0.2

pkg/apis/pipeline/v1/container_types.go

pkg/apis/pipeline/v1/result_types.go

pkg/apis/pipeline/v1/task_validation_test.go

pkg/apis/pipeline/v1beta1/container_types.go

pkg/apis/pipeline/v1beta1/result_types.go

tekton-robot · 2023-11-22T16:43:28Z

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/apis/pipeline/v1/task_validation.go	98.2%	98.4%	0.2
pkg/apis/pipeline/v1alpha1/stepaction_validation.go	99.0%	98.9%	-0.1
pkg/apis/pipeline/v1beta1/task_validation.go	98.2%	98.3%	0.1

tekton-robot · 2023-11-22T16:44:02Z

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/apis/pipeline/v1/task_validation.go	98.2%	98.4%	0.2
pkg/apis/pipeline/v1alpha1/stepaction_validation.go	99.0%	98.9%	-0.1
pkg/apis/pipeline/v1beta1/task_validation.go	98.2%	98.3%	0.1

pkg/apis/pipeline/v1/result_types.go

tekton-robot · 2023-11-22T17:35:41Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jerop, Yongxuanzhang

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~~ [Yongxuanzhang,jerop]

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

This PR introduces `StepResults` in Steps.

Yongxuanzhang

/lgtm

tekton-robot · 2023-11-22T17:49:33Z

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/apis/pipeline/v1/task_validation.go	98.2%	98.4%	0.2
pkg/apis/pipeline/v1alpha1/stepaction_validation.go	99.0%	98.9%	-0.1
pkg/apis/pipeline/v1beta1/task_validation.go	98.2%	98.3%	0.1

tekton-robot · 2023-11-22T17:51: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/apis/pipeline/v1/task_validation.go	98.2%	98.4%	0.2
pkg/apis/pipeline/v1alpha1/stepaction_validation.go	99.0%	98.9%	-0.1
pkg/apis/pipeline/v1beta1/task_validation.go	98.2%	98.3%	0.1

tekton-robot added the release-note-none Denotes a PR that doesnt merit a release note. label Nov 17, 2023

tekton-robot requested review from bobcatfish and vdemeester November 17, 2023 02:30

tekton-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Nov 17, 2023

chitrangpatel requested review from Yongxuanzhang and removed request for vdemeester and bobcatfish November 17, 2023 02:31

tekton-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesnt merit a release note. labels Nov 17, 2023

chitrangpatel changed the title ~~TEP-0142: Introduce StepResults in Steps~~ TEP-0142: Introduce StepResults in Steps, StepActions Nov 17, 2023

chitrangpatel force-pushed the step-action-step-results-api branch from 85df806 to 9fb20a4 Compare November 17, 2023 02:57

chitrangpatel mentioned this pull request Nov 17, 2023

TEP-0142: Surface step results via termination message #7349

Merged

7 tasks

tekton-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Nov 17, 2023

Yongxuanzhang reviewed Nov 17, 2023

View reviewed changes

pkg/apis/pipeline/v1/task_validation.go Outdated Show resolved Hide resolved

pkg/apis/pipeline/v1/task_validation_test.go Show resolved Hide resolved

pkg/apis/pipeline/v1/container_types.go Show resolved Hide resolved

chitrangpatel force-pushed the step-action-step-results-api branch from 9fb20a4 to 75ddccd Compare November 17, 2023 16:27

chitrangpatel force-pushed the step-action-step-results-api branch from 75ddccd to 6d86b30 Compare November 17, 2023 19:56

wlynch requested changes Nov 17, 2023

View reviewed changes

chitrangpatel force-pushed the step-action-step-results-api branch from 6d86b30 to 06d4709 Compare November 20, 2023 18:29

jerop reviewed Nov 21, 2023

View reviewed changes

pkg/apis/pipeline/v1/container_types.go Outdated Show resolved Hide resolved

chitrangpatel force-pushed the step-action-step-results-api branch from 2f3843a to 13f391d Compare November 21, 2023 14:52

chitrangpatel requested a review from jerop November 21, 2023 15:27

wlynch requested changes Nov 21, 2023

View reviewed changes

pkg/apis/pipeline/v1/result_types.go Show resolved Hide resolved

pkg/apis/pipeline/v1/container_types.go Outdated Show resolved Hide resolved

pkg/apis/pipeline/v1/container_types.go Outdated Show resolved Hide resolved

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

chitrangpatel force-pushed the step-action-step-results-api branch from 13f391d to 485ff46 Compare November 22, 2023 15:02

chitrangpatel requested a review from wlynch November 22, 2023 15:08

jerop mentioned this pull request Nov 22, 2023

TEP-0142: Enable Step Reusability - Implementation #7259

Closed

20 tasks

jerop reviewed Nov 22, 2023

View reviewed changes

chitrangpatel force-pushed the step-action-step-results-api branch from 485ff46 to 6499d9b Compare November 22, 2023 16:36

chitrangpatel requested a review from jerop November 22, 2023 16:57

jerop approved these changes Nov 22, 2023

View reviewed changes

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

Yongxuanzhang approved these changes Nov 22, 2023

View reviewed changes

pkg/apis/pipeline/v1/result_types.go Outdated Show resolved Hide resolved

TEP-0142: Introduce StepResults in Steps

00ac630

This PR introduces `StepResults` in Steps.

chitrangpatel force-pushed the step-action-step-results-api branch from 6499d9b to 00ac630 Compare November 22, 2023 17:43

Yongxuanzhang reviewed Nov 22, 2023

View reviewed changes

tekton-robot assigned Yongxuanzhang Nov 22, 2023

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

tekton-robot merged commit 140b633 into tektoncd:main Nov 22, 2023
12 of 13 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TEP-0142: Introduce StepResults in Steps, StepActions #7382

TEP-0142: Introduce StepResults in Steps, StepActions #7382

chitrangpatel commented Nov 17, 2023 •

edited

Loading

chitrangpatel commented Nov 17, 2023

tekton-robot commented Nov 17, 2023

tekton-robot commented Nov 17, 2023

tekton-robot commented Nov 17, 2023

tekton-robot commented Nov 17, 2023

tekton-robot commented Nov 17, 2023

chitrangpatel commented Nov 17, 2023

chitrangpatel commented Nov 17, 2023

tekton-robot commented Nov 17, 2023

tekton-robot commented Nov 17, 2023

tekton-robot commented Nov 17, 2023

tekton-robot commented Nov 17, 2023

wlynch Nov 17, 2023

chitrangpatel Nov 20, 2023

chitrangpatel Nov 20, 2023

chitrangpatel Nov 20, 2023

wlynch Nov 21, 2023

chitrangpatel Nov 22, 2023

tekton-robot commented Nov 21, 2023

tekton-robot commented Nov 21, 2023

chitrangpatel commented Nov 21, 2023

tekton-robot commented Nov 22, 2023

tekton-robot commented Nov 22, 2023

tekton-robot commented Nov 22, 2023

tekton-robot commented Nov 22, 2023

tekton-robot commented Nov 22, 2023

Yongxuanzhang left a comment

tekton-robot commented Nov 22, 2023

tekton-robot commented Nov 22, 2023

TEP-0142: Introduce StepResults in Steps, StepActions #7382

TEP-0142: Introduce StepResults in Steps, StepActions #7382

Conversation

chitrangpatel commented Nov 17, 2023 • edited Loading

Changes

Submitter Checklist

Release Notes

chitrangpatel commented Nov 17, 2023

tekton-robot commented Nov 17, 2023

tekton-robot commented Nov 17, 2023

tekton-robot commented Nov 17, 2023

tekton-robot commented Nov 17, 2023

tekton-robot commented Nov 17, 2023

chitrangpatel commented Nov 17, 2023

chitrangpatel commented Nov 17, 2023

tekton-robot commented Nov 17, 2023

tekton-robot commented Nov 17, 2023

tekton-robot commented Nov 17, 2023

tekton-robot commented Nov 17, 2023

wlynch Nov 17, 2023

Choose a reason for hiding this comment

chitrangpatel Nov 20, 2023

Choose a reason for hiding this comment

chitrangpatel Nov 20, 2023

Choose a reason for hiding this comment

chitrangpatel Nov 20, 2023

Choose a reason for hiding this comment

wlynch Nov 21, 2023

Choose a reason for hiding this comment

chitrangpatel Nov 22, 2023

Choose a reason for hiding this comment

tekton-robot commented Nov 21, 2023

tekton-robot commented Nov 21, 2023

chitrangpatel commented Nov 21, 2023

tekton-robot commented Nov 22, 2023

tekton-robot commented Nov 22, 2023

tekton-robot commented Nov 22, 2023

tekton-robot commented Nov 22, 2023

tekton-robot commented Nov 22, 2023

Yongxuanzhang left a comment

Choose a reason for hiding this comment

tekton-robot commented Nov 22, 2023

tekton-robot commented Nov 22, 2023

chitrangpatel commented Nov 17, 2023 •

edited

Loading