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 a version command #108

Closed
amwat opened this issue Mar 9, 2021 · 9 comments · Fixed by #162
Closed

Add a version command #108

amwat opened this issue Mar 9, 2021 · 9 comments · Fixed by #162
Assignees
Labels
sig/testing Categorizes an issue or PR as relevant to SIG Testing.
Milestone

Comments

@amwat
Copy link
Contributor

amwat commented Mar 9, 2021

kubetest2 --version should output the tag/commit that it's built from.

This is especially important since currently kubetest2 is used in a lot of place by go getting (mostly latest)
so knowing which exact binary(commit) was used is useful.
This will be further useful if it gets surfaced in testgrid metadata for any kubetest2 based job.

Additionally, this should be done for all the in-tree deployers and testers.

The version will also reflect the release once we move to tagged releases.
related: #17

A typical useful pattern is to inject and set package variables at build time through linker flags.
https://blog.alexellis.io/inject-build-time-vars-golang/

A good example can be found in the kind repo

https://github.com/kubernetes-sigs/kind/blob/fbf6cdcf5240c6b994657c9a4f85657ee0591331/Makefile#L57

https://github.com/kubernetes-sigs/kind/blob/fbf6cdcf5240c6b994657c9a4f85657ee0591331/pkg/cmd/kind/version/version.go

xref: kubernetes/enhancements#2464

/help
/good-first-issue

@k8s-ci-robot k8s-ci-robot added good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. labels Mar 9, 2021
@supriya-premkumar
Copy link
Contributor

/assign

@amwat
Copy link
Contributor Author

amwat commented Mar 11, 2021

/remove-help
Please ping with any questions if you need help :)

@k8s-ci-robot k8s-ci-robot removed help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. labels Mar 11, 2021
@amwat amwat added this to the v0.1.0 milestone Mar 17, 2021
@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 15, 2021
@BenTheElder BenTheElder removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 25, 2021
@kubernetes-sigs kubernetes-sigs deleted a comment from fejta-bot Jun 25, 2021
@BenTheElder
Copy link
Member

#111 is in but needs follow-ups (I think)

@coryrc
Copy link

coryrc commented Jul 7, 2021

since currently kubetest2 is used in a lot of place by go getting

I don't believe there's any way the callee (kubetest2 repo) can inject the version when something uses go install.

@BenTheElder
Copy link
Member

I don't believe there's any way the callee (kubetest2 repo) can inject the version when something uses go install.

we have a version when you call kind even with go get. the solution is to have a placeholder hardcoded value and then sometimes override those symbols.

https://github.com/kubernetes-sigs/kind/blob/e0f885dbd565cf6449aa806e5093440db69d002d/pkg/cmd/kind/version/version.go#L1

https://github.com/kubernetes-sigs/kind/blob/c029e821c07b3c821ee3bde7b7386b8859d968ed/Makefile#L57

@coryrc
Copy link

coryrc commented Jul 7, 2021

Sure, with go get you get VersionCore-VersionPreRelease which certainly narrows it down, but this sentence cannot be fulfilled to my knowledge by the callee:

since currently kubetest2 is used in a lot of place by go getting (mostly latest)
so knowing which exact binary(commit) was used is useful.

The caller can (they could set that linker flag w/ go install, or they could use your makefile), but not the callee, right?

@BenTheElder
Copy link
Member

BenTheElder commented Jul 7, 2021

Yes, I should have looked back at the rest of the context 👍

With kind IMO we instead encourage:

  • binaries from head are available to curl (which set this, and is cheaper/quicker/simpler than go-get anyhow)
  • most people should use releases

IMO it's an anti-pattern that kubetest2 encourages go get at HEAD still, as soon as we tag a version go get sigs.k8s.io/kubetset/... will be a stable version which can have the version hard-coded, and we should also encourage using stable versions.

I think the problem has just been getting to a point ready to tag as a version, which seems to be getting overdue for at least v0.1.x

@amwat
Copy link
Contributor Author

amwat commented Jul 7, 2021

+1 yeah we should release a v0.1.0 soon.

For the record I don't think we "encourage" folks to go get it from HEAD :) that's just how I've seen most folks use it. /shrug
Definitely a stable tag and/or atleast a fixed commit is better. Pre-built curlable binaries (that would come with the release) would be the best.

We already have them for CI: https://gcsweb.k8s.io/gcs/k8s-staging-kubetest2/

@coryrc as you said the caller can inject build time variables during go get as well

go get -ldflags 'pathToVariable=$COMMIT' sigs.k8s.io/kubetest2@$COMMIT

Agreed this is complicated for a human to do, but possible to setup in CI as part of getting the binary.
Anyway this issue is to have such functionality(injectable version) in the first place and not necessarily about inventing a mechanism for setting the version, while go getting latest, by the callee :)

@BenTheElder
Copy link
Member

For the record I don't think we "encourage" folks to go get it from HEAD :) that's just how I've seen most folks use it. /shrug

right now go get is got get from HEAD until there is a tagged version, encouraging go get is the same as encouraging this until we tag it.

@spiffxp spiffxp added the sig/testing Categorizes an issue or PR as relevant to SIG Testing. label Aug 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/testing Categorizes an issue or PR as relevant to SIG Testing.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants