8000 Dockerfile: update to runc v1.2.2 by thaJeztah · Pull Request #47666 · moby/moby · GitHub
[go: up one dir, main page]

Skip to content

Conversation

thaJeztah
Copy link
Member
@thaJeztah thaJeztah commented Apr 3, 2024

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.

- What I did

- How I did it

- How to verify it

- Description for the changelog

Upgrade `runc` to [v1.2.2](https://github.com/opencontainers/runc/releases/tag/v1.2.2)

- A picture of a cute animal (not mandatory but encouraged)

@thaJeztah
Copy link
Member Author

Hm... something broke with the way we compile runc on non-x86

 > [runc-build 3/3] RUN --mount=from=runc-src,src=/usr/src/runc,rw     --mount=type=cache,target=/root/.cache/go-build,id=runc-build-linux/arm/v6 <<EOT (set -e...):
0.415 rm -f libcontainer/dmz/binary/runc-dmz
0.417 go generate -tags "seccomp urfave_cli_no_docs " ./libcontainer/dmz
0.420 make[1]: Entering directory '/go/src/github.com/opencontainers/runc/libcontainer/dmz'
0.427 gcc -fno-asynchronous-unwind-tables -fno-ident -s -Os -nostdlib -lgcc -static -o binary/runc-dmz _dmz.c
0.554 strip -gs binary/runc-dmz
0.556 make[1]: Leaving directory '/go/src/github.com/opencontainers/runc/libcontainer/dmz'
0.557 go build -trimpath   -tags "seccomp urfave_cli_no_docs  netgo osusergo" -ldflags "-X main.gitCommit=v1.2.0-rc.1-0-g275e6d8 -X main.version=1.2.0-rc.1 -extldflags -static " -o runc .
7.518 # runtime/cgo
7.518 gcc: error: unrecognized command-line option '-marm'; did you mean '-mabm'?
37.89 make: *** [Makefile:98: static-bin] Error 1
------
Dockerfile:305
--------------------
 304 |     ARG DOCKER_STATIC
 305 | >>> RUN --mount=from=runc-src,src=/usr/src/runc,rw \
 306 | >>>     --mount=type=cache,target=/root/.cache/go-build,id=runc-build-$TARGETPLATFORM <<EOT
 307 | >>>   set -e
 308 | >>>   xx-go --wrap
 309 | >>>   CGO_ENABLED=1 make "$([ "$DOCKER_STATIC" = "1" ] && echo "static" || echo "runc")"
 310 | >>>   xx-verify $([ "$DOCKER_STATIC" = "1" ] && echo "--static") runc
 311 | >>>   mkdir /build
 312 | >>>   mv runc /build/
 313 | >>> EOT
 314 |     
--------------------
ERROR: failed to solve: process "/bin/sh -c   set -e\n  xx-go --wrap\n  CGO_ENABLED=1 make \"$([ \"$DOCKER_STATIC\" = \"1\" ] && echo \"static\" || echo \"runc\")\"\n  xx-verify $([ \"$DOCKER_STATIC\" = \"1\" ] && echo \"--static\") runc\n  mkdir /build\n  mv runc /build/\n" did not complete successfully: exit code: 2
Error: buildx bake failed with: ERROR: failed to solve: process "/bin/sh -c   set -e\n  xx-go --wrap\n  CGO_ENABLED=1 make \"$([ \"$DOCKER_STATIC\" = \"1\" ] && echo \"static\" || echo \"runc\")\"\n  xx-verify $([ \"$DOCKER_STATIC\" = \"1\" ] && echo \"--static\") runc\n  mkdir /build\n  mv runc /build/\n" did not complete successfully: exit code: 2

@rata
Copy link
Contributor
rata commented Apr 8, 2024

@thaJeztah can you try using the buildtag runc_nodmz when compiling runc?

We want to fix the issue, of course, but knowing that can help to narrow it down.

@thaJeztah
Copy link
Member Author

@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 vendor update in my other PR 🙈🙈 ). Will give it a try!

@thaJeztah thaJeztah force-pushed the update_runc_1.2.0 branch 2 times, most recently from bedd143 to b91f320 Compare April 9, 2024 09:35
@rata
Copy link
Contributor
rata commented Apr 29, 2024

@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:

7.156 gcc: error: unrecognized command-line option '-marm'; did you mean '-mabm'?

In the runc repo, -marm only matches this file vendor/golang.org/x/sys/unix/zerrors_netbsd_arm.go, which seems for netbsd and not linux. Maybe some platform setting is not being honored or not passed at some step (and for some reason that step didn't need the platform before)? Just from github logs is hard to be sure if that is the issue, though.

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? :)

@thaJeztah thaJeztah force-pushed the update_runc_1.2.0 branch from b91f320 to dcb0804 Compare June 26, 2024 23:11
@thaJeztah thaJeztah changed the title Dockerfile: update to runc v1.2.0-rc.1 Dockerfile: update to runc v1.2.0-rc.2 Jun 26, 2024
@thaJeztah thaJeztah self-assigned this Jun 26, 2024
@thaJeztah
Copy link
Member Author

Hmm lots of failures (cross)compiling;

arm;

o runc .
7.158 # runtime/cgo
7.158 gcc: error: unrecognized command-line option '-marm'; did you mean '-mabm'?
38.64 make: *** [Makefile:105: static-bin] Error 1

ppc64le

