-
Notifications
You must be signed in to change notification settings - Fork 50
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
Contrib: Deprecate torcx, ship containerd / docker as sysexts #1216
Conversation
build_library/build_image_util.sh
Outdated
|
||
sudo "${SCRIPTS_DIR}/build_sysext" --board="${BOARD}" --image_builddir=${BUILD_DIR} --squashfs_base="${BUILD_DIR}/${image_sysext_base}" --manglefs_script="${SCRIPTS_DIR}/manglefs_containerd" containerd-flatcar app-containers/containerd | ||
sudo install -m 0644 -D "${BUILD_DIR}/containerd-flatcar.raw" "${root_fs_dir}"/usr/share/flatcar/ | ||
sudo ln -sf /usr/share/flatcar/containerd-flatcar.raw "${root_fs_dir}"/etc/extensions/containerd-flatcar.raw |
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.
@pothos does this work?
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.
I also don't quite understand why we first create a docker sysext pretending containerd is installed via ${PORTAGE_CONFIGROOT}/etc/portage/profile/package.provided
and then create a containerd sysext anyway.
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.
That was done to have a split containerd and Docker sysext setup. This way users can disable Docker but keep our containerd.
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 pretending of containerd being installed is to build the Docker sysext without containerd in it.)
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.
I mean whether the symlinking to etc at this phase of the build will work
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.
@jepio to answer your question this works because of
scripts/build_library/build_image_util.sh
Lines 793 to 803 in 7c25545
# Backup the /etc contents to /usr/share/flatcar/etc to serve as | |
# source for creating missing files. Make sure that the preexisting | |
# /usr/share/flatcar/etc does not have any meaningful (non-empty) | |
# files, so we remove nothing important. There shouldn't be any | |
# symlinks either. Add "! -type d" to exclude directories as "stat" | |
# usually returns a size of a directory being 4096 or so. | |
if [[ $(sudo find "${root_fs_dir}/usr/share/flatcar/etc" -size +0 ! -type d 2>/dev/null | wc -l) -gt 0 ]]; then | |
die "Unexpected non-empty files in ${root_fs_dir}/usr/share/flatcar/etc" | |
fi | |
sudo rm -rf "${root_fs_dir}/usr/share/flatcar/etc" | |
sudo cp -a "${root_fs_dir}/etc" "${root_fs_dir}/usr/share/flatcar/etc" |
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.
That was done to have a split containerd and Docker sysext setup. This way users can disable Docker but keep our containerd.
Oh I understand the intent, but I'd argue that if we want stackable / dependent sysexts then we should make the packages shipped with each layer explicit, and re-use these when we build the next "upper" layer.
I've extended build_sysext.sh to cover this, and updated build_image_util.sh to use it.
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.
That looks more complex than it needs to be. How about just mount the whole previous sysext when building the next one.
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.
Sysexts do not contain package information since they only cover /usr
(and/or /opt
). Installed packages are registered in sub-directories below /var/db/pkg
.
(I wish this was easier though!)
build_library/dev_container_util.sh
Outdated
@@ -110,7 +110,7 @@ create_dev_container() { | |||
# The remount services are provided by coreos-base/coreos-init | |||
systemd_enable "${root_fs_dir}" "multi-user.target" "remount-usr.service" | |||
|
|||
finish_image "${image_name}" "${disk_layout}" "${root_fs_dir}" "${image_contents}" "${image_contents_wtd}" | |||
DEVCONTAINER=1 finish_image "${image_name}" "${disk_layout}" "${root_fs_dir}" "${image_contents}" "${image_contents_wtd}" |
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.
this looks unused
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.
Yeah, the whole thing needs some cleaning up. This was added in a no-comment commit without justification, and the environment variable does not appear anywhere else in the code.
The PR in its current state:
|
Build action triggered: https://github.com/flatcar/scripts/actions/runs/6688993401 |
Added SBOM, licenses, packages and file listing / disk usage inventory information for sysexts.
Sample files attached - @krnowak @jepio @pothos could you please have a look and tell me if that works for you? flatcar_production_image_contents.txt |
672f019
to
1ea4333
Compare
1ea4333
to
0a2d1f9
Compare
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.
Please change the name app-containers_docker.raw
and app-containers_containerd.raw
to docker-flatcar.raw
and containerd-flatcar.raw
. We have documented those for ages in
https://www.flatcar.org/docs/latest/container-runtimes/use-a-custom-docker-or-containerd-version/
and
https://www.flatcar.org/docs/latest/provisioning/sysext/#supplying-your-sysext-image-from-ignition
for a smooth upgrade experience which doesn't break user setups.
Also, app-containers
is a gentooism that isn't really meaningful to users. While finally better than app-emulation
we just shouldn't use Gentoo packaging categories for user-facing names. Also, we might want to add more things into the mix and have other Flatcar extensions as well, and the use of Gentoo packaging categories is not a good idea for consistency.
The concatenation is not good for automated processing. We use this file for the image change reports and would have to split all these |
Listing sysext contents in these files in separate sections was an explicit ask. We additionally ship separate files per sysext, so processing everything should be rather simple: stop parsing |
Got it, will update. It's a bit sad because this convention will make it harder to generically split more apps from the base image into sysexts since we need to maintain app -> name relationships for each, while the current implementation guarantees unique names. |
Or maybe even better - create a separate file, like |
That's what I had initially, then the request came up to create merged lists. I'm not complaining though, the way you propose it is simpler and requires less code. Will update the PR. |
Maybe the communication about the files got confusing because they serve different purposes and land on different places. For the license JSON it makes sense to have a merged view while for the contents txt files it rather doesn't because they are developer-focused. For the SBOM I think the merged view makes sense, too (even if it doesn't truly reflect what the user finds when opening the image, but I think that live SSH interaction for a scanner would also be a valid use case, so better list the files than not). |
0a2d1f9
to
892ea57
Compare
Most notably I extended the format of |
005d2ab
to
b3b45a1
Compare
This change removes torcx libraries, references, and commandline options from build automation scripts and from build_library/. Containerd and docker are shipped via sysexts which are included in the base image. Signed-off-by: Thilo Fromm <thilofromm@microsoft.com>
This change refactors sysext builds during build_image and generalises the code (no hard-coded containerd and docker anymore). A command line option is added to build_image for sysexts to include in the OS image. It defaults to containerd and docker but may be set to arbitrary packages. The command line supports simple depenencies, i.e. the "docker" sysext will re-use package information from the "containerd" sysext and not include another containerd. Signed-off-by: Thilo Fromm <thilofromm@microsoft.com>
Signed-off-by: Thilo Fromm <thilofromm@microsoft.com>
This change refactors base OS sysext builds to use a separate build script `build_library/sysext_prod_builder`, which is called from `build_library/prod_image_util.sh` when `build_image` runs. This allows for better separation of cleanup traps: prod image sysext builds need its own trap / cleanup function for temporary build directories and loopback mounts. Prod sysext builds properly generate lincense and SBOM information, and provide detailed file listings and disk space usage stats. - SBOM / licenses JSON now include all packages of the final image, i.e. a combined list of base image and all base OS sysexts. - Packages lists, files list and detailed files list include the sysext squashfs files for the base image, and separate sections with files / packages lists for each sysext. - Disk usage contains both final disk image usage as well as usage of each individual sysext squashfs.
Signed-off-by: Thilo Fromm <thilofromm@microsoft.com>
This change adds run_local_tests.sh, a script to run tests on local builds. It's a comfort wrapper around ci-automation scripts and uses the latest local build. Signed-off-by: Thilo Fromm <thilofromm@microsoft.com>
Signed-off-by: Thilo Fromm <thilofromm@microsoft.com>
This change makes QEMU_UPDATE_PAYLOAD configurable via ci-automation/settings.env where it was hard-wired before. The change also fixes fall-out in qemu_update.sh by ensuring a local tmp directory is created before it is used by the test. Signed-off-by: Thilo Fromm <thilofromm@microsoft.com>
This change bumps the image ref of the mantle container to ghcr.io/flatcar/mantle:git-20a2f8ffee8c8a1a042b1da99f0f59312110f285. This version includes 2 PRs (flatcar/mantle#465 and flatcar/mantle#466) which add support for sysext docker / torcx removal in the OS image. Signed-off-by: Thilo Fromm <thilofromm@microsoft.com>
This change adds a -U flag to run_sdk_container. If provided, the script will not regenerate version.txt but instead use the existing file as-is. Signed-off-by: Thilo Fromm <thilofromm@microsoft.com>
Include PR flatcar/update_engine#30 to un-break updates when torcx was removed in favour of sysext. Signed-off-by: Thilo Fromm <thilofromm@microsoft.com>
Signed-off-by: Thilo Fromm <thilofromm@microsoft.com>
Signed-off-by: Thilo Fromm <thilofromm@microsoft.com>
- updated github actions for runc, containerd, and docker to not handle nonexistent ebuilds in app-torcx/ anymore - removed spurious package_run_dependencies from build_image_util.sh - build_sysext: generate pkginfo before mangle script runs use zstd for compression; add cli flag to select compression - ci_automation_common.sh: remove spurious `/` from match string - coreos, board-packages, bootengine: bump ebuild revisions - kernel commonconfig: add squashfs zstd support Signed-off-by: Thilo Fromm <thilofromm@microsoft.com>
Thank you Krzesimir! Co-authored-by: Krzesimir Nowak <knowak@microsoft.com>
This change enables USE flags for all supported compression formats. zstd specifically is required to build zstd sysexts.
Signed-off-by: Thilo Fromm <thilofromm@microsoft.com>
Turns out using ${var@Q} instead of ${var} ends up with paths like /work/foo/'amd64'-usr/... instead of /work/foo/amd64-usr/... which breaks the script. So we revert it. Signed-off-by: Thilo Fromm <thilofromm@microsoft.com>
Co-authored-by: Krzesimir Nowak <knowak@microsoft.com>
Signed-off-by: Thilo Fromm <thilofromm@microsoft.com>
bc8ba62
to
f81bbeb
Compare
Addressed Krzesimir's comments, rebased to latest main, restarted Ci tests. |
==> Merging PR. Thank you so much @pothos and @krnowak for your help and support with improving this PR and getting it merged! Also big thanks to @tormath1 for help with the test suite and to @krishjainx for starting this work! |
Thanks, somehow I assumed the kernel config change would be part of last week's releases and also that we should have this here in Alpha. Eagerly waiting for the next Alpha then^^ |
This PR supersedes #982 which has been idling for a while. Main motivation of re-issuing the PR is to provide a branch directly on the scripts repo for Flatcar maintainers to collaborate on.
Features added
The PR includes a number of improvements to streamline the developer experience with sysexts. The improvements are:
build_sysext
can now handle dependencies and create sysexts that depend on packages of other sysextsbuild_image
takes a command line option of the sysexts to include in the base OS, defaulting tocontainerd
anddocker
. Motivation is to make it easier down the road to move base OS packages into sysexts, and to build leaner OS images.Testing
Unrelated minor improvements
run_local_tests.sh
to lower the bar of running tests locally for the latest image builtqemu_update.sh
test case. Formerly it was always fetched from bincache; fetch failures were silently ignored and lead to this test being skipped.run_sdk_container
to not touch / update the version file (-U
)docker container prune
).Dependencies (squashfs zstd support for Jenkins CI nodes)
[ALL DONE] Todo items
build_packages
,build_image
, and friends/etc/systemd/system/multi-user.target.wants/docker.service => /usr/lib/systemd/system/docker.service
for sysext based docker. Seekola/tests/docker/enable_docker.go
for details. : Torcx removal: update documentation flatcar-archive/flatcar-docs#343