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.

Sign up for GitHub

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

Jump to bottom

Update Moby to 20.10.17; containerd to 1.6.6; runc to 1.1.2 #310

Merged
merged 145 commits into from
Feb 20, 2023
Merged

Update Moby to 20.10.17; containerd to 1.6.6; runc to 1.1.2 #310

merged 145 commits into from
Feb 20, 2023

Conversation

lmbarros
Copy link
Contributor

@lmbarros lmbarros commented Nov 3, 2022 •
edited
Loading

The main motivation for this PR is to update containerd to the 1.6 release (as the version currently bundled with balenaOS, 1.4, is EOL). The following components were updated:

  • Moby to 20.10.17 (which the Moby release using the containerd version we wanted)
  • containerd to 1.6.6
  • runc to 1.1.2
  • All of these components own dependencies

The PR as a whole is huge, but most of its bulk comes wither from commits merged from upstream Moby or from the one commit re-vendoring the dependencies. All other commits created by me are comparatively simple. I suggest reviewing the commits individually.

Testing:

  • All unit tests are passing. A number of integration tests of the "CLI suite" are failing, but they are all regressions introduced during our latest major update (this is being tracked here: Regressions in integration tests since the 20.10 upgrade #309). In other words, all tests that were passing after the initial 20.10 upgrade are still passing.
  • Built OS for a Pi Zero, ran a stress test app for ~22 hours during which:
    • Ran ~10 app updates.
    • Measured the time needed to run a balena info and start a hello-world container. No performance differences found compared to the current balenaEngine release.
  • I created a draft PR in meta-balena using this branch, to check for integration side effects. All passes -- except for the qemu-generic-aarch64 tests which are currently unstable. (Anyway, I ran the os, e2e, and hup suites locally for qemu-generic-aarch64 and they passed)
  • We also created a draft PR in balena-raspberrrypi for running the automated test suite on actual devices. We had some failures there, but nothing that appeared to be related with the Engine.

Resolves #300

Unblocks:

@lmbarros lmbarros changed the title Update containerd to 1.6.6, runc to 1.1.2 Update Moby to 20.10.17; containerd to 1.6.6; runc to 1.1.2 Nov 3, 2022
@lmbarros lmbarros force-pushed the 20.10.17-balena branch 3 times, most recently from 258492b to 6a2fc55 Compare November 4, 2022 12:13
@lmbarros
Copy link
Contributor Author

lmbarros commented Nov 4, 2022

@balena-ci retest

@lmbarros lmbarros force-pushed the 20.10.17-balena branch 3 times, most recently from 4a3557f to 812f87f Compare November 4, 2022 12:59
@lmbarros lmbarros self-assigned this Nov 11, 2022
@lmbarros lmbarros marked this pull request as ready for review November 11, 2022 14:44
@lmbarros lmbarros force-pushed the 20.10.17-balena branch 5 times, most recently from 54951df to 65384c6 Compare November 22, 2022 17:38
@alexgg
Copy link
Contributor

alexgg commented Nov 28, 2022

@lmbarros the changes look good to me - I skipped the upstream and vendor refactoring, and scanned though your integration ones. The update is clean and well structured.
I am wondering how best to test it so we don't introduce nasty regressions. I am thinking you should maybe branch balena-raspberrypi with this update and launch some leviathan jobs on all supported device types.
It would also be good to do some manual checks in the RPI0 to see how performance is. We may also want to leave a couple of devices running some application to make sure everything is smooth.
What do you think?

@lmbarros
Copy link
Contributor Author

Thanks @alexgg ! I have a branch on meta-balena that I created to leverage the OS tests. I think I never got all tests there to pass on a single run, but every time I checked it was caused by one of the few known cases of flaky OS tests.

I'll do more extensive tests running on a device. (So far I have just tried very basic things, like starting a container.)

@lmbarros
Copy link
Contributor Author

lmbarros commented Dec 1, 2022

This will fix #284.

Hi @tianyuanhao ! We are currently defaulting to the shim runtime v1, which supports only cgroups v1. A colleague has tried to use the shim runtime v2 but run into some issues, so I believe we have some more work to do before we can support cgroups v2.

@lmbarros lmbarros force-pushed the 20.10.17-balena branch 2 times, most recently from 9a84ae1 to c8e8ac3 Compare December 8, 2022 18:05
@balena-ci balena-ci enabled auto-merge December 21, 2022 16:42
@ab77
Copy link
Contributor

ab77 commented Jan 9, 2023

@lmbarros any updates on this? We have a non-functional engine in balena-builder right now.

cc @Page- @thgreasi

@lmbarros
Copy link
Contributor Author

lmbarros commented Jan 9, 2023

It's ongoing, @ab77 . This update has broken our aarch64 CI tests, apparently because of the AppArmor config on the kernel used for generic-aarch64. This specif point is being discussed here: https://balena.zulipchat.com/#narrow/stream/345889-loop.2Fbalena-os/topic/qemu-generic-aarch64.20and.20AppArmor

@lmbarros
Copy link
Contributor Author

lmbarros commented Jan 9, 2023

What's wrong with the Engine on balena-builder now?

@ab77
Copy link
Contributor

ab77 commented Jan 9, 2023

It's ongoing, @ab77 . This update has broken our aarch64 CI tests, apparently because of the AppArmor config on the kernel used for generic-aarch64. This specif point is being discussed here: https://balena.zulipchat.com/#narrow/stream/345889-loop.2Fbalena-os/topic/qemu-generic-aarch64.20and.20AppArmor

Is there a quick way we can build the binaries for v19.03.16 manually (47b47a6) and publish a GitHub release? that would unblock the builder without having to roll back its Dockerfile and build the engine in there from source.

@lmbarros
Copy link
Contributor Author

lmbarros commented Jan 9, 2023

Is there a quick way we can build the binaries for v19.03.16 manually?

Tried a quick local build of this release and it failed (didn't dig into why, apparently some dependencies weren't found).

Why do you need this version specifically? It's almost 2 years old.

Why did you asked about this specific PR? If this PR would work for you, a newer Engine version should work, too.

@ab77
Copy link
Contributor

ab77 commented Jan 9, 2023

@lmbarros this is the last builder commit known to work: https://github.com/balena-io/balena-builder/commit/d6af534f0cb9c5733b5a1a095bea70894a9c8aa2

It contains engine build from the commit I posted.

@balena-ci balena-ci disabled auto-merge January 17, 2023 18:18
thaJeztah and others added 22 commits February 7, 2023 15:22
@thaJeztah @lmbarros
[20.10] update golang to 1.17.10
3a7a7d6 
go1.17.10 (released 2022-05-10) includes security fixes to the syscall package,
as well as bug fixes to the compiler, runtime, and the crypto/x509 and net/http/httptest
packages. See the Go 1.17.10 milestone on the issue tracker for details:

https://github.com/golang/go/issues?q=milestone%3AGo1.17.10+label%3ACherryPickApproved

Full diff: http://github.com/golang/go/compare/go1.17.9...go1.17.10

Includes fixes for:

- CVE-2022-29526 (http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-29526);
  (description at https://go.dev/issue/52313).

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah @lmbarros
[20.10] vendor: golang.org/x/sys v0.0.0-20220412211240-33da011f77ad
d663107 
Includes fixes for:

- CVE-2022-29526 (http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-29526);
  (description at https://go.dev/issue/52313).

full diff: golang/sys@63515b4...33da011

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@rumpl @lmbarros
Rename Reservation to Reservations in the open API
3c60544 
The correct name for this property is, and always was "Reservations"

Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
@thaJeztah @lmbarros
docs: api: /containers/{id}/attach/ws: remove unsupported query-args …
dd09822 
…< v1.42

These query-args were documented, but not actually supported until
ea67601 (API v1.42).

This removes them from the documentation, as these arguments were ignored
(and defaulted to `true` (enabled))

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit a5a7797)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@ameyag @lmbarros
vendor: libnetwork 2dab5620d4462865c6151e573b3e7fa5d3b8458b
81250b9 
Signed-off-by: Ameya Gawde <agawde@mirantis.com>
@ameyag @lmbarros
vendor: hcsshim a11a2c44e8a4aa9d66314b1d759ef582df5ab5e8
d58c11d 
Signed-off-by: Ameya Gawde <agawde@mirantis.com>
@thaJeztah @lmbarros
[20.10] update golang to 1.17.11
4802893 
go1.17.11 (released 2022-06-01) includes security fixes to the crypto/rand,
crypto/tls, os/exec, and path/filepath packages, as well as bug fixes to the
crypto/tls package. See the Go 1.17.11 milestone on our issue tracker for details.

https://github.com/golang/go/issues?q=milestone%3AGo1.17.11+label%3ACherryPickApproved

Hello gophers,

We have just released Go versions 1.18.3 and 1.17.11, minor point releases.

These minor releases include 4 security fixes following the security policy:

- crypto/rand: rand.Read hangs with extremely large buffers
  On Windows, rand.Read will hang indefinitely if passed a buffer larger than
  1 << 32 - 1 bytes.

  Thanks to Davis Goodin and Quim Muntal, working at Microsoft on the Go toolset,
  for reporting this issue.

  This is [CVE-2022-30634][CVE-2022-30634] and Go issue https://go.dev/issue/52561.
- crypto/tls: session tickets lack random ticket_age_add
  Session tickets generated by crypto/tls did not contain a randomly generated
  ticket_age_add. This allows an attacker that can observe TLS handshakes to
  correlate successive connections by comparing ticket ages during session
  resumption.

  Thanks to GitHub user nervuri for reporting this.

  This is [CVE-2022-30629][CVE-2022-30629] and Go issue https://go.dev/issue/52814.
- `os/exec`: empty `Cmd.Path` can result in running unintended binary on Windows

  If, on Windows, `Cmd.Run`, `cmd.Start`, `cmd.Output`, or `cmd.CombinedOutput`
  are executed when Cmd.Path is unset and, in the working directory, there are
  binaries named either "..com" or "..exe", they will be executed.

  Thanks to Chris Darroch, brian m. carlson, and Mikhail Shcherbakov for reporting
  this.

  This is [CVE-2022-30580][CVE-2022-30580] and Go issue https://go.dev/issue/52574.
- `path/filepath`: Clean(`.\c:`) returns `c:` on Windows

  On Windows, the `filepath.Clean` function could convert an invalid path to a
  valid, absolute path. For example, Clean(`.\c:`) returned `c:`.

  Thanks to Unrud for reporting this issue.

  This is [CVE-2022-29804][CVE-2022-29804] and Go issue https://go.dev/issue/52476.

[CVE-2022-30634]: https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-30634
[CVE-2022-30629]: https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-30629
[CVE-2022-30580]: https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-30580
[CVE-2022-29804]: https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-29804

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@neersighted @lmbarros
vendor: libnetwork f6ccccb1c082a432c2a5814aaedaca56af33d9ea
ed3effa 
Signed-off-by: Bjorn Neergaard <bneergaard@mirantis.com>
@akhramov @lmbarros
Port pkg/system/mknod.go to FreeBSD
bd70a11 
Because FreeBSD uses 64-bit device nodes (see
https://reviews.freebsd.org/rS318736), Linux implementation of
`system.Mknod` & `system.Mkdev` is not sufficient.

This change adds freebsd-specific implementations for `Mknod` and
Mkdev`.

Signed-off-by: Artem Khramov <akhramov@pm.me>
(cherry picked from commit f3d3994)
Signed-off-by: Doug Rabson <dfr@rabson.org>
@tonistiigi @lmbarros
system: unbreak build for darwin
75bc086 
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
(cherry picked from commit 26dafe4)
Signed-off-by: Doug Rabson <dfr@rabson.org>
@thaJeztah @lmbarros
update runc binary to v1.1.2
2acd5c2 
This is the second patch release of the runc 1.1 release branch. It
fixes CVE-2022-29162, a minor security issue (which appears to not be
exploitable) related to process capabilities.

This is a similar bug to the ones found and fixed in Docker and
containerd recently (CVE-2022-24769).

- A bug was found in runc where runc exec --cap executed processes with
  non-empty inheritable Linux process capabilities, creating an atypical Linux
  environment. For more information, see GHSA-f3fp-gc8g-vw66 and CVE-2022-29162.
- runc spec no longer sets any inheritable capabilities in the created
  example OCI spec (config.json) file.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit bc0fd3f)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah @lmbarros
update containerd binary to v1.6.5
01c58c6 
Welcome to the v1.6.5 release of containerd!

The fifth patch release for containerd 1.6 includes a few fixes and updated
version of runc.

Notable Updates

- Fix for older CNI plugins not reporting version
- Fix mount path handling for CRI plugin on Windows

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit a747cd370234fb9c15353e62903fa91fad734682)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah @lmbarros
update containerd binary to v1.6.6
3da5310 
Welcome to the v1.6.6 release of containerd!

The sixth patch release for containerd 1.6 includes a fix for
[CVE-2022-31030](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-31030).

Notable Updates

- Fix ExecSync handler to cap console output size ([GHSA-5ffw-gxpp-mxpf](GHSA-5ffw-gxpp-mxpf))

full diff: containerd/containerd@v1.6.5...v1.6.6

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit a7e3182757ca20217032cdf5bc04e61e528c2dfa)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@lmbarros
Update dependencies to match containerd 1.6.6 and runc 1.1.2
52750c7 
The versions of containerd and runc we are updating to depend upon new
versions of several packages. This commit updates these packages.

Signed-off-by: Leandro Motta Barros <leandro@balena.io>
Change-type: patch
@lmbarros
Vendor dependencies
631882a 
Re-vendor after updating several dependencies in the previous commit.

Signed-off-by: Leandro Motta Barros <leandro@balena.io>
Change-type: patch
@lmbarros
Post-merge tweaks
4069923 
Small changes to keep our code closer to Moby's.
@lmbarros
Fix compilation errors after dependencies update
7d29f8e 
Some of the updated dependencies introduced breaking changes. This
commit deals with the compilation errors that thus arose in our code.

Signed-off-by: Leandro Motta Barros <leandro@balena.io>
Change-type: patch
@lmbarros
Disable new tests that depend on swarm and plugins
c1a516d 
This is just disabling integration tests that rely on features we do not
support. We did this previously (see
088d3ee),
here we just doing the same thing for test cases introduced since then.

Signed-off-by: Leandro Motta Barros <leandro@balena.io>
Change-type: patch
@lmbarros
Fix errors with balena-engine update
7451222 
Many uses of `balena-engine update` were failing, including a bunch of
integration tests. The reason is explained below.

The new version of runc we are using changed the way it handles
command-line arguments -- particularly, how one needs to pass `-` to
mean "read the desired container configuration from the standard input".
So, the Engine code was still invoking `runc` using the old syntax,
which caused `runc` to fail.

This commit just updates the `go-runc` dependency, which is the layer we
use to invoke `runc` from Go. This new `go-runc` version contains an
update to use the new syntax.

Signed-off-by: Leandro Motta Barros <leandro@balena.io>
Change-type: patch
@thaJeztah @lmbarros
Use containerd's apparmor package to detect if apparmor can be used
f146fa6 
The runc/libcontainer apparmor package on master no longer checks if apparmor_parser
is enabled, or if we are running docker-in-docker.

While those checks are not relevant to runc (as it doesn't load the profile), these
checks _are_ relevant to us (and containerd). So switching to use the containerd
apparmor package, which does include the needed checks.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

(Cherry-picked from 2834f84, to avoid problems with app armor in some
environments -- including balenaOS' CI.)
@lmbarros
Use Go 1.17.11, as upstream
013d027 
Also, Go 1.17.x is what Yocto/OpenEmbedded kirkstone uses, so this shall
make integration with meta-balena smoother.

Signed-off-by: Leandro Motta Barros <leandro@balena.io>
Change-type: patch
@lmbarros
Add upstream changelog
c5a6085 
Signed-off-by: Leandro Motta Barros <leandro@balena.io>
Change-type: patch
@lmbarros
Disable builds for linux/386
ae2500b 
This is currently broken (apparently also upstream).

Signed-off-by: Leandro Motta Barros <leandro@balena.io>
Change-type: patch
@ab77
Copy link
Contributor

ab77 commented Feb 14, 2023

@lmbarros I've manually adjusted the pre-release and testing it:
https://github.com/balena-os/balena-engine/releases/tag/v20.10.28

@ab77 ab77 self-requested a review February 14, 2023 23:19
ab77
ab77 approved these changes Feb 15, 2023
View reviewed changes
Copy link
Contributor

@ab77 ab77 left a comment

Choose a reason for hiding this comment

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

See Zulip thread.

@lmbarros lmbarros merged commit 644c733 into master Feb 20, 2023
@lmbarros lmbarros deleted the 20.10.17-balena branch February 20, 2023 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ab77 ab77 ab77 approved these changes

Assignees

@lmbarros lmbarros

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Should update to moby v20.10.13 to get containerd v1.5.10