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.

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

Add version label to pipelines controller deployment #1543

Closed
skaegi opened this issue Nov 8, 2019 · 14 comments · Fixed by #1650
Closed

Add version label to pipelines controller deployment #1543

skaegi opened this issue Nov 8, 2019 · 14 comments · Fixed by #1650
Labels
kind/feature Categorizes issue or PR as related to a new feature.

Comments

@skaegi
Copy link
Contributor

skaegi commented Nov 8, 2019

Figuring out which version of Tekton you're running can be a challenge of looking up image SHAs and other equally fun things.

As per https://kubernetes.io/docs/concepts/overview/working-with-objects/common-labels/ we should add an an app.kubernetes.io/version label to the tekton-pipelines-controller deployment and figure out how we set it both for formal versions and ad-hoc builds.

@vdemeester vdemeester added this to the Pipelines 1.0/beta 🐱 milestone Nov 8, 2019
@vdemeester
Copy link
Member

/kind feature

@tekton-robot tekton-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Nov 8, 2019
@josephlewis42
Copy link
Contributor

josephlewis42 commented Nov 12, 2019

Another place I'd like to see this is on the namespace; if you're sharing a cluster a scrupulous operator would probably lock down the controllers, but it's more likely users would have access to read namespaces.

For example, Knative Serving does this with a serving.knative.dev/release label on their namespace. (I like the idea of using the standard K8s label instead of re-inventing the wheel, however.)

@waveywaves
Copy link
Member

waveywaves commented Nov 29, 2019

@vdemeester @chmouel Kubernetes exposes versions leveraging git metadata, the following being the scaffold to it https://github.com/kubernetes/kubernetes/tree/7f23a743e8c23ac6489340bbb34fa6f1d392db9d/staging/src/k8s.io/client-go/pkg/version
This approach would be quite reliable

This doesn't count as a hack right ? 🤔

@chmouel
Copy link
Member

chmouel commented Nov 29, 2019

@waveywaves Git tagging is good enough for me,

There is multiple part where we can expose this information :

  • On the controller
  • On the namespace
  • as an API (for tools to show)

@vdemeester
Copy link
Member

We should definitely expose it using labels on the namespace and/or on the controller/webhook deployments. I'm not sure how we would do "as an API" as I don't think it is supported for CRDs.

@chmouel
Copy link
Member

chmouel commented Nov 29, 2019

Yes you are right but how can we do to get the version from the tooling tho? (it doesnt know where the namespace is and there can be many pipeline in different installs)

waveywaves added a commit to waveywaves/tekton-pipeline that referenced this issue Nov 29, 2019
Fixes tektoncd#1543
Introduces variable for storing the pipeline version
waveywaves added a commit to waveywaves/tekton-pipeline that referenced this issue Nov 29, 2019
Fixes tektoncd#1543
Introduces variable for storing the pipeline version
waveywaves added a commit to waveywaves/tekton-pipeline that referenced this issue Dec 3, 2019
Fixes tektoncd#1543
Introduces variable for storing the pipeline version
waveywaves added a commit to waveywaves/tekton-pipeline that referenced this issue Dec 3, 2019
Fixes tektoncd#1543
Introduces variable for storing the pipeline version
waveywaves added a commit to waveywaves/tekton-pipeline that referenced this issue Dec 3, 2019
Fixes tektoncd#1543
Introduces variable for storing the pipeline version
waveywaves added a commit to waveywaves/tekton-pipeline that referenced this issue Dec 3, 2019
Fixes tektoncd#1543
Introduces variable for storing the pipeline version
waveywaves added a commit to waveywaves/tekton-pipeline that referenced this issue Dec 3, 2019
Fixes tektoncd#1543
Introduces variable for storing the pipeline version
waveywaves added a commit to waveywaves/tekton-pipeline that referenced this issue Dec 3, 2019
Fixes tektoncd#1543
Introduces variable for storing the pipeline version
waveywaves added a commit to waveywaves/tekton-pipeline that referenced this issue Dec 3, 2019
Fixes tektoncd#1543
Introduces variable for storing the pipeline version
waveywaves added a commit to waveywaves/tekton-pipeline that referenced this issue Dec 3, 2019
Fixes tektoncd#1543
Introduces variable for storing the pipeline version
waveywaves added a commit to waveywaves/tekton-pipeline that referenced this issue Dec 3, 2019
Fixes tektoncd#1543
Introduces variable for storing the pipeline version
waveywaves added a commit to waveywaves/tekton-pipeline that referenced this issue Dec 3, 2019
Fixes tektoncd#1543
Introduces variable for storing the pipeline version
tekton-robot pushed a commit that referenced this issue Dec 4, 2019
Fixes #1543
Introduces variable for storing the pipeline version
@chmouel
Copy link
Member

chmouel commented Dec 4, 2019

I think we should reopen this since there is a bit more to it than f85c53f

/reopen

@vdemeester vdemeester reopened this Dec 4, 2019
@xcoulon
Copy link
Contributor

xcoulon commented Feb 7, 2020

Not sure if this would help, but you could use ldflags during build time to set some variables with the commit hash (and optionally tag) and reuse them in the controller at runtime. Something like:

go build ${V_FLAG} \
-ldflags "-X ${GO_PACKAGE_PATH}/version.Commit=${GIT_COMMIT_ID} -X ${GO_PACKAGE_PATH}/version.BuildTime=${BUILD_TIME}" \
-o $(OUT_DIR)/bin/host-operator \
cmd/manager/main.go

see https://github.com/codeready-toolchain/host-operator/blob/master/make/go.mk#L13-L19, https://github.com/codeready-toolchain/host-operator/blob/master/make/git.mk and https://github.com/codeready-toolchain/host-operator/blob/master/version/version.go

@vdemeester
Copy link
Member

To recap:

  • We have the ldflags thingy (here)
  • We have versions labels in our deployments (controller and webhook) (that are updated at release time)

Given the title of the issue, I think we can close 😉

@vdemeester
Copy link
Member

/close

@tekton-robot
Copy link
Collaborator

@vdemeester: Closing this issue.

In response to this:

/close

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.

@xcoulon
Copy link
Contributor

xcoulon commented Feb 7, 2020

To recap:

  • We have the ldflags thingy (here)
  • We have versions labels in our deployments (controller and webhook) (that are updated at release time)

Given the title of the issue, I think we can close 😉

doh! I should have checked the actual codebase before making this suggestion ;) 🤦‍♂

@chmouel
Copy link
Member

chmouel commented Feb 7, 2020

we still need an implementation in CLI tho 💃

(but that's nothing to do with this issue, just looking for some volunteers who wants to tackle it since it seems that it has stagnated ;))

@chmouel
Copy link
Member

chmouel commented Feb 12, 2020

fyi we have an implementation for CLI now!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants