-
Notifications
You must be signed in to change notification settings - Fork 37
updatecli: converge both golang versions together #634
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
Conversation
this can avoid situations when the golang version is released earlier than the microsoft golang version
|
This pull request does not have a backport label. Could you fix it @v1v? 🙏
|
Co-authored-by: Dimitrios Liappis <dimitrios.liappis@gmail.com>
…astic/golang-crossbuild into feature/fix-golang-crossbuild * 'feature/fix-golang-crossbuild' of https://github.com/elastic/golang-crossbuild: Apply suggestions from code review
thank you @v1v that is more than enough on my end. Now I believe that the situation that we are in is clear to everybody.
@ebeahan @pierrehilbert @cmacknz @dliappis please advise; Is the requirement of having a go version released first for the MSFT fork and only then being able to get golang-crossbuild images for upstream, acceptable? For the record, I wanna replay @v1v statistics here, indeed the last MSFT release was delayed only by 1 day. However, if anything changes with the MSFT fork or there is a delay in general for reasons we don't know/control, we won't get an upstream version without manual intervention |
I think that won't be even possible manually for new minor or patches, the current implementation relies on two different sources. Unless I miss something here |
hmm you are 100% right @v1v this even requires manual intervention today, and this is yet another surprise to me 😆 Clearly I am not involved as much with FIPS and the MSFT fork and what are the expectations here. Thinking out loud since we produce different |
|
Adding @simitt @ycombinator if they have context about FIPS, the MSFT fork, and what the expectations are here. Will the eventual certification of FIPS 140-3 help simplify this situation with the two sources? |
|
I'm in favor of using the version of https://github.com/microsoft/go as the source of truth.
Yes, looking at their last few releases, they seem to generally be within a day of the upstream Go releases so I don't think we're introducing a huge delay. Also, while we rarely have bugs due to changes in Golang, from a support perspective, if we use the same version of Golang for FIPS and non-FIPS builds, it would be one less variable we have to think about. I also think it's easier from a development / maintenance perspective. If we kept the two versions independent, would we then need two variants of the |
|
@ycombinator @ebeahan definitely I am not trying to delay this PR, it just happened for me yesterday to realise the sequencing and I needed to make sure that we are on the same page. I feel that we all are now, so @ycombinator I leave the PR review up to you 🙂 |
|
@ycombinator @ebeahan and @pkoutsovasilis , can I get an approval here? This should solve the problem with partial updates. I've just found a new golang release, it worked partially for 1.24 but failed for 1.23. |
pkoutsovasilis
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 am not sure why noone approves this 😄 LGTM since I read all the comments of the stakeholders above
Co-authored-by: Dimitrios Liappis <dimitrios.liappis@gmail.com> (cherry picked from commit 1b40a6d) # Conflicts: # .github/updatecli.d/bump-go-version.sh
…er (#643) Co-authored-by: Victor Martinez <victormartinezrubio@gmail.com>
What
Use Microsoft Golang releases as the source of truth.
Why
This can avoid situations when the Go version is released earlier than the Microsoft Go version. And also simplify the automation.
Background
We use
updateclito determine when a new Golang release will be out.FIPS requirementsenabled a new dependency; see #495. That introduced a change in where to look for releases and required fixing some issues: #571.However, the changes that were applied didn't work smoothly, and PRs were partially created, see #623 (comment).
Consequently, I decided to change the implementation and rely on only the
microsoft/goversion; it might take a bit to be available, but that's the dependency we have by adding FIPS support.Caveats
If this PR is merged, this will delay the golang crosbuild images for a new minor/patch until the Microsoft Golang release is out - last time took 1 day for
1.24.4-1to be available.If no actions are taken, and a new patch or minor release, then the golang autobump PR in this repository will be incomplete.
As far as I know, as long as the FIPS Microsoft is used in the golang crossbuild we cannot use a different approach, the autobuses PRs are atomic and should contain the golang versión and the sha for the amd64 and arm64 tar.gz files.
@pkoutsovasilis , let me know if this section provided enough context 🙇
Test
I created a branch called
test-1.24.new-versiointhat had the version1.23.5so I can test it works for1.24.4Produced v1v#20
and if I retry it: