Print Version Number on startup #1659

ViktorLindgren95 · 2022-07-19T21:27:36Z

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

…ions workflow and pass it to the main.go file Added a function in main.go to fetch the enviroment varible and to have a fallback if the env variable isnt there Added a test for the version to use for this branch only

ViktorLindgren95 · 2022-07-19T21:29:16Z

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.

main.go

toast-gear · 2022-07-20T15:13:53Z

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

ViktorLindgren95 · 2022-07-23T09:16:29Z

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

…by toast-gear Added version as issue#1161 requests. Decided to use a docker tag structure for the userAgent string, with : being a seperator of the name and version

Dockerfile

github/github.go

mumoshu · 2022-07-26T09:42:08Z

@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 ☺️

Changed Dockerfile to use $VERSION from the workflow Added version.go and the build package Removed the getVersion function as we can just get the value directly

ViktorLindgren95 · 2022-07-29T08:07:39Z

@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 🥰

toast-gear

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.

ViktorLindgren95

~~ignore~~

ViktorLindgren95 · 2022-08-13T15:53:24Z

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 · 2022-08-15T08:29:25Z

mmmm looks like some changes are needed:

https://github.com/actions-runner-controller/actions-runner-controller/blob/master/Makefile#L7 needs to become dev instead of latest
https://github.com/actions-runner-controller/actions-runner-controller/blob/master/Makefile#L121 a --build-arg added for passing in VERSION to the Dockerfile
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
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 · 2022-08-17T07:07:58Z

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 ?

* Changed version from latest to dev inside makefile * Added buildarg for version to the dockerfile in the makerfile * Added VERSION with default dev value as arg inside dockerfile * Cleaned up inside dockerfile

toast-gear · 2022-08-17T08:11:40Z

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 · 2022-08-17T08:38:53Z

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

Changes have been made

mumoshu

LGTM. Thank you so much for your efforts and contribution @ViktorLindgren95!!

TingluoHuang · 2022-08-23T04:13:13Z

Dockerfile

@@ -25,7 +24,7 @@ RUN go mod download
 # With the above commmand,
 # TARGETOS can be "linux", TARGETARCH can be "amd64", "arm64", and "arm", TARGETVARIANT can be "v7".

-ARG TARGETPLATFORM TARGETOS TARGETARCH TARGETVARIANT
+ARG TARGETPLATFORM TARGETOS TARGETARCH TARGETVARIANT VERSION=dev


Do we need to also update the workflow yaml file to set the VERSION arg?
https://github.com/actions-runner-controller/actions-runner-controller/blob/master/.github/workflows/publish-arc.yaml

nvm... build.Version=${VERSION} will read from the ENV.

@TingluoHuang Ahhh, that's actually a good catch!
Although our go-build snippet refers to the VERSION env, I don't think we now pass that env as a build-arg for the docker build.
Probably we might need to add something like build-args: VERSION=${{env.VERSION}} to the docker-build-push action step.

Addressed in da14999

…d arg in docker-build-push step

mumoshu · 2022-08-23T04:38:44Z

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 · 2022-08-23T04:41:34Z

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.

ViktorLindgren95 added 4 commits July 19, 2022 23:14

Update test-version.yaml

c95b633

Update test-version.yaml

6fdef9a

Removed the test because its not needed when we push upstream

98cefd1

ViktorLindgren95 requested review from mumoshu and toast-gear as code owners July 19, 2022 21:27

toast-gear reviewed Jul 20, 2022

View reviewed changes

main.go Outdated Show resolved Hide resolved

Moved the version print in main.go to the Log codeblock as requested …

b968016

…by toast-gear Added version as issue#1161 requests. Decided to use a docker tag structure for the userAgent string, with : being a seperator of the name and version

ViktorLindgren95 requested a review from toast-gear July 23, 2022 09:29

mumoshu reviewed Jul 26, 2022

View reviewed changes

Dockerfile Outdated Show resolved Hide resolved

mumoshu reviewed Jul 26, 2022

View reviewed changes

github/github.go Outdated Show resolved Hide resolved

S8338C added 2 commits July 29, 2022 10:04

Used ldflags instead like mumoshu recommended

c43f683

Changed Dockerfile to use $VERSION from the workflow Added version.go and the build package Removed the getVersion function as we can just get the value directly

Used ldflags instead like mumoshu recommended

3f4999c