14.27 gcc_linux_ppc64x.S:36:            Info: macro invoked from here
14.27 gcc_linux_ppc64x.S:74:             Info: macro invoked from here
14.27 gcc_linux_ppc64x.S:76: Error: no such instruction: `ld %r2,24(%r1)'
14.27 gcc_linux_ppc64x.S:77: Error: no such instruction: `addi %r1,%r1,FRAME_SIZE'
14.27 gcc_linux_ppc64x.S:78: Error: no such instruction: `ld %r0,16(%r1)'
14.27 gcc_linux_ppc64x.S:79: Error: no such instruction: `mtlr %r0'
14.27 gcc_linux_ppc64x.S:80: Error: no such instruction: `ld %r0,8(%r1)'
14.27 gcc_linux_ppc64x.S:81: Error: no such instruction: `mtcr %r0'
14.27 gcc_linux_ppc64x.S:82: Error: no such instruction: `blr'
39.60 make: *** [Makefile:105: static-bin] Error 1

s390x

0.424 rm -f libcontainer/dmz/binary/runc-dmz
0.429 go generate -tags "secccomp runc_nodmz" ./libcontainer/dmz
0.440 go build -trimpath   -tags "secccomp runc_nodmz netgo osusergo" -ldflags "-X main.gitCommit=v1.2.0-rc.2-0-gf2d2ee5 -X main.version=1.2.0-rc.2 -extldflags -static " -o runc .
9.251 # runtime/cgo
9.251 cc1: error: bad value 'z196' for '-march=' switch
9.251 cc1: note: valid arguments to '-march=' switch are: nocona core2 nehalem corei7 westmere sandybridge corei7-avx ivybridge core-avx-i haswell core-avx2 broadwell skylake skylake-avx512 cannonlake icelake-client rocketlake icelake-server cascadelake tigerlake cooperlake sapphirerapids alderlake bonnell atom silvermont slm goldmont goldmont-plus tremont knl knm x86-64 x86-64-v2 x86-64-v3 x86-64-v4 eden-x2 nano nano-1000 nano-2000 nano-3000 nano-x2 eden-x4 nano-x4 k8 k8-sse3 opteron opteron-sse3 athlon64 athlon64-sse3 athlon-fx amdfam10 barcelona bdver1 bdver2 bdver3 bdver4 znver1 znver2 znver3 btver1 btver2 native

@thaJeztah
Copy link
Member Author

cc @kolyshkin

Copy link
Contributor
@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.

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")"
Copy link
Contributor

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 cs.

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).

Copy link
Contributor
@kolyshkin kolyshkin left a 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")"
Copy link
Contributor

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:

Suggested change
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")"

Copy link
Contributor

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)

@cyphar
Copy link
Contributor
cyphar commented Jul 14, 2024

What happens if you do export CC=xx-clang? While cc_platform.mk is an unfortunate hack, it only changes the CC value if it wasn't specified by anyone and is just the Make default:

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 HOST=xx would work? FWIW, I would've expect setting CC explicitly (either with export or on the cmdline) to be fairly standard for any cross-compilation system to do?

What does xx-go --wrap do? EDIT: Ah, so the build is replacing all of the binaries with --wrap rather than setting variables. Yeah, that's quite unfortunate...

@kolyshkin
Copy link
Contributor

@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))

@thaJeztah thaJeztah changed the title Dockerfile: update to runc v1.2.0-rc.2 Dockerfile: update to runc v1.2.2 Nov 19, 2024
@thaJeztah thaJeztah added this to the 28.0.0 milestone Nov 19, 2024
- 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>
@thaJeztah
Copy link
Member Author

Alright; all green now, no_dmz not needed indeed; I'll drop those commits

@thaJeztah thaJeztah marked this pull request as ready for review November 19, 2024 14:17
@thaJeztah thaJeztah requested a review from tianon as a code owner November 19, 2024 14:17
@thaJeztah
Copy link
Member Author

Hm... looks like flaky test;

Loaded image: arm32v7/hello-world:latest
    build_userns_linux_test.go:132: run produced invalid output: "", expected "/bin/sleep cap_net_bind_service=eip"
--- FAIL: TestBuildUserNamespaceValidateCapabilitiesAreV2 (8.02s)
FAIL

@austinvazquez
Copy link
Contributor
austinvazquez commented Nov 21, 2024

Any concerns for taking this back across the release branches? I can help carry the weight if not. :)

@thaJeztah
Copy link
Member Author

It's probably fine; this is the version we use for the static binaries and for CI. The runc version used in our deb and rpm packages use the one bundled with the containerd.io packages.

@thaJeztah
Copy link
Member Author

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)

@thaJeztah
Copy link
Member Author

Let me bring this one one in 👍

@thaJeztah thaJeztah merged commit 21f7414 into moby:master Nov 21, 2024
155 checks passed
@thaJeztah thaJeztah deleted the update_runc_1.2.0 branch November 21, 2024 16:12
@austinvazquez
Copy link
Contributor
austinvazquez commented Nov 21, 2024

The runc version used in our deb and rpm packages use the one bundled with the containerd.io packages.

Oh TIL, the CI check is still good enough for me. Maybe can take it back with the containerd update as well.

@thaJeztah
Copy link
Member Author
thaJeztah commented Nov 21, 2024

Ah, yes, so the containerd.io packages are built using the https://github.com/docker/containerd-packaging repository. We build using our internal release pipeline, but use that repository to perform the builds (the internal build pipeline is effectively just a matrix for selecting which distros to build for).

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);

@austinvazquez
Copy link
Contributor

https://github.com/docker/containerd-packaging

Oh a new friend has appeared! Let me pin that one; might be good place for me to learn Debian packaging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants
0