Print Version Number on startup

[ViktorLindgren95]

Based on Issue 1654

Built a function that will print the current ARC version by fetching the enviroment variable ARC_VERSION with a fallback incase the enviroment variable isnt set yet.

ARC_VERSION is set inside the dockerfile which gets its value from the Publish ARC workflow

Works on my local windows machine, Havent confirmed on either my linux machine or K8s cluster. I will in future contributions setup a local test enviroment

[ViktorLindgren95]

Im not totally sure how the docker build and push works as i personally use buildah but it should be the same logic, as you set the enviroment variable on the runner machine and the docker daemon is hosted on the same "container" so it should inherit the enviorment variable. If not then maybe we can pass it.

[toast-gear]

It's probably worth doing https://github.com/actions-runner-controller/actions-runner-controller/issues/1161 as part of this PR or in a follow up PR

[ViktorLindgren95]

It's probably worth doing #1161 as part of this PR or in a follow up PR

I have a quick question then. Shall i put the get version function inside github.go. I dont feel like it belongs there really, as its not really related to github per say. But it makes sense inheritance wise, that way we can have one function that both github.go calls internally and the same function that main.go calls, so we dont have two diffrent functions / code blocks. Could you make a "design" call on this kindly

EDIT: We also dont need it to be a function it could just be two diffrent code blocks that get the ENV but with a function i creates one source of truth etc etc

[mumoshu]

@ViktorLindgren95 Thanks a lot for your effort! The direction looks very good. Implementation-wise, I have two comments- would you mind reviewing those comments and, if that makes sense to you, make corresponding changes, so that we can merge this before 0.26.0? Thanks in advance for your help ☺️

[ViktorLindgren95]

@ViktorLindgren95 Thanks a lot for your effort! The direction looks very good. Implementation-wise, I have two comments- would you mind reviewing those comments and, if that makes sense to you, make corresponding changes, so that we can merge this before 0.26.0? Thanks in advance for your help ☺️

No problem im just happy to do some coding in my free time 🥰

[ViktorLindgren95]

The default didn't seem to take when I tested (built using the Makefile make docker-buildx). Additionally, the Makefile needs adjusting to support passing in a version if desired (I still expected to see the verison as dev but instead I got version": "",). We can probably move the default value being set from the Go code to being set via an ARG in the Dockerfile with a default value if no arg is passed in. The project is designed to run from the image produced from its Dockerfile so I don't see an issue with doing that.

Ok i think i know what happens. If the $VERSION variable is empty, aka not set, the version becomes empty instead of dev. I just confirmed this in my local enviroment.

Because i got the version from the github actions variable its not fetched from any local instance. If you set version tag inside your own enviroment do you see a version being printed or is it still empty?

I unfortunately dont know much about makefiles but, Would the makefile create it with the dockerfile? then try setting the version env in your enviroment and confirm for me .

EDIT: Alternatively make a len < 1 return dev function , like a getter function but that seems hacky

[toast-gear]

mmmm looks like some changes are needed:

1. https://github.com/actions-runner-controller/actions-runner-controller/blob/master/Makefile#L7 needs to become dev instead of latest
2. https://github.com/actions-runner-controller/actions-runner-controller/blob/master/Makefile#L121 a --build-arg added for passing in VERSION to the Dockerfile
3. https://github.com/actions-runner-controller/actions-runner-controller/blob/master/Dockerfile#L28 VERSION=dev added to the ARG list with a default for those not using the Makefile
4. The default removed from the Go code (build/version.go)

EDIT @ViktorLindgren95 really appreciate your work so far, we're keen to get this in the current milestone v0.26.0 so if you do find some time to make those changes sooner rather than later we'd very much appreciate it 🙏

[ViktorLindgren95]

mmmm looks like some changes are needed:

1. https://github.com/actions-runner-controller/actions-runner-controller/blob/master/Makefile#L7 needs to become `dev` instead of latest

2. https://github.com/actions-runner-controller/actions-runner-controller/blob/master/Makefile#L121 a --build-arg added for passing in VERSION to the Dockerfile

3. https://github.com/actions-runner-controller/actions-runner-controller/blob/master/Dockerfile#L28 `VERSION=dev` added to the ARG list with a default for those not using the Makefile

4. The default removed from the Go code (build/version.go)

EDIT @ViktorLindgren95 really appreciate your work so far, we're keen to get this in the current milestone v0.26.0 so if you do find some time to make those changes sooner rather than later we'd very much appreciate it 🙏

I will try to get a push before tommorow evening. When is 0.26 deadline?

Also thanks for the patience on your end. This is all pretty new to me, the contributing to opensource and all that ^^

EDIT: @toast-gear "The default removed from the code" Do you mean just remove the dev part inside the build/version and leave it blank ?

[toast-gear]

I will try to get a push before tommorow evening. When is 0.26 deadline?

We don't have specific deadlines for releases as the maintainers have full time jobs. There's only this + a small refactor to do (https://github.com/actions-runner-controller/actions-runner-controller/milestone/9) and then we are ready to release so we are hoping in the next week or 2.

EDIT: @toast-gear "The default removed from the code" Do you mean just remove the dev part inside the build/version and leave it blank ?

yeh, leave it blank was what I was thinking

[ViktorLindgren95]

I set it as N/A to make debugging easier instead of an empty message.

Changes have been made

[mumoshu]

Updated the N/A to NA so that it looks a bit more natural in the default User-Agent string actions-runner-controller/N/A vs actions-runner-controller/NA. I don't think NA as an abbrev for Not-Available is not so common but I think it's a bit better than breaking UA string 😁

Also fixed the failing test related to the UA string generation in a4b3019

[mumoshu]

Merged. The next release of our canary image which will be available within 10 minutes or so would show canary as the version number. It would be great if anyone could confirm it by looking into the controller logs or seeing outgoing HTTP requests or etc.