TEP-0076 (array results and indexing into array) promoted to beta 🎓 #6103

pritidesai · 2023-02-02T20:39:26Z

Changes

Pipelines 0.38 introduced two alpha features - producing array results and indexing into array param (proposed in TEP-0076). This commit is promoting these two features to beta such that these features can be used by the task authors and pipeline authors in a cluster when enable-api-fields is either set to beta or alpha.

These features are still not available in stable API i.e. these features will not work when enable-api-fields set to stable

Before we can merge this PR, I have following three PRs to update the doc and examples. Please review the outstanding PRs first. These PRs are merged now.

Fixes part of #5688.

Signed-off-by: pritidesai pdesai@us.ibm.com

/kind feature

Submitter Checklist

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

Has Docs included if any changes are user facing
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)
Release notes contains the string "action required" if the change requires additional action from users switching to the new release

Release Notes

TEP-0076 promoted to beta - Array results and indexing into an array is now possible with enable-api-fields set to beta.

tekton-robot · 2023-02-02T20:44:24Z

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/config/feature_flags.go	89.5%	87.9%	-1.5
pkg/reconciler/pipelinerun/resources/apply.go	97.5%	97.5%	-0.0
pkg/reconciler/pipelinerun/resources/validate_params.go	96.2%	96.2%	-0.0
pkg/reconciler/taskrun/resources/apply.go	99.3%	99.3%	-0.0

tekton-robot · 2023-02-02T20:47:47Z

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/config/feature_flags.go	89.5%	87.9%	-1.5
pkg/reconciler/pipelinerun/resources/apply.go	97.5%	97.5%	-0.0
pkg/reconciler/pipelinerun/resources/validate_params.go	96.2%	96.2%	-0.0
pkg/reconciler/taskrun/resources/apply.go	99.3%	99.3%	-0.0

tekton-robot · 2023-02-03T17:33:28Z

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/config/feature_flags.go	89.5%	87.9%	-1.5
pkg/reconciler/pipelinerun/resources/apply.go	97.5%	97.5%	-0.0
pkg/reconciler/pipelinerun/resources/validate_params.go	96.2%	96.2%	-0.0
pkg/reconciler/taskrun/resources/apply.go	99.3%	99.3%	-0.0

tekton-robot · 2023-02-03T17:35:59Z

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/config/feature_flags.go	89.5%	87.9%	-1.5
pkg/reconciler/pipelinerun/resources/apply.go	97.5%	97.5%	-0.0
pkg/reconciler/pipelinerun/resources/validate_params.go	96.2%	96.2%	-0.0
pkg/reconciler/taskrun/resources/apply.go	99.3%	99.3%	-0.0

tekton-robot · 2023-02-03T18:00: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/config/feature_flags.go	89.5%	94.0%	4.5
pkg/reconciler/pipelinerun/resources/apply.go	97.5%	97.5%	-0.0
pkg/reconciler/pipelinerun/resources/validate_params.go	96.2%	96.2%	-0.0
pkg/reconciler/taskrun/resources/apply.go	99.3%	99.3%	-0.0

tekton-robot · 2023-02-03T18:30:04Z

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/config/feature_flags.go	89.5%	94.0%	4.5
pkg/reconciler/pipelinerun/resources/apply.go	97.5%	97.5%	-0.0
pkg/reconciler/pipelinerun/resources/validate_params.go	96.2%	96.2%	-0.0
pkg/reconciler/taskrun/resources/apply.go	99.3%	99.3%	-0.0

tekton-robot · 2023-02-07T16:52: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/config/feature_flags.go	88.6%	93.5%	4.9
pkg/reconciler/pipelinerun/resources/apply.go	97.5%	97.5%	-0.0
pkg/reconciler/pipelinerun/resources/validate_params.go	96.2%	96.2%	-0.0
pkg/reconciler/taskrun/resources/apply.go	99.3%	99.3%	-0.0

JeromeJu · 2023-02-07T17:41:03Z

