-
Notifications
You must be signed in to change notification settings - Fork 24
🐛 Override the Golang version by config #822
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: main
Are you sure you want to change the base?
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: cardil The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/cc @dsimansk |
9513c95 to
a59443c
Compare
Previously, the `resolveGolangVersion` function would error out if it could not find a repository-specific `.yaml` configuration file. This made the config file a mandatory requirement for generating a Dockerfile for a Go project.
creydr
left a comment
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 like this approach more as just overriding the version hardly in #821. To not need to specify the same Golang version for each config/repo, we could also default to the goVersion from the SO config and only use the repo specific one, in case it is set.
e.g.
serverless-operator-config: golang 1.24
ekb: golang unset
func: golang 1.25
->
ekb: use golang 1.24 (same as in SO config)
func: use golang 1.25 (as custom repo config overrides SO default config)
This may backfire. Firstly, to accomplish that, a version match needs to be implemented. The versioning on S-O is different from on other components ( IMHO, with long support branches and other changes to the program, we may hit a similar situation soon. |
|
We have some logic to get the SO branch/release from a component branch/release and vice versa: https://github.com/openshift-knative/hack/blob/main/pkg/soversion/version.go |
|
It looks good at a first glance. I like the way we can influence golang version from the config. I'll expand a bit more on my idea about global config. IMO we are missing descriptor that could capture S-O release version and then versions like go, knative, rhel (not definitive list). A data structure translated from entries we track in s spreadsheet for every release. It's been doable so far, most of values carried-over from one release to next. But it started to fall apart. Wrt/ to the actual PR. I'm fine to proceed with it right now. |
Remove the duplicated `golangVersion` key from all component configuration files for the `release-v1.15` branch. The Go version resolution logic in the dockerfile generator has been refactored to be more intelligent and reduce configuration duplication. Previously, the Go version for a specific release had to be defined in every single component's config file. The new logic introduces a fallback mechanism: 1. It first checks for a `golangVersion` in the component's specific configuration for the given branch (preserving override capability). 2. If not found, it calculates the corresponding Serverless Operator release branch from the upstream Knative release (e.g., `release-v1.15` -> `release-1.36`). 3. It then looks for the `golangVersion` in the `serverless-operator.yaml` configuration for that calculated branch. This allows us to define the Go version for an entire release train in a single location (`serverless-operator.yaml`), simplifying future updates. As part of this change, the `soversion` package was also improved to return errors from version parsing instead of panicking, increasing the robustness of the tooling.
Resolves (partially) SRVCOM-4037
Related to #821