remove beta flag check for v1beta1 object param,results and array result #6644

Yongxuanzhang · 2023-05-10T15:50:14Z

Changes

This commit removes the beta feature flag check for v1beta1 object param,results and array result. Beta features are considered to be 'stable' for v1beta1 CRDs, so we shouldn't check them.

Related TEP for more context: https:/tektoncd/community/blob/main/teps/0033-tekton-feature-gates.md#new-api-fields

/kind bug

Signed-off-by: Yongxuan Zhang [email protected]

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

Remove beta feature flag check for v1beta1 object param, results and array result. Object param, results and array result will be enabled if the  enable-api-fields feature flag is not alpha for v1beta1 CRDs (e.g. Tasks and Pipelines)

tekton-robot · 2023-05-10T15:50:16Z

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

tekton-robot · 2023-05-10T15:58: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/v1beta1/task_validation.go	97.3%	97.3%	-0.0
pkg/apis/pipeline/v1beta1/taskrun_validation.go	97.8%	97.7%	-0.0

tekton-robot · 2023-05-10T15:58:48Z

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/v1beta1/task_validation.go	97.3%	97.3%	-0.0
pkg/apis/pipeline/v1beta1/taskrun_validation.go	97.8%	97.7%	-0.0

lbernick · 2023-05-11T14:22:48Z

@Yongxuanzhang i think there are still some tests sticking around that use "alpha" for object params unnecessarily, e.g.

pipeline/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go

Line 111 in ca608f2

wc: config.EnableAlphaAPIFields,

. Can you look into this?

Yongxuanzhang · 2023-05-11T16:35:12Z

@Yongxuanzhang i think there are still some tests sticking around that use "alpha" for object params unnecessarily, e.g.

pipeline/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go

Line 111 in ca608f2

wc: config.EnableAlphaAPIFields,

. Can you look into this?

Thanks! I didn't realize that propagate params tests have this

tekton-robot · 2023-05-11T16:59:23Z

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/v1beta1/task_validation.go	97.3%	97.3%	-0.0
pkg/apis/pipeline/v1beta1/taskrun_validation.go	97.8%	97.7%	-0.0

tekton-robot · 2023-05-11T17:00: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/apis/pipeline/v1beta1/task_validation.go	97.3%	97.3%	-0.0
pkg/apis/pipeline/v1beta1/taskrun_validation.go	97.8%	97.7%	-0.0

tekton-robot · 2023-05-15T18:53: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/pipeline/v1beta1/task_validation.go	97.2%	97.2%	-0.0
pkg/apis/pipeline/v1beta1/taskrun_validation.go	97.8%	97.7%	-0.0

tekton-robot · 2023-05-15T18:54:14Z

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/v1beta1/task_validation.go	97.2%	97.2%	-0.0
pkg/apis/pipeline/v1beta1/taskrun_validation.go	97.8%	97.7%	-0.0

Yongxuanzhang · 2023-05-15T20:07:31Z

/retest

JeromeJu

Thanks @Yongxuanzhang , happy to lgtm after rebased
would you mind adding a link to TEP033 here just for reference as context of the PR?

tekton-robot · 2023-05-17T11:44:47Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JeromeJu, lbernick

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:

~~pkg/apis/OWNERS~~ [lbernick]

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

JeromeJu · 2023-05-17T15:02:41Z

/lgtm

tekton-robot · 2023-05-17T15:08:30Z

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/v1beta1/task_validation.go	97.3%	97.3%	-0.0
pkg/apis/pipeline/v1beta1/taskrun_validation.go	97.8%	97.8%	-0.0

JeromeJu · 2023-05-17T15:11:18Z

/lgtm

tekton-robot · 2023-05-17T15:13:50Z

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/v1beta1/task_validation.go	97.3%	97.3%	-0.0
pkg/apis/pipeline/v1beta1/taskrun_validation.go	97.8%	97.8%	-0.0

tekton-robot · 2023-05-17T15:14: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/v1beta1/task_validation.go	97.3%	97.3%	-0.0
pkg/apis/pipeline/v1beta1/taskrun_validation.go	97.8%	97.8%	-0.0

lbernick

it looks like there are also some places in task_validation_test.go where the alpha flag is used unnecessarily, e.g.

pipeline/pkg/apis/pipeline/v1beta1/task_validation_test.go

Line 1330 in 5abf7c2

ctx := config.EnableAlphaAPIFields(context.Background())

Can those be removed as well?

Yongxuanzhang · 2023-05-17T15:45:23Z

/hold
let me take a look

This commit removes the beta feature flag check for v1beta1 object param,results and array result. Beta features are considered to be 'stable' for v1beta1 CRDs, so we shouldn't check them. Signed-off-by: Yongxuan Zhang [email protected]

Yongxuanzhang · 2023-05-17T15:52:38Z

it looks like there are also some places in task_validation_test.go where the alpha flag is used unnecessarily, e.g.

pipeline/pkg/apis/pipeline/v1beta1/task_validation_test.go

Line 1330 in 5abf7c2

ctx := config.EnableAlphaAPIFields(context.Background())

Can those be removed as well?

