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: bump go version to 1.23 #11

Merged
merged 1 commit into from
Jan 5, 2025

Conversation

azych
Copy link
Contributor

@azych azych commented Jan 2, 2025

When using tilt to run operator-controller / catalogd and having latest stable version of go (1.23.4), both controllers end up in a failed state because of the incompatible version of delve defined in tilt-support/Tiltfile builder:

operator-con… │ [event: pod olmv1-system/operator-controller-controller-manager-778d45c685-vwrxt] Back-off restarting failed container manager in pod operator-controller-controller-manager-778d45c685-vwrxt_olmv1-system(1f47d6f8-6b8e-44c0-a945-ad676e38ad6a)
...
catalogd-con… │ Version of Delve is too old for Go version go1.23.4 (maximum supported version 1.22, suppress this error with --check-go-version=false)

This PR bumps the builder go version to 1.23 which is also used for dlv installation version and does not have compatibility issue.

An alternative solution could be to separate GO_VERSION and DLV_VERSION if the builder go version should not be updated for some reason (I did not found one).

@azych azych requested review from a team as code owners January 2, 2025 15:31
@camilamacedo86
Copy link
Contributor

I am ok with, but for we update operator-controller and etc to use go 1.23 we need bump the next/future release of controller-runtime: https://github.com/kubernetes-sigs/controller-runtime

Copy link
Contributor

@camilamacedo86 camilamacedo86 left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 2, 2025
@camilamacedo86
Copy link
Contributor

camilamacedo86 commented Jan 2, 2025

/lgtm cancel

I want to approve to run the tests and not get it merged now

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jan 2, 2025
@azych
Copy link
Contributor Author

azych commented Jan 3, 2025

I am ok with, but for we update operator-controller and etc to use go 1.23 we need bump the next/future release of controller-runtime: https://github.com/kubernetes-sigs/controller-runtime

could you please explain why the version of controller-runtime will have to be updated here as well? And also, do you mean that it would have to be updated in controller-operator?

I thought this only influenced the project developer convenience tooling connected with Tilt. If it isn't the case, I think it might be better to introduce DLV_VERSION that will be separate from GO_VERSION, WDYT?

@camilamacedo86
Copy link
Contributor

Hi @azych

IHMO: We can move forward with this one. 👍 🥇
I LGTM this one already.

The hassle is that if we upgrade the operator-controller and other projects, it would be better if we bump the next controller-runtime release with go 1.23 first then we can do it all.

@camilamacedo86 camilamacedo86 added this pull request to the merge queue Jan 5, 2025
Merged via the queue into operator-framework:main with commit 3c93ebd Jan 5, 2025
2 checks passed
@azych
Copy link
Contributor Author

azych commented Jan 6, 2025

Hi @azych

IHMO: We can move forward with this one. 👍 🥇 I LGTM this one already.

The hassle is that if we upgrade the operator-controller and other projects, it would be better if we bump the next controller-runtime release with go 1.23 first then we can do it all.

@camilamacedo86
thanks for approval, but to make sure I understand correctly - you're saying that if we upgraded go version in operator-controller or catalogd (ie. set new version in go.mod) for example, then we would have to bump controller-runtime version in our dependencies of that project as well?

@azych azych deleted the bump-go-version branch January 6, 2025 23:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants