-
Notifications
You must be signed in to change notification settings - Fork 13
go1.24 #1963
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
base: master
Are you sure you want to change the base?
Conversation
@@ -36,7 +36,7 @@ var nameRegexp = regexp.MustCompile(`([[:graph:]]+)-([[:digit:]][\-.[:alnum:]]*( | |||
// checkName returns the extracted package name from the above regexp. | |||
func checkName(name string) string { | |||
m := nameRegexp.FindStringSubmatch(name) | |||
if m == nil || len(m) < 2 { | |||
if len(m) < 2 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was called out after the linter update
@@ -1,5 +1,5 @@ | |||
# Compiling scanner binaries and staging repo2cpe and genesis manifests | |||
FROM brew.registry.redhat.io/rh-osbs/openshift-golang-builder:rhel_8_1.23@sha256:0a070e4a8f2698b6aba3630a49eb995ff1b0a182d0c5fa264888acf9d535f384 AS builder | |||
FROM brew.registry.redhat.io/rh-osbs/openshift-golang-builder:rhel_8_1.24@sha256:021ab8e6e4af3d1683b52ec985996f9421edb2111dc697b5781dee3c7595ae00 AS builder |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added konflux-build
label because the builds don't trigger in PRs without it (we can revisit).
Please push any/empty commit to this PR to trigger Konflux CI.
Let's make sure openshift-golang-builder
is also happy with the change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I forgot the Konflux builds didn't run by default
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like they changed the name of the tag, so I had to update that, too. I chose the option I figured would be best
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem is that both el9
https://brewweb.engineering.redhat.com/brew/buildinfo?buildID=3679446 and el8
https://brewweb.engineering.redhat.com/brew/buildinfo?buildID=3679501 push the floating tag v1.24
. That's not good because if our builds pick up 9-based builder, the resulting executables will have problems with FIPS or will just not work.
We have to select tags which have rhel_8
in them. The only one like this available in that image build is rhel_8_golang_1.24_test
which reveals they are testing something. This warrants to find out how suitable do they consider these images to be for us to release built artifacts to production.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test
images can be used now, but must not be shipped (non production ready). In "early July", golang builder images will be updated to have a floating tag without _test
suffix, ie production ready.
I would suggest to hold until the production ready image is available.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Related internal discussion https://redhat-internal.slack.com/archives/C05TS9N0S7L/p1751971387754689
9b10d05
to
2af62d4
Compare
|
||
toolchain go1.23.6 | ||
toolchain go1.24.3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I learned we MUST have a toolchain
version set to some version available downstream (Konflux kept failing at the prefetch stage until I made this change).
I originally removed the toolchain
line and just set go 1.24
(to match the stackrox repo).
1.24 doesn't exist there, but 1.24.3 does. We could just update go 1.24
to go 1.24.3
and remove the toolchain; however, I prefer not requiring anyone who may use this repo as a library be forced to use some specific minor version of Go unless we absolutely require it. I opted to keep go 1.24
and just update the the toolchain to toolchain go1.24.3
Just a heads up we'll probably have to do this for the stackrox repo @janisz @msugakov
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Simply, openshift-golang-builder
has no ability to download any toolchains because that feature is disabled. The only one it can use is what's provided. If the image comes with 1.24.3, the highest toolchain we can have (when building with openshift-golang-builder
) is 1.24.3.
@RTann: The following tests 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. |
See stackrox/rox-ci-image@0.4.8...0.4.9 for the updated Go version.
Also updated the linters, as they support go1.24 now