Feature to serve the version using /version endpoint #948
Conversation
knative-prow-robot
added
size/M
|
Hi @waveywaves. Thanks for your PR. I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the 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. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: waveywaves The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Basically, I need to be able to serve the release version of the controller in use in tektoncd/pipline so that it can be consumed by the cli for showing the version. |
|
|
||
| type versionRequestHandler struct { | ||
| version string | ||
| } |
There was a problem hiding this comment.
This type is impossible to create, and the function below is impossible to call 🤔
We have a convention where we have resources labeled with serving.knative.dev/release: devel and rewrite devel to a version when we ./hack/release.sh. Perhaps we should have this projected into an EnvVar or volume that this serves?
There was a problem hiding this comment.
My bad, the StartVersionSrv is supposed to be public so that it can be called from tektoncd/pipeline. As pkg is used there as well for serving metrics. I have taken inspiration from there wrt serving the version.
There was a problem hiding this comment.
If the version is reflected in the endpoints it would be easier for cli to query it for the release version.
There was a problem hiding this comment.
The type is only consumed by StartVersionSrv, so it should be alright I suppose.
There was a problem hiding this comment.
Hmm, not unless there's a publicly exposed network endpoint. Perhaps it could key off of the label I gave above on a fixed resource (e.g. the tekton namespace)?
There was a problem hiding this comment.
@mattmoor To clarify, you are suggesting that the tekton release be given by a label on a namespace or as a volume correct ? If that is so, should I make changes here or open a PR on tektoncd for this ?
There was a problem hiding this comment.
Okay @mattmoor It makes more sense to do this directly in tekton then in terms of the label on the resource.
https://github.com/knative/serving/blob/916ab9f5ac54c9cd041ad7c502e02b3bc8f6ea17/config/500-webhook-configuration.yaml#L20
There was a problem hiding this comment.
I could see value in having /version that keyed off of a label like what our convention is, but I doubt that is adequate for your use case since unless there's an externally accessible endpoint you can't hit it without tunneling into the cluster (unless you are running on-cluster).
If the CLI just looked at labels in a fixed place (e.g. on the Tekton namespace of CR defintion) then you could piggyback on the API server as your endpoint.
There was a problem hiding this comment.
Was planning to have the release show up on the pods running the controller and the webhook. In that case I would be able to use the cli to pick the labels up from there.
There was a problem hiding this comment.
@mattmoor I have opened up a PR directly in tektoncd/pipeline for setting annotation on the controller pod for the release. If I am not wrong it should work with CLI for querying the annotation off of the pod for getting the version.
Using this feature it should be possible for knative products to be able to leverage this featureto serve the curent version of product in use. This would help in debugging based on the version as well as facilitate checking cli + controller compatibility. Signed-off-by: Vibhav Bobade <[email protected]>
waveywaves
force-pushed
the
feature/serve-version
branch
from
f0213fb to
c0651c9
Compare
|
/close |
|
@waveywaves: Closed this PR. In response to this:
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. |
Using this feature it should be possible for knative products
to be able to leverage this featureto serve the curent version
of product in use. This would help in debugging based on the
version as well as facilitate checking cli + controller
compatibility.
Related to
tektoncd/pipeline#1650
tektoncd/cli#503
cc @vdemeester
Signed-off-by: Vibhav Bobade [email protected]