RFC: Makefile: fetch GOLANG_VERSION from go.mod #1047
Conversation
A bit of automation for golang version update. For local builds make it possible to set it only once in go.mod (it actually means minimal required version, but in the project it is used as the version for builds) and then reuse it for Dockerfiles: - pass it as the build argument for normal image build (Dockerfile); - use it for TOOLBOX_GOLANG_VERSION for toolbox.Dockerfile. For compatibility to keep workflow of overriding IMAGE_BUILD_FLAGS --build-arg USE_LOCAL working introduce another variable IMAGE_BUILD_FLAGS_ALL to combine GOLANG_VERSION build argument with the passed IMAGE_BUILD_FLAGS. Add target golang-version-update to update the default version in the Dockerfiles to avoid doing it manually in all of the places and just commit then for the case of using Dockerfile without Makefile. This change does not affect integration, version their should still be updated separatly. Signed-off-by: Yauheni Kaliuta <[email protected]>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
ykaliuta
requested review from
bartoszmajsak,
zdtsw and
VaishnaviHire
and removed request for
etirelli and
asanzgom
| for f in Dockerfiles/*; do \ | ||
| sed -i -e 's/\(ARG GOLANG_VERSION=\).*/\1$(GOLANG_VERSION)/g' $$f ; \ | ||
| done | ||
|
|
There was a problem hiding this comment.
but "make build" is still using the local Go, which can be different version from go.mod, right?
There was a problem hiding this comment.
Yes. But it is out of scope of go updates (#811) anyway
There was a problem hiding this comment.
@ykaliuta Can you list steps one needs to take now in order to perform a correct golang version bump for this project?
There was a problem hiding this comment.
Sure:
- update the version in go.mod
- run make golang-version-update
- commit the changes to git and create a pull request
There was a problem hiding this comment.
I personally do not see it really necessary to have default version in the Dockerfiles, #811 (comment) but I was told that people can use it
| @@ -73,6 +73,7 @@ YQ_VERSION ?= v4.12.2 | |||
| ENVTEST_K8S_VERSION = 1.24.2 | |||
| ENVTEST_PACKAGE_VERSION = v0.0.0-20240320141353-395cfc7486e6 | |||
| CRD_REF_DOCS_VERSION = 0.0.11 | |||
| GOLANG_VERSION := $(shell sed -n '/^go/s/go \(.*\)/\1/p' < go.mod) | |||
There was a problem hiding this comment.
I wonder if the name shouldn't be REQUIRED_GOLANG_VERSION. IIUC the goal here is to ensure we build against this exact version.
| @@ -93,6 +94,7 @@ E2E_TEST_FLAGS = "--skip-deletion=false" -timeout 20m # See README.md, default g | |||
| # set to "true" to use local instead | |||
| # see target "image-build" | |||
| IMAGE_BUILD_FLAGS ?= --build-arg USE_LOCAL=false | |||
There was a problem hiding this comment.
I don't see this one used anywhere after rename. Am I missing some other piece that depends on IMAGE_BUILD_FLAGS? Or is it intended now to be a customization point for the caller?
There was a problem hiding this comment.
I don't see this one used anywhere after rename. Am I missing some other piece that depends on
IMAGE_BUILD_FLAGS? Or is it intended now to be a customization point for the caller?
Yes, for you :) I remember you told that you have it in some alias.
| for f in Dockerfiles/*; do \ | ||
| sed -i -e 's/\(ARG GOLANG_VERSION=\).*/\1$(GOLANG_VERSION)/g' $$f ; \ | ||
| done | ||
|
|
There was a problem hiding this comment.
@ykaliuta Can you list steps one needs to take now in order to perform a correct golang version bump for this project?
|
PR needs rebase. 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-sigs/prow repository. |
A bit of automation for golang version update.
For local builds make it possible to set it only once in go.mod (it actually means minimal required version, but in the project it is used as the version for builds) and then reuse it for Dockerfiles:
For compatibility to keep workflow of overriding IMAGE_BUILD_FLAGS --build-arg USE_LOCAL working introduce another variable IMAGE_BUILD_FLAGS_ALL to combine GOLANG_VERSION build argument with the passed IMAGE_BUILD_FLAGS.
Add target golang-version-update to update the default version in the Dockerfiles to avoid doing it manually in all of the places and just commit then for the case of using Dockerfile without Makefile. So, to do the version upgrade in the repo one should:
make golang-version-updateThis change does not affect integration, version their should still be updated separatly.
Description
How Has This Been Tested?
Merge criteria: