Reflect tektoncd/pipeline release version as an annotation on pod #1758

waveywaves · 2019-12-17T03:13:04Z

Signed-off-by: Vibhav Bobade [email protected]

Changes

Based on suggesstion in knative/pkg#948 have opened this PR as an alternative fix for reflecting the release version for tektoncd/pipline.

This solution reflects the version as value to the annotation tekton.dev/release on the controller pod.

Followup of #1650 and tektoncd/cli#503

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

Includes tests (if functionality changed/added)
Includes docs (if user facing)
Commit messages follow commit message best practices

See the contribution guide for more details.

Double check this list of stuff that's easy to miss:

If you are adding a new binary/image to the cmd dir, please update
the release Task to build and release this image.

Reviewer Notes

If API changes are included, additive changes must be approved by at least two OWNERS and backwards incompatible changes must be approved by more than 50% of the OWNERS, and they must first be added in a backwards compatible way.

Release Notes

tekton.dev/release annotation will be added to all pods created by the controller which would let us know what release of tekton is running. 
Controller Pod and Pods in general should have the following annotation after subsequent release
tekton.dev/release: <version tag>

tekton-robot · 2019-12-17T03:13:19Z

Hi @waveywaves. Thanks for your PR.

I'm waiting for a tektoncd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

afrittoli · 2019-12-17T08:05:08Z

pkg/pod/pod.go

