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

chore: Update to Go 1.22 #1400

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions Dockerfiles/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
# Build the manager binary
ARG GOLANG_VERSION=1.21

################################################################################
FROM registry.access.redhat.com/ubi8/toolbox as manifests
ARG USE_LOCAL=false
Expand All @@ -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
ARG CGO_ENABLED=1
Copy link
Member Author

@grdryn grdryn Nov 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 go directive 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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

USER root
WORKDIR /workspace
Expand Down
2 changes: 1 addition & 1 deletion Dockerfiles/toolbox.Dockerfile
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
FROM registry.fedoraproject.org/fedora-toolbox:38

ARG GOLANG_VERSION=1.21
ARG GOLANG_VERSION=1.22.9
grdryn marked this conversation as resolved.
Show resolved Hide resolved
ARG OPERATOR_SDK_VERSION=1.31.0

ENV GOLANG_VERSION=$GOLANG_VERSION \
Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ catalog-build: opm ## Build a catalog image.
catalog-push: ## Push a catalog image.
$(MAKE) image-push IMG=$(CATALOG_IMG)

TOOLBOX_GOLANG_VERSION := 1.21
TOOLBOX_GOLANG_VERSION := 1.22.9

# Generate a Toolbox container for locally testing changes easily
.PHONY: toolbox
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ Additionally installing `Authorino operator` & `Service Mesh operator` enhances

#### Pre-requisites

- Go version **go1.21**
- Go version **go1.22**
- operator-sdk version can be updated to **v1.31.1**

#### Download manifests
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module github.com/opendatahub-io/opendatahub-operator/v2

go 1.21
go 1.22

require (
github.com/blang/semver/v4 v4.0.0
Expand Down
Loading