-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
update to go 1.16 #2144
update to go 1.16 #2144
Conversation
Runc Compile is failing;
|
Looks like I get a more detailed error when running locally;
|
More details above: https://github.com/moby/buildkit/pull/2144/checks?check_run_id=2719843639#step:7:287 |
Ah! Looks like I didn't scroll back far enough. Slightly confusing that that error is not included in the final error that buildkit returns (perhaps it should be?) |
Need https://pkgs.alpinelinux.org/package/v3.13/main/aarch64/libseccomp-static as alpine 3.13 does not contain static library in https://pkgs.alpinelinux.org/contents?branch=v3.13&name=libseccomp-dev&arch=aarch64&repo=main anymore |
Ugh.. more fun with modules;
|
1ddca36
to
4045235
Compare
All green now 🎉 ✅ I'll rebase, and move this out of draft once #2146 is merged |
This updates all occurrences of Go 1.13 to Go 1.16; also updated the code that's used to redact credentials in URLs to use the Go implementation. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@@ -151,6 +150,7 @@ RUN --mount=from=containerd-src,src=/usr/src/containerd,readwrite --mount=target | |||
# containerd v1.4 for integration tests | |||
FROM containerd-base as containerd-alt | |||
ARG CONTAINERD_ALT_VERSION | |||
ARG GO111MODULE=off |
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.
TBH, not sure why go1.16 didn't detect vendor here, and complains about no go.mod
being present; guess it still requires a go.mod
even if a vendor
is present 🤷♂️
I'lll have a look at adding this to the containerd makefile in the 1.4 branch
Or would it be better to build this version of containerd with Go 1.15? https://github.com/containerd/containerd/blob/v1.4.6/.github/workflows/ci.yml#L29
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 doesn't matter until the tests pass. These binaries are never released.
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.
We will probably have to mess around with the EINTR stuff, but anyway building is flaky by nature mostly due to network flakiness, so possible extra flakiness added by EINTR is not a huge deal 😛
So LGTM.
Looks like the "vendor" directory of BuildKit in moby is ok, but perhaps it'd be good to run Tibor's script on the BuildKit codebase itself (script is here: https://gist.github.com/tiborvass/786c66dfa614db4a60805c05bdf8a923#file-syscalls-go) Here's an Excel export of that file (for others to view) - I think the conditional formatting got lost in the export from Google docs though); |
@thaJeztah In a follow-up, wonder if we should start produce riscv64 arch bin which is available since 1.14. |
@crazy-max There are no alpine packages (for runc and image) |
@tonistiigi LGTY? |
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.
Merging. But please do run the script above and create follow-up issues for any discoveries.
I've seen "flightcontrol: exceeded retry timeout" errors in ci 2 times in the past day that I've never seen before. Wondering if it can be related somehow. |
depends on #2146
relates to moby/moby#40353 moby/moby#40353 (comment)
This updates all occurrences of Go 1.13 to Go 1.16; also updated the code that's used to redact credentials in URLs to use the Go implementation (based on golang/go@e3323f5, it's available in Go 1.15 and up).