Changed Dockerfile to use $VERSION from the workflow Added version.go and the build package Removed the getVersion function as we can just get the value directly

ViktorLindgren95 requested a review from mumoshu July 29, 2022 08:06

toast-gear requested changes Aug 13, 2022

View reviewed changes

ViktorLindgren95 requested a review from toast-gear August 13, 2022 15:06

ViktorLindgren95 commented Aug 13, 2022

View reviewed changes

* Removed the default from the go code (set it as N/A)

826bad3

* Changed version from latest to dev inside makefile * Added buildarg for version to the dockerfile in the makerfile * Added VERSION with default dev value as arg inside dockerfile * Cleaned up inside dockerfile

mumoshu previously approved these changes Aug 23, 2022

View reviewed changes

Merge branch 'master' into master

46a86fa

mumoshu dismissed their stale review via 46a86fa August 23, 2022 04:03

TingluoHuang reviewed Aug 23, 2022

View reviewed changes

mumoshu added 2 commits August 23, 2022 04:28

Fix failing test

a4b3019

Fix possible missing VERSION in the ARC UA suffix due to missing buil…

da14999

…d arg in docker-build-push step

mumoshu approved these changes Aug 23, 2022

View reviewed changes

mumoshu merged commit ca97f39 into actions:master Aug 23, 2022

This was referenced Aug 24, 2022

Print the ARC version number on startup #1654

Closed

Add the ARC release version into the UserAgent #1161

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Print Version Number on startup #1659

Print Version Number on startup #1659

ViktorLindgren95 commented Jul 19, 2022

ViktorLindgren95 commented Jul 19, 2022

toast-gear commented Jul 20, 2022

ViktorLindgren95 commented Jul 23, 2022 •

edited

Loading

mumoshu commented Jul 26, 2022

ViktorLindgren95 commented Jul 29, 2022

toast-gear left a comment •

edited

Loading

ViktorLindgren95 left a comment •

edited

Loading

ViktorLindgren95 commented Aug 13, 2022 •

edited

Loading

toast-gear commented Aug 15, 2022 •

edited

Loading

ViktorLindgren95 commented Aug 17, 2022 •

edited

Loading

toast-gear commented Aug 17, 2022 •

edited

Loading

ViktorLindgren95 commented Aug 17, 2022

mumoshu left a comment

TingluoHuang Aug 23, 2022

TingluoHuang Aug 23, 2022

mumoshu Aug 23, 2022 •

edited

Loading

mumoshu Aug 23, 2022

mumoshu commented Aug 23, 2022

mumoshu commented Aug 23, 2022

Print Version Number on startup #1659

Print Version Number on startup #1659

Conversation

ViktorLindgren95 commented Jul 19, 2022

ViktorLindgren95 commented Jul 19, 2022

toast-gear commented Jul 20, 2022

ViktorLindgren95 commented Jul 23, 2022 • edited Loading

mumoshu commented Jul 26, 2022

ViktorLindgren95 commented Jul 29, 2022

toast-gear left a comment • edited Loading

Choose a reason for hiding this comment

ViktorLindgren95 left a comment • edited Loading

Choose a reason for hiding this comment

ViktorLindgren95 commented Aug 13, 2022 • edited Loading

toast-gear commented Aug 15, 2022 • edited Loading

ViktorLindgren95 commented Aug 17, 2022 • edited Loading

toast-gear commented Aug 17, 2022 • edited Loading

ViktorLindgren95 commented Aug 17, 2022

mumoshu left a comment

Choose a reason for hiding this comment

TingluoHuang Aug 23, 2022

Choose a reason for hiding this comment

TingluoHuang Aug 23, 2022

Choose a reason for hiding this comment

mumoshu Aug 23, 2022 • edited Loading

Choose a reason for hiding this comment

mumoshu Aug 23, 2022

Choose a reason for hiding this comment

mumoshu commented Aug 23, 2022

mumoshu commented Aug 23, 2022

ViktorLindgren95 commented Jul 23, 2022 •

edited

Loading

toast-gear left a comment •

edited

Loading

ViktorLindgren95 left a comment •

edited

Loading

ViktorLindgren95 commented Aug 13, 2022 •

edited

Loading

toast-gear commented Aug 15, 2022 •

edited

Loading

ViktorLindgren95 commented Aug 17, 2022 •

edited

Loading

toast-gear commented Aug 17, 2022 •

edited

Loading

mumoshu Aug 23, 2022 •

edited

Loading