osbuild: use bootc install to deploy the container#4224
osbuild: use bootc install to deploy the container#4224jbtrystram wants to merge 7 commits intocoreos:mainfrom
Conversation
|
Skipping CI for Draft Pull Request. |
There was a problem hiding this comment.
Code Review
The pull request introduces changes to use bootc install to deploy the container, which simplifies the image build process. There are a few critical issues in the YAML manifest related to copy-paste errors that lead to incorrect configurations for the 4k image builds and missing options for loopback devices. These issues need to be addressed.
|
I switched the CI on this to run against |
|
A few diffs picked up by We should probably profile each diff (maybe in coreos/fedora-coreos-tracker#1827) and evaluate whether it's a change we want to make or not. |
|
I can't get a built qemu image to boot. I suspect probably the root= and boot= UUIDs added on the kernel command line? |
do you mind sharing more logs ? What I am getting locally is ignition failing on coreos/fedora-coreos-tracker#1250 |
|
Ahh. I see that too now: |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
looks like removing those make the boot process go further (ignition completes), and out of the initramfs but fail to mount the boot partition. |
|
Blocked on bootc-dev/bootc#1441 |
|
ok this works with the following PRs :
for the bootc PR, it can be built then added into the image through |
59f1061 to
254f877
Compare
|
follow-up : either find a way to get the boot components inside cosa, or change the bootc code to call bootupd from the deployed root . I think the latter is preferable. |
Made bootc-dev/bootc#1460 |
bb4270f to
310bd60
Compare
|
Alright, marking this as ready for review as all the bits are in place. This will need a release of bootc. |
dustymabe
left a comment
There was a problem hiding this comment.
Some comments.
I think there are a few things we need to iron out before we can really move forward with this:
- supporting both old and new paths at the same time
Do we need to? Usually when we make a change this large we roll it out slowly, which means we have to support both ways for some time.
This PR is ignoring that fact, but TBH looking at OSBuild configs that support both would be pretty intimidating, so I'm not excited about trying to do that either. I'd be interested in @jlebon or @travier's thoughts.
- We need to make sure any/every diff that exists between images generated this way and the old way are considered and acknowleged as acceptable before we'd make this change.
bb4e221 to
3772cfb
Compare
|
If this can save a bit of time reviewing this, attached to this comment is the output for the following: |
3772cfb to
99384c3
Compare
|
I did get back to this today. Doing some local testing. |
| # TODO move this to an overlay in fedora-coreos-config | ||
| # so it get baked into the container at build time. We | ||
| # want the container to be the source of truth as much as possible. |
There was a problem hiding this comment.
agree. I guess this is something we should go ahead and do?
Though I do have a question, will bootc work either way?
- /usr/lib/bootc/install/10-ostree.toml in buildroot
- /usr/lib/bootc/install/10-ostree.toml in target container
Does one take precedence over the other?
There was a problem hiding this comment.
There was a problem hiding this comment.
Ok I did some more testing on this to answer Dusty's question above.
Until we move to image builder we have to keep those in COSA, because we call bootc from the COSA and not the target container, so bootc won't read the configs.
See coreos/fedora-coreos-config#4093 (comment) for more details.
I filed bootc-dev/bootc#2122
btw : Yet another thing that would be easy if we had python in.
I will update that comment to be clearer.
Though I do have a question, will bootc work either way?
/usr/lib/bootc/install/10-ostree.toml in buildroot
/usr/lib/bootc/install/10-ostree.toml in target container
In this PR, only buildroot matters. With image-builder, the target container one will override the buildroot. (Note: only if they have the same name, otherwise they'll merge)
|
OK did a deep dive here today. Got distracted with a few things like python :( (added a comment to 1730) and also simplifying our manifests just in general to make them more maintainable (AI actually gave me some good insights today on this). Here's some general comments: Trying to do The file inside the disk image is: and I had to hack diff --git a/src/cmd-diff b/src/cmd-diff
index 5424fa497..78051e544 100755
--- a/src/cmd-diff
+++ b/src/cmd-diff
@@ -568,6 +568,7 @@ def diff_metal_helper(diff_from, diff_to):
# in filenames with XXXXXXXXXXXXXXXX so that we can get a real diff between
# two of the same files in different builds.
runcmd(['tar', '-xf', tmp_tar.name, '-C', diff_dir,
+ '--exclude', '*backingFsBlockDev',
'--transform', 's|[[:xdigit:]]{64}|XXXXXXXXXXXXXXXX|gx'])
except Exception as e:The most disturbing thing I see in the actual diff is the origin file has changed: diff --git a/tmp/diff-cache/metal/43.20260309.20.dev0/ostree/deploy/fedora-coreos/deploy/XXXXXXXXXXXXXXXX.0.origin b/tmp/diff-cache/metal/43.20260309.20.dev1/ostree/deploy/fedora-coreos/deploy/XXXXXXXXXXXXXXXX.0.origin
index b1f437b..60d27fc 100644
--- a/tmp/diff-cache/metal/43.20260309.20.dev0/ostree/deploy/fedora-coreos/deploy/XXXXXXXXXXXXXXXX.0.origin
+++ b/tmp/diff-cache/metal/43.20260309.20.dev1/ostree/deploy/fedora-coreos/deploy/XXXXXXXXXXXXXXXX.0.origin
@@ -1,2 +1,2 @@
[origin]
-container-image-reference=ostree-image-signed:docker://quay.io/fedora/fedora-coreos:testing-devel
+container-image-reference=ostree-unverified-registry:ostree-image-signed:docker://quay.io/fedora/fedora-coreos:testing-develIf I'm not mistaken doesn't zincati use the origin file and this might throw it off? |
The insight it gave me was this: mpp-if already works at a lower level than stage/node level. Looking at the implementation, there's nothing restricting it to stages. The _process_format method (line 1317) recursively walks the entire YAML tree — dicts and lists alike — and
In other words I think there may be a way to break up all the duplicated code in each of the architecture files into something more shared. I started working on this today but ran out of time. |
|
Thanks Dusty for looking into this.
Zincati uses the output of sigh. I should have tested this |
It's not worth spending this time IMHO. We will switch to generating those manifest at build-time in the near future anyway. |
Ok I quickly hacked this : This will require bootc-dev/bootc#2112 diff --git a/image-base.yaml b/image-base.yaml
index cbda166b..b61801ec 100644
--- a/image-base.yaml
+++ b/image-base.yaml
@@ -38,7 +38,10 @@ platform-compressor:
digitalocean: gzip
# Set container-imgref
-container-imgref: "ostree-image-signed:docker://quay.io/fedora/fedora-coreos:{stream}"
+# container-imgref: "ostree-image-signed:docker://quay.io/fedora/fedora-coreos:{stream}"
+# For bootc install we just specify the pullspec, because bootc automatically preffix
+# the `ostree-image-signed:docker://`
+container-imgref: "quay.io/fedora/fedora-coreos:{stream}" |
| should_use_bootc_install() { | ||
| _should_enable_feature "COSA_OSBUILD_USE_BOOTC_INSTALL" "use_bootc_install" | ||
| } | ||
|
|
There was a problem hiding this comment.
I'm thinking we should enable this via an image.yaml setting versus a manifest or environment variable. WDYT?
if we do change this to image.yaml, honestly all of this code can go away now because the build_with_buildah config knob is obsolete and we should be able to delete all of the relevant code now and overwrite the old code with mv cmd-build-with-buildah cmd-build.
|
ok I opened #4519 to simplify the manifests and reduce duplication... and rebased this PR on top (since I know that's extra work I don't want you to have to do because of something I decided) if you'd like to use it: https://github.com/dustymabe/coreos-assembler/tree/dusty-bootc-install |
Introduce a new overlay to ship configuration files for bootc and image-builder. These file are sourced from the container during `bootc install to-filesystem`. We can also use this later to ship other bits as we make the container more and more the source of truth, e.g. the partition table definition. This is prep work for [1] [1] coreos/fedora-coreos-tracker#1827 See also coreos/coreos-assembler#4224
Introduce a new overlay to ship configuration files for bootc and image-builder. These file are sourced from the container during `bootc install to-filesystem`. We can also use this later to ship other bits as we make the container more and more the source of truth, e.g. the partition table definition. This is prep work for [1] [1] coreos/fedora-coreos-tracker#1827 See also coreos/coreos-assembler#4224
The log disk usage message comming every 10 seconds is quite noisy, hide it when we are in a shell in osbuild. I aslo added a couple of helpful tips in comments given by @dustymabe to work with osbuild.
Prep work to add a knob for using bootc install in osbuild. Refactor the override logic in a helper function so we can easily add those knobs down the line.
This adds raw-{,4k}-image-bootc manifests that are alternative versions
of the raw-{,4k}-image manifests. This will allow to keep the legacy build
path with a new path that leverages bootc install to filesystem.
In this mode instead of deploying the container to the tree then copy all
the contents to the disk image, use bootc to directly manage the installation
to the target filesystems.
We can conditionalize this until we are confident to roll this to all streams
or move to image-builder.
Requires:
bootc-dev/bootc#1460
bootc-dev/bootc#1451
osbuild/osbuild#2149
osbuild/osbuild#2152
bootc-dev/bootc#1978
bootc-dev/bootc#1909
Create symlinks to the aleph file created by bootc so our tests and tooling find the aleph at the expected path. Note that when moving to image-build we will likely move this to an overlay in the config, that's way easier than having to wire up a blueprint option to allow creating arbitrary symlinks.
By default bootc calls bootups with the `--write-uuid` option that write a stamp file with the boot partition UUID in the UEFI parition. We want to restamp those UUID at first boot, so adding this option make sure bootc does not pass that flag to bootupd. See bootc-dev/bootc#1978
Bootc is looking for the prepare-root config file in the buildroot environnement because the main assumption is that it's run from the target container. However, in osbuild, it's run from te buildroot, because podman inside bwrap (inside supermin in our case) causes issues. It's fine for RHCOS and SCOS where we use the target container as the buildroot but we cannot do that for FCOS because we require python in the buildroot. For now, insert a prepare-root file in the supermin VM (use as the buildroot for osbuild) until either : - bootc learn to look into the container for it [1] - we ship python in our images and can use them as buildroot. Another approach would be to layer python and the osbuild dependencies on top of our image and use that as the buildroot, but that would create room for packages drift (what was in the repos at build time?). At least using COSA it's easier to keep track of versions. [1] bootc-dev/bootc#1410
Add a bootc install config file[1] to set ostree repo options so we inject the `grub_users` config on non-default entries. [1] https://bootc-dev.github.io/bootc/man/bootc-install-config.5.html#ostree See bootc-dev/bootc#1909
99384c3 to
e39271a
Compare
Instead of deploying the container to the tree then copy all the contents to the disk image, use bootc to directly manage the installation to the target filesystems.
Right now this requires to use the image as the buildroot so this requires python (for osbuild). This is tracked in [1].
[1] bootc-dev/bootc#1410 Requires osbuild/osbuild#2149