TEP-0107: propagate results to embedded task spec #7100

chengjoey · 2023-09-11T08:18:19Z

Changes

add container type substitution expresions to pipeline task result reference
propagate results to embedded task spec

Part of work on issue #7086 , passing-results

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

Results are propagated in embedded specifications without mutations.

chengjoey · 2023-09-11T08:20:20Z

/kind feature

chengjoey · 2023-09-11T08:20:58Z

/hold

tekton-robot · 2023-09-11T08:24:32Z

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/container_types.go	31.2%	30.2%	-1.0
pkg/apis/pipeline/v1/pipeline_validation.go	99.1%	98.6%	-0.5
pkg/apis/pipeline/v1/resultref.go	98.2%	98.3%	0.1
pkg/reconciler/pipelinerun/pipelinerun.go	92.7%	92.7%	0.0
pkg/reconciler/pipelinerun/resources/apply.go	98.1%	97.7%	-0.4

tekton-robot · 2023-09-11T08:56:56Z

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/container_types.go	31.2%	30.2%	-1.0
pkg/apis/pipeline/v1/pipeline_validation.go	99.1%	98.6%	-0.5
pkg/apis/pipeline/v1/resultref.go	98.2%	98.3%	0.1
pkg/reconciler/pipelinerun/pipelinerun.go	92.7%	92.7%	0.0
pkg/reconciler/pipelinerun/resources/apply.go	98.1%	97.7%	-0.4

tekton-robot · 2023-09-11T09:01: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/container_types.go	31.2%	30.2%	-1.0
pkg/apis/pipeline/v1/pipeline_validation.go	99.1%	98.6%	-0.5
pkg/apis/pipeline/v1/resultref.go	98.2%	98.3%	0.1
pkg/reconciler/pipelinerun/pipelinerun.go	92.7%	92.7%	0.0
pkg/reconciler/pipelinerun/resources/apply.go	98.1%	97.7%	-0.4

chitrangpatel · 2023-09-11T15:48:49Z

/assign

docs/pipelineruns.md

pkg/apis/pipeline/v1/pipeline_validation.go

pkg/apis/pipeline/v1/container_types.go

chengjoey · 2023-09-21T14:23:58Z

/hold cancel

tekton-robot · 2023-09-21T14:28:08Z

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/container_types.go	31.2%	30.2%	-1.0
pkg/apis/pipeline/v1/pipeline_validation.go	99.1%	98.6%	-0.5
pkg/reconciler/pipelinerun/pipelinerun.go	92.7%	92.7%	0.0
pkg/reconciler/pipelinerun/resources/apply.go	98.1%	97.7%	-0.4

tekton-robot · 2023-09-21T14:29: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/container_types.go	31.2%	30.2%	-1.0
pkg/apis/pipeline/v1/pipeline_validation.go	99.1%	98.6%	-0.5
pkg/reconciler/pipelinerun/pipelinerun.go	92.7%	92.7%	0.0
pkg/reconciler/pipelinerun/resources/apply.go	98.1%	97.7%	-0.4

JeromeJu · 2023-09-28T14:36:45Z

docs/pipelineruns.md

+Metadata:
+  ...
+Spec:
+  Task Spec:


nit: the yaml seems to need some formatting: apiVersion, taskSpec and etc.

JeromeJu · 2023-09-28T15:01:15Z

pkg/reconciler/pipelinerun/resources/apply.go

+				stringReplacements[fmt.Sprintf("tasks.%s.results.%s", taskName, res.Name)] = res.Value.StringVal
+			case v1.ResultsTypeArray:
+				arrayReplacements[fmt.Sprintf("tasks.%s.results.%s", taskName, res.Name)] = res.Value.ArrayVal
+			case v1.ResultsTypeObject:


Wondering what is propagation for ResultsTypeObject?

I put the ResultsTypeObject into stringReplacements

In the following way:

for k, v := range res.Value.ObjectVal { stringReplacements[fmt.Sprintf("tasks.%s.results.%s.%s", taskName, res.Name, k)] = v }

JeromeJu · 2023-09-28T15:43:52Z

pkg/apis/pipeline/v1/pipeline_validation.go

@@ -497,6 +497,21 @@ func (pt *PipelineTask) extractAllParams() Params {
 	return allParams
 }