/assign @afrittoli @JeromeJu

Yongxuanzhang · 2023-02-07T18:07:59Z

/assign

pritidesai · 2023-02-09T19:36:57Z

@afrittoli @JeromeJu @Yongxuanzhang I appreciate if you can review the changes in this PR, my team is looking for this feature as part of 0.45. Thanks!

pkg/apis/config/feature_flags_test.go

Yongxuanzhang

/lgtm
Thanks! Sorry I thought we want to merge other issues fixes before this one. I think it's ok to get this merged in v0.45

I will update other PRs (like #6120) once we get this merged.

vdemeester · 2023-02-14T09:57:51Z

/hold

vdemeester · 2023-02-14T09:58:24Z

Cc @abayer @afrittoli @jerop to remove the hold 😛 but yay 🎉

afrittoli

Thanks @pritidesai 🎉 - this looks mostly good to me, only a couple of minor things - it would be nice to have the docs updated, especially given the fact that we don't have v1/v1beta separate docs, so the docs should default to the latest API which is v1

afrittoli · 2023-02-14T10:28:13Z

docs/pipelines.md

-Both are alpha features and can be enabled by setting `enable-api-fields` to `alpha`.
+Object result is alpha feature and can be enabled by setting `enable-api-fields` to `alpha`.


NIT: shouldn't we say that "array results are a beta feature. They can be enabled in the v1 API by setting the enable-api-fields to beta

I am not sure, with v1beta1 API, for this feature to work, enable-api-fields does not have to be alpha anymore which means it can be set to either beta or stable.

@chitrangpatel how did you approach this when we promoted propagated params/workspaces features? 🤔 Or these features were not classified as beta? will appreciate any guidance. Thanks!

Propagated Params was moved straight to stable since stable and beta were same at the time.
Propagated workspaces is currently beta meaning that it will work if enable-api-fields is alpha or beta but not stable.

yea, we should clarify that array results is an beta feature

@pritidesai it seems that in both v1 and v1beta1, we check that beta feature flag is enabled -- @lbernick we may need clarification in the TEP about whether the flag should be the same in both versions

Propagated workspaces is currently beta meaning that it will work if enable-api-fields is alpha or beta but not stable.

Thanks @chitrangpatel for getting back. How did we document that propagated workspace is beta feature? At the same time, I am checking docs. I just want to make sure its consistent across many different features.

https://github.com/tektoncd/pipeline/pull/6030/files is all I had done.

So I just checked the PR #6030, there is no specific guidelines other than the replacing alpha with beta in the docs 🙃 I will update the docs here but it's really very confusing for folks using v1beta1 APIs.

afrittoli · 2023-02-14T10:29:40Z

docs/tasks.md

@@ -833,7 +833,7 @@ or [at the `Pipeline` level](./pipelines.md#emitting-results-from-a-pipeline).
 #### Emitting Array `Results`

 Tekton Task also supports defining a result of type `array` and `object` in addition to `string`.
-Emitting a task result of type `array` is an `alpha` feature implemented based on the
+Emitting a task result of type `array` is implemented based on the


Ditto: shouldn't me mention that arrays are a beta feature?

afrittoli · 2023-02-14T10:32:27Z

pkg/apis/config/feature_flags_test.go

+		expectedResult: true,
+	}, {
+		name:           "when enable-api-fields is set to stable",
+		c:              config.EnableStableAPIFields(ctx),


NIT: stable is the default (and will always be), so this feels a bit redundant.

Yup I completely agree, stable is a default. its just added to be consistent with the

/approve

Good by me!

thanks @abayer 🙏

@vdemeester can we cancel the hold? I do not think there is anything pending here now. If there is a concern of array params in a matrix and when expressions, we had examples and doc updated for both features in PR #6092 and PR #6077. @Yongxuanzhang please confirm!

afrittoli · 2023-02-14T10:35:47Z

pkg/apis/config/feature_flags.go

+// EnableStableAPIFields enables stable features in an existing context (for use in testing)
+func EnableStableAPIFields(ctx context.Context) context.Context {
+	return setEnableAPIFields(ctx, StableAPIFields)
+}
+


Why do we need this? As far as I can tell it's only used in the unit test for CheckAlphaOrBetaAPIFields - and it doesn't look like it's really needed there either as stable is the default value.

pkg/apis/pipeline/v1/result_validation.go

afrittoli · 2023-02-14T10:45:37Z

pkg/apis/pipeline/v1beta1/result_validation.go

-	// By default the result type is string
-	if tr.Type != ResultsTypeString {
+	// By default, the result type is string
+	case tr.Type != ResultsTypeString:


Ditto: same as above, I would prefer using default here

pkg/apis/pipeline/v1beta1/result_validation_test.go

pritidesai · 2023-02-14T16:49:51Z

Thanks @pritidesai 🎉 - this looks mostly good to me, only a couple of minor things - it would be nice to have the docs updated, especially given the fact that we don't have v1/v1beta separate docs, so the docs should default to the latest API which is v1

Thank you @afrittoli, how can we resolve this?

pritidesai · 2023-02-14T16:51:20Z

Cc @abayer @afrittoli @jerop to remove the hold 😛 but yay 🎉

thanks @vdemeester 🙏

@abayer @jerop anything pending to cancel the hold? 😕

abayer · 2023-02-14T16:59:58Z

/approve

Good by me!

pkg/apis/config/feature_flags.go

Pipelines 0.38 introduced two alpha features - producing array results and indexing into array param. This commit is promoting these two features to beta such that these features can be used by the task authors and pipeline authors by setting enable-api-fields to either beta or alpha. These features are still not availalbe in stable API i.e. these features will not work with enable-api-fields set to stable Signed-off-by: pritidesai <pdesai@us.ibm.com>

tekton-robot · 2023-02-14T18:40:22Z

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/config/feature_flags.go	88.6%	93.5%	4.9
pkg/reconciler/pipelinerun/resources/apply.go	97.5%	97.5%	-0.0
pkg/reconciler/pipelinerun/resources/validate_params.go	96.2%	96.2%	-0.0
pkg/reconciler/taskrun/resources/apply.go	99.3%	99.3%	-0.0

tekton-robot · 2023-02-14T18:41:26Z

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/config/feature_flags.go	88.6%	93.5%	4.9
pkg/reconciler/pipelinerun/resources/apply.go	97.5%	97.5%	-0.0
pkg/reconciler/pipelinerun/resources/validate_params.go	96.2%	96.2%	-0.0
pkg/reconciler/taskrun/resources/apply.go	99.3%	99.3%	-0.0

pritidesai · 2023-02-14T18:48:58Z

thank you @Yongxuanzhang @afrittoli @vdemeester @abayer @JeromeJu @jerop and @chitrangpatel 🙏

I have updated this PR to document that this feature is beta feature. I am looking for lgtm unless there is anything else pending. Thanks a bunch!

tekton-robot · 2023-02-14T18:52:59Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abayer, JeromeJu, jerop, vdemeester

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

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

Yongxuanzhang · 2023-02-14T18:59:44Z

/lgtm
Thanks!

afrittoli · 2023-02-15T12:04:51Z

/lgtm

afrittoli · 2023-02-15T12:41:14Z

/hold cancel

This commit includes two changes to the table with beta features: 1. Alpha Release: an additional column to keep history of when the feature was first available as alpha 2. TEP-0076: Array results was promoted to beta in PR - tektoncd#6103 Signed-off-by: pritidesai <pdesai@us.ibm.com>

This commit includes two changes to the table with beta features: 1. Alpha Release: an additional column to keep history of when the feature was first available as alpha 2. TEP-0076: Array results was promoted to beta in PR - #6103 Signed-off-by: pritidesai <pdesai@us.ibm.com>

tekton-robot added kind/feature Categorizes issue or PR as related to a new feature. release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Feb 2, 2023

tekton-robot requested review from afrittoli and dibyom February 2, 2023 20:39

tekton-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 2, 2023

jerop added this to the Pipelines v0.45 milestone Feb 3, 2023

pritidesai force-pushed the promote-0076-to-beta-1 branch from 36d8c5c to def864e Compare February 3, 2023 17:28

pritidesai force-pushed the promote-0076-to-beta-1 branch from def864e to a9f8f6e Compare February 3, 2023 17:55

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

pritidesai force-pushed the promote-0076-to-beta-1 branch from a9f8f6e to 9f7716c Compare February 7, 2023 16:47

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

tekton-robot assigned afrittoli and JeromeJu Feb 7, 2023

tekton-robot assigned Yongxuanzhang Feb 7, 2023

JeromeJu approved these changes Feb 9, 2023

View reviewed changes

pkg/apis/config/feature_flags_test.go Show resolved Hide resolved

Yongxuanzhang reviewed Feb 9, 2023

View reviewed changes

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

vdemeester approved these changes Feb 14, 2023

View reviewed changes

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

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

afrittoli reviewed Feb 14, 2023

View reviewed changes

afrittoli mentioned this pull request Feb 14, 2023

add feature gate check for param array indexing #6120

Closed

7 tasks

jerop reviewed Feb 14, 2023

View reviewed changes

pkg/apis/config/feature_flags.go Show resolved Hide resolved

pritidesai force-pushed the promote-0076-to-beta-1 branch from 9f7716c to 2aa5ac9 Compare February 14, 2023 18:32

tekton-robot removed the lgtm Indicates that a PR is ready to be merged. label Feb 14, 2023

jerop approved these changes Feb 14, 2023

View reviewed changes

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

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

tekton-robot merged commit 9c1ecae into tektoncd:main Feb 15, 2023

pritidesai deleted the promote-0076-to-beta-1 branch February 15, 2023 16:21

pritidesai mentioned this pull request Feb 16, 2023

updating the list of beta features #6181

Merged

7 tasks

Yongxuanzhang mentioned this pull request May 1, 2023

ValidateParamArrayIndex for taskSpec should check beta flag. #6607

Closed

Yongxuanzhang mentioned this pull request May 30, 2023

move tep75 tep76 and tep 107 examples from alpha to beta #6747

Merged

7 tasks

		Both are alpha features and can be enabled by setting `enable-api-fields` to `alpha`.
		Object result is alpha feature and can be enabled by setting `enable-api-fields` to `alpha`.

TEP-0076 (array results and indexing into array) promoted to beta 🎓 #6103

TEP-0076 (array results and indexing into array) promoted to beta 🎓 #6103

Conversation

pritidesai commented Feb 2, 2023 • edited Loading

Changes

Submitter Checklist

Release Notes

tekton-robot commented Feb 2, 2023

tekton-robot commented Feb 2, 2023

tekton-robot commented Feb 3, 2023

tekton-robot commented Feb 3, 2023

tekton-robot commented Feb 3, 2023

tekton-robot commented Feb 3, 2023

tekton-robot commented Feb 7, 2023

JeromeJu commented Feb 7, 2023

Yongxuanzhang commented Feb 7, 2023

pritidesai commented Feb 9, 2023

Yongxuanzhang left a comment • edited Loading

Choose a reason for hiding this comment

vdemeester commented Feb 14, 2023

vdemeester commented Feb 14, 2023

afrittoli 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

pritidesai commented Feb 14, 2023

pritidesai commented Feb 14, 2023

abayer commented Feb 14, 2023

tekton-robot commented Feb 14, 2023

tekton-robot commented Feb 14, 2023

pritidesai commented Feb 14, 2023

tekton-robot commented Feb 14, 2023

Yongxuanzhang commented Feb 14, 2023

afrittoli commented Feb 15, 2023

afrittoli commented Feb 15, 2023

pritidesai commented Feb 2, 2023 •

edited

Loading

Yongxuanzhang left a comment •

edited

Loading