@@ -42,6 +43,9 @@ const (

 // These are effectively const, but Go doesn't have such an annotation.
 var (
+	releaseAnnotation      = "tekon.dev/release"


Tekon - > tekton

afrittoli · 2019-12-17T08:05:47Z

/ok-to-test

dibyom

This approach looks interesting! Have a couple of questions/requests:

Could we add tests to verify that the annotation is being set?
Could we add some release notes to the PR description? https://github.com/tektoncd/pipeline/blob/master/.github/pull_request_template.md#release-notes
Question: This seems to add the release version to all pods that are being created by the controller. Is there a way to add this annotation to the controller itself?

waveywaves · 2019-12-18T08:05:18Z

@dibyom Thanks for chekcing this out. I was planning on also adding it to the controller yaml file and then overriding the value there during build/release, similar to what has been done in knative/serving.

afrittoli · 2019-12-19T15:02:53Z

@dibyom Thanks for chekcing this out. I was planning on also adding it to the controller yaml file and then overriding the value there during build/release, similar to what has been done in knative/serving.

Thanks!
I wonder if we should set the annotation on all resources i.e. Task, Pipelines, TaskRuns, PipelineRuns, Conditions, PipelineResources?
I'm not sure if the annotation would be cascaded to the pod then.
It might be useful though to know that a certain resource was created with a specific version of Tekton, when doing upgrades. @vdemeester thoughts?

vdemeester · 2019-12-19T15:06:38Z

I wonder if we should set the annotation on all resources i.e. Task, Pipelines, TaskRuns, PipelineRuns, Conditions, PipelineResources?
I'm not sure if the annotation would be cascaded to the pod then.
It might be useful though to know that a certain resource was created with a specific version of Tekton, when doing upgrades. @vdemeester thoughts?

I think it would cascade to the pod. Honestly I think it's a nice to have to know which version of Tekton did create a certain resource, so I am 👍 on having the annotation on all the things — it doesn't cost too much either 👼 .

/cc @bobcatfish

pkg/pod/entrypoint.go

pkg/pod/pod.go

tekton/publish.yaml

vdemeester

LGTM 👍

chmouel · 2019-12-30T12:31:13Z

/lgtm

vdemeester · 2020-01-02T15:29:02Z

@waveywaves needs a rebase 👼

waveywaves · 2020-01-03T03:44:19Z

@chmouel @vdemeester Done !

waveywaves · 2020-01-03T04:14:38Z

/retest

As a followup to tektoncd#1650 , the `tekton.dev/release` annotation is set on the pod to reflect the value of version.PipelineVersion Signed-off-by: Vibhav Bobade <[email protected]>

vdemeester

/lgtm

tekton-robot · 2020-01-06T10:05:41Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 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~~ [vdemeester]

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

tekton-robot requested review from afrittoli and bobcatfish December 17, 2019 03:13

googlebot added the cla: yes Trying to make the CLA bot happy with ppl from different companies work on one commit label Dec 17, 2019

tekton-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Dec 17, 2019

tekton-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Dec 17, 2019

waveywaves force-pushed the update/release-label branch from 0d10efc to 90b4048 Compare December 17, 2019 03:58

tekton-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Dec 17, 2019

waveywaves force-pushed the update/release-label branch from 90b4048 to 33471ea Compare December 17, 2019 04:00

afrittoli reviewed Dec 17, 2019

View reviewed changes

tekton-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Dec 17, 2019

dibyom reviewed Dec 17, 2019

View reviewed changes

waveywaves force-pushed the update/release-label branch from 33471ea to 264bcc4 Compare December 20, 2019 09:14

vdemeester reviewed Dec 20, 2019

View reviewed changes

pkg/pod/entrypoint.go Outdated Show resolved Hide resolved

pkg/pod/pod.go Outdated Show resolved Hide resolved

waveywaves force-pushed the update/release-label branch 4 times, most recently from 26ea656 to 97f5e07 Compare December 20, 2019 10:17

vdemeester reviewed Dec 20, 2019

View reviewed changes

tekton/publish.yaml Show resolved Hide resolved

waveywaves force-pushed the update/release-label branch from 97f5e07 to 813e54f Compare December 20, 2019 12:45

vdemeester approved these changes Dec 20, 2019

View reviewed changes

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

tekton-robot assigned chmouel Dec 30, 2019

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 30, 2019

waveywaves force-pushed the update/release-label branch from 51a1d6e to 7549c69 Compare January 3, 2020 03:43

tekton-robot removed the lgtm Indicates that a PR is ready to be merged. label Jan 3, 2020

Reflect tektoncd/pipeline release version as an annotation on pod

c60516e

As a followup to tektoncd#1650 , the `tekton.dev/release` annotation is set on the pod to reflect the value of version.PipelineVersion Signed-off-by: Vibhav Bobade <[email protected]>

waveywaves force-pushed the update/release-label branch from 7549c69 to c60516e Compare January 6, 2020 05:04

vdemeester approved these changes Jan 6, 2020

View reviewed changes

tekton-robot assigned vdemeester Jan 6, 2020

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 6, 2020

tekton-robot merged commit ebb071e into tektoncd:master Jan 6, 2020

vdemeester mentioned this pull request Jan 22, 2020

Easily access installed version information #1919

Closed

waveywaves mentioned this pull request Mar 19, 2020

Add waveywaves (Vibhav Bobade (me)) to the org tektoncd/community#87

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reflect tektoncd/pipeline release version as an annotation on pod #1758

Reflect tektoncd/pipeline release version as an annotation on pod #1758

waveywaves commented Dec 17, 2019 •

edited

Loading

tekton-robot commented Dec 17, 2019

afrittoli Dec 17, 2019

afrittoli commented Dec 17, 2019

dibyom left a comment

waveywaves commented Dec 18, 2019

afrittoli commented Dec 19, 2019

vdemeester commented Dec 19, 2019

vdemeester left a comment

chmouel commented Dec 30, 2019

vdemeester commented Jan 2, 2020

waveywaves commented Jan 3, 2020

waveywaves commented Jan 3, 2020

vdemeester left a comment

tekton-robot commented Jan 6, 2020

Reflect tektoncd/pipeline release version as an annotation on pod #1758

Reflect tektoncd/pipeline release version as an annotation on pod #1758

Conversation

waveywaves commented Dec 17, 2019 • edited Loading

Changes

Submitter Checklist

Reviewer Notes

Release Notes

tekton-robot commented Dec 17, 2019

afrittoli Dec 17, 2019

Choose a reason for hiding this comment

afrittoli commented Dec 17, 2019

dibyom left a comment

Choose a reason for hiding this comment

waveywaves commented Dec 18, 2019

afrittoli commented Dec 19, 2019

vdemeester commented Dec 19, 2019

vdemeester left a comment

Choose a reason for hiding this comment

chmouel commented Dec 30, 2019

vdemeester commented Jan 2, 2020

waveywaves commented Jan 3, 2020

waveywaves commented Jan 3, 2020

vdemeester left a comment

Choose a reason for hiding this comment

tekton-robot commented Jan 6, 2020

waveywaves commented Dec 17, 2019 •

edited

Loading