-
Notifications
You must be signed in to change notification settings - Fork 18.8k
Dockerfile: update to runc v1.2.2 #47666
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
Hm... something broke with the way we compile runc on non-x86
|
@thaJeztah can you try using the buildtag We want to fix the issue, of course, but knowing that can help to narrow it down. |
@rata oh! Yes, I can update later today. In all honesty, I'm quite behind on my runc notifications, and for these updates, I didn't have time yet to spend any time investigating 🙈 (also need to look at the |
bedd143
to
b91f320
Compare
@thaJeztah I was checking this, I'm not sure I understand what is happening here. Do you know any simple way to run it locally, so I can help to debug this? The linux/arm/v6 and v7 builds fail with:
In the runc repo, The other failing builds might be due to the wrong platform used too, though (operand size mismatch, instruction not recognized, etc.). Runc main is using version 0.18 of golang.org/x/sys and 0.19 is available, but the same -marm is used still in the latest version (https://cs.opensource.google/go/x/sys/+/master:unix/zerrors_netbsd_arm.go;l=7). runc 1.1 is using 0.19, though. @thaJeztah Do you have any pointers on what to check? Or how to run this locally? :) |
b91f320
to
dcb0804
Compare
Hmm lots of failures (cross)compiling; arm;
ppc64le
s390x
|
cc @kolyshkin |
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 other issue, causing the compilation to fail, seems to be caused by including cc_platform.mk in runc. If we don't include it when we don't use runc-dmz, the compilation works.
I've debugged this here, in case you want to look: #48161
Dockerfile
Outdated
set -e | ||
xx-go --wrap | ||
CGO_ENABLED=1 make "$([ "$DOCKER_STATIC" = "1" ] && echo "static" || echo "runc")" | ||
CGO_ENABLED=1 make BUILDTAGS="secccomp runc_nodmz" "$([ "$DOCKER_STATIC" = "1" ] && echo "static" || echo "runc")" |
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.
Typo here: it is using seccomp with 3 c
s.
It took me a long while when doing c&p of this, to see why it was failing saying seccomp is not supported.
But there is another bug in runc breaking the compilation (more or that in another 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.
The issue is hopefully fixed by opencontainers/runc#4346, currently being tested at #48160
Dockerfile
Outdated
set -e | ||
xx-go --wrap | ||
CGO_ENABLED=1 make "$([ "$DOCKER_STATIC" = "1" ] && echo "static" || echo "runc")" | ||
CGO_ENABLED=1 make BUILDTAGS="secccomp runc_nodmz" "$([ "$DOCKER_STATIC" = "1" ] && echo "static" || echo "runc")" |
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.
s/secccomp/seccomp/
Yet better, use EXTRA_BUILDTAGS:
CGO_ENABLED=1 make BUILDTAGS="secccomp runc_nodmz" "$([ "$DOCKER_STATIC" = "1" ] && echo "static" || echo "runc")" | |
CGO_ENABLED=1 EXTRA_BUILDTAGS="runc_nodmz" make "$([ "$DOCKER_STATIC" = "1" ] && echo "static" || echo "runc")" |
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.
(see the full patch here: fed7477)
What happens if you do ifeq ($(origin CC),$(filter $(origin CC),undefined default))
# Override CC if it's undefined or just the default value set by Make.
CC := $(HOST)gcc
export CC
endif
STRIP ?= $(HOST)strip
export STRIP (Maybe doing `STRIP ?= $(HOST)strip' unconditionally is also wrong...) Alternatively, I wonder if
|
@thaJeztah I figured it out in #48160, runc now compiles fine, no need to set extra tags either. Feel free to incorporate my changes here. I'm not sure about CI failures though, they seem to be consistent (for a subset of failed tests, see #48160 (comment)) |
dcb0804
to
5e73b66
Compare
- 1.2.2 release notes: https://github.com/opencontainers/runc/releases/tag/v1.2.2 - 1.2.1 release notes: https://github.com/opencontainers/runc/releases/tag/v1.2.1 - 1.2.0 release notes: https://github.com/opencontainers/runc/releases/tag/v1.2.0 Breaking changes and deprecations are included below; Breaking changes: Several aspects of how mount options work has been adjusted in a way that could theoretically break users that have very strange mount option strings. This was necessary to fix glaring issues in how mount options were being treated. The key changes are: - Mount options on bind-mounts that clear a mount flag are now always applied. Previously, if a user requested a bind-mount with only clearing options (such as rw,exec,dev) the options would be ignored and the original bind-mount options would be set. Unfortunately this also means that container configurations which specified only clearing mount options will now actually get what they asked for, which could break existing containers (though it seems unlikely that a user who requested a specific mount option would consider it "broken" to get the mount options they asked foruser who requested a specific mount option would consider it "broken" to get the mount options they asked for). This also allows us to silently add locked mount flags the user did not explicitly request to be cleared in rootless mode, allowing for easier use of bind-mounts for rootless containers. - Container configurations using bind-mounts with superblock mount flags (i.e. filesystem-specific mount flags, referred to as "data" in mount(2), as opposed to VFS generic mount flags like MS_NODEV) will now return an error. This is because superblock mount flags will also affect the host mount (as the superblock is shared when bind-mounting), which is obviously not acceptable. Previously, these flags were silently ignored so this change simply tells users that runc cannot fulfil their request rather than just ignoring it. Deprecated - runc option --criu is now ignored (with a warning), and the option will be removed entirely in a future release. Users who need a non-standard criu binary should rely on the standard way of looking up binaries in $PATH. - runc kill option -a is now deprecated. Previously, it had to be specified to kill a container (with SIGKILL) which does not have its own private PID namespace (so that runc would send SIGKILL to all processes). Now, this is done automatically. - github.com/opencontainers/runc/libcontainer/user is now deprecated, please use github.com/moby/sys/user instead. It will be removed in a future release. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Alright; all green now, |
bf91d8b
to
e257856
Compare
Hm... looks like flaky test;
|
Any concerns for taking this back across the release branches? I can help carry the weight if not. :) |
It's probably fine; this is the version we use for the static binaries and for CI. The runc version used in our |
I haven't checked yet though if it's green in the 27.x branch; there were a couple of changes needed before we got this to go green in CI (but I think most fixes were on the runc side, so maybe not an issue) |
Let me bring this one one in 👍 |
Oh TIL, the CI check is still good enough for me. Maybe can take it back with the containerd update as well. |
Ah, yes, so the The version of runc we include in those packages defaults to what's defined in containerd's repository (we use the https://github.com/containerd/containerd/blob/v1.7.24/script/setup/runc-version file for this). For the Go version to build with, we use what's defined in the containerd repository as well (it's a bit more hacky, but we use the version from https://github.com/containerd/containerd/blob/v1.7.24/contrib/Dockerfile.test#L32). Those are the defaults though; we have some parameters we can set to override such versions. I just pushed a PR to update the changelog / specs, which is needed for doing a release (the deb/rpm scripts deduct the version from those); |
Oh a new friend has appeared! Let me pin that one; might be good place for me to learn Debian packaging. |
Breaking changes and deprecations are included below;
Breaking changes:
Several aspects of how mount options work has been adjusted in a way that could theoretically break users that have very strange mount option strings. This was necessary to fix glaring issues in how mount options were being treated. The key changes are:
Deprecated
- What I did
- How I did it
- How to verify it
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)