osbuild: conditionally use the built payload as the buildroot #3808

dustymabe · 2024-05-21T17:30:05Z

With this we now use a buildroot that is derived from the OCI container that was built by the pipeline. This allows us to use the exact same versions of software from the payload we built when we construct the images that we will ship, which will be better for us over time.

The benefits of this are immediately apparent in this commit as we are able to drop configuration that tries to set feature flags for our ext4 filesystems based on what we think are the current defaults in RHEL.

For now we aren't able to do this with FCOS because FCOS doesn't have python in it. This should be OK for now because COSA is almost always based on the latest version of Fedora. Though one benefit we would have if we did switch to doing this for FCOS is that we would test newer versions of "build tools" from rawhide alongside the rawhide pipeline builds that we do.

These were missed copy/pastes from d3a4551 and b74e2f9.

The copy/paste from 1d51ca4 didn't account for the fact that the image filename is renamed to disk.raw for GCP. Fix it and also add a comment with some context on why the filename differs here from the others.

Drop a few patches here: - 0004-fscache-add-eviction-log-statement.patch - 0001-stages-qemu-sanity-check-created-image.patch The fscache eviction log statement was just informational anyway and the qemu sanity check should no longer be needed since we got a fixed XFS/kernel in RHEL 9.4 and Fedora. We also drop OSBuild version locking since things are a lot more stable now and it's probably better to follow latest and find issues immediately rather than forget to update this for long periods of time.

We'll now make deploying the OSTree directly conditional on if ostree_repo is passed in or not. The only use case today where we are deploying via OSTree and not via container is our FCOS non-rawhide streams, so this shouldn't affect any non FCOS users out there today. This is prep for some re-work to create a buildroot from the container. We always have the container image in our pipelines so we want to unconditionally pass it in to OSBuild now (i.e. so we can create the buildroot from it).

dustymabe · 2024-05-21T17:30:40Z

Implementing this we think will mean issues like openshift/os#1504 will go away.

dustymabe · 2024-05-21T17:58:21Z

This will fix #3801

jlebon · 2024-05-21T18:27:04Z

build.sh

-    cat /usr/lib/coreos-assembler/0004-fscache-add-eviction-log-statement.patch                   \
-        /usr/lib/coreos-assembler/0001-stages-qemu-sanity-check-created-image.patch               \
-            | patch -d /usr/lib/osbuild -p1
+   #cat your.patch \


I think we should keep a comment here that says to freeze osbuild if we ever need to add patches here again. Or alternatively, delete all of patch_osbuild().

Reverting the commit to add patches will also revert the freeze that can then be updated.

depending on the situation I don't think it's required that we pin OSBuild if we patch.

If it's an area of code that changes a lot then it might be worth it to pin, if not then maybe not.

I'm OK with deleting the whole function, but thought for now it might be worth keeping the scaffolding in place.

Let me know what you think is best.

Leaning towards nuking it for now with the understanding that it's only a git revert away if we ever need to carry patches again in the future.

sorry I missed this in the final upload

followup in #3809

jlebon · 2024-05-21T18:52:12Z

src/osbuild-manifests/platform.qemu.ipp.yaml

@@ -79,6 +81,8 @@ pipelines:
                mpp-format-int: '{image.layout[''boot''].partnum}'
              target: /boot
  - name: qemu
+    build:
+      mpp-format-string: '{qemu_stage_buildroot}'


I'm confused why we need this. Since the buildroot is scoped to the pipeline and here we're defining a different pipeline, isn't it going to use the host buildroot if we don't specify anything?

yes. it will use the "host" buildroot if we don't specify anything, but I do like the explicitness that goes along with the comment where this is defined:

# For the qemu stages we'll use the host buildroot (COSA) # because we definitely don't include qemu-img in CoreOS qemu_stage_buildroot: ""

OK to leave a comment but maybe let's make it clearer that that's the default anyway and we're just being explicit?

Sounds good. I'll enhance the comment further.

dustymabe · 2024-05-21T19:34:00Z

so coreos.unique.boot.ignition.failure is failing here in ci/prow/rhcos. I can also reproduce this locally.

