chore: Update to Go 1.22 #1400
chore: Update to Go 1.22 #1400
Conversation
|
[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 |
Dockerfiles/Dockerfile
Outdated
| @@ -15,7 +12,7 @@ RUN if [ "${USE_LOCAL}" != "true" ]; then \ | |||
| fi | |||
|
|
|||
| ################################################################################ | |||
| FROM registry.access.redhat.com/ubi8/go-toolset:$GOLANG_VERSION as builder | |||
| FROM registry.access.redhat.com/ubi8/go-toolset:latest as builder | |||
There was a problem hiding this comment.
This might be a controversial part, and happy to revert it if there's consensus in that direction, but I think it's a good change for the following reasons:
- UBI go-toolset is a single-stream repo, so now that it has a 1.22 version, the 1.21 version is no longer maintained. By default, I don't think we should ever want anything but the latest version of this image.
- We can rebuild the image to pick up on new Go fixes without having to necessarily update the
godirective in the go.mod file.
I think these reasons are probably even more important further downstream where Konflux is already in use, since it will block releases if there are CVEs of a certain severity or higher.
There was a problem hiding this comment.
The only drawback I see would probably be to have to watch for the go-toolset to move to go 1.23 ? (not sure if it would even happen and pretty sure quite long time form now)
There was a problem hiding this comment.
The only drawback I see would probably be to have to watch for the
go-toolsetto move to go 1.23 ? (not sure if it would even happen and pretty sure quite long time form now)
That's too. I remember some conversation about latest, but do not remember details. It was decided not to use it. I'll try to find the thread.
There was a problem hiding this comment.
i agree with this.
unless the images are released on major version, we should not use "latest" in general.
also, to have a gap if downstream keeps with certain verson when ODH already moved to newer one, could cause backwards compability issue for the same code running between ODH and downstreaml
There was a problem hiding this comment.
The only drawback I see would probably be to have to watch for the go-toolset to move to go 1.23 ?
@lburgazzoli Why would we need to watch for this though? It will be able to build older code, and we don't need to synchronously bump to 1.23 in go.mod at the same time: that can be done asynchronously at any point after (it just can't be done before). If it causes a build to break for some reason, we'll get notified of that ASAP and fix (and a temporary fix at that point might be to hold back to a previous minor version, but it's not a sustainable fix, because it's a single-stream image that we're basing on).
unless the images are released on major version, we should not use "latest" in general.
I don't understand this point, could you elaborate further?
also, to have a gap if downstream keeps with certain verson when ODH already moved to newer one, could cause backwards compability issue for the same code running between ODH and downstream
Given that the UBI go-toolset image is single stream, and is used both upstream and downstream here, downstream will also want/need to keep updated to the latest version also. Any new downstream Z release even (think 2.8, 2.13, 2.16, etc) will eventually want to be built with the latest version of this image (meaning 1.22, where it's currently max 1.21).
By specifying the latest tag here, it means that Konflux/mintmaker will be able to take care of that for us automatically. Currently though, that's kinda "stuck" on each release, to the minor version, which means that manual intervention is needed to change that every time to a new minor tag. E.g. https://github.com/red-hat-data-services/rhods-operator/blob/main/Dockerfiles/Dockerfile.konflux#L4
grdryn
force-pushed
the
go-1.22
branch
2 times, most recently
from
80af060 to
ba42dd3
Compare
|
Related to the change, I personally would like some automation of the action itself, like #1047 But people did not accept it. What would be your opinion? |
|
ref here https://github.com/red-hat-data-services/rhods-operator/blob/main/Dockerfiles/Dockerfile.konflux#L4 if we do uplift, we need update ^ as well |
|
@grdryn: The following test failed, say
Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
|
I've created openshift/release#59338 in an attempt to make the changes needed to fix the failing e2e test |
|
Closing this, will make an equivalent PR against main |
Description
The UBI8 go-toolset image that we use for building is already on 1.22, so I don't think there's any reason to remain bound to 1.21.
Also, upstream Go only support the two most recent minor versions: currently that means 1.23 and 1.22; 1.21 is no longer receiving any updates from the Go developers.
At the same time, there's nothing forcing us to update to this version at this point: leaving it at 1.21 will just mean that the builder image won't receive any more updates (UBI go-toolset doesn't maintain multiple streams)
How Has This Been Tested?
Built and run with the new changes
Screenshot or short clip
Merge criteria