Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

TEP-0106: Support Specifying Metadata per Task in Runtime #4834

Merged
merged 1 commit into from
May 27, 2022
Merged

TEP-0106: Support Specifying Metadata per Task in Runtime #4834

merged 1 commit into from
May 27, 2022

Conversation

austinzhao-go
Copy link
Contributor

@austinzhao-go austinzhao-go commented May 4, 2022 •
edited
Loading

Changes

TEP PR: TEP-0106

This work will support a user specifying the required metadata (annotations and/or labels) for a referenced Task in a PipelineRun. So the metadata depending on an execution context can be added in the runtime when they can not be statically defined in a Task during the "authoring" time.

TODO

  • Confirm annotations propagating details
  • Update unit tests
  • Fix integration tests
  • Update docs
  • Apply similar logics on labels

Submitter Checklist

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

  • Docs included if any changes are user facing
  • Tests included if any functionality added or changed
  • Follows the commit message standard
  • Meets the Tekton contributor standards (including
    functionality, content, code)
  • Release notes block below has been filled in
    (if there are no user facing changes, use release note "NONE")

Release Notes

Support Specifying Metadata per Task in Runtime (PipelineRun)

@tekton-robot tekton-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 4, 2022
@austinzhao-go austinzhao-go mentioned this pull request May 4, 2022
Closed
@austinzhao-go
Copy link
Contributor Author

/assign @dibyom
/kind feature

@tekton-robot tekton-robot added the kind/feature Categorizes issue or PR as related to a new feature. label May 4, 2022
@austinzhao-go austinzhao-go marked this pull request as draft May 4, 2022 17:04
@austinzhao-go austinzhao-go changed the title [WIP] Add Metadata Support [WIP] TEP-0106: Support Specifying Metadata per Task in Runtime May 4, 2022
@austinzhao-go austinzhao-go force-pushed the tep-0106-support-metadata-runtime branch from c69c670 to e53e2c1 Compare May 5, 2022 03:27
@tekton-robot 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 5, 2022
@tekton-robot
Copy link
Collaborator

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/pipelinerun_types.go 80.9% 81.1% 0.2
pkg/reconciler/pipelinerun/pipelinerun.go 86.0% 86.0% -0.0

@austinzhao-go austinzhao-go mentioned this pull request May 5, 2022
Merged
@austinzhao-go austinzhao-go marked this pull request as ready for review May 5, 2022 15:42
@austinzhao-go austinzhao-go changed the title [WIP] TEP-0106: Support Specifying Metadata per Task in Runtime TEP-0106: Support Specifying Metadata per Task in Runtime May 5, 2022
@tekton-robot 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 5, 2022
@austinzhao-go
Copy link
Contributor Author

/test pull-tekton-pipeline-alpha-integration-tests

@austinzhao-go austinzhao-go force-pushed the tep-0106-support-metadata-runtime branch from e53e2c1 to 1f9a5d0 Compare May 5, 2022 16:06
@tekton-robot
Copy link
Collaborator

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/pipelinerun_types.go 80.9% 81.1% 0.2
pkg/reconciler/pipelinerun/pipelinerun.go 86.0% 86.0% -0.0

@austinzhao-go austinzhao-go force-pushed the tep-0106-support-metadata-runtime branch from 1f9a5d0 to 0c728f8 Compare May 5, 2022 18:57
@tekton-robot
Copy link
Collaborator

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/pipelinerun_types.go 80.9% 81.1% 0.2
pkg/reconciler/pipelinerun/pipelinerun.go 86.0% 86.0% -0.0

@austinzhao-go austinzhao-go force-pushed the tep-0106-support-metadata-runtime branch 2 times, most recently from b7a2a67 to 1a97932 Compare May 5, 2022 19:59
@tekton-robot
Copy link
Collaborator

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/pipelinerun_types.go 80.9% 81.1% 0.2
pkg/reconciler/pipelinerun/pipelinerun.go 86.0% 86.0% -0.0

@austinzhao-go
Copy link
Contributor Author

/test pull-tekton-pipeline-alpha-integration-tests

@dibyom
Copy link
Member

dibyom commented May 6, 2022

/assign @lbernick