It looks like the Ensure Unique 'boot' Filesystem Label in the console is happening before Ignition even runs, but the coreos.unique.boot.ignition.failure adds a boot labeled filesystem using Ignition.

Is it really required for this unit to run after Ignition is complete? I guess so since that's a test case we want to cover, but we'll probably have to strengthen the unit dependencies.

dustymabe · 2024-05-21T19:52:53Z

ahh. interestingly enough we have two units that check if boot is unique, but they both have very similar descriptions so the log messages are hard to distinguish to the untrained eye.

so it looks like maybe the coreos-ignition-unique-boot.service somehow isn't doing it's job here.

dustymabe · 2024-05-21T19:59:12Z

ok I think I found the real error earlier up in the log:

[^[[0;32m  OK  ^[[0m] Finished ^[[0;1;39mGenerate New UUID For Boot Disk GPT^[[0m.^M                                                                                                                        
[    4.126289] systemd[1]: Finished Generate New UUID For Boot Disk GPT.^M                                                                                                                                  
[    4.137089] ignition-ostree-transposefs[908]: Moving bootfs to RAM...^M                                                                                                                                  
         Starting ^[[0;1;39mIgnition OSTree: Save Partitions^[[0m...[    4.137822] systemd[1]: Starting Ignition OSTree: Save Partitions...^M                                                               
^M                                                                                                                                                                                                          
[    4.140833] ignition-ostree-transposefs[908]: Mounting /dev/disk/by-label/boot ro (/dev/vdb3) to /var/tmp/mnt^M                                                                                          
         Starting ^[[0;1;39mIgnition OSTree: …rate Filesystem UUID (boot)^[[0m...[    4.144472] systemd[1]: Starting Ignition OSTree: Regenerate Filesystem UUID (boot)...^M                                
^M                                                                                                                                                                                                          
[    4.155819] ignition-ostree-firstboot-uuid[918]: e2fsck 1.46.5 (30-Dec-2021)^M                                                                                                                           
[    4.155947] ignition-ostree-firstboot-uuid[918]: /dev/disk/by-label/boot is in use.^M                                                                                                                    
[    4.155973] ignition-ostree-firstboot-uuid[918]: e2fsck: Cannot continue, aborting.^M                                                                                                                    
[    4.158304] EXT4-fs (vdb3): mounted filesystem 96d15588-3596-4b3c-adca-a2ff7279ea63 ro with ordered data mode. Quota mode: none.^M                                                                       
[^[[0;1;31mFAILED^[[0m] Failed to start ^[[0;1;39mIgnition O…nerate Filesystem UUID (boot)^[[0m.^M

jlebon · 2024-05-21T20:15:45Z

src/osbuild-manifests/coreos.osbuild.aarch64.mpp.yaml

+  # really use FCOS as the buildroot so we'll set it to "" so that
+  # the host (COSA) gets used as the buildroot there.
+  buildroot:
+    mpp-if: osname == 'rhcos'


Not related to the current CI failure, but shouldn't this be

Suggested change

mpp-if: osname == 'rhcos'

mpp-if: osname in ['rhcos', 'scos']

?

jlebon · 2024-05-21T20:42:41Z

I think the CI issue is roughly:

metadata_csum_seed is still off by default in el9
so with this patch the feature in RHCOS will go from on to off
changing the filesystem UUID while it's mounted doesn't work without the metadata_csum_seed feature so we'd have to serialize things more to ensure there's no mounts happening at the same time

But also, this would make us enter a completely different path in ignition-ostree-firstboot-uuid.

I think it'd be safer to still enable the feature? If we just hard enable it and then we can simplify ignition-ostree-firstboot-uuid.

With this we now use a buildroot that is derived from the OCI container that was built by the pipeline. This allows us to use the exact same versions of software from the payload we built when we construct the images that we will ship, which will be better for us over time. The benefits of this are immediately apparent in this commit as we are able to drop configuration that tries to set feature flags for our ext4 filesystems based on what we think are the current defaults in RHEL. For now we aren't able to do this with FCOS because FCOS doesn't have python in it. This should be OK for now because COSA is almost always based on the latest version of Fedora. Though one benefit we would have if we did switch to doing this for FCOS is that we would test newer versions of "build tools" from `rawhide` alongside the `rawhide` pipeline builds that we do.

Since we are now storing the OSBuild buildroot in the cache2.qcow2 let's bump the size again.

Here we just unconditionally set it because otherwise changing the filesystem while it is mounted doesn't work and we get errors like this during startup: ``` [ 4.155947] ignition-ostree-firstboot-uuid[918]: /dev/disk/by-label/boot is in use. ``` Anyways, we are going to delete the fallback code there anyway coreos/fedora-coreos-config#3008 so it's required to be set so let's set it here and then we can drop this once RHEL has caught up.

dustymabe · 2024-05-21T23:53:34Z

CI is fixed and all comments should be addressed now.

jlebon

LGTM!

- Reflect changes done in coreos/coreos-assembler#3808 Signed-off-by: Renata Ravanelli <[email protected]>

dustymabe added 4 commits May 21, 2024 12:14

osbuild: add a few pieces from previous commits to multiarch manifests

8f9cb6a

These were missed copy/pastes from d3a4551 and b74e2f9.

osbuild/gcp: fix manifest to use disk.raw

529891c

The copy/paste from 1d51ca4 didn't account for the fact that the image filename is renamed to disk.raw for GCP. Fix it and also add a comment with some context on why the filename differs here from the others.

This was referenced May 21, 2024

s390x: ext.config.shared.kdump.crash test flakes with corrupted filesystem openshift/os#1504

Closed

osbuild should use a buildroot that matches the target system #3801

Closed

jlebon reviewed May 21, 2024

View reviewed changes

dustymabe added 3 commits May 21, 2024 17:00

bump the cache2.qcow2 size

2d6e76e

Since we are now storing the OSBuild buildroot in the cache2.qcow2 let's bump the size again.

dustymabe force-pushed the dusty-osbuild branch from ac870ef to 0fb2faa Compare May 21, 2024 21:14

dustymabe enabled auto-merge (rebase) May 21, 2024 23:53

jlebon approved these changes May 22, 2024

View reviewed changes

dustymabe merged commit f9f9c5b into coreos:main May 22, 2024
5 checks passed

dustymabe deleted the dusty-osbuild branch May 22, 2024 01:02

dustymabe mentioned this pull request May 24, 2024

buildroot: support OSTree trees as buildroots osbuild/osbuild#1790

Closed

ravanelli mentioned this pull request May 29, 2024

osbuild: Fix changes done causing podman to fail #3814

Merged

ravanelli added a commit to ravanelli/fedora-coreos-pipeline that referenced this pull request May 29, 2024

jobs: Reflect changes done in cosa for osbuild

c6c0ad1

- Reflect changes done in coreos/coreos-assembler#3808 Signed-off-by: Renata Ravanelli <[email protected]>

ravanelli mentioned this pull request May 29, 2024

jobs: Reflect changes done in cosa for osbuild coreos/fedora-coreos-pipeline#997

Merged

jlebon pushed a commit to coreos/fedora-coreos-pipeline that referenced this pull request Jun 3, 2024

jobs: Reflect changes done in cosa for osbuild

ed70563

- Reflect changes done in coreos/coreos-assembler#3808 Signed-off-by: Renata Ravanelli <[email protected]>

	mpp-if: osname == 'rhcos'
	mpp-if: osname in ['rhcos', 'scos']

osbuild: conditionally use the built payload as the buildroot #3808

osbuild: conditionally use the built payload as the buildroot #3808

Uh oh!

Conversation

dustymabe commented May 21, 2024

Uh oh!

dustymabe commented May 21, 2024

Uh oh!

dustymabe commented May 21, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dustymabe commented May 21, 2024

Uh oh!

dustymabe commented May 21, 2024

Uh oh!

dustymabe commented May 21, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jlebon commented May 21, 2024

Uh oh!

dustymabe commented May 21, 2024

Uh oh!

jlebon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!