+func (pt *PipelineTask) GetVarSubstitutionExpressions() ([]string, bool) {


Shall we add some more docString for this since it is an exported func?

added

GetVarSubstitutionExpressions extract all values between the parameters "$(" and ")" of steps and sidecars

JeromeJu · 2023-09-28T15:45:25Z

pkg/apis/pipeline/v1/pipeline_validation.go

+	var allExpressions []string
+	if pt.TaskSpec != nil {
+		for _, step := range pt.TaskSpec.Steps {
+			stepExpressions, _ := step.GetVarSubstitutionExpressions()


Could you help me to understand why is the bool return value needed here? It seems that this is implying whether the expressions is of len 0, but the actual returned values are unused.

the returned bool value is indeed not used. I modified the function and only returned expressions.

tekton-robot · 2023-10-20T06:49:54Z

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/container_types.go	31.2%	62.5%	31.2
pkg/apis/pipeline/v1/pipeline_validation.go	99.2%	99.2%	0.0
pkg/reconciler/pipelinerun/pipelinerun.go	92.4%	92.4%	0.0
pkg/reconciler/pipelinerun/resources/apply.go	98.3%	98.4%	0.1

tekton-robot · 2023-10-20T06:50:25Z

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/container_types.go	31.2%	62.5%	31.2
pkg/apis/pipeline/v1/pipeline_validation.go	99.2%	99.2%	0.0
pkg/reconciler/pipelinerun/pipelinerun.go	92.4%	92.4%	0.0
pkg/reconciler/pipelinerun/resources/apply.go	98.3%	98.4%	0.1

tekton-robot · 2023-10-20T07:30:55Z

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/container_types.go	31.2%	62.5%	31.2
pkg/apis/pipeline/v1/pipeline_validation.go	99.2%	99.2%	0.0
pkg/reconciler/pipelinerun/pipelinerun.go	92.4%	92.4%	0.0
pkg/reconciler/pipelinerun/resources/apply.go	98.3%	98.4%	0.1

tekton-robot · 2023-10-20T07:51:49Z

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/container_types.go	31.2%	62.5%	31.2
pkg/apis/pipeline/v1/pipeline_validation.go	99.2%	99.2%	0.0
pkg/reconciler/pipelinerun/pipelinerun.go	92.4%	92.4%	0.0
pkg/reconciler/pipelinerun/resources/apply.go	98.3%	98.4%	0.1

tekton-robot · 2023-10-20T08:31:58Z

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/container_types.go	31.2%	62.5%	31.2
pkg/apis/pipeline/v1/pipeline_validation.go	99.2%	99.2%	0.0
pkg/reconciler/pipelinerun/pipelinerun.go	92.4%	92.4%	0.0
pkg/reconciler/pipelinerun/resources/apply.go	98.3%	98.4%	0.1

tekton-robot · 2023-10-20T08:35:08Z

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/container_types.go	31.2%	62.5%	31.2
pkg/apis/pipeline/v1/pipeline_validation.go	99.2%	99.2%	0.0
pkg/reconciler/pipelinerun/pipelinerun.go	92.4%	92.4%	0.0
pkg/reconciler/pipelinerun/resources/apply.go	98.3%	98.4%	0.1

tekton-robot · 2023-10-20T08:44:17Z

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/container_types.go	31.2%	62.5%	31.2
pkg/apis/pipeline/v1/pipeline_validation.go	99.2%	99.2%	0.0
pkg/reconciler/pipelinerun/pipelinerun.go	92.4%	92.4%	0.0
pkg/reconciler/pipelinerun/resources/apply.go	98.3%	98.4%	0.1

… reference 2. propagate results to embedded task spec Part of work on issue tektoncd#7086 Signed-off-by: chengjoey <zchengjoey@gmail.com>

tekton-robot · 2023-10-20T15:01: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/container_types.go	31.2%	62.5%	31.2
pkg/apis/pipeline/v1/pipeline_validation.go	99.2%	99.2%	0.0
pkg/reconciler/pipelinerun/pipelinerun.go	92.4%	92.4%	0.0
pkg/reconciler/pipelinerun/resources/apply.go	98.3%	98.4%	0.1

tekton-robot · 2023-10-20T15:12:41Z

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/container_types.go	31.2%	62.5%	31.2
pkg/apis/pipeline/v1/pipeline_validation.go	99.2%	99.2%	0.0
pkg/reconciler/pipelinerun/pipelinerun.go	92.4%	92.4%	0.0
pkg/reconciler/pipelinerun/resources/apply.go	98.3%	98.4%	0.1

chengjoey · 2023-10-20T15:34:31Z

@chengjoey I can help. It seems like your test is fighting the feature flags. You need to set "enable-api-fields": "alpha or beta" for this to work. My guess is that's because you are using object results.

Some of the other e2e tests that use enable-api-fields might help here.

Thanks! @chitrangpatel , it worked

jerop

thank you @chengjoey 🎉

tekton-robot · 2023-10-20T17:02:34Z

[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

chitrangpatel · 2023-10-20T18:10:42Z

/lgtm

PR tektoncd#7100 introduced a flaky test. When comparing the pipelineRuns and TaskRuns, the results were not sorted. As a result, `cmp.Diff` throws an error sometimes when the order does not match. This PR sorts the TaskRunResults by providing a sort function to `cmp.Diff`. After the fix, running this 20 times consecutively, no flakes were found.

PR #7100 introduced a flaky test. When comparing the pipelineRuns and TaskRuns, the results were not sorted. As a result, `cmp.Diff` throws an error sometimes when the order does not match. This PR sorts the TaskRunResults by providing a sort function to `cmp.Diff`. After the fix, running this 20 times consecutively, no flakes were found.

tekton-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 11, 2023

tekton-robot requested review from bobcatfish and pritidesai September 11, 2023 08:18

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

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

chengjoey force-pushed the feat/TEP-107-results-propagation branch from 1155c16 to 9f28178 Compare September 11, 2023 08:50

tekton-robot assigned chitrangpatel Sep 11, 2023

chengjoey mentioned this pull request Sep 12, 2023

Results "interpolation" in embedded spec (or "results" propagation) #7086

Closed

jerop reviewed Sep 15, 2023

View reviewed changes

jerop added this to the Pipelines v0.53 milestone Sep 20, 2023

jerop added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Sep 21, 2023

chengjoey force-pushed the feat/TEP-107-results-propagation branch from 9f28178 to 39bfe9f Compare September 21, 2023 14:20

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

JeromeJu reviewed Sep 28, 2023

View reviewed changes

chengjoey force-pushed the feat/TEP-107-results-propagation branch from 39bfe9f to 07e1121 Compare October 7, 2023 02:59

tekton-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 7, 2023

chengjoey requested a review from JeromeJu October 7, 2023 03:04

chengjoey force-pushed the feat/TEP-107-results-propagation branch from 98f857f to fa87932 Compare October 20, 2023 06:41

chengjoey force-pushed the feat/TEP-107-results-propagation branch from fa87932 to 7cb7b9c Compare October 20, 2023 07:25

chengjoey force-pushed the feat/TEP-107-results-propagation branch 2 times, most recently from e80923b to 2539118 Compare October 20, 2023 08:25

1. add container type substitution expresions to pipeline task result…

9be1652

… reference 2. propagate results to embedded task spec Part of work on issue tektoncd#7086 Signed-off-by: chengjoey <zchengjoey@gmail.com>

chengjoey force-pushed the feat/TEP-107-results-propagation branch from 2539118 to 9be1652 Compare October 20, 2023 14:51

chengjoey requested a review from jerop October 20, 2023 15:34

jerop approved these changes Oct 20, 2023

View reviewed changes

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

jerop modified the milestones: Pipelines v0.54, Pipelines v0.53 LTS Oct 20, 2023

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

tekton-robot merged commit 5066b40 into tektoncd:main Oct 20, 2023
12 checks passed

Yongxuanzhang mentioned this pull request Oct 20, 2023

[TEP-0145] Add CEL field to WhenExpression, and feature flag to guard the field #7247

Merged

7 tasks

chitrangpatel mentioned this pull request Oct 20, 2023

Flake Test fix: Sort TaskRunResults #7254

Merged

7 tasks

chitrangpatel mentioned this pull request Apr 5, 2024

feat: introduce when expressions to steps #7746

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TEP-0107: propagate results to embedded task spec #7100

TEP-0107: propagate results to embedded task spec #7100

chengjoey commented Sep 11, 2023 •

edited

Loading

chengjoey commented Sep 11, 2023

chengjoey commented Sep 11, 2023

tekton-robot commented Sep 11, 2023

tekton-robot commented Sep 11, 2023

tekton-robot commented Sep 11, 2023

chitrangpatel commented Sep 11, 2023

chengjoey commented Sep 21, 2023

tekton-robot commented Sep 21, 2023

tekton-robot commented Sep 21, 2023

JeromeJu Sep 28, 2023

chengjoey Oct 7, 2023

JeromeJu Sep 28, 2023

chengjoey Oct 7, 2023

JeromeJu Sep 28, 2023

chengjoey Oct 7, 2023

JeromeJu Sep 28, 2023

chengjoey Oct 7, 2023

tekton-robot commented Oct 20, 2023

tekton-robot commented Oct 20, 2023

tekton-robot commented Oct 20, 2023

tekton-robot commented Oct 20, 2023

tekton-robot commented Oct 20, 2023

tekton-robot commented Oct 20, 2023

tekton-robot commented Oct 20, 2023

tekton-robot commented Oct 20, 2023

tekton-robot commented Oct 20, 2023

chengjoey commented Oct 20, 2023

jerop left a comment

tekton-robot commented Oct 20, 2023

chitrangpatel commented Oct 20, 2023

TEP-0107: propagate results to embedded task spec #7100

TEP-0107: propagate results to embedded task spec #7100

Conversation

chengjoey commented Sep 11, 2023 • edited Loading

Changes

Submitter Checklist

Release Notes

chengjoey commented Sep 11, 2023

chengjoey commented Sep 11, 2023

tekton-robot commented Sep 11, 2023

tekton-robot commented Sep 11, 2023

tekton-robot commented Sep 11, 2023

chitrangpatel commented Sep 11, 2023

chengjoey commented Sep 21, 2023

tekton-robot commented Sep 21, 2023

tekton-robot commented Sep 21, 2023

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

tekton-robot commented Oct 20, 2023

tekton-robot commented Oct 20, 2023

tekton-robot commented Oct 20, 2023

tekton-robot commented Oct 20, 2023

tekton-robot commented Oct 20, 2023

tekton-robot commented Oct 20, 2023

tekton-robot commented Oct 20, 2023

tekton-robot commented Oct 20, 2023

tekton-robot commented Oct 20, 2023

chengjoey commented Oct 20, 2023

jerop left a comment

Choose a reason for hiding this comment

tekton-robot commented Oct 20, 2023

chitrangpatel commented Oct 20, 2023

chengjoey commented Sep 11, 2023 •

edited

Loading