Thanks! Just removed 2 places, I may wrongly thought they are used for other features.
I searched the folder again, I think the left ones should be used for sidecar & propogation feature

Yongxuanzhang · 2023-05-17T15:52:56Z

/hold cancel

tekton-robot · 2023-05-17T15:56: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/apis/pipeline/v1beta1/task_validation.go	97.3%	97.3%	-0.0
pkg/apis/pipeline/v1beta1/taskrun_validation.go	97.8%	97.8%	-0.0

tekton-robot · 2023-05-17T15:58:18Z

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/v1beta1/task_validation.go	97.3%	97.3%	-0.0
pkg/apis/pipeline/v1beta1/taskrun_validation.go	97.8%	97.8%	-0.0

Yongxuanzhang · 2023-05-18T14:32:32Z

/retest

JeromeJu · 2023-05-18T18:28:36Z

/lgtm

tekton-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/bug Categorizes issue or PR as related to a bug. release-note-none Denotes a PR that doesnt merit a release note. labels May 10, 2023

tekton-robot requested review from abayer and afrittoli May 10, 2023 15:50

tekton-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. 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 May 10, 2023

Yongxuanzhang marked this pull request as ready for review May 10, 2023 15:52

tekton-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 10, 2023

tekton-robot requested review from bobcatfish and wlynch May 10, 2023 15:52

Yongxuanzhang force-pushed the fix-v1beta1-object branch from 90e5410 to e82ec77 Compare May 11, 2023 16:53

tekton-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 11, 2023

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

lbernick approved these changes May 15, 2023

View reviewed changes

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

Yongxuanzhang force-pushed the fix-v1beta1-object branch from e82ec77 to 86ea75d Compare May 15, 2023 18:47

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

JeromeJu approved these changes May 17, 2023

View reviewed changes

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

Yongxuanzhang force-pushed the fix-v1beta1-object branch from 86ea75d to 4ebcc01 Compare May 17, 2023 15:01

tekton-robot removed lgtm Indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels May 17, 2023

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

Yongxuanzhang force-pushed the fix-v1beta1-object branch from 4ebcc01 to f8f6a3f Compare May 17, 2023 15:07

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

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

lbernick reviewed May 17, 2023

View reviewed changes

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

Yongxuanzhang force-pushed the fix-v1beta1-object branch from f8f6a3f to 9f77b3c Compare May 17, 2023 15:50

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

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

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

tekton-robot merged commit cec2422 into tektoncd:main May 18, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

remove beta flag check for v1beta1 object param,results and array result #6644

remove beta flag check for v1beta1 object param,results and array result #6644

Yongxuanzhang commented May 10, 2023 •

edited

Loading

tekton-robot commented May 10, 2023

tekton-robot commented May 10, 2023

tekton-robot commented May 10, 2023

lbernick commented May 11, 2023

Yongxuanzhang commented May 11, 2023

tekton-robot commented May 11, 2023

tekton-robot commented May 11, 2023

tekton-robot commented May 15, 2023

tekton-robot commented May 15, 2023

Yongxuanzhang commented May 15, 2023

JeromeJu left a comment •

edited

Loading

tekton-robot commented May 17, 2023

JeromeJu commented May 17, 2023

tekton-robot commented May 17, 2023

JeromeJu commented May 17, 2023

tekton-robot commented May 17, 2023

tekton-robot commented May 17, 2023

lbernick left a comment •

edited

Loading

Yongxuanzhang commented May 17, 2023

Yongxuanzhang commented May 17, 2023

Yongxuanzhang commented May 17, 2023

tekton-robot commented May 17, 2023

tekton-robot commented May 17, 2023

Yongxuanzhang commented May 18, 2023

JeromeJu commented May 18, 2023

remove beta flag check for v1beta1 object param,results and array result #6644

remove beta flag check for v1beta1 object param,results and array result #6644

Conversation

Yongxuanzhang commented May 10, 2023 • edited Loading

Changes

Submitter Checklist

Release Notes

tekton-robot commented May 10, 2023

tekton-robot commented May 10, 2023

tekton-robot commented May 10, 2023

lbernick commented May 11, 2023

Yongxuanzhang commented May 11, 2023

tekton-robot commented May 11, 2023

tekton-robot commented May 11, 2023

tekton-robot commented May 15, 2023

tekton-robot commented May 15, 2023

Yongxuanzhang commented May 15, 2023

JeromeJu left a comment • edited Loading

Choose a reason for hiding this comment

tekton-robot commented May 17, 2023

JeromeJu commented May 17, 2023

tekton-robot commented May 17, 2023

JeromeJu commented May 17, 2023

tekton-robot commented May 17, 2023

tekton-robot commented May 17, 2023

lbernick left a comment • edited Loading

Choose a reason for hiding this comment

Yongxuanzhang commented May 17, 2023

Yongxuanzhang commented May 17, 2023

Yongxuanzhang commented May 17, 2023

tekton-robot commented May 17, 2023

tekton-robot commented May 17, 2023

Yongxuanzhang commented May 18, 2023

JeromeJu commented May 18, 2023

Yongxuanzhang commented May 10, 2023 •

edited

Loading

JeromeJu left a comment •

edited

Loading

lbernick left a comment •

edited

Loading