@austinzhao-go austinzhao-go force-pushed the tep-0106-support-metadata-runtime branch from 4efa93f to 664d1c1 Compare May 11, 2022 19:34
@tekton-robot
Copy link
Collaborator

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/pipelinerun_types.go 80.9% 81.1% 0.2
pkg/reconciler/pipelinerun/pipelinerun.go 86.0% 86.0% -0.0

@tekton-robot
Copy link
Collaborator

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/pipelinerun_types.go 80.9% 81.1% 0.2
pkg/reconciler/pipelinerun/pipelinerun.go 86.0% 86.0% -0.0

@austinzhao-go
Copy link
Contributor Author

austinzhao-go commented May 17, 2022 •
edited
Loading

/remove-hold
as the related TEP was merged - tektoncd/community#695

@austinzhao-go
Copy link
Contributor Author

Hey @lbernick, could you help me cancel the "hold" label? seems only can be done by raiser.

@lbernick
Copy link
Member

/hold cancel

^ Just FYI anyone can do this :)

@tekton-robot tekton-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 18, 2022
@austinzhao-go
Copy link
Contributor Author

/hold cancel

^ Just FYI anyone can do this :)

thanks Lee! perhaps a space line I need to add between the cmd line and other comments. tried several diff cancel/unhold cmds.

@austinzhao-go
Copy link
Contributor Author

Hi @pritidesai @dlorenc @jerop could I have one more review for a lgtm to go?

@tekton-robot tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 26, 2022
@austinzhao-go austinzhao-go force-pushed the tep-0106-support-metadata-runtime branch from 664d1c1 to ba200ca Compare May 26, 2022 19:48
@tekton-robot tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 26, 2022
@tekton-robot
Copy link
Collaborator

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/pipelinerun_types.go 80.9% 81.1% 0.2
pkg/reconciler/pipelinerun/pipelinerun.go 86.4% 86.4% -0.0

@jerop
Copy link
Member

jerop commented May 26, 2022

/assign

jerop
jerop approved these changes May 27, 2022
View reviewed changes
Copy link
Member

@jerop jerop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @austinzhao-go!

docs/pipelineruns.md Show resolved Hide resolved
docs/pipelineruns.md Outdated Show resolved Hide resolved
@tekton-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jerop, 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:

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

@austinzhao-go
TEP-0106: Support Specifying Metadata per Task in Runtime
cfda928 
While currently Tekton supports specifying metadata for a Task in a Pipeline during "authoring time", specific requirements from a running context lead to this work to offer a flexibility to add metadata during "runtime".

The chosen approach is to add a Metadata field under PipelineTaskRunSpec as for its proper runtime context and easier for a user to follow and utilize, rather than a whole new field. Further consideration and tradeoff can be found in the TEP-0106.
@austinzhao-go austinzhao-go force-pushed the tep-0106-support-metadata-runtime branch from ba200ca to cfda928 Compare May 27, 2022 15:08
@austinzhao-go
Copy link
Contributor Author

thanks @jerop for this round reviews. pushed updates.

@tekton-robot
Copy link
Collaborator

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/pipelinerun_types.go 80.9% 81.1% 0.2
pkg/reconciler/pipelinerun/pipelinerun.go 86.4% 86.4% -0.0

jerop
jerop reviewed May 27, 2022
View reviewed changes
Copy link
Member

@jerop jerop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks again @austinzhao-go!

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label May 27, 2022
@tekton-robot tekton-robot merged commit b19a9ab into tektoncd:main May 27, 2022
This was referenced May 27, 2022
Closed
Merged
@austinzhao-go austinzhao-go mentioned this pull request May 30, 2022
Merged
@lbernick lbernick mentioned this pull request Jul 21, 2022
Closed
@JeromeJu JeromeJu mentioned this pull request Nov 23, 2022
Closed
5 tasks
@JeromeJu JeromeJu mentioned this pull request Jul 17, 2023
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Yongxuanzhang Yongxuanzhang Yongxuanzhang left review comments

@jerop jerop jerop approved these changes

@lbernick lbernick lbernick approved these changes

@dlorenc dlorenc Awaiting requested review from dlorenc

@pritidesai pritidesai Awaiting requested review from pritidesai

Assignees

@dibyom dibyom

@jerop jerop

@lbernick lbernick

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/feature Categorizes issue or PR as related to a new feature. lgtm Indicates that a PR is ready to be merged. 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.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@austinzhao-go @tekton-robot @dibyom @lbernick @jerop @Yongxuanzhang