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

Stop blacklisting Go 1.22+, drop Go < 1.21 support, use Go 1.22 in CI #4292

Merged
merged 7 commits into from
Jun 8, 2024

Conversation

kolyshkin
Copy link
Contributor

@kolyshkin kolyshkin commented May 25, 2024

High level overview:

  • remove nsenter compile restriction since Go 1.22.4 and Go 1.23 can now be used. THIS PART NEEDS TO BE BACKPORTED TO 1.1.
  • switch to Go 1.22 in CI whether possible (everywhere except Ubuntu 20.04 and CentOS 7 and 8).
  • set minimum requirement to Go 1.21
  • rm some old code.

See individual commits for details.

@kolyshkin kolyshkin force-pushed the go122 branch 4 times, most recently from c20f7e5 to 6b132ac Compare May 30, 2024 22:28
@kolyshkin kolyshkin marked this pull request as ready for review May 30, 2024 23:10
Copy link
Member

@rata rata left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

It took me a while to test locally that the go version dance was fine, but it indeed seem fine :)

btw, it seems the ubuntu builds are stalled.

README.md Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@kolyshkin
Copy link
Contributor Author

We'll probably need to port the first 3 patches to release-1.1 (so that it will work with Go 1.22).

@kolyshkin kolyshkin added the backport/1.1-todo A PR in main branch which needs to be backported to release-1.1 label Jun 1, 2024
@kolyshkin
Copy link
Contributor Author

btw, it seems the ubuntu builds are stalled.

This PR drops go 1.20.x tests, but they are marked as required in repo config -- this is why they are shown as "Expected -- waiting ...". I will fix the repo config accordingly (rm go 1.20.x, require go 1.22.x) once this PR will be ready to be merged (I guess once Go 1.21.4 is out).

@kolyshkin
Copy link
Contributor Author

@opencontainers/runc-maintainers wdyt?

@@ -27,7 +27,7 @@ A third party security audit was performed by Cure53, you can see the full repor

## Building

`runc` only supports Linux. It must be built with Go version 1.19 or higher.
`runc` only supports Linux. It must be built with Go version 1.21 or higher.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe add some note about Go 1.22

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had this originally but got it removed as explained above: #4292 (comment) (TL;DR: the error message is explanatory enough).

Copy link
Member

@rata rata left a comment

Choose a reason for hiding this comment

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

LGTM, one minor (optional) nit. Very optional, as that code will cease to exist in a few months :)

Makefile Outdated Show resolved Hide resolved
@kolyshkin
Copy link
Contributor Author

@lifubang @cyphar PTAL

@AkihiroSuda
Copy link
Member

CentOS tests seem failing

@thaJeztah
Copy link
Member

CentOS tests seem failing

For CentOS stream 8, that may be expected; it's EOL since May 31 (just got hit by that in some CI pipeline that als had it in the Matrix)

@AkihiroSuda
Copy link
Member

CentOS tests seem failing

For CentOS stream 8, that may be expected; it's EOL since May 31 (just got hit by that in some CI pipeline that als had it in the Matrix)

I guess we now have to replace CentOS with Alma, Rocky, or Oracle Linux?

@lifubang
Copy link
Member

lifubang commented Jun 4, 2024

This one should be merged after go 1.22.4 has released.

Copy link
Member

@rata rata left a comment

Choose a reason for hiding this comment

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

LGTM

README.md Outdated Show resolved Hide resolved
@kolyshkin kolyshkin changed the title Drop Go < 1.21 support, use Go 1.22 in CI, relax go122+glibc check Stop blacklisting Go 1.22+, drop Go < 1.21 support, use Go 1.22 in CI Jun 7, 2024
Go 1.23 includes a fix (https://go.dev/cl/587919) so go1.23.x can be
used. This fix is also backported to 1.22.4, so go1.22.x can also be
used (when x >= 4). Finally, for glibc >= 2.32 it doesn't really matter.

Add a note about Go 1.22.x > 1.22.4 to README as well.

Signed-off-by: Kir Kolyshkin <[email protected]>
Now when Go 1.22.4 is out it should no longer be a problem.

Leave Go 1.21 for CentOS testing (CentOS 7 and 8 have older glibc)
and Dockerfile (Debian 11 have older glibc).

Signed-off-by: Kir Kolyshkin <[email protected]>
Go 1.20 was released in February 2023 and is no longer supported since
February 2024. Time to move on.

Signed-off-by: Kir Kolyshkin <[email protected]>
It is not needed since Go 1.20 (which was released in February 2023 and
is no longer supported since February 2024).

Signed-off-by: Kir Kolyshkin <[email protected]>
As we're no longer supporting Go < 1.21.

Signed-off-by: Kir Kolyshkin <[email protected]>
As we no longer support Go < 1.21.

Signed-off-by: Kir Kolyshkin <[email protected]>
@kolyshkin
Copy link
Contributor Author

Assuming that by the time 1.2.0 is out, everyone will use Go 1.22.x >= 1.22.4, I just dropped the restriction (and added a note to README).

I think this is ready to be merged. WDYT @cyphar?

@kolyshkin kolyshkin added backport/1.1-done A PR in main branch which has been backported to release-1.1 and removed backport/1.1-todo A PR in main branch which needs to be backported to release-1.1 labels Jun 7, 2024
README.md Outdated Show resolved Hide resolved
@AkihiroSuda AkihiroSuda merged commit 865224d into opencontainers:main Jun 8, 2024
39 checks passed
@lifubang lifubang mentioned this pull request Jun 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.1-done A PR in main branch which has been backported to release